linux-kernel - Re: [PATCH] peci: PECI should depend on ARCH

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, 30 Mar 2022 10:37:26 +0000
From:   "Winiarska, Iwona" <iwona.winiarska@...el.com>
To:     "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "geert@...ux-m68k.org" <geert@...ux-m68k.org>,
        "patrick@...cx.xyz" <patrick@...cx.xyz>
CC:     "pierre-louis.bossart@...ux.intel.com" 
        <pierre-louis.bossart@...ux.intel.com>,
        "jason.m.bills@...ux.intel.com" <jason.m.bills@...ux.intel.com>,
        "openbmc@...ts.ozlabs.org" <openbmc@...ts.ozlabs.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "jae.hyun.yoo@...ux.intel.com" <jae.hyun.yoo@...ux.intel.com>,
        "joel@....id.au" <joel@....id.au>
Subject: Re: [PATCH] peci: PECI should depend on ARCH_ASPEED

On Tue, 2022-03-29 at 12:08 -0500, Patrick Williams wrote:
> On Tue, Mar 29, 2022 at 12:33:14PM +0200, Greg Kroah-Hartman wrote:
> > On Tue, Mar 29, 2022 at 11:21:37AM +0200, Geert Uytterhoeven wrote:
> > > The Platform Environment Control Interface (PECI) is only available on
> > > Baseboard Management Controllers (BMC) for Intel processors.  Currently
> > > the only supported BMCs are ASpeed BMC SoCs.  Hence add a dependency on
> > > ARCH_ASPEED, to prevent asking the user about the PECI subsystem when
> > > configuring a kernel without ASpeed SoC support.
> > > 
> > > Fixes: 6523d3b2ffa238ac ("peci: Add core infrastructure")
> > > Signed-off-by: Geert Uytterhoeven <geert@...ux-m68k.org>
> > > ---
> > >  drivers/peci/Kconfig | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/peci/Kconfig b/drivers/peci/Kconfig
> > > index 89872ad833201510..0d3ef8ba0998d649 100644
> > > --- a/drivers/peci/Kconfig
> > > +++ b/drivers/peci/Kconfig
> > > @@ -2,6 +2,7 @@
> > >  
> > >  menuconfig PECI
> > >         tristate "PECI support"
> > > +       depends on ARCH_ASPEED || COMPILE_TEST
> > 
> > I hate ARCH_ dependencies as there is nothing specific with that one
> > platform that means that this driver subsystem will only work on that
> > one.
> 
> The motivation in the commit message isn't even accurate because the chips
> under ARCH_NPCM are usually used as a BMC as well and PECI is just as important
> for them.  HPE also has a custom chip they use as BMC and they've started the
> process for upstreaming that support.

"Currently the only supported BMCs are ASpeed BMC SoCs."
From PECI subsystem perspective (not BMC support in general), it is technically
true for now - but I agree with Greg and Patrick, this is just artificially
introducing build-time dependencies where in fact there are none. And yes - we
would then have to add the "depends on ARCH_YET_ANOTHER_ARCH" to generic
subsystem anytime we add a new PECI controller. This belongs in the controller
(and the ASPEED one depends on ARCH_ASPEED).

> 
> > I'm all for fixing build dependancies, but it should be fine to build
> > all drivers for all arches.
> 
> There are a few drivers, like PECI and FSI, that are likely only useful
> when being used in the BMC space.  Is it worth having a "config BMC" for
> drivers which are likely only useful in a BMC environment and that we can
> "if BMC" around these drivers so they get hidden from the menuconfig for
> typical use cases?

We don't have "config SERVER/config DESKTOP/config IOT".
I don't think there's anything special about BMCs to benefit from introducing
that (ultimately, this would be yet another "artificial build-time dependency").

Thanks
-Iwona

>