[PATCH v4 0/8] [PATCH v3 0/8] ACPI: Buffer property and reference as string support

[PATCH v4 0/8] [PATCH v3 0/8] ACPI: Buffer property and reference as string support

[Sakari Ailus]

Hello everyone,

This set adds support for _DSD buffer properties (specified by DSD Guide
<URL:https://github.com/UEFI/DSD-Guide/blob/main/dsd-guide.md>) as well as
support for references as strings. Reference property type was previously
supported for device objects only, whereas string references enable
referencing also _DSD sub-node objects --- also included in the set.

The ACPICA patch has been submitted to upstream but not merged yet.

This set currently prepares for data node string reference support and
does not add it anymore.

since v3:

- Use bool return type for acpi_tie_nondev_subnodes. Return false on
  failure.

- Print ACPI buffer parsing error references as %*pE (was %s).

- Unwrap a few lines (i.e. non-functional changes).

- Declare local variables for acpi_copy_property_array_uint().

- Clean up buffer size check in acpi_data_prop_read(), add break at the
  end of the default case.

since v2:

- Use C99 _Generic() in patch unifying reading integer arrays.

since v1:

- Drop the ACPICA, data node child list initialisation and data node
  string reference patches.

Sakari Ailus (8):
  ACPI: property: Return type of acpi_add_nondev_subnodes() should be
    bool
  ACPI: property: Tie data nodes to acpi handles
  ACPI: property: Use acpi_object_type consistently in property ref
    parsing
  ACPI: property: Move property ref argument parsing into a new function
  ACPI: property: Switch node property referencing from ifs to a switch
  ACPI: property: Unify integer value reading functions
  ACPI: property: Add support for parsing buffer property UUID
  ACPI: property: Read buffer properties as integers

 drivers/acpi/property.c | 465 +++++++++++++++++++++++++++-------------
 include/acpi/acpi_bus.h |   3 +-
 include/linux/acpi.h    |   2 +-
 3 files changed, 318 insertions(+), 152 deletions(-)

-- 
2.30.2

[PATCH v4 1/8] ACPI: property: Return type of acpi_add_nondev_subnodes() should be bool

[Sakari Ailus]

The value acpi_add_nondev_subnodes() returns is bool so change the return
type of the function to match that.

Fixes: 445b0eb058f5 ("ACPI / property: Add support for data-only subnodes")
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/acpi/property.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index e764f9ac9cf8f..6d698f3f83a0a 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -155,10 +155,10 @@ static bool acpi_nondev_subnode_ok(acpi_handle scope,
 	return acpi_nondev_subnode_data_ok(handle, link, list, parent);
 }
 
-static int acpi_add_nondev_subnodes(acpi_handle scope,
-				    const union acpi_object *links,
-				    struct list_head *list,
-				    struct fwnode_handle *parent)
+static bool acpi_add_nondev_subnodes(acpi_handle scope,
+				     const union acpi_object *links,
+				     struct list_head *list,
+				     struct fwnode_handle *parent)
 {
 	bool ret = false;
 	int i;
-- 
2.30.2

[PATCH v4 2/8] ACPI: property: Tie data nodes to acpi handles

[Sakari Ailus]

ACPICA allows associating additional information (i.e. pointers with
specific tag) to acpi_handles. The acpi_device's are associated to
acpi_handle's in acpi_tie_acpi_dev() in scan.c, do the same here for the
_DSD data nodes.

This allows direct data node references in properties, implemented later on
in the series.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c | 42 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 6d698f3f83a0a..08b5292d2f89b 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -340,6 +340,43 @@ acpi_data_add_props(struct acpi_device_data *data, const guid_t *guid,
 	return props;
 }
 
+static void acpi_nondev_subnode_tag(acpi_handle handle, void *context)
+{
+}
+
+static void acpi_untie_nondev_subnodes(struct acpi_device_data *data)
+{
+	struct acpi_data_node *dn;
+
+	list_for_each_entry(dn, &data->subnodes, sibling) {
+		acpi_detach_data(dn->handle, acpi_nondev_subnode_tag);
+
+		acpi_untie_nondev_subnodes(&dn->data);
+	}
+}
+
+static bool acpi_tie_nondev_subnodes(struct acpi_device_data *data)
+{
+	struct acpi_data_node *dn;
+
+	list_for_each_entry(dn, &data->subnodes, sibling) {
+		acpi_status status;
+		bool ret;
+
+		status = acpi_attach_data(dn->handle, acpi_nondev_subnode_tag, dn);
+		if (ACPI_FAILURE(status)) {
+			acpi_handle_err(dn->handle, "Can't tag data node\n");
+			return false;
+		}
+
+		ret = acpi_tie_nondev_subnodes(&dn->data);
+		if (!ret)
+			return ret;
+	}
+
+	return true;
+}
+
 static bool acpi_extract_properties(const union acpi_object *desc,
 				    struct acpi_device_data *data)
 {
@@ -419,7 +456,9 @@ void acpi_init_properties(struct acpi_device *adev)
 					&adev->data, acpi_fwnode_handle(adev)))
 		adev->data.pointer = buf.pointer;
 
-	if (!adev->data.pointer) {
+	if (!adev->data.pointer ||
+	    !acpi_tie_nondev_subnodes(&adev->data)) {
+		acpi_untie_nondev_subnodes(&adev->data);
 		acpi_handle_debug(adev->handle, "Invalid _DSD data, skipping\n");
 		ACPI_FREE(buf.pointer);
 	}
@@ -462,6 +501,7 @@ static void acpi_destroy_nondev_subnodes(struct list_head *list)
 
 void acpi_free_properties(struct acpi_device *adev)
 {
+	acpi_untie_nondev_subnodes(&adev->data);
 	acpi_destroy_nondev_subnodes(&adev->data.subnodes);
 	ACPI_FREE((void *)adev->data.pointer);
 	adev->data.of_compatible = NULL;
-- 
2.30.2

[PATCH v4 3/8] ACPI: property: Use acpi_object_type consistently in property ref parsing

[Sakari Ailus]

The type of union acpi_object field type is acpi_object_type. Use that
instead of int.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/acpi/property.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 08b5292d2f89b..8d757ca2a58f3 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -793,7 +793,7 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
 			 * nor integer, return an error, we can't parse it.
 			 */
 			for (i = 0; element + i < end && i < num_args; i++) {
-				int type = element[i].type;
+				acpi_object_type type = element[i].type;
 
 				if (type == ACPI_TYPE_LOCAL_REFERENCE)
 					break;
-- 
2.30.2

[PATCH v4 4/8] ACPI: property: Move property ref argument parsing into a new function

[Sakari Ailus]

Split out property reference argument parsing out of the
__acpi_node_get_property_reference() function into a new one,
acpi_get_ref_args(). The new function will be needed also for parsing
string references soon.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c | 103 ++++++++++++++++++++++------------------
 1 file changed, 58 insertions(+), 45 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 8d757ca2a58f3..a2733f8981c7f 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -673,6 +673,58 @@ acpi_fwnode_get_named_child_node(const struct fwnode_handle *fwnode,
 	return NULL;
 }
 
+static int acpi_get_ref_args(struct fwnode_reference_args *args,
+			     struct fwnode_handle *ref_fwnode,
+			     const union acpi_object **element,
+			     const union acpi_object *end, size_t num_args)
+{
+	u32 nargs = 0, i;
+
+	/*
+	 * Find the referred data extension node under the
+	 * referred device node.
+	 */
+	for (; *element < end && (*element)->type == ACPI_TYPE_STRING;
+	     (*element)++) {
+		const char *child_name = (*element)->string.pointer;
+
+		ref_fwnode = acpi_fwnode_get_named_child_node(ref_fwnode, child_name);
+		if (!ref_fwnode)
+			return -EINVAL;
+	}
+
+	/*
+	 * Assume the following integer elements are all args. Stop counting on
+	 * the first reference or end of the package arguments. In case of
+	 * neither reference, nor integer, return an error, we can't parse it.
+	 */
+	for (i = 0; (*element) + i < end && i < num_args; i++) {
+		acpi_object_type type = (*element)[i].type;
+
+		if (type == ACPI_TYPE_LOCAL_REFERENCE)
+			break;
+
+		if (type == ACPI_TYPE_INTEGER)
+			nargs++;
+		else
+			return -EINVAL;
+	}
+
+	if (nargs > NR_FWNODE_REFERENCE_ARGS)
+		return -EINVAL;
+
+	if (args) {
+		args->fwnode = ref_fwnode;
+		args->nargs = nargs;
+		for (i = 0; i < nargs; i++)
+			args->args[i] = (*element)[i].integer.value;
+	}
+
+	(*element) += nargs;
+
+	return 0;
+}
+
 /**
  * __acpi_node_get_property_reference - returns handle to the referenced object
  * @fwnode: Firmware node to get the property from
@@ -761,61 +813,22 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
 	end = element + obj->package.count;
 
 	while (element < end) {
-		u32 nargs, i;
-
 		if (element->type == ACPI_TYPE_LOCAL_REFERENCE) {
-			struct fwnode_handle *ref_fwnode;
-
 			device = acpi_fetch_acpi_dev(element->reference.handle);
 			if (!device)
 				return -EINVAL;
 
-			nargs = 0;
 			element++;
 
-			/*
-			 * Find the referred data extension node under the
-			 * referred device node.
-			 */
-			for (ref_fwnode = acpi_fwnode_handle(device);
-			     element < end && element->type == ACPI_TYPE_STRING;
-			     element++) {
-				ref_fwnode = acpi_fwnode_get_named_child_node(
-					ref_fwnode, element->string.pointer);
-				if (!ref_fwnode)
-					return -EINVAL;
-			}
-
-			/*
-			 * Assume the following integer elements are all args.
-			 * Stop counting on the first reference or end of the
-			 * package arguments. In case of neither reference,
-			 * nor integer, return an error, we can't parse it.
-			 */
-			for (i = 0; element + i < end && i < num_args; i++) {
-				acpi_object_type type = element[i].type;
-
-				if (type == ACPI_TYPE_LOCAL_REFERENCE)
-					break;
-				if (type == ACPI_TYPE_INTEGER)
-					nargs++;
-				else
-					return -EINVAL;
-			}
-
-			if (nargs > NR_FWNODE_REFERENCE_ARGS)
-				return -EINVAL;
-
-			if (idx == index) {
-				args->fwnode = ref_fwnode;
-				args->nargs = nargs;
-				for (i = 0; i < nargs; i++)
-					args->args[i] = element[i].integer.value;
+			ret = acpi_get_ref_args(idx == index ? args : NULL,
+						acpi_fwnode_handle(device),
+						&element, end, num_args);
+			if (ret < 0)
+				return ret;
 
+			if (idx == index)
 				return 0;
-			}
 
-			element += nargs;
 		} else if (element->type == ACPI_TYPE_INTEGER) {
 			if (idx == index)
 				return -ENOENT;
-- 
2.30.2

[PATCH v4 5/8] ACPI: property: Switch node property referencing from ifs to a switch

[Sakari Ailus]

__acpi_node_get_property_reference() uses a series of if () statements for
testing the same variable. There's soon going to be one more value to be
tested.

Switch to use switch() instead.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/acpi/property.c | 41 ++++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index a2733f8981c7f..3411cd0433b88 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -778,11 +778,9 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
 	if (ret)
 		return ret == -EINVAL ? -ENOENT : -EINVAL;
 
-	/*
-	 * The simplest case is when the value is a single reference.  Just
-	 * return that reference then.
-	 */
-	if (obj->type == ACPI_TYPE_LOCAL_REFERENCE) {
+	switch (obj->type) {
+	case ACPI_TYPE_LOCAL_REFERENCE:
+		/* Plain single reference without arguments. */
 		if (index)
 			return -ENOENT;
 
@@ -793,19 +791,21 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
 		args->fwnode = acpi_fwnode_handle(device);
 		args->nargs = 0;
 		return 0;
+	case ACPI_TYPE_PACKAGE:
+		/*
+		 * If it is not a single reference, then it is a package of
+		 * references followed by number of ints as follows:
+		 *
+		 *  Package () { REF, INT, REF, INT, INT }
+		 *
+		 * The index argument is then used to determine which reference
+		 * the caller wants (along with the arguments).
+		 */
+		break;
+	default:
+		return -EINVAL;
 	}
 
-	/*
-	 * If it is not a single reference, then it is a package of
-	 * references followed by number of ints as follows:
-	 *
-	 *  Package () { REF, INT, REF, INT, INT }
-	 *
-	 * The index argument is then used to determine which reference
-	 * the caller wants (along with the arguments).
-	 */
-	if (obj->type != ACPI_TYPE_PACKAGE)
-		return -EINVAL;
 	if (index >= obj->package.count)
 		return -ENOENT;
 
@@ -813,7 +813,8 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
 	end = element + obj->package.count;
 
 	while (element < end) {
-		if (element->type == ACPI_TYPE_LOCAL_REFERENCE) {
+		switch (element->type) {
+		case ACPI_TYPE_LOCAL_REFERENCE:
 			device = acpi_fetch_acpi_dev(element->reference.handle);
 			if (!device)
 				return -EINVAL;
@@ -829,11 +830,13 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
 			if (idx == index)
 				return 0;
 
-		} else if (element->type == ACPI_TYPE_INTEGER) {
+			break;
+		case ACPI_TYPE_INTEGER:
 			if (idx == index)
 				return -ENOENT;
 			element++;
-		} else {
+			break;
+		default:
 			return -EINVAL;
 		}
 
-- 
2.30.2

[PATCH v4 6/8] ACPI: property: Unify integer value reading functions

[Sakari Ailus]

Unify functions reading ACPI property integer values into a single macro
using C99 _Generic().

Also use size_t for the counter instead of int.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c | 96 +++++++++++++----------------------------
 1 file changed, 31 insertions(+), 65 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 3411cd0433b88..236a847f1bfbd 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -908,67 +908,33 @@ static int acpi_data_prop_read_single(const struct acpi_device_data *data,
 	return ret;
 }
 
-static int acpi_copy_property_array_u8(const union acpi_object *items, u8 *val,
-				       size_t nval)
-{
-	int i;
-
-	for (i = 0; i < nval; i++) {
-		if (items[i].type != ACPI_TYPE_INTEGER)
-			return -EPROTO;
-		if (items[i].integer.value > U8_MAX)
-			return -EOVERFLOW;
-
-		val[i] = items[i].integer.value;
-	}
-	return 0;
-}
-
-static int acpi_copy_property_array_u16(const union acpi_object *items,
-					u16 *val, size_t nval)
-{
-	int i;
-
-	for (i = 0; i < nval; i++) {
-		if (items[i].type != ACPI_TYPE_INTEGER)
-			return -EPROTO;
-		if (items[i].integer.value > U16_MAX)
-			return -EOVERFLOW;
-
-		val[i] = items[i].integer.value;
-	}
-	return 0;
-}
-
-static int acpi_copy_property_array_u32(const union acpi_object *items,
-					u32 *val, size_t nval)
-{
-	int i;
-
-	for (i = 0; i < nval; i++) {
-		if (items[i].type != ACPI_TYPE_INTEGER)
-			return -EPROTO;
-		if (items[i].integer.value > U32_MAX)
-			return -EOVERFLOW;
-
-		val[i] = items[i].integer.value;
-	}
-	return 0;
-}
-
-static int acpi_copy_property_array_u64(const union acpi_object *items,
-					u64 *val, size_t nval)
-{
-	int i;
-
-	for (i = 0; i < nval; i++) {
-		if (items[i].type != ACPI_TYPE_INTEGER)
-			return -EPROTO;
-
-		val[i] = items[i].integer.value;
-	}
-	return 0;
-}
+#define acpi_copy_property_array_uint(items, val, nval)			\
+	({								\
+		typeof(items) __items = items;				\
+		typeof(val) __val = val;				\
+		typeof(nval) __nval = nval;				\
+		size_t i;						\
+		int ret = 0;						\
+									\
+		for (i = 0; i < __nval; i++) {				\
+			if (__items[i].type != ACPI_TYPE_INTEGER) {	\
+				ret = -EPROTO;				\
+				break;					\
+			}						\
+			if (__items[i].integer.value > _Generic(__val,	\
+								u8: U8_MAX, \
+								u16: U16_MAX, \
+								u32: U32_MAX, \
+								u64: U64_MAX, \
+								default: 0U)) { \
+				ret = -EOVERFLOW;			\
+				break;					\
+			}						\
+									\
+			__val[i] = __items[i].integer.value;		\
+		}							\
+		ret;							\
+	})
 
 static int acpi_copy_property_array_string(const union acpi_object *items,
 					   char **val, size_t nval)
@@ -1025,16 +991,16 @@ static int acpi_data_prop_read(const struct acpi_device_data *data,
 
 	switch (proptype) {
 	case DEV_PROP_U8:
-		ret = acpi_copy_property_array_u8(items, (u8 *)val, nval);
+		ret = acpi_copy_property_array_uint(items, (u8 *)val, nval);
 		break;
 	case DEV_PROP_U16:
-		ret = acpi_copy_property_array_u16(items, (u16 *)val, nval);
+		ret = acpi_copy_property_array_uint(items, (u16 *)val, nval);
 		break;
 	case DEV_PROP_U32:
-		ret = acpi_copy_property_array_u32(items, (u32 *)val, nval);
+		ret = acpi_copy_property_array_uint(items, (u32 *)val, nval);
 		break;
 	case DEV_PROP_U64:
-		ret = acpi_copy_property_array_u64(items, (u64 *)val, nval);
+		ret = acpi_copy_property_array_uint(items, (u64 *)val, nval);
 		break;
 	case DEV_PROP_STRING:
 		ret = acpi_copy_property_array_string(
-- 
2.30.2

[PATCH v4 7/8] ACPI: property: Add support for parsing buffer property UUID

[Sakari Ailus]

Add support for newly added buffer property UUID, as defined in the DSD
guide.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c | 142 ++++++++++++++++++++++++++++++++++++----
 include/acpi/acpi_bus.h |   3 +-
 include/linux/acpi.h    |   2 +-
 3 files changed, 132 insertions(+), 15 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 236a847f1bfbd..7621f684212ff 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -55,14 +55,19 @@ static const guid_t ads_guid =
 	GUID_INIT(0xdbb8e3e6, 0x5886, 0x4ba6,
 		  0x87, 0x95, 0x13, 0x19, 0xf5, 0x2a, 0x96, 0x6b);
 
+static const guid_t buffer_prop_guid =
+	GUID_INIT(0xedb12dd0, 0x363d, 0x4085,
+		  0xa3, 0xd2, 0x49, 0x52, 0x2c, 0xa1, 0x60, 0xc4);
+
 static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
-					   const union acpi_object *desc,
+					   union acpi_object *desc,
 					   struct acpi_device_data *data,
 					   struct fwnode_handle *parent);
-static bool acpi_extract_properties(const union acpi_object *desc,
+static bool acpi_extract_properties(acpi_handle handle,
+				    union acpi_object *desc,
 				    struct acpi_device_data *data);
 
-static bool acpi_nondev_subnode_extract(const union acpi_object *desc,
+static bool acpi_nondev_subnode_extract(union acpi_object *desc,
 					acpi_handle handle,
 					const union acpi_object *link,
 					struct list_head *list,
@@ -81,7 +86,7 @@ static bool acpi_nondev_subnode_extract(const union acpi_object *desc,
 	INIT_LIST_HEAD(&dn->data.properties);
 	INIT_LIST_HEAD(&dn->data.subnodes);
 
-	result = acpi_extract_properties(desc, &dn->data);
+	result = acpi_extract_properties(handle, desc, &dn->data);
 
 	if (handle) {
 		acpi_handle scope;
@@ -156,7 +161,7 @@ static bool acpi_nondev_subnode_ok(acpi_handle scope,
 }
 
 static bool acpi_add_nondev_subnodes(acpi_handle scope,
-				     const union acpi_object *links,
+				     union acpi_object *links,
 				     struct list_head *list,
 				     struct fwnode_handle *parent)
 {
@@ -164,7 +169,7 @@ static bool acpi_add_nondev_subnodes(acpi_handle scope,
 	int i;
 
 	for (i = 0; i < links->package.count; i++) {
-		const union acpi_object *link, *desc;
+		union acpi_object *link, *desc;
 		acpi_handle handle;
 		bool result;
 
@@ -204,7 +209,7 @@ static bool acpi_add_nondev_subnodes(acpi_handle scope,
 }
 
 static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
-					   const union acpi_object *desc,
+					   union acpi_object *desc,
 					   struct acpi_device_data *data,
 					   struct fwnode_handle *parent)
 {
@@ -212,7 +217,8 @@ static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
 
 	/* Look for the ACPI data subnodes GUID. */
 	for (i = 0; i < desc->package.count; i += 2) {
-		const union acpi_object *guid, *links;
+		const union acpi_object *guid;
+		union acpi_object *links;
 
 		guid = &desc->package.elements[i];
 		links = &desc->package.elements[i + 1];
@@ -325,7 +331,7 @@ static bool acpi_is_property_guid(const guid_t *guid)
 
 struct acpi_device_properties *
 acpi_data_add_props(struct acpi_device_data *data, const guid_t *guid,
-		    const union acpi_object *properties)
+		    union acpi_object *properties)
 {
 	struct acpi_device_properties *props;
 
@@ -377,7 +383,104 @@ static bool acpi_tie_nondev_subnodes(struct acpi_device_data *data)
 	return true;
 }
 
-static bool acpi_extract_properties(const union acpi_object *desc,
+static void acpi_data_add_buffer_props(acpi_handle handle,
+				       struct acpi_device_data *data,
+				       union acpi_object *properties)
+{
+	struct acpi_device_properties *props;
+	union acpi_object *package;
+	size_t alloc_size;
+	unsigned int i;
+	u32 *count;
+
+	if (check_mul_overflow((size_t)properties->package.count,
+			       sizeof(*package) + sizeof(void *),
+			       &alloc_size) ||
+	    check_add_overflow(sizeof(*props) + sizeof(*package), alloc_size,
+			       &alloc_size)) {
+		acpi_handle_warn(handle,
+				 "can't allocate memory for %u buffer props",
+				 properties->package.count);
+		return;
+	}
+
+	props = kvzalloc(alloc_size, GFP_KERNEL);
+	if (!props)
+		return;
+
+	props->guid = &buffer_prop_guid;
+	props->bufs = (void *)(props + 1);
+	props->properties = (void *)(props->bufs + properties->package.count);
+
+	/* Outer package */
+	package = props->properties;
+	package->type = ACPI_TYPE_PACKAGE;
+	package->package.elements = package + 1;
+	count = &package->package.count;
+	*count = 0;
+
+	/* Inner packages */
+	package++;
+
+	for (i = 0; i < properties->package.count; i++) {
+		struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
+		union acpi_object *property = &properties->package.elements[i];
+		union acpi_object *prop, *obj, *buf_obj;
+		acpi_status status;
+
+		if (property->type != ACPI_TYPE_PACKAGE ||
+		    property->package.count != 2) {
+			acpi_handle_warn(handle,
+					 "buffer property %u has %u entries\n",
+					 i, property->package.count);
+			continue;
+		}
+
+		prop = &property->package.elements[0];
+		obj = &property->package.elements[1];
+
+		if (prop->type != ACPI_TYPE_STRING ||
+		    obj->type != ACPI_TYPE_STRING) {
+			acpi_handle_warn(handle,
+					 "wrong object types %u and %u\n",
+					 prop->type, obj->type);
+			continue;
+		}
+
+		status = acpi_evaluate_object_typed(handle, obj->string.pointer,
+						    NULL, &buf,
+						    ACPI_TYPE_BUFFER);
+		if (ACPI_FAILURE(status)) {
+			acpi_handle_warn(handle,
+					 "can't evaluate \"%*pE\" as buffer\n",
+					 obj->string.length,
+					 obj->string.pointer);
+			continue;
+		}
+
+		package->type = ACPI_TYPE_PACKAGE;
+		package->package.elements = prop;
+		package->package.count = 2;
+
+		buf_obj = buf.pointer;
+
+		/* Replace the string object with a buffer object */
+		obj->type = ACPI_TYPE_BUFFER;
+		obj->buffer.length = buf_obj->buffer.length;
+		obj->buffer.pointer = buf_obj->buffer.pointer;
+
+		props->bufs[i] = buf.pointer;
+		package++;
+		(*count)++;
+	}
+
+	if (*count)
+		list_add(&props->list, &data->properties);
+	else
+		kvfree(props);
+}
+
+static bool acpi_extract_properties(acpi_handle scope, union acpi_object *desc,
 				    struct acpi_device_data *data)
 {
 	int i;
@@ -387,7 +490,8 @@ static bool acpi_extract_properties(const union acpi_object *desc,
 
 	/* Look for the device properties GUID. */
 	for (i = 0; i < desc->package.count; i += 2) {
-		const union acpi_object *guid, *properties;
+		const union acpi_object *guid;
+		union acpi_object *properties;
 
 		guid = &desc->package.elements[i];
 		properties = &desc->package.elements[i + 1];
@@ -401,6 +505,12 @@ static bool acpi_extract_properties(const union acpi_object *desc,
 		    properties->type != ACPI_TYPE_PACKAGE)
 			break;
 
+		if (guid_equal((guid_t *)guid->buffer.pointer,
+			       &buffer_prop_guid)) {
+			acpi_data_add_buffer_props(scope, data, properties);
+			continue;
+		}
+
 		if (!acpi_is_property_guid((guid_t *)guid->buffer.pointer))
 			continue;
 
@@ -447,7 +557,7 @@ void acpi_init_properties(struct acpi_device *adev)
 	if (ACPI_FAILURE(status))
 		goto out;
 
-	if (acpi_extract_properties(buf.pointer, &adev->data)) {
+	if (acpi_extract_properties(adev->handle, buf.pointer, &adev->data)) {
 		adev->data.pointer = buf.pointer;
 		if (acpi_of)
 			acpi_init_of_compatible(adev);
@@ -477,8 +587,14 @@ static void acpi_free_device_properties(struct list_head *list)
 	struct acpi_device_properties *props, *tmp;
 
 	list_for_each_entry_safe(props, tmp, list, list) {
+		u32 i;
+
 		list_del(&props->list);
-		kfree(props);
+		/* Buffer data properties were separately allocated */
+		if (props->bufs)
+			for (i = 0; i < props->properties->package.count; i++)
+				ACPI_FREE(props->bufs[i]);
+		kvfree(props);
 	}
 }
 
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 54c5566df9fe1..88a17fce49fd0 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -344,8 +344,9 @@ struct acpi_device_physical_node {
 
 struct acpi_device_properties {
 	const guid_t *guid;
-	const union acpi_object *properties;
+	union acpi_object *properties;
 	struct list_head list;
+	void **bufs;
 };
 
 /* ACPI Device Specific Data (_DSD) */
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 4f82a5bc6d987..93a695cab9d8a 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1243,7 +1243,7 @@ static inline bool acpi_dev_has_props(const struct acpi_device *adev)
 
 struct acpi_device_properties *
 acpi_data_add_props(struct acpi_device_data *data, const guid_t *guid,
-		    const union acpi_object *properties);
+		    union acpi_object *properties);
 
 int acpi_node_prop_get(const struct fwnode_handle *fwnode, const char *propname,
 		       void **valptr);
-- 
2.30.2

[PATCH v4 8/8] ACPI: property: Read buffer properties as integers

[Sakari Ailus]

Instead of adding a new property type, read buffer properties as integers.
Even though the internal representation in ACPI is different, the data
type is the same (byte) than on 8-bit integers.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 7621f684212ff..9711482014a60 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -1033,6 +1033,10 @@ static int acpi_data_prop_read_single(const struct acpi_device_data *data,
 		int ret = 0;						\
 									\
 		for (i = 0; i < __nval; i++) {				\
+			if (__items->type == ACPI_TYPE_BUFFER) {	\
+				__val[i] = __items->buffer.pointer[i];	\
+				continue;				\
+			}						\
 			if (__items[i].type != ACPI_TYPE_INTEGER) {	\
 				ret = -EPROTO;				\
 				break;					\
@@ -1092,18 +1096,41 @@ static int acpi_data_prop_read(const struct acpi_device_data *data,
 	}
 
 	ret = acpi_data_get_property_array(data, propname, ACPI_TYPE_ANY, &obj);
+	if (ret && proptype >= DEV_PROP_U8 && proptype <= DEV_PROP_U64)
+		ret = acpi_data_get_property(data, propname, ACPI_TYPE_BUFFER,
+					     &obj);
 	if (ret)
 		return ret;
 
-	if (!val)
+	if (!val) {
+		if (obj->type == ACPI_TYPE_BUFFER)
+			return obj->buffer.length;
+
 		return obj->package.count;
+	}
 
-	if (proptype != DEV_PROP_STRING && nval > obj->package.count)
-		return -EOVERFLOW;
+	switch (proptype) {
+	case DEV_PROP_STRING:
+		break;
+	case DEV_PROP_U8 ... DEV_PROP_U64:
+		if (obj->type == ACPI_TYPE_BUFFER) {
+			if (nval > obj->buffer.length)
+				return -EOVERFLOW;
+			break;
+		}
+		fallthrough;
+	default:
+		if (nval > obj->package.count)
+			return -EOVERFLOW;
+		break;
+	}
 	if (nval == 0)
 		return -EINVAL;
 
-	items = obj->package.elements;
+	if (obj->type != ACPI_TYPE_BUFFER)
+		items = obj->package.elements;
+	else
+		items = obj;
 
 	switch (proptype) {
 	case DEV_PROP_U8:
-- 
2.30.2

Re: [PATCH v4 7/8] ACPI: property: Add support for parsing buffer property UUID

[Rafael J. Wysocki]

On Mon, Jul 11, 2022 at 1:24 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Add support for newly added buffer property UUID, as defined in the DSD
> guide.

A link to the DSD guide and a more specific reference to the relevant
part of it (eg. section number) would be handy.

> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/acpi/property.c | 142 ++++++++++++++++++++++++++++++++++++----
>  include/acpi/acpi_bus.h |   3 +-
>  include/linux/acpi.h    |   2 +-
>  3 files changed, 132 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> index 236a847f1bfbd..7621f684212ff 100644
> --- a/drivers/acpi/property.c
> +++ b/drivers/acpi/property.c
> @@ -55,14 +55,19 @@ static const guid_t ads_guid =
>         GUID_INIT(0xdbb8e3e6, 0x5886, 0x4ba6,
>                   0x87, 0x95, 0x13, 0x19, 0xf5, 0x2a, 0x96, 0x6b);
>
> +static const guid_t buffer_prop_guid =
> +       GUID_INIT(0xedb12dd0, 0x363d, 0x4085,
> +                 0xa3, 0xd2, 0x49, 0x52, 0x2c, 0xa1, 0x60, 0xc4);
> +
>  static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
> -                                          const union acpi_object *desc,
> +                                          union acpi_object *desc,
>                                            struct acpi_device_data *data,
>                                            struct fwnode_handle *parent);
> -static bool acpi_extract_properties(const union acpi_object *desc,
> +static bool acpi_extract_properties(acpi_handle handle,
> +                                   union acpi_object *desc,
>                                     struct acpi_device_data *data);
>
> -static bool acpi_nondev_subnode_extract(const union acpi_object *desc,
> +static bool acpi_nondev_subnode_extract(union acpi_object *desc,
>                                         acpi_handle handle,
>                                         const union acpi_object *link,
>                                         struct list_head *list,
> @@ -81,7 +86,7 @@ static bool acpi_nondev_subnode_extract(const union acpi_object *desc,
>         INIT_LIST_HEAD(&dn->data.properties);
>         INIT_LIST_HEAD(&dn->data.subnodes);
>
> -       result = acpi_extract_properties(desc, &dn->data);
> +       result = acpi_extract_properties(handle, desc, &dn->data);
>
>         if (handle) {
>                 acpi_handle scope;
> @@ -156,7 +161,7 @@ static bool acpi_nondev_subnode_ok(acpi_handle scope,
>  }
>
>  static bool acpi_add_nondev_subnodes(acpi_handle scope,
> -                                    const union acpi_object *links,
> +                                    union acpi_object *links,
>                                      struct list_head *list,
>                                      struct fwnode_handle *parent)
>  {
> @@ -164,7 +169,7 @@ static bool acpi_add_nondev_subnodes(acpi_handle scope,
>         int i;
>
>         for (i = 0; i < links->package.count; i++) {
> -               const union acpi_object *link, *desc;
> +               union acpi_object *link, *desc;
>                 acpi_handle handle;
>                 bool result;
>
> @@ -204,7 +209,7 @@ static bool acpi_add_nondev_subnodes(acpi_handle scope,
>  }
>
>  static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
> -                                          const union acpi_object *desc,
> +                                          union acpi_object *desc,
>                                            struct acpi_device_data *data,
>                                            struct fwnode_handle *parent)
>  {
> @@ -212,7 +217,8 @@ static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
>
>         /* Look for the ACPI data subnodes GUID. */
>         for (i = 0; i < desc->package.count; i += 2) {
> -               const union acpi_object *guid, *links;
> +               const union acpi_object *guid;
> +               union acpi_object *links;
>
>                 guid = &desc->package.elements[i];
>                 links = &desc->package.elements[i + 1];
> @@ -325,7 +331,7 @@ static bool acpi_is_property_guid(const guid_t *guid)
>
>  struct acpi_device_properties *
>  acpi_data_add_props(struct acpi_device_data *data, const guid_t *guid,
> -                   const union acpi_object *properties)
> +                   union acpi_object *properties)
>  {
>         struct acpi_device_properties *props;
>
> @@ -377,7 +383,104 @@ static bool acpi_tie_nondev_subnodes(struct acpi_device_data *data)
>         return true;
>  }
>
> -static bool acpi_extract_properties(const union acpi_object *desc,
> +static void acpi_data_add_buffer_props(acpi_handle handle,
> +                                      struct acpi_device_data *data,
> +                                      union acpi_object *properties)
> +{
> +       struct acpi_device_properties *props;
> +       union acpi_object *package;
> +       size_t alloc_size;
> +       unsigned int i;
> +       u32 *count;
> +
> +       if (check_mul_overflow((size_t)properties->package.count,
> +                              sizeof(*package) + sizeof(void *),
> +                              &alloc_size) ||
> +           check_add_overflow(sizeof(*props) + sizeof(*package), alloc_size,
> +                              &alloc_size)) {
> +               acpi_handle_warn(handle,
> +                                "can't allocate memory for %u buffer props",
> +                                properties->package.count);
> +               return;
> +       }
> +
> +       props = kvzalloc(alloc_size, GFP_KERNEL);
> +       if (!props)
> +               return;
> +
> +       props->guid = &buffer_prop_guid;
> +       props->bufs = (void *)(props + 1);
> +       props->properties = (void *)(props->bufs + properties->package.count);
> +
> +       /* Outer package */
> +       package = props->properties;
> +       package->type = ACPI_TYPE_PACKAGE;
> +       package->package.elements = package + 1;
> +       count = &package->package.count;
> +       *count = 0;
> +
> +       /* Inner packages */
> +       package++;
> +
> +       for (i = 0; i < properties->package.count; i++) {
> +               struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
> +               union acpi_object *property = &properties->package.elements[i];
> +               union acpi_object *prop, *obj, *buf_obj;
> +               acpi_status status;
> +
> +               if (property->type != ACPI_TYPE_PACKAGE ||
> +                   property->package.count != 2) {
> +                       acpi_handle_warn(handle,
> +                                        "buffer property %u has %u entries\n",
> +                                        i, property->package.count);
> +                       continue;
> +               }
> +
> +               prop = &property->package.elements[0];
> +               obj = &property->package.elements[1];
> +
> +               if (prop->type != ACPI_TYPE_STRING ||
> +                   obj->type != ACPI_TYPE_STRING) {
> +                       acpi_handle_warn(handle,
> +                                        "wrong object types %u and %u\n",
> +                                        prop->type, obj->type);
> +                       continue;
> +               }
> +
> +               status = acpi_evaluate_object_typed(handle, obj->string.pointer,
> +                                                   NULL, &buf,
> +                                                   ACPI_TYPE_BUFFER);
> +               if (ACPI_FAILURE(status)) {
> +                       acpi_handle_warn(handle,
> +                                        "can't evaluate \"%*pE\" as buffer\n",
> +                                        obj->string.length,
> +                                        obj->string.pointer);
> +                       continue;
> +               }
> +
> +               package->type = ACPI_TYPE_PACKAGE;
> +               package->package.elements = prop;
> +               package->package.count = 2;
> +
> +               buf_obj = buf.pointer;
> +
> +               /* Replace the string object with a buffer object */
> +               obj->type = ACPI_TYPE_BUFFER;
> +               obj->buffer.length = buf_obj->buffer.length;
> +               obj->buffer.pointer = buf_obj->buffer.pointer;
> +
> +               props->bufs[i] = buf.pointer;
> +               package++;
> +               (*count)++;
> +       }
> +
> +       if (*count)
> +               list_add(&props->list, &data->properties);
> +       else
> +               kvfree(props);
> +}
> +
> +static bool acpi_extract_properties(acpi_handle scope, union acpi_object *desc,
>                                     struct acpi_device_data *data)
>  {
>         int i;
> @@ -387,7 +490,8 @@ static bool acpi_extract_properties(const union acpi_object *desc,
>
>         /* Look for the device properties GUID. */
>         for (i = 0; i < desc->package.count; i += 2) {
> -               const union acpi_object *guid, *properties;
> +               const union acpi_object *guid;
> +               union acpi_object *properties;
>
>                 guid = &desc->package.elements[i];
>                 properties = &desc->package.elements[i + 1];
> @@ -401,6 +505,12 @@ static bool acpi_extract_properties(const union acpi_object *desc,
>                     properties->type != ACPI_TYPE_PACKAGE)
>                         break;
>
> +               if (guid_equal((guid_t *)guid->buffer.pointer,
> +                              &buffer_prop_guid)) {
> +                       acpi_data_add_buffer_props(scope, data, properties);
> +                       continue;
> +               }
> +
>                 if (!acpi_is_property_guid((guid_t *)guid->buffer.pointer))
>                         continue;
>
> @@ -447,7 +557,7 @@ void acpi_init_properties(struct acpi_device *adev)
>         if (ACPI_FAILURE(status))
>                 goto out;
>
> -       if (acpi_extract_properties(buf.pointer, &adev->data)) {
> +       if (acpi_extract_properties(adev->handle, buf.pointer, &adev->data)) {
>                 adev->data.pointer = buf.pointer;
>                 if (acpi_of)
>                         acpi_init_of_compatible(adev);
> @@ -477,8 +587,14 @@ static void acpi_free_device_properties(struct list_head *list)
>         struct acpi_device_properties *props, *tmp;
>
>         list_for_each_entry_safe(props, tmp, list, list) {
> +               u32 i;
> +
>                 list_del(&props->list);
> -               kfree(props);
> +               /* Buffer data properties were separately allocated */
> +               if (props->bufs)
> +                       for (i = 0; i < props->properties->package.count; i++)
> +                               ACPI_FREE(props->bufs[i]);
> +               kvfree(props);
>         }
>  }
>
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 54c5566df9fe1..88a17fce49fd0 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -344,8 +344,9 @@ struct acpi_device_physical_node {
>
>  struct acpi_device_properties {
>         const guid_t *guid;
> -       const union acpi_object *properties;
> +       union acpi_object *properties;
>         struct list_head list;
> +       void **bufs;
>  };
>
>  /* ACPI Device Specific Data (_DSD) */
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 4f82a5bc6d987..93a695cab9d8a 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1243,7 +1243,7 @@ static inline bool acpi_dev_has_props(const struct acpi_device *adev)
>
>  struct acpi_device_properties *
>  acpi_data_add_props(struct acpi_device_data *data, const guid_t *guid,
> -                   const union acpi_object *properties);
> +                   union acpi_object *properties);
>
>  int acpi_node_prop_get(const struct fwnode_handle *fwnode, const char *propname,
>                        void **valptr);
> --
> 2.30.2
>

Re: [PATCH v4 7/8] ACPI: property: Add support for parsing buffer property UUID

[Sakari Ailus]

Hi Rafael,

Thanks for the review.

On Fri, Jul 15, 2022 at 07:59:21PM +0200, Rafael J. Wysocki wrote:
> On Mon, Jul 11, 2022 at 1:24 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > Add support for newly added buffer property UUID, as defined in the DSD
> > guide.
> 
> A link to the DSD guide and a more specific reference to the relevant
> part of it (eg. section number) would be handy.

That would be 3.3
<URL:https://github.com/UEFI/DSD-Guide/blob/main/src/dsd-guide.adoc#buffer-data-extension-uuid>.

I'll add that for v5.

-- 
Sakari Ailus

Re: [PATCH v4 7/8] ACPI: property: Add support for parsing buffer property UUID

[Rafael J. Wysocki]

On Sat, Jul 16, 2022 at 10:00 AM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Rafael,
>
> Thanks for the review.
>
> On Fri, Jul 15, 2022 at 07:59:21PM +0200, Rafael J. Wysocki wrote:
> > On Mon, Jul 11, 2022 at 1:24 PM Sakari Ailus
> > <sakari.ailus@linux.intel.com> wrote:
> > >
> > > Add support for newly added buffer property UUID, as defined in the DSD
> > > guide.
> >
> > A link to the DSD guide and a more specific reference to the relevant
> > part of it (eg. section number) would be handy.
>
> That would be 3.3
> <URL:https://github.com/UEFI/DSD-Guide/blob/main/src/dsd-guide.adoc#buffer-data-extension-uuid>.
>
> I'll add that for v5.

The rest of the patches in the series look fine to me, so please just
send an update of this one.

[PATCH v5 7/8] ACPI: property: Add support for parsing buffer property UUID

[Sakari Ailus]

Add support for newly added buffer property UUID, as defined in the DSD
guide section 3.3
<URL:https://github.com/UEFI/DSD-Guide/blob/main/src/dsd-guide.adoc#buffer-data-extension-uuid>.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
since v4:

Refer to DSD guide section 3.3, with URL.

 drivers/acpi/property.c | 142 ++++++++++++++++++++++++++++++++++++----
 include/acpi/acpi_bus.h |   3 +-
 include/linux/acpi.h    |   2 +-
 3 files changed, 132 insertions(+), 15 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 236a847f1bfbd..7621f684212ff 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -55,14 +55,19 @@ static const guid_t ads_guid =
 	GUID_INIT(0xdbb8e3e6, 0x5886, 0x4ba6,
 		  0x87, 0x95, 0x13, 0x19, 0xf5, 0x2a, 0x96, 0x6b);
 
+static const guid_t buffer_prop_guid =
+	GUID_INIT(0xedb12dd0, 0x363d, 0x4085,
+		  0xa3, 0xd2, 0x49, 0x52, 0x2c, 0xa1, 0x60, 0xc4);
+
 static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
-					   const union acpi_object *desc,
+					   union acpi_object *desc,
 					   struct acpi_device_data *data,
 					   struct fwnode_handle *parent);
-static bool acpi_extract_properties(const union acpi_object *desc,
+static bool acpi_extract_properties(acpi_handle handle,
+				    union acpi_object *desc,
 				    struct acpi_device_data *data);
 
-static bool acpi_nondev_subnode_extract(const union acpi_object *desc,
+static bool acpi_nondev_subnode_extract(union acpi_object *desc,
 					acpi_handle handle,
 					const union acpi_object *link,
 					struct list_head *list,
@@ -81,7 +86,7 @@ static bool acpi_nondev_subnode_extract(const union acpi_object *desc,
 	INIT_LIST_HEAD(&dn->data.properties);
 	INIT_LIST_HEAD(&dn->data.subnodes);
 
-	result = acpi_extract_properties(desc, &dn->data);
+	result = acpi_extract_properties(handle, desc, &dn->data);
 
 	if (handle) {
 		acpi_handle scope;
@@ -156,7 +161,7 @@ static bool acpi_nondev_subnode_ok(acpi_handle scope,
 }
 
 static bool acpi_add_nondev_subnodes(acpi_handle scope,
-				     const union acpi_object *links,
+				     union acpi_object *links,
 				     struct list_head *list,
 				     struct fwnode_handle *parent)
 {
@@ -164,7 +169,7 @@ static bool acpi_add_nondev_subnodes(acpi_handle scope,
 	int i;
 
 	for (i = 0; i < links->package.count; i++) {
-		const union acpi_object *link, *desc;
+		union acpi_object *link, *desc;
 		acpi_handle handle;
 		bool result;
 
@@ -204,7 +209,7 @@ static bool acpi_add_nondev_subnodes(acpi_handle scope,
 }
 
 static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
-					   const union acpi_object *desc,
+					   union acpi_object *desc,
 					   struct acpi_device_data *data,
 					   struct fwnode_handle *parent)
 {
@@ -212,7 +217,8 @@ static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
 
 	/* Look for the ACPI data subnodes GUID. */
 	for (i = 0; i < desc->package.count; i += 2) {
-		const union acpi_object *guid, *links;
+		const union acpi_object *guid;
+		union acpi_object *links;
 
 		guid = &desc->package.elements[i];
 		links = &desc->package.elements[i + 1];
@@ -325,7 +331,7 @@ static bool acpi_is_property_guid(const guid_t *guid)
 
 struct acpi_device_properties *
 acpi_data_add_props(struct acpi_device_data *data, const guid_t *guid,
-		    const union acpi_object *properties)
+		    union acpi_object *properties)
 {
 	struct acpi_device_properties *props;
 
@@ -377,7 +383,104 @@ static bool acpi_tie_nondev_subnodes(struct acpi_device_data *data)
 	return true;
 }
 
-static bool acpi_extract_properties(const union acpi_object *desc,
+static void acpi_data_add_buffer_props(acpi_handle handle,
+				       struct acpi_device_data *data,
+				       union acpi_object *properties)
+{
+	struct acpi_device_properties *props;
+	union acpi_object *package;
+	size_t alloc_size;
+	unsigned int i;
+	u32 *count;
+
+	if (check_mul_overflow((size_t)properties->package.count,
+			       sizeof(*package) + sizeof(void *),
+			       &alloc_size) ||
+	    check_add_overflow(sizeof(*props) + sizeof(*package), alloc_size,
+			       &alloc_size)) {
+		acpi_handle_warn(handle,
+				 "can't allocate memory for %u buffer props",
+				 properties->package.count);
+		return;
+	}
+
+	props = kvzalloc(alloc_size, GFP_KERNEL);
+	if (!props)
+		return;
+
+	props->guid = &buffer_prop_guid;
+	props->bufs = (void *)(props + 1);
+	props->properties = (void *)(props->bufs + properties->package.count);
+
+	/* Outer package */
+	package = props->properties;
+	package->type = ACPI_TYPE_PACKAGE;
+	package->package.elements = package + 1;
+	count = &package->package.count;
+	*count = 0;
+
+	/* Inner packages */
+	package++;
+
+	for (i = 0; i < properties->package.count; i++) {
+		struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
+		union acpi_object *property = &properties->package.elements[i];
+		union acpi_object *prop, *obj, *buf_obj;
+		acpi_status status;
+
+		if (property->type != ACPI_TYPE_PACKAGE ||
+		    property->package.count != 2) {
+			acpi_handle_warn(handle,
+					 "buffer property %u has %u entries\n",
+					 i, property->package.count);
+			continue;
+		}
+
+		prop = &property->package.elements[0];
+		obj = &property->package.elements[1];
+
+		if (prop->type != ACPI_TYPE_STRING ||
+		    obj->type != ACPI_TYPE_STRING) {
+			acpi_handle_warn(handle,
+					 "wrong object types %u and %u\n",
+					 prop->type, obj->type);
+			continue;
+		}
+
+		status = acpi_evaluate_object_typed(handle, obj->string.pointer,
+						    NULL, &buf,
+						    ACPI_TYPE_BUFFER);
+		if (ACPI_FAILURE(status)) {
+			acpi_handle_warn(handle,
+					 "can't evaluate \"%*pE\" as buffer\n",
+					 obj->string.length,
+					 obj->string.pointer);
+			continue;
+		}
+
+		package->type = ACPI_TYPE_PACKAGE;
+		package->package.elements = prop;
+		package->package.count = 2;
+
+		buf_obj = buf.pointer;
+
+		/* Replace the string object with a buffer object */
+		obj->type = ACPI_TYPE_BUFFER;
+		obj->buffer.length = buf_obj->buffer.length;
+		obj->buffer.pointer = buf_obj->buffer.pointer;
+
+		props->bufs[i] = buf.pointer;
+		package++;
+		(*count)++;
+	}
+
+	if (*count)
+		list_add(&props->list, &data->properties);
+	else
+		kvfree(props);
+}
+
+static bool acpi_extract_properties(acpi_handle scope, union acpi_object *desc,
 				    struct acpi_device_data *data)
 {
 	int i;
@@ -387,7 +490,8 @@ static bool acpi_extract_properties(const union acpi_object *desc,
 
 	/* Look for the device properties GUID. */
 	for (i = 0; i < desc->package.count; i += 2) {
-		const union acpi_object *guid, *properties;
+		const union acpi_object *guid;
+		union acpi_object *properties;
 
 		guid = &desc->package.elements[i];
 		properties = &desc->package.elements[i + 1];
@@ -401,6 +505,12 @@ static bool acpi_extract_properties(const union acpi_object *desc,
 		    properties->type != ACPI_TYPE_PACKAGE)
 			break;
 
+		if (guid_equal((guid_t *)guid->buffer.pointer,
+			       &buffer_prop_guid)) {
+			acpi_data_add_buffer_props(scope, data, properties);
+			continue;
+		}
+
 		if (!acpi_is_property_guid((guid_t *)guid->buffer.pointer))
 			continue;
 
@@ -447,7 +557,7 @@ void acpi_init_properties(struct acpi_device *adev)
 	if (ACPI_FAILURE(status))
 		goto out;
 
-	if (acpi_extract_properties(buf.pointer, &adev->data)) {
+	if (acpi_extract_properties(adev->handle, buf.pointer, &adev->data)) {
 		adev->data.pointer = buf.pointer;
 		if (acpi_of)
 			acpi_init_of_compatible(adev);
@@ -477,8 +587,14 @@ static void acpi_free_device_properties(struct list_head *list)
 	struct acpi_device_properties *props, *tmp;
 
 	list_for_each_entry_safe(props, tmp, list, list) {
+		u32 i;
+
 		list_del(&props->list);
-		kfree(props);
+		/* Buffer data properties were separately allocated */
+		if (props->bufs)
+			for (i = 0; i < props->properties->package.count; i++)
+				ACPI_FREE(props->bufs[i]);
+		kvfree(props);
 	}
 }
 
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 54c5566df9fe1..88a17fce49fd0 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -344,8 +344,9 @@ struct acpi_device_physical_node {
 
 struct acpi_device_properties {
 	const guid_t *guid;
-	const union acpi_object *properties;
+	union acpi_object *properties;
 	struct list_head list;
+	void **bufs;
 };
 
 /* ACPI Device Specific Data (_DSD) */
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 4f82a5bc6d987..93a695cab9d8a 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1243,7 +1243,7 @@ static inline bool acpi_dev_has_props(const struct acpi_device *adev)
 
 struct acpi_device_properties *
 acpi_data_add_props(struct acpi_device_data *data, const guid_t *guid,
-		    const union acpi_object *properties);
+		    union acpi_object *properties);
 
 int acpi_node_prop_get(const struct fwnode_handle *fwnode, const char *propname,
 		       void **valptr);
-- 
2.30.2

Re: [PATCH v4 0/8] [PATCH v3 0/8] ACPI: Buffer property and reference as string support

[Rafael J. Wysocki]

On Mon, Jul 11, 2022 at 1:24 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hello everyone,
>
> This set adds support for _DSD buffer properties (specified by DSD Guide
> <URL:https://github.com/UEFI/DSD-Guide/blob/main/dsd-guide.md>) as well as
> support for references as strings. Reference property type was previously
> supported for device objects only, whereas string references enable
> referencing also _DSD sub-node objects --- also included in the set.
>
> The ACPICA patch has been submitted to upstream but not merged yet.
>
> This set currently prepares for data node string reference support and
> does not add it anymore.
>
> since v3:
>
> - Use bool return type for acpi_tie_nondev_subnodes. Return false on
>   failure.
>
> - Print ACPI buffer parsing error references as %*pE (was %s).
>
> - Unwrap a few lines (i.e. non-functional changes).
>
> - Declare local variables for acpi_copy_property_array_uint().
>
> - Clean up buffer size check in acpi_data_prop_read(), add break at the
>   end of the default case.
>
> since v2:
>
> - Use C99 _Generic() in patch unifying reading integer arrays.
>
> since v1:
>
> - Drop the ACPICA, data node child list initialisation and data node
>   string reference patches.
>
> Sakari Ailus (8):
>   ACPI: property: Return type of acpi_add_nondev_subnodes() should be
>     bool
>   ACPI: property: Tie data nodes to acpi handles
>   ACPI: property: Use acpi_object_type consistently in property ref
>     parsing
>   ACPI: property: Move property ref argument parsing into a new function
>   ACPI: property: Switch node property referencing from ifs to a switch
>   ACPI: property: Unify integer value reading functions
>   ACPI: property: Add support for parsing buffer property UUID
>   ACPI: property: Read buffer properties as integers
>
>  drivers/acpi/property.c | 465 +++++++++++++++++++++++++++-------------
>  include/acpi/acpi_bus.h |   3 +-
>  include/linux/acpi.h    |   2 +-
>  3 files changed, 318 insertions(+), 152 deletions(-)
>
> --

The series (including the update of patch [7/8]) has been applied as
5.20 material, thanks!

Re: [PATCH v4 0/8] [PATCH v3 0/8] ACPI: Buffer property and reference as string support

[Sakari Ailus]

On Wed, Jul 27, 2022 at 09:20:15PM +0200, Rafael J. Wysocki wrote:
> >  drivers/acpi/property.c | 465 +++++++++++++++++++++++++++-------------
> >  include/acpi/acpi_bus.h |   3 +-
> >  include/linux/acpi.h    |   2 +-
> >  3 files changed, 318 insertions(+), 152 deletions(-)
> >
> > --
> 
> The series (including the update of patch [7/8]) has been applied as
> 5.20 material, thanks!

Thank you!

Thanks to Andy for reviews, too!

-- 
Sakari Ailus