| 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: Wed, 28 Sep 2016 23:36:10 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Mika Westerberg <mika.westerberg@...ux.intel.com>
Cc: "Rafael J. Wysocki" <rjw@...ysocki.net>,
Linus Walleij <linus.walleij@...aro.org>,
Alexandre Courbot <gnurou@...il.com>,
ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/5] ACPI / property: Allow holes in reference properties
On Fri, Sep 23, 2016 at 4:57 PM, Mika Westerberg
<mika.westerberg@...ux.intel.com> wrote:
> DT allows holes or empty phandles for references. This is used for example
> in SPI subsystem where some chip selects are native and others are regular
> GPIOs. In ACPI _DSD we currently do not support this but instead the
> preceding reference "consumes" all following integer arguments.
>
> For example we would like to support something like the below ASL fragment
> for SPI:
>
> Package () {
> "cs-gpios",
> Package () {
> ^GPIO, 19, 0, 0, // GPIO CS0
> 0, // Native CS
> ^GPIO, 20, 0, 0, // GPIO CS1
> }
> }
>
> The zero in the middle means "no entry" or NULL reference. To support this
> we add a new function acpi_node_get_property_reference_fixed_args() that
> takes number of expected arguments as parameter. Function returns -ENOENT
> if the corresponding index resolves to the "no entry" reference.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@...ux.intel.com>
> ---
> drivers/acpi/property.c | 116 +++++++++++++++++++++++++++++++++++-------------
> include/linux/acpi.h | 3 ++
> 2 files changed, 89 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> index f2fd3fee588a..b255c4442a05 100644
> --- a/drivers/acpi/property.c
> +++ b/drivers/acpi/property.c
> @@ -472,6 +472,7 @@ static int acpi_data_get_property_array(struct acpi_device_data *data,
> * @data: ACPI device data object containing the property
> * @propname: Name of the property
> * @index: Index of the reference to return
> + * @num_args: Maximum number of arguments after each reference
> * @args: Location to store the returned reference with optional arguments
> *
> * Find property with @name, verifify that it is a package containing at least
> @@ -485,8 +486,8 @@ static int acpi_data_get_property_array(struct acpi_device_data *data,
> * Return: %0 on success, negative error code on failure.
> */
> static int acpi_data_get_property_reference(struct acpi_device_data *data,
> - const char *propname, size_t index,
> - struct acpi_reference_args *args)
> + const char *propname, size_t index, size_t num_args,
> + struct acpi_reference_args *args)
> {
> const union acpi_object *element, *end;
> const union acpi_object *obj;
> @@ -532,41 +533,52 @@ static int acpi_data_get_property_reference(struct acpi_device_data *data,
> while (element < end) {
> u32 nargs, i;
>
> - if (element->type != ACPI_TYPE_LOCAL_REFERENCE)
> - return -EPROTO;
> -
> - ret = acpi_bus_get_device(element->reference.handle, &device);
> - if (ret)
> - return -ENODEV;
> -
> - element++;
> - nargs = 0;
> -
> - /* assume following integer elements are all args */
> - for (i = 0; element + i < end; i++) {
> - int type = element[i].type;
> + if (element->type == ACPI_TYPE_LOCAL_REFERENCE) {
> + ret = acpi_bus_get_device(element->reference.handle,
> + &device);
> + if (ret)
> + return -ENODEV;
> +
> + nargs = 0;
> + element++;
> +
> + /* assume following integer elements are all args */
> + for (i = 0; element + i < end && i < num_args; i++) {
> + int type = element[i].type;
> +
> + if (type == ACPI_TYPE_INTEGER)
> + nargs++;
> + else if (type == ACPI_TYPE_LOCAL_REFERENCE)
> + break;
> + else
> + return -EPROTO;
> + }
>
> - if (type == ACPI_TYPE_INTEGER)
> - nargs++;
> - else if (type == ACPI_TYPE_LOCAL_REFERENCE)
> - break;
> - else
> + if (nargs > MAX_ACPI_REFERENCE_ARGS)
> return -EPROTO;
> - }
>
> - if (idx++ == index) {
> - args->adev = device;
> - args->nargs = nargs;
> - for (i = 0; i < nargs; i++)
> - args->args[i] = element[i].integer.value;
> + if (idx == index) {
> + args->adev = device;
> + args->nargs = nargs;
> + for (i = 0; i < nargs; i++)
> + args->args[i] = element[i].integer.value;
>
> - return 0;
> + return 0;
> + }
> +
> + element += nargs;
> + } else if (element->type == ACPI_TYPE_INTEGER) {
> + if (idx == index)
> + return -ENOENT;
> + element++;
> + } else {
> + return -EPROTO;
> }
>
> - element += nargs;
> + idx++;
> }
>
> - return -EPROTO;
> + return -ENODATA;
> }
>
> /**
> @@ -582,10 +594,54 @@ int acpi_node_get_property_reference(struct fwnode_handle *fwnode,
> {
> struct acpi_device_data *data = acpi_device_data_of_node(fwnode);
>
> - return data ? acpi_data_get_property_reference(data, name, index, args) : -EINVAL;
> + if (!data)
> + return -EINVAL;
> +
> + return acpi_data_get_property_reference(data, name, index,
> + MAX_ACPI_REFERENCE_ARGS, args);
> }
> EXPORT_SYMBOL_GPL(acpi_node_get_property_reference);
gpiolib-acpi is the only user of this AFAICS, so why don't we change
the definition of acpi_node_get_property_reference() to take one extra
argument and update the caller to pass MAX_ACPI_REFERENCE_ARGS?
We'd avoid having to define the thing below then.
>
> +/**
> + * acpi_node_get_property_reference_fixed_args - get a reference from property
> + * @fwnode: Firmware node to get the property from.
> + * @propname: Name of the property.
> + * @index: Index of the reference to return.
> + * @num_args: Expected number of arguments following each reference.
> + * @args: Location to store the returned reference with optional arguments.
> + *
> + * Using this function it is possible to leave "holes" in the property
> + * value set like in an example below:
> + *
> + * Package () {
> + * "cs-gpios",
> + * Package () {
> + * ^GPIO, 19, 0, 0,
> + * ^GPIO, 20, 0, 0,
> + * 0,
> + * ^GPIO, 21, 0, 0,
> + * }
> + * }
> + *
> + * Calling this function with index %2 return %-ENOENT and with index %3
> + * returns the last entry. If the property does not contain any more values
> + * %-ENODATA is returned. The NULL entry must be single integer and
> + * preferably contain value %0.
> + */
> +int acpi_node_get_property_reference_fixed_args(struct fwnode_handle *fwnode,
> + const char *name, size_t index, size_t num_args,
> + struct acpi_reference_args *args)
> +{
> + struct acpi_device_data *data = acpi_device_data_of_node(fwnode);
> +
> + if (!data)
> + return -EINVAL;
> +
> + return acpi_data_get_property_reference(data, name, index, num_args,
> + args);
> +}
> +EXPORT_SYMBOL_GPL(acpi_node_get_property_reference_fixed_args);
> +
> static int acpi_data_prop_read_single(struct acpi_device_data *data,
> const char *propname,
> enum dev_prop_type proptype, void *val)
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index c5eaf2f80a4c..c5d7c503843f 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -930,6 +930,9 @@ int acpi_dev_get_property(struct acpi_device *adev, const char *name,
> int acpi_node_get_property_reference(struct fwnode_handle *fwnode,
> const char *name, size_t index,
> struct acpi_reference_args *args);
> +int acpi_node_get_property_reference_fixed_args(struct fwnode_handle *fwnode,
> + const char *name, size_t index, size_t num_args,
> + struct acpi_reference_args *args);
>
> int acpi_node_prop_get(struct fwnode_handle *fwnode, const char *propname,
> void **valptr);
> --
Thanks,
Rafael
Powered by blists - more mailing lists