linux-kernel - Re: [PATCH] pci: do not try to assign irq 255

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]

Message-ID: <512CBD88.5020008@suse.de>
Date:	Tue, 26 Feb 2013 14:50:00 +0100
From:	Hannes Reinecke <hare@...e.de>
To:	"Rafael J. Wysocki" <rjw@...k.pl>
Cc:	Yinghai Lu <yinghai@...nel.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	linux-kernel@...r.kernel.org, Frederik Himpe <fhimpe@....ac.be>,
	Oliver Neukum <oneukum@...e.de>, linux-usb@...r.kernel.org,
	linux-pci@...r.kernel.org, Len Brown <lenb@...nel.org>
Subject: Re: [PATCH] pci: do not try to assign irq 255

On 02/26/2013 02:29 PM, David Härdeman wrote:
> On Thu, Feb 21, 2013 at 07:53:14AM +0100, Hannes Reinecke wrote:
>> On 02/20/2013 05:57 PM, Yinghai Lu wrote:
>>> it seems you mess pin with interrupt line.
>>>
>>> current code:
>>>          unsigned char irq;
>>>
>>>          pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &irq);
>>>          dev->pin = irq;
>>>          if (irq)
>>>                  pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
>>>          dev->irq = irq;
>>>
>>> so if the device does not have interrupt pin implemented, pin should be zero.
>>> and  pin and irq in dev should
>>> be all 0.
>>>
>> But the device _has_ an interrupt pin implemented.
>> The whole point here is that the interrupt line is _NOT_ zero.
>>
> ...
>>
>> So at one point we have to decide that ->irq is not valid, despite it
>> being not set to zero.
>> An alternative fix would be this:
>>
>> diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
>> index 68a921d..4a480cb 100644
>> --- a/drivers/acpi/pci_irq.c
>> +++ b/drivers/acpi/pci_irq.c
>> @@ -469,6 +469,7 @@ int acpi_pci_irq_enable(struct pci_dev *dev)
>>                 } else {
>>                         dev_warn(&dev->dev, "PCI INT %c: no GSI\n",
>>                                  pin_name(pin));
>> +                       dev->irq = 0;
>>                 }
>>                 return 0;
>>         }
>>
>> Which probably is a better solution, as here ->irq is _definitely_
>> not valid, so we should reset it to '0' to avoid confusion on upper
>> layers.
>>
>
> Is there any agreement on how to proceed?
>
I would actually prefer the second solution, as the ACPI code gives
some better guarantees here. With the original solution it _might_ 
be that on non-ACPI systems an interrupt 255 is valid, so it might
incur unwanted regressions.

However, for an ACPI system we only have the two choices, assigning
an interrupt via ACPI tables or use a default GSI value.
If both failed the interrupt definitely is not valid and can safely
be reset to 0.

But this would need a formal ACK from the ACPI gods ...
Len? Rafael?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@...e.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/