linux-kernel - Re: [PATCH v3 3/9] PCI: keystone: Use hwirq to get the legacy IRQ number offset

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: <86ftsqsfk6.wl-marc.zyngier@arm.com>
Date:   Thu, 14 Feb 2019 12:46:49 +0000
From:   Marc Zyngier <marc.zyngier@....com>
To:     Lorenzo Pieralisi <lorenzo.pieralisi@....com>
Cc:     Kishon Vijay Abraham I <kishon@...com>,
        Murali Karicheri <m-karicheri2@...com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Jingoo Han <jingoohan1@...il.com>,
        Gustavo Pimentel <gustavo.pimentel@...opsys.com>,
        <linux-pci@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 3/9] PCI: keystone: Use hwirq to get the legacy IRQ number offset

Hi Lirenzo,

On Thu, 14 Feb 2019 10:31:38 +0000,
Lorenzo Pieralisi <lorenzo.pieralisi@....com> wrote:
> 
> [CC'ed MarcZ]
> 
> On Thu, Feb 14, 2019 at 10:27:19AM +0530, Kishon Vijay Abraham I wrote:
> > Hi Lorenzo,
> > 
> > On 13/02/19 10:27 PM, Lorenzo Pieralisi wrote:
> > > On Wed, Feb 13, 2019 at 06:56:23PM +0530, Kishon Vijay Abraham I wrote:
> > >> ks_pcie_legacy_irq_handler() uses 'virq' to get the IRQ number offset.
> > >> This offset is used to get the correct IRQ_STATUS register
> > >> corresponding to the IRQ line that raised the interrupt.
> > >> There is no guarantee that 'virq' assigned for consecutive hardware
> > >> IRQ will be contiguous. And this might get us an incorrect IRQ number
> > >> offset.
> > >>
> > >> Fix it here by using 'hwirq' to get the IRQ number offset.
> > >>
> > >> Link: https://lkml.kernel.org/r/bb081d21-7c03-0357-4294-7e92d95d838c@arm.com
> > >> Signed-off-by: Kishon Vijay Abraham I <kishon@...com>
> > >> ---
> > >>  drivers/pci/controller/dwc/pci-keystone.c | 17 +++++++++++++----
> > >>  1 file changed, 13 insertions(+), 4 deletions(-)
> > >>
> > >> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> > >> index e8b1d8eca78e..d35ac712a9f8 100644
> > >> --- a/drivers/pci/controller/dwc/pci-keystone.c
> > >> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> > >> @@ -87,7 +87,7 @@ struct keystone_pcie {
> > >>  	struct dw_pcie		*pci;
> > >>  	/* PCI Device ID */
> > >>  	u32			device_id;
> > >> -	int			legacy_host_irqs[PCI_NUM_INTX];
> > >> +	int			legacy_host_irq;
> > >>  	struct			device_node *legacy_intc_np;
> > >>  
> > >>  	int			msi_host_irqs[MAX_MSI_HOST_IRQS];
> > >> @@ -582,11 +582,11 @@ static void ks_pcie_msi_irq_handler(struct irq_desc *desc)
> > >>   */
> > >>  static void ks_pcie_legacy_irq_handler(struct irq_desc *desc)
> > >>  {
> > >> -	unsigned int irq = irq_desc_get_irq(desc);
> > >> +	unsigned int irq = desc->irq_data.hwirq;
> > >>  	struct keystone_pcie *ks_pcie = irq_desc_get_handler_data(desc);
> > >>  	struct dw_pcie *pci = ks_pcie->pci;
> > >>  	struct device *dev = pci->dev;
> > >> -	u32 irq_offset = irq - ks_pcie->legacy_host_irqs[0];
> > >> +	u32 irq_offset = irq - ks_pcie->legacy_host_irq;
> > > 
> > > I think you should use the plain hwirq number (that if I understand
> > > correctly range in [0,3]) and drop legacy_host_irq.
> > 
> > The hwirq is [80, 83] for Keystone. We store legacy_host_irq (for Keystone it
> > is 80) to get the correct offset in the range [0, 3].
> 
> IIUC the _parent_ hw_irq number should be what you need here, Marc (if
> he has time) can correct me if I am wrong (or what I am suggesting is
> a violation of IRQ domain bstraction usage).

It isn't pretty, but it is something we already do in some cases (see
irq-gic-v3-mbi.c::mbi_compose_msi_msg as an infamous example).

Thanks,

	M.

-- 
Jazz is not dead, it just smell funny.