linux-kernel - Re: [PATCH v2 -next] crypto: inside-secure

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]
Date:   Wed, 4 Sep 2019 05:10:34 -0700
From:   Ard Biesheuvel <ard.biesheuvel@...aro.org>
To:     Pascal Van Leeuwen <pvanleeuwen@...imatrix.com>
Cc:     YueHaibing <yuehaibing@...wei.com>,
        "antoine.tenart@...tlin.com" <antoine.tenart@...tlin.com>,
        "herbert@...dor.apana.org.au" <herbert@...dor.apana.org.au>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "pvanleeuwen@...idesecure.com" <pvanleeuwen@...idesecure.com>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI

On Wed, 4 Sep 2019 at 04:57, Pascal Van Leeuwen
<pvanleeuwen@...imatrix.com> wrote:
>
>
> > -----Original Message-----
> > From: linux-crypto-owner@...r.kernel.org <linux-crypto-owner@...r.kernel.org> On Behalf Of
> > YueHaibing
> > Sent: Tuesday, September 3, 2019 3:45 AM
> > To: antoine.tenart@...tlin.com; herbert@...dor.apana.org.au; davem@...emloft.net;
> > pvanleeuwen@...idesecure.com
> > Cc: linux-crypto@...r.kernel.org; linux-kernel@...r.kernel.org; YueHaibing
> > <yuehaibing@...wei.com>
> > Subject: [PATCH v2 -next] crypto: inside-secure - Fix build error without CONFIG_PCI
> >
> > If CONFIG_PCI is not set, building fails:
> >
> > rivers/crypto/inside-secure/safexcel.c: In function safexcel_request_ring_irq:
> > drivers/crypto/inside-secure/safexcel.c:944:9: error: implicit declaration of function
> > pci_irq_vector;
> >  did you mean rcu_irq_enter? [-Werror=implicit-function-declaration]
> >    irq = pci_irq_vector(pci_pdev, irqid);
> >          ^~~~~~~~~~~~~~
> >
> > Use #ifdef block to guard this.
> >
> Actually, this is interesting. My *original* implementation was using
> straight #ifdefs, but then I got review feedback stating that I should not
> do that, as it's not compile testable, suggesting to use regular C if's
> instead. Then there was quite some back-and-forth on the actual
> implementation and I ended up with this.
>
> So now it turns out that doesn't work and I'm suggested to go full-circle
> back to straight #ifdef's? Or is there some other way to make this work?
> Because I don't know where to go from here ...
>


C conditionals are preferred over preprocessor conditional, but if the
conditional code refers to symbols that are not declared when the
Kconfig symbol is not defined, preprocessor conditionals are the only
option.

This is the reason we have so many empty static inline functions in
header files - it ensures that the symbols are declared even if the
only invocations are from dead code.


> > Fixes: 625f269a5a7a ("crypto: inside-secure - add support for PCI based FPGA development
> > board")
> > Signed-off-by: YueHaibing <yuehaibing@...wei.com>
> > ---
> > v2: use 'ifdef' instead of 'IS_ENABLED'
> > ---
> >  drivers/crypto/inside-secure/safexcel.c | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-
> > secure/safexcel.c
> > index e12a2a3..5253900 100644
> > --- a/drivers/crypto/inside-secure/safexcel.c
> > +++ b/drivers/crypto/inside-secure/safexcel.c
> > @@ -937,7 +937,8 @@ static int safexcel_request_ring_irq(void *pdev, int irqid,
> >       int ret, irq;
> >       struct device *dev;
> >
> > -     if (IS_ENABLED(CONFIG_PCI) && is_pci_dev) {
> > +#ifdef CONFIG_PCI
> > +     if (is_pci_dev) {
> >               struct pci_dev *pci_pdev = pdev;
> >
> >               dev = &pci_pdev->dev;
> > @@ -947,7 +948,10 @@ static int safexcel_request_ring_irq(void *pdev, int irqid,
> >                               irqid, irq);
> >                       return irq;
> >               }
> > -     } else if (IS_ENABLED(CONFIG_OF)) {
> > +     } else
> > +#endif
> > +     {
> > +#ifdef CONFIG_OF
> >               struct platform_device *plf_pdev = pdev;
> >               char irq_name[6] = {0}; /* "ringX\0" */
> >
> > @@ -960,6 +964,7 @@ static int safexcel_request_ring_irq(void *pdev, int irqid,
> >                               irq_name, irq);
> >                       return irq;
> >               }
> > +#endif
> >       }
> >
> >       ret = devm_request_threaded_irq(dev, irq, handler,
> > @@ -1137,7 +1142,8 @@ static int safexcel_probe_generic(void *pdev,
> >
> >       safexcel_configure(priv);
> >
> > -     if (IS_ENABLED(CONFIG_PCI) && priv->version == EIP197_DEVBRD) {
> > +#ifdef CONFIG_PCI
> > +     if (priv->version == EIP197_DEVBRD) {
> >               /*
> >                * Request MSI vectors for global + 1 per ring -
> >                * or just 1 for older dev images
> > @@ -1153,6 +1159,7 @@ static int safexcel_probe_generic(void *pdev,
> >                       return ret;
> >               }
> >       }
> > +#endif
> >
> >       /* Register the ring IRQ handlers and configure the rings */
> >       priv->ring = devm_kcalloc(dev, priv->config.rings,
> > --
> > 2.7.4
> >
>
> Regards,
> Pascal van Leeuwen
> Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> www.insidesecure.com