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
| ||
|
Date: Thu, 06 Apr 2017 10:26:12 +0300 From: Luca Coelho <luca@...lho.fi> To: "Luis R. Rodriguez" <mcgrof@...nel.org>, gregkh@...uxfoundation.org Cc: wagi@...om.org, dwmw2@...radead.org, rafal@...ecki.pl, arend.vanspriel@...adcom.com, rjw@...ysocki.net, yi1.li@...ux.intel.com, atull@...nsource.altera.com, moritz.fischer@...us.com, pmladek@...e.com, johannes.berg@...el.com, emmanuel.grumbach@...el.com, kvalo@...eaurora.org, luto@...nel.org, takahiro.akashi@...aro.org, dhowells@...hat.com, pjones@...hat.com, linux-kernel@...r.kernel.org Subject: Re: [PATCH v6 1/5] firmware: add extensible driver data params On Wed, 2017-03-29 at 20:25 -0700, Luis R. Rodriguez wrote: > As the firmware API evolves we keep extending functions with more arguments. > Stop this nonsense by proving an extensible data structure which can be used > to represent both user parameters and private internal parameters. > > We introduce 3 data structures: > > o struct driver_data_req_params - used for user specified parameters > o struct driver_data_priv_params - used for internal use > o struct driver_data_params - stiches both of the the above together, > also only for internal use > > This starts off by just making the existing APIs use the new data > structures, it will make subsequent changes easier to review which will > be adding new flexible APIs. > > This commit should introduces no functional changes (TM). > > Signed-off-by: Luis R. Rodriguez <mcgrof@...nel.org> > --- Looks fine with a few nitpicks. > diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c > index 54fc4c42f126..f702566554e1 100644 > --- a/drivers/base/firmware_class.c > +++ b/drivers/base/firmware_class.c [...] > @@ -40,6 +41,77 @@ MODULE_AUTHOR("Manuel Estrada Sainz"); > MODULE_DESCRIPTION("Multi purpose firmware loading support"); > MODULE_LICENSE("GPL"); > > +struct driver_data_priv_params { > + bool use_fallback; > + bool use_fallback_uevent; > + bool no_cache; > + void *alloc_buf; > + size_t alloc_buf_size; > +}; > + > +struct driver_data_params { > + const struct firmware *driver_data; > + const struct driver_data_req_params req_params; > + struct driver_data_priv_params priv_params; > +}; > + > +/* > + * These are kept to remain backward compatible with old behaviour. Do not > + * modify them unless you know what you are doing. These are to be used only > + * by the old API, so: > + * > + * Old sync APIs: > + * o request_firmware(): __DATA_REQ_FIRMWARE() > + * o request_firmware_direct(): __DATA_REQ_FIRMWARE_DIRECT() > + * o request_firmware_into_buf(): __DATA_REQ_FIRMWARE_BUF() > + * > + * Old async API: > + * o request_firmware_nowait(): __DATA_REQ_FIRMWARE_NOWAIT() > + */ > +#define __DATA_REQ_FIRMWARE() \ > + .priv_params = { \ > + .use_fallback = !!FW_OPT_FALLBACK, \ use_fallback is a boolean, so you don't need the double negation here. [...] > @@ -1332,12 +1433,15 @@ request_firmware_into_buf(const struct firmware **firmware_p, const char *name, > struct device *device, void *buf, size_t size) > { > int ret; > + struct driver_data_params data_params = { > + __DATA_REQ_FIRMWARE_BUF(buf, size), > + }; > > __module_get(THIS_MODULE); > - ret = _request_firmware(firmware_p, name, device, buf, size, > - FW_OPT_UEVENT | FW_OPT_FALLBACK | > - FW_OPT_NOCACHE); > + ret = _request_firmware(firmware_p, name, &data_params, device); > module_put(THIS_MODULE); > + > + Double empty-lines here. > return ret; > } > EXPORT_SYMBOL(request_firmware_into_buf); > [...] > diff --git a/include/linux/driver_data.h b/include/linux/driver_data.h > new file mode 100644 > index 000000000000..c3d3a4129838 > --- /dev/null > +++ b/include/linux/driver_data.h > @@ -0,0 +1,88 @@ > +#ifndef _LINUX_DRIVER_DATA_H > +#define _LINUX_DRIVER_DATA_H > + > +#include <linux/types.h> > +#include <linux/compiler.h> > +#include <linux/gfp.h> > +#include <linux/device.h> > + > +/* > + * Driver Data internals > + * > + * Copyright (C) 2017 Luis R. Rodriguez <mcgrof@...nel.org> > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of copyleft-next (version 0.3.1 or later) as published > + * at http://copyleft-next.org/. > + */ > + > +/** > + * enum driver_data_mode - driver data mode of operation > + * > + * DRIVER_DATA_SYNC: your call to request driver data is synchronous. We will > + * look for the driver data file you have requested immediatley. > + * DRIVER_DATA_ASYNC: your call to request driver data is asynchronous. We will It should be @DRIVER_DATA_SYNC and @DRIVER_DATA_ASYNC here. -- Cheers, Luca.
Powered by blists - more mailing lists