linux-kernel - Re: [PATCH V3 1/3] ACPI, PCI IRQ: add PCI

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: <b4a8995e-1454-3b84-0681-1491788087ec@codeaurora.org>
Date:   Tue, 18 Oct 2016 08:20:55 -0700
From:   Sinan Kaya <okaya@...eaurora.org>
To:     Bjorn Helgaas <helgaas@...nel.org>
Cc:     linux-acpi@...r.kernel.org, rjw@...ysocki.net, bhelgaas@...gle.com,
        ravikanth.nalla@....com, linux@...nbow-software.org,
        timur@...eaurora.org, cov@...eaurora.org, jcm@...hat.com,
        alex.williamson@...hat.com, linux-pci@...r.kernel.org,
        agross@...eaurora.org, linux-kernel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, wim@....tudelft.nl,
        linux-arm-kernel@...ts.infradead.org, Len Brown <lenb@...nel.org>
Subject: Re: [PATCH V3 1/3] ACPI, PCI IRQ: add PCI_USING penalty for ISA
 interrupts

On 10/18/2016 6:59 AM, Bjorn Helgaas wrote:
>> However, this function only gets called if the IRQ number is greater than
>> > 16 and acpi_irq_get_penalty function gets called before ACPI start in
>> > acpi_isa_irq_available and acpi_penalize_isa_irq functions. We can't rely
>> > on iterating the link list.

Maybe, I am missing context here. I can add this paragraph to the commit. 

When we started cleaning the code we got rid of the acpi_irq_penalty_init function
in favor of acpi_irq_pci_sharing_penalty function as it does have some fair
amount of code duplication.

I tried putting back the acpi_irq_pci_sharing_penalty function into the ISA
IRQ path again during the debug and the machine died way too early. We couldn't
collect any debug message.

This is telling me that we can't even iterate the link list when these two API
is called. ISA IRQ need to be handled with special care due to calling order.


> It seems wrong to me that we call acpi_irq_get_penalty() from
> acpi_irq_penalty_update() and acpi_penalize_isa_irq().  It seems like they
> should just manipulate acpi_isa_irq_penalty[irq] directly.
> 
> acpi_irq_penalty_update() is for command-line parameters, so it certainly
> doesn't need the acpi_irq_pci_sharing_penalty() information (the
> acpi_link_list should be empty at the time we process the command-line
> parameters).
> 
> acpi_penalize_isa_irq() is telling us that a PNP or ACPI device is using
> the IRQ -- this should modify the IRQ's penalty, but it shouldn't depend on
> the acpi_irq_pci_sharing_penalty() value at all.
> 

acpi_irq_get_penalty function knows how to deal with ISA IRQ. So, it is harmless
to call it. Also, reading the acpi_isa_irq_penalty array directly isn't also right.
It doesn't contain the SCI penalty. So, it returns incorrect penalty value.

The rule of thumb is:
- all PCI/SCI penalty reads need to go through acpi_isa_irq_penalty function
- all ISA penalty writes need to go through acpi_isa_irq_penalty array directly.
- we do not support modifying the PCI IRQ penalties greater than the ISA IRQ numbers.
The original code supported this.


-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.