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: <155026608236.115909.9007817366859489104@swboyd.mtv.corp.google.com>
Date:   Fri, 15 Feb 2019 13:28:02 -0800
From:   Stephen Boyd <sboyd@...nel.org>
To:     Brian Masney <masneyb@...tation.org>
Cc:     andy.gross@...aro.org, bjorn.andersson@...aro.org,
        lee.jones@...aro.org, linus.walleij@...aro.org,
        marc.zyngier@....com, tglx@...utronix.de, shawnguo@...nel.org,
        dianders@...omium.org, linux-gpio@...r.kernel.org,
        nicolas.dechesne@...aro.org, niklas.cassel@...aro.org,
        david.brown@...aro.org, robh+dt@...nel.org, mark.rutland@....com,
        thierry.reding@...il.com, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v2 05/11] mfd: pm8xxx: disassociate old virq if hwirq mapping already exists

Quoting Brian Masney (2019-02-15 05:47:33)
> On Thu, Feb 14, 2019 at 09:51:26PM -0800, Stephen Boyd wrote:
> > > diff --git a/drivers/mfd/qcom-pm8xxx.c b/drivers/mfd/qcom-pm8xxx.c
> > > index 8eb2528793f9..2f99a98ccee5 100644
> > > --- a/drivers/mfd/qcom-pm8xxx.c
> > > +++ b/drivers/mfd/qcom-pm8xxx.c
> > > @@ -380,6 +380,12 @@ static void pm8xxx_irq_domain_map(struct pm_irq_chip *chip,
> > >                                   struct irq_domain *domain, unsigned int irq,
> > >                                   irq_hw_number_t hwirq, unsigned int type)
> > >  {
> > > +       unsigned int old_virq;
> > > +
> > > +       old_virq = irq_find_mapping(domain, hwirq);
> > > +       if (old_virq)
> > > +               irq_domain_disassociate(domain, old_virq);
> > 
> > Is it possible to pass 'true' for the 'realloc' argument to
> > __irq_domain_alloc_irqs() and then this disassociate change isn't
> > needed?
> 
> The kernel doc for __irq_domain_alloc_irqs() says that the realloc
> parameter is mainly to support legacy IRQs. I don't think its a good
> idea to add new code that'll stay past the end of this patch series
> on top of that legacy interface.
> 

Ok. The other side of the argument is that this is the only user of
irq_domain_disassociate(), which may also be some sort of legacy
interface that isn't supposed to be used. Looking at the commit that
exposed it, it seems to be that it's there for legacy reasons.

commit 43a775916d63d1c822107b39987192ca5ced445c
Author: Jiang Liu <jiang.liu@...ux.intel.com>
Date:   Mon Jun 9 16:20:05 2014 +0800

    genirq: Export irq_domain_disassociate() to architecture interrupt drivers
    
    Export irq_domain_disassociate() to architecture interrupt drivers,
    so it could be used to handle legacy IRQ descriptors on x86.

So maybe we should just use the realloc argument and bury the
disassociate API in irqdomain.c because it's not supposed to be used?
Or does the realloc path not work for some reason?

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ