[v2,18/35] acpi: Support writing a UUID

Message ID 20200510203409.203520-16-sjg@chromium.org
State Accepted
Commit 29df845204e6b67583491b3c9883432c3a74d923
Headers show
Series
  • dm: Add programmatic generation of ACPI tables (part B)
Related show

Commit Message

Simon Glass May 10, 2020, 8:33 p.m.
ACPI supports writing a UUID in a special format. Add a function to handle
this.

Signed-off-by: Simon Glass <sjg at chromium.org>
---

Changes in v2: None
Changes in v1: None

 include/acpi/acpigen.h | 13 +++++++++++++
 lib/acpi/acpigen.c     | 39 +++++++++++++++++++++++++++++++++++++++
 test/dm/acpigen.c      | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 85 insertions(+)

Comments

Wolfgang Wallner May 28, 2020, 9:57 a.m. | #1
Hi Simon,

-----"Simon Glass" <sjg at chromium.org> schrieb: -----
> Betreff: [PATCH v2 18/35] acpi: Support writing a UUID
> 
> ACPI supports writing a UUID in a special format. Add a function to handle
> this.
> 
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
> 
> Changes in v2: None
> Changes in v1: None
> 
>  include/acpi/acpigen.h | 13 +++++++++++++
>  lib/acpi/acpigen.c     | 39 +++++++++++++++++++++++++++++++++++++++
>  test/dm/acpigen.c      | 33 +++++++++++++++++++++++++++++++++
>  3 files changed, 85 insertions(+)
> 
> diff --git a/include/acpi/acpigen.h b/include/acpi/acpigen.h
> index 0fecb7d57a..f35467f029 100644
> --- a/include/acpi/acpigen.h
> +++ b/include/acpi/acpigen.h
> @@ -24,6 +24,7 @@ enum {
>  	DWORD_PREFIX		= 0x0c,
>  	STRING_PREFIX		= 0x0d,
>  	QWORD_PREFIX		= 0x0e,
> +	BUFFER_OP		= 0x11,
>  	PACKAGE_OP		= 0x12,
>  	DUAL_NAME_PREFIX	= 0x2e,
>  	MULTI_NAME_PREFIX	= 0x2f,
> @@ -182,4 +183,16 @@ void acpigen_emit_namestring(struct acpi_ctx *ctx, const char *namepath);
>   * @namepath: Name / path to emit
>   */
>  void acpigen_write_name(struct acpi_ctx *ctx, const char *namepath);
> +
> +/**
> + * acpigen_write_uuid() - Write a UUID
> + *
> + * This writes out a UUID in the format used by ACPI, with a BUFFER_OP prefix.
> + *
> + * @ctx: ACPI context pointer
> + * @uuid: UUID to write in the form aabbccdd-eeff-gghh-iijj-kkllmmnnoopp
> + * @return 0 if OK, -EINVAL if the format is incorrect
> + */
> +int acpigen_write_uuid(struct acpi_ctx *ctx, const char *uuid);
> +
>  #endif
> diff --git a/lib/acpi/acpigen.c b/lib/acpi/acpigen.c
> index eae2f605ed..f781ad4d87 100644
> --- a/lib/acpi/acpigen.c
> +++ b/lib/acpi/acpigen.c
> @@ -11,6 +11,7 @@
>  #include <common.h>
>  #include <dm.h>
>  #include <log.h>
> +#include <uuid.h>
>  #include <acpi/acpigen.h>
>  #include <dm/acpi.h>
>  
> @@ -248,3 +249,41 @@ void acpigen_write_name(struct acpi_ctx *ctx, const char *namepath)
>  	acpigen_emit_byte(ctx, NAME_OP);
>  	acpigen_emit_namestring(ctx, namepath);
>  }
> +
> +/*
> + * ToUUID(uuid)
> + *
> + * ACPI 6.3 Section 19.6.142 table 19-438 defines a special output order for the
> + * bytes that make up a UUID Buffer object:
> + *
> + * UUID byte order for input to this function:
> + *   aabbccdd-eeff-gghh-iijj-kkllmmnnoopp
> + *
> + * UUID byte order output by this function:
> + *   ddccbbaa-ffee-hhgg-iijj-kkllmmnnoopp
> + */
> +#define UUID_LEN 16

Nit: To me it looks strange that this is defined within the ACPI module.
Could this be moved to e.g. include/uuid.h?

It seems this define could be useful elsewhere, as I could not find an
existing define like this, but "grep -r -i -e uuid | grep -e 16" shows
several places where the fixed length of 16 bytes for UUIDs is hardcoded.

> +int acpigen_write_uuid(struct acpi_ctx *ctx, const char *uuid)
> +{
> +	u8 buf[UUID_LEN];
> +	int ret;
> +
> +	/* Parse UUID string into bytes */
> +	ret = uuid_str_to_bin(uuid, buf, UUID_STR_FORMAT_GUID);
> +	if (ret)
> +		return log_msg_ret("bad hex", -EINVAL);
> +
> +	/* BufferOp */
> +	acpigen_emit_byte(ctx, BUFFER_OP);
> +	acpigen_write_len_f(ctx);
> +
> +	/* Buffer length in bytes */
> +	acpigen_write_word(ctx, UUID_LEN);
> +
> +	/* Output UUID in expected order */
> +	acpigen_emit_stream(ctx, (char *)buf, UUID_LEN);
> +
> +	acpigen_pop_len(ctx);
> +
> +	return 0;
> +}
> diff --git a/test/dm/acpigen.c b/test/dm/acpigen.c
> index 0f56262a28..db8cad47d8 100644
> --- a/test/dm/acpigen.c
> +++ b/test/dm/acpigen.c
> @@ -588,3 +588,36 @@ static int dm_test_acpi_name(struct unit_test_state *uts)
>  	return 0;
>  }
>  DM_TEST(dm_test_acpi_name, 0);
> +
> +/* Test writing a UUID */
> +static int dm_test_acpi_uuid(struct unit_test_state *uts)
> +{
> +	struct acpi_ctx *ctx;
> +	u8 *ptr;
> +
> +	ut_assertok(alloc_context(&ctx));
> +
> +	ptr = acpigen_get_current(ctx);
> +
> +	ut_assertok(acpigen_write_uuid(ctx,
> +				       "dbb8e3e6-5886-4ba6-8795-1319f52a966b"));
> +	ut_asserteq(23, acpigen_get_current(ctx) - ptr);
> +	ut_asserteq(BUFFER_OP, ptr[0]);
> +	ut_asserteq(22, get_length(ptr + 1));
> +	ut_asserteq(0xdbb8e3e6, get_unaligned((u32 *)(ptr + 7)));
> +	ut_asserteq(0x5886, get_unaligned((u16 *)(ptr + 11)));
> +	ut_asserteq(0x4ba6, get_unaligned((u16 *)(ptr + 13)));
> +	ut_asserteq(0x9587, get_unaligned((u16 *)(ptr + 15)));
> +	ut_asserteq(0x2af51913, get_unaligned((u32 *)(ptr + 17)));
> +	ut_asserteq(0x6b96, get_unaligned((u16 *)(ptr + 21)));
> +
> +	/* Try a bad UUID */
> +	ut_asserteq(-EINVAL,
> +		    acpigen_write_uuid(ctx,
> +				       "dbb8e3e6-5886-4ba6x8795-1319f52a966b"));
> +
> +	free_context(&ctx);
> +
> +	return 0;
> +}
> +DM_TEST(dm_test_acpi_uuid, 0);
> -- 
> 2.26.2.645.ge9eca65c58-goog

I would also be fine with keeping the UUID_LEN define where it is,
and the rest of the patch looks good to me:

Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>

regards, Wolfgang

Patch

diff --git a/include/acpi/acpigen.h b/include/acpi/acpigen.h
index 0fecb7d57a..f35467f029 100644
--- a/include/acpi/acpigen.h
+++ b/include/acpi/acpigen.h
@@ -24,6 +24,7 @@  enum {
 	DWORD_PREFIX		= 0x0c,
 	STRING_PREFIX		= 0x0d,
 	QWORD_PREFIX		= 0x0e,
+	BUFFER_OP		= 0x11,
 	PACKAGE_OP		= 0x12,
 	DUAL_NAME_PREFIX	= 0x2e,
 	MULTI_NAME_PREFIX	= 0x2f,
@@ -182,4 +183,16 @@  void acpigen_emit_namestring(struct acpi_ctx *ctx, const char *namepath);
  * @namepath: Name / path to emit
  */
 void acpigen_write_name(struct acpi_ctx *ctx, const char *namepath);
+
+/**
+ * acpigen_write_uuid() - Write a UUID
+ *
+ * This writes out a UUID in the format used by ACPI, with a BUFFER_OP prefix.
+ *
+ * @ctx: ACPI context pointer
+ * @uuid: UUID to write in the form aabbccdd-eeff-gghh-iijj-kkllmmnnoopp
+ * @return 0 if OK, -EINVAL if the format is incorrect
+ */
+int acpigen_write_uuid(struct acpi_ctx *ctx, const char *uuid);
+
 #endif
diff --git a/lib/acpi/acpigen.c b/lib/acpi/acpigen.c
index eae2f605ed..f781ad4d87 100644
--- a/lib/acpi/acpigen.c
+++ b/lib/acpi/acpigen.c
@@ -11,6 +11,7 @@ 
 #include <common.h>
 #include <dm.h>
 #include <log.h>
+#include <uuid.h>
 #include <acpi/acpigen.h>
 #include <dm/acpi.h>
 
@@ -248,3 +249,41 @@  void acpigen_write_name(struct acpi_ctx *ctx, const char *namepath)
 	acpigen_emit_byte(ctx, NAME_OP);
 	acpigen_emit_namestring(ctx, namepath);
 }
+
+/*
+ * ToUUID(uuid)
+ *
+ * ACPI 6.3 Section 19.6.142 table 19-438 defines a special output order for the
+ * bytes that make up a UUID Buffer object:
+ *
+ * UUID byte order for input to this function:
+ *   aabbccdd-eeff-gghh-iijj-kkllmmnnoopp
+ *
+ * UUID byte order output by this function:
+ *   ddccbbaa-ffee-hhgg-iijj-kkllmmnnoopp
+ */
+#define UUID_LEN 16
+int acpigen_write_uuid(struct acpi_ctx *ctx, const char *uuid)
+{
+	u8 buf[UUID_LEN];
+	int ret;
+
+	/* Parse UUID string into bytes */
+	ret = uuid_str_to_bin(uuid, buf, UUID_STR_FORMAT_GUID);
+	if (ret)
+		return log_msg_ret("bad hex", -EINVAL);
+
+	/* BufferOp */
+	acpigen_emit_byte(ctx, BUFFER_OP);
+	acpigen_write_len_f(ctx);
+
+	/* Buffer length in bytes */
+	acpigen_write_word(ctx, UUID_LEN);
+
+	/* Output UUID in expected order */
+	acpigen_emit_stream(ctx, (char *)buf, UUID_LEN);
+
+	acpigen_pop_len(ctx);
+
+	return 0;
+}
diff --git a/test/dm/acpigen.c b/test/dm/acpigen.c
index 0f56262a28..db8cad47d8 100644
--- a/test/dm/acpigen.c
+++ b/test/dm/acpigen.c
@@ -588,3 +588,36 @@  static int dm_test_acpi_name(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_acpi_name, 0);
+
+/* Test writing a UUID */
+static int dm_test_acpi_uuid(struct unit_test_state *uts)
+{
+	struct acpi_ctx *ctx;
+	u8 *ptr;
+
+	ut_assertok(alloc_context(&ctx));
+
+	ptr = acpigen_get_current(ctx);
+
+	ut_assertok(acpigen_write_uuid(ctx,
+				       "dbb8e3e6-5886-4ba6-8795-1319f52a966b"));
+	ut_asserteq(23, acpigen_get_current(ctx) - ptr);
+	ut_asserteq(BUFFER_OP, ptr[0]);
+	ut_asserteq(22, get_length(ptr + 1));
+	ut_asserteq(0xdbb8e3e6, get_unaligned((u32 *)(ptr + 7)));
+	ut_asserteq(0x5886, get_unaligned((u16 *)(ptr + 11)));
+	ut_asserteq(0x4ba6, get_unaligned((u16 *)(ptr + 13)));
+	ut_asserteq(0x9587, get_unaligned((u16 *)(ptr + 15)));
+	ut_asserteq(0x2af51913, get_unaligned((u32 *)(ptr + 17)));
+	ut_asserteq(0x6b96, get_unaligned((u16 *)(ptr + 21)));
+
+	/* Try a bad UUID */
+	ut_asserteq(-EINVAL,
+		    acpigen_write_uuid(ctx,
+				       "dbb8e3e6-5886-4ba6x8795-1319f52a966b"));
+
+	free_context(&ctx);
+
+	return 0;
+}
+DM_TEST(dm_test_acpi_uuid, 0);