linux-kernel - Re: [PATCH 1/4] cdx: add support for bus enable and disable

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: <2023071223-unstopped-marsupial-9c89@gregkh>
Date:   Wed, 12 Jul 2023 17:09:13 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     "Gangurde, Abhijit" <abhijit.gangurde@....com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Gupta, Nipun" <Nipun.Gupta@....com>,
        "Agarwal, Nikhil" <nikhil.agarwal@....com>,
        "Gupta, Puneet (DCG-ENG)" <puneet.gupta@....com>,
        "git (AMD-Xilinx)" <git@....com>,
        "Simek, Michal" <michal.simek@....com>,
        "Jansen Van Vuuren, Pieter" <pieter.jansen-van-vuuren@....com>
Subject: Re: [PATCH 1/4] cdx: add support for bus enable and disable

On Wed, Jul 12, 2023 at 01:21:13PM +0000, Gangurde, Abhijit wrote:
> [AMD Official Use Only - General]
> 
> > > CDX bus needs to be disabled before updating/writing devices
> > > in the FPGA. Once the devices are written, the bus shall be
> > > enabled. This change provides sysfs entry to enable/disable the
> > > CDX bus.
> > >
> > > Co-developed-by: Nipun Gupta <nipun.gupta@....com>
> > > Signed-off-by: Nipun Gupta <nipun.gupta@....com>
> > > Signed-off-by: Abhijit Gangurde <abhijit.gangurde@....com>
> > > Reviewed-by: Pieter Jansen van Vuuren <pieter.jansen-van-
> > vuuren@....com>
> > > Tested-by: Nikhil Agarwal <nikhil.agarwal@....com>
> > > ---
> > >  Documentation/ABI/testing/sysfs-bus-cdx | 11 +++++
> > >  drivers/cdx/cdx.c                       | 26 ++++++++++++
> > >  drivers/cdx/controller/cdx_controller.c | 27 +++++++++++++
> > >  drivers/cdx/controller/mc_cdx_pcol.h    | 54
> > +++++++++++++++++++++++++
> > >  drivers/cdx/controller/mcdi_functions.c | 24 +++++++++++
> > >  drivers/cdx/controller/mcdi_functions.h | 16 ++++++++
> > >  include/linux/cdx/cdx_bus.h             |  6 +++
> > >  7 files changed, 164 insertions(+)
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-bus-cdx
> > b/Documentation/ABI/testing/sysfs-bus-cdx
> > > index 7af477f49998..0afa85b3c63b 100644
> > > --- a/Documentation/ABI/testing/sysfs-bus-cdx
> > > +++ b/Documentation/ABI/testing/sysfs-bus-cdx
> > > @@ -11,6 +11,17 @@ Description:
> > >
> > >               # echo 1 > /sys/bus/cdx/rescan
> > >
> > > +What:              /sys/bus/cdx/enable
> > > +Date:              July 2023
> > > +Contact:   nipun.gupta@....com
> > > +Description:
> > > +           Writing y/1/on to this file enables the CDX bus and
> > > +           writing n/0/off disables the bus.
> > > +
> > > +           For example to disable CDX bus::
> > > +
> > > +             # echo 0 > /sys/bus/cdx/enable
> >
> > What could go wrong!  :)
> >
> > You don't say why disabling / enabling the bus is needed, this feels
> > like a very huge stick, why is this for all busses, and not just an
> > individual CDX bus?
> >
> 
> As said in the description of the patch, disabling/enabling is needed when FPGA is being reprogrammed.

Ok, why would this not also be in the description for when you need to
look up what this file does?  You will not be able to track it back to
the commit log very easily.

thanks,

greg k-h