linux-kernel - Re: [PATCH 1/4] acpi,pci,irq: reduce resource requirements

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 14 Mar 2016 16:01:47 -0500
From:	Bjorn Helgaas <helgaas@...nel.org>
To:	Sinan Kaya <okaya@...eaurora.org>
Cc:	linux-acpi@...r.kernel.org, timur@...eaurora.org,
	cov@...eaurora.org, linux-pci@...r.kernel.org,
	ravikanth.nalla@....com, lenb@...nel.org, harish.k@....com,
	ashwin.reghunandanan@....com, bhelgaas@...gle.com,
	rjw@...ysocki.net, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] acpi,pci,irq: reduce resource requirements

On Mon, Mar 14, 2016 at 04:37:51PM -0400, Sinan Kaya wrote:
> Hi Bjorn,
> 
> On 3/14/2016 2:52 PM, Bjorn Helgaas wrote:
> >> bool acpi_isa_irq_available(int irq)
> >> > @@ -840,13 +881,6 @@ bool acpi_isa_irq_available(int irq)
> >> >   */
> >> >  void acpi_penalize_sci_irq(int irq, int trigger, int polarity)
> >> >  {
> >> > -	if (irq >= 0 && irq < ARRAY_SIZE(acpi_irq_penalty)) {
> >> > -		if (trigger != ACPI_MADT_TRIGGER_LEVEL ||
> >> > -		    polarity != ACPI_MADT_POLARITY_ACTIVE_LOW)
> >> > -			acpi_irq_penalty[irq] += PIRQ_PENALTY_ISA_ALWAYS;
> >> > -		else
> >> > -			acpi_irq_penalty[irq] += PIRQ_PENALTY_PCI_USING;
> >> > -	}
> > I think we lost the validation of trigger mode and polarity, didn't
> > we?
> > 
> 
> This function gets called to inform ACPI that this is the SCI interrupt
> and, trigger and polarity are their attributes.
> 
> The return value is void and the caller is not interested in what ACPI thinks
> about. 
> 
> This function adjusts the SCI penalty based on correct attributes passed
> (ISA_ALWAYS vs. PCI_USING).
> 
> I agree that we lost this validation. 
> 
> I can keep sci_trigger/sci_polarity somewhere and keep that into the calculation
> in get function.
> 
> Like this for instance,
> 
> 	if (irq == acpi_gbl_FADT.sci_interrupt) {
> +		if (sci_trigger != ACPI_MADT_TRIGGER_LEVEL ||
> +		    sci_polarity != ACPI_MADT_POLARITY_ACTIVE_LOW)
> +			penalty += PIRQ_PENALTY_ISA_ALWAYS;
> +		else
> 			penalty += PIRQ_PENALTY_PCI_USING; 
> 	}
> 
> Then, we can't get rid of the function just we can reduce the contents.

I think it's important to keep that check.

I raised the possibility of using irq_get_trigger_type() for all IRQs
(not just the SCI).  Did you have a chance to look into that at all?

Bjorn