linux-kernel - Re: [PATCH] cdx: add support for bus mastering

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] [day] [month] [year] [list]

Message-ID: <2023071939-delegator-kebab-51b8@gregkh>
Date:   Wed, 19 Jul 2023 15:36:15 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     "Gupta, Nipun" <nipun.gupta@....com>
Cc:     linux-kernel@...r.kernel.org, git@....com,
        pieter.jansen-van-vuuren@....com, nikhil.agarwal@....com,
        michal.simek@....com, abhijit.gangurde@....com
Subject: Re: [PATCH] cdx: add support for bus mastering

On Wed, Jul 19, 2023 at 06:06:40PM +0530, Gupta, Nipun wrote:
> 
> 
> On 7/18/2023 7:16 PM, Greg KH wrote:
> > On Tue, Jul 18, 2023 at 03:36:51PM +0530, Nipun Gupta wrote:
> > > Introduce cdx_set_master() and cdx_clear_master() APIs
> > > to support enable and disable of bus mastering. Drivers
> > > need to use these APIs to enable/disable DMAs from the
> > > CDX devices.
> > 
> > You do have a full 72 columns, why not use that?
> 
> sure, will update accordingly.
> 
> > 
> > > Signed-off-by: Nipun Gupta <nipun.gupta@....com>
> > > Reviewed-by: Pieter Jansen van Vuuren <pieter.jansen-van-vuuren@....com>
> > > ---
> > >   drivers/cdx/cdx.c                       | 32 ++++++++++++++
> > >   drivers/cdx/controller/cdx_controller.c |  4 ++
> > >   drivers/cdx/controller/mcdi_functions.c | 57 +++++++++++++++++++++++++
> > >   drivers/cdx/controller/mcdi_functions.h | 13 ++++++
> > >   include/linux/cdx/cdx_bus.h             | 16 +++++++
> > >   5 files changed, 122 insertions(+)
> > > 
> > > diff --git a/drivers/cdx/cdx.c b/drivers/cdx/cdx.c
> > > index d2cad4c670a0..efb24672b7d9 100644
> > > --- a/drivers/cdx/cdx.c
> > > +++ b/drivers/cdx/cdx.c
> > > @@ -182,6 +182,38 @@ cdx_match_id(const struct cdx_device_id *ids, struct cdx_device *dev)
> > >   	return NULL;
> > >   }
> > > +int cdx_set_master(struct cdx_device *cdx_dev)
> > > +{
> > > +	struct cdx_controller *cdx = cdx_dev->cdx;
> > > +	struct cdx_device_config dev_config;
> > > +	int ret;
> > > +
> > > +	dev_config.type = CDX_DEV_BUS_MASTER_CONF;
> > > +	dev_config.bme = true;
> > 
> > What is "bme"?
> 
> This is bus master enable. I will add a comment on the structure definition.

Better yet, spell it out "bus_master_enable" so no one has to look up
the comment, no need to try to make cryptic variable names :)

thanks,

greg k-h