linux-kernel - Re: [PATCH 30/39] PCI: Bulk conversion to generic_handle_domain

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: <87tumswmqc.wl-maz@kernel.org>
Date:   Mon, 24 May 2021 09:38:03 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     Rob Herring <robh@...nel.org>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Michael Ellerman <mpe@...erman.id.au>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Ley Foon Tan <ley.foon.tan@...el.com>,
        Chris Zankel <chris@...kel.net>,
        Max Filippov <jcmvbkbc@...il.com>,
        Vineet Gupta <vgupta@...opsys.com>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Robert Jarzmik <robert.jarzmik@...e.fr>,
        Russell King <linux@...linux.org.uk>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>,
        Yoshinori Sato <ysato@...rs.sourceforge.jp>,
        Rich Felker <dalias@...c.org>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Alex Deucher <alexander.deucher@....com>,
        Christian König <christian.koenig@....com>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Rob Clark <robdclark@...il.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Lee Jones <lee.jones@...aro.org>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Android Kernel Team <kernel-team@...roid.com>
Subject: Re: [PATCH 30/39] PCI: Bulk conversion to generic_handle_domain_irq()

On Thu, 20 May 2021 18:47:06 +0100,
Rob Herring <robh@...nel.org> wrote:
> 
> On Thu, May 20, 2021 at 11:57 AM Marc Zyngier <maz@...nel.org> wrote:
> >
> > Wherever possible, replace constructs that match either
> > generic_handle_irq(irq_find_mapping()) or
> > generic_handle_irq(irq_linear_revmap()) to a single call to
> > generic_handle_domain_irq().
> >
> > Signed-off-by: Marc Zyngier <maz@...nel.org>
> > ---
> >  drivers/pci/controller/dwc/pci-dra7xx.c        | 14 +++++---------
> >  drivers/pci/controller/dwc/pci-keystone.c      |  5 ++---
> >  .../pci/controller/dwc/pcie-designware-host.c  |  9 ++++-----
> >  drivers/pci/controller/dwc/pcie-uniphier.c     |  6 ++----
> >  .../controller/mobiveil/pcie-mobiveil-host.c   | 15 ++++++---------
> >  drivers/pci/controller/pci-aardvark.c          |  5 ++---
> >  drivers/pci/controller/pci-ftpci100.c          |  2 +-
> >  drivers/pci/controller/pci-tegra.c             |  8 +++-----
> >  drivers/pci/controller/pci-xgene-msi.c         |  9 +++------
> >  drivers/pci/controller/pcie-altera-msi.c       | 10 ++++------
> >  drivers/pci/controller/pcie-altera.c           | 10 ++++------
> >  drivers/pci/controller/pcie-brcmstb.c          |  9 ++++-----
> >  drivers/pci/controller/pcie-iproc-msi.c        |  4 +---
> >  drivers/pci/controller/pcie-mediatek-gen3.c    | 13 ++++---------
> >  drivers/pci/controller/pcie-mediatek.c         | 12 ++++--------
> >  drivers/pci/controller/pcie-microchip-host.c   | 18 +++++++-----------
> >  drivers/pci/controller/pcie-rcar-host.c        |  8 +++-----
> >  drivers/pci/controller/pcie-rockchip-host.c    |  8 +++-----
> >  drivers/pci/controller/pcie-xilinx-cpm.c       |  4 ++--
> >  drivers/pci/controller/pcie-xilinx-nwl.c       | 13 +++----------
> >  drivers/pci/controller/pcie-xilinx.c           |  9 ++++-----
> >  21 files changed, 71 insertions(+), 120 deletions(-)
> 
> 
> > diff --git a/drivers/pci/controller/pci-xgene-msi.c b/drivers/pci/controller/pci-xgene-msi.c
> > index 1c34c897a7e2..cf3832b905e8 100644
> > --- a/drivers/pci/controller/pci-xgene-msi.c
> > +++ b/drivers/pci/controller/pci-xgene-msi.c
> > @@ -291,8 +291,7 @@ static void xgene_msi_isr(struct irq_desc *desc)
> >         struct irq_chip *chip = irq_desc_get_chip(desc);
> >         struct xgene_msi_group *msi_groups;
> >         struct xgene_msi *xgene_msi;
> > -       unsigned int virq;
> > -       int msir_index, msir_val, hw_irq;
> > +       int msir_index, msir_val, hw_irq, ret;
> >         u32 intr_index, grp_select, msi_grp;
> >
> >         chained_irq_enter(chip, desc);
> > @@ -330,10 +329,8 @@ static void xgene_msi_isr(struct irq_desc *desc)
> >                          * CPU0
> >                          */
> >                         hw_irq = hwirq_to_canonical_hwirq(hw_irq);
> > -                       virq = irq_find_mapping(xgene_msi->inner_domain, hw_irq);
> > -                       WARN_ON(!virq);
> > -                       if (virq != 0)
> > -                               generic_handle_irq(virq);
> > +                       ret = generic_handle_domain_irq(xgene_msi->inner_domain, hw_irq);
> > +                       WARN_ON(ret);
> 
> There's various error prints in some of the handlers. I think they
> should be moved to the core. I can't imagine handling the irq is ever
> optional.

Printing stuff like this is a sure recipe for disaster, and there is
no way I'm moving such crap into core code. If the interrupt handling
fails (most likely because there is no mapping for this interrupt), it
is the driver's responsibility to handle the error (either disabling
the input or the output of the secondary irqchip). There isn't much
the core code can do about it.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.