Re: [PATCH v3 13/35] acpigen: Support writing a length

On Sun, Jun 14, 2020 at 10:55 AM Simon Glass sjg@chromium.org wrote:

It is convenient to write a length value for preceding a block of data. Of course the length is not known or is hard to calculate a priori. So add a way to mark the start on a stack, so the length can be updated when known.

Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com

---

Changes in v3:

• Add a reference to the ACPI spec
• Add a define for the 0x80 constant
• Move the function comments into this patch

include/acpi/acpigen.h | 38 +++++++++++++++++++++++++ include/dm/acpi.h | 7 +++++ lib/acpi/acpigen.c | 33 ++++++++++++++++++++++ test/dm/acpigen.c | 64 ++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 140 insertions(+), 2 deletions(-)

diff --git a/include/acpi/acpigen.h b/include/acpi/acpigen.h index 7365cce738..12cd1bd578 100644 --- a/include/acpi/acpigen.h +++ b/include/acpi/acpigen.h @@ -14,6 +14,9 @@

struct acpi_ctx;

+/* Top 4 bits of the value used to indicate a three-byte length value */ +#define ACPI_PKG_LEN_3_BYTES 0x80

/**

• acpigen_get_current() - Get the current ACPI code output pointer

@@ -65,4 +68,39 @@ void acpigen_emit_stream(struct acpi_ctx *ctx, const char *data, int size); */ void acpigen_emit_string(struct acpi_ctx *ctx, const char *str);

+/**

• • acpigen_write_len_f() - Write a 'forward' length placeholder
• • This adds space for a length value in the ACPI stream and pushes the current
• • position (before the length) on the stack. After calling this you can write
• • some data and then call acpigen_pop_len() to update the length value.
• • Usage:
• • acpigen_write_len_f() ------\
• • acpigen_write...() |
• • acpigen_write...() |

• •  acpigen_write_len_f() --\ |
    

• •  acpigen_write...()      | |
    

• •  acpigen_write...()      | |
    

• •  acpigen_pop_len() ------/ |
    

• • acpigen_write...() |
• • acpigen_pop_len() ----------/
• • See ACPI 6.3 section 20.2.4 Package Length Encoding

It's better we put some more comments here mentioning that we are hardcoding a 3 byte packet length encoding that can represent up to 2*20 packet length in total.

Any chance to optimize this routine not to use a hardcode 3 byte encoding?

• • @ctx: ACPI context pointer
• */

+void acpigen_write_len_f(struct acpi_ctx *ctx);

+/**

• • acpigen_pop_len() - Update the previously stacked length placeholder
• • Call this after the data for the block gas been written. It updates the

typo: gas -> has

• • top length value in the stack and pops it off.
• • @ctx: ACPI context pointer
• */

+void acpigen_pop_len(struct acpi_ctx *ctx);

#endif diff --git a/include/dm/acpi.h b/include/dm/acpi.h index 696b1a96a0..f27ca64507 100644 --- a/include/dm/acpi.h +++ b/include/dm/acpi.h @@ -22,6 +22,9 @@ /* Length of an ACPI name string including nul terminator */

nits: nul -> null

#define ACPI_NAME_MAX (ACPI_NAME_LEN + 1)

+/* Number of nested objects supported */ +#define ACPIGEN_LENSTACK_SIZE 10

#if !defined(__ACPI__)

/** @@ -35,6 +38,8 @@

• adding a new table. The RSDP holds pointers to the RSDT and XSDT.
  

• @rsdt: Pointer to the Root System Description Table
• @xsdt: Pointer to the Extended System Description Table

• • @len_stack: Stack of 'length' words to fix up later
• • @ltop: Points to current top of stack (0 = empty)
  */

struct acpi_ctx { void *base; @@ -42,6 +47,8 @@ struct acpi_ctx { struct acpi_rsdp *rsdp; struct acpi_rsdt *rsdt; struct acpi_xsdt *xsdt;

•   char *len_stack[ACPIGEN_LENSTACK_SIZE];
  

•   int ltop;
  

};

/** diff --git a/lib/acpi/acpigen.c b/lib/acpi/acpigen.c index 1223f0d1c4..2c2b604d80 100644 --- a/lib/acpi/acpigen.c +++ b/lib/acpi/acpigen.c @@ -10,6 +10,7 @@

#include <common.h> #include <dm.h> +#include <log.h> #include <acpi/acpigen.h> #include <dm/acpi.h>

@@ -37,6 +38,38 @@ void acpigen_emit_dword(struct acpi_ctx *ctx, uint data) acpigen_emit_byte(ctx, (data >> 24) & 0xff); }

+/*

• • Maximum length for an ACPI object generated by this code,
• • If you need to change this, change acpigen_write_len_f(ctx) and
• • acpigen_pop_len(ctx)
• */

+#define ACPIGEN_MAXLEN 0xfffff

+void acpigen_write_len_f(struct acpi_ctx *ctx) +{

•   assert(ctx->ltop < (ACPIGEN_LENSTACK_SIZE - 1));
  

•   ctx->len_stack[ctx->ltop++] = ctx->current;
  

•   acpigen_emit_byte(ctx, 0);
  

•   acpigen_emit_byte(ctx, 0);
  

•   acpigen_emit_byte(ctx, 0);
  

+}

+void acpigen_pop_len(struct acpi_ctx *ctx) +{

•   int len;
  

•   char *p;
  

•   assert(ctx->ltop > 0);
  

•   p = ctx->len_stack[--ctx->ltop];
  

•   len = ctx->current - (void *)p;
  

•   assert(len <= ACPIGEN_MAXLEN);
  

•   /* generate store length for 0xfffff max */
  

•   p[0] = ACPI_PKG_LEN_3_BYTES | (len & 0xf);
  

•   p[1] = len >> 4 & 0xff;
  

•   p[2] = len >> 12 & 0xff;
  

+}

void acpigen_emit_stream(struct acpi_ctx *ctx, const char *data, int size) { int i; diff --git a/test/dm/acpigen.c b/test/dm/acpigen.c index 571510722d..71f70ebf71 100644 --- a/test/dm/acpigen.c +++ b/test/dm/acpigen.c @@ -22,7 +22,7 @@ #define TEST_STRING "frogmore" #define TEST_STREAM2 "\xfa\xde"

-static int alloc_context(struct acpi_ctx **ctxp) +static int alloc_context_size(struct acpi_ctx **ctxp, int size) { struct acpi_ctx *ctx;

@@ -30,17 +30,23 @@ static int alloc_context(struct acpi_ctx **ctxp) ctx = malloc(sizeof(*ctx)); if (!ctx) return -ENOMEM;

•   ctx->base = malloc(150);
  

•   ctx->base = malloc(size);
    if (!ctx->base) {
            free(ctx);
            return -ENOMEM;
    }
  

•   ctx->ltop = 0;
    ctx->current = ctx->base;
    *ctxp = ctx;
  
    return 0;
  

}

+static int alloc_context(struct acpi_ctx **ctxp) +{

•   return alloc_context_size(ctxp, 150);
  

+}

static void free_context(struct acpi_ctx **ctxp) { free((*ctxp)->base); @@ -338,3 +344,57 @@ static int dm_test_acpi_spi(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_acpi_spi, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);

+/**

• • get_length() - decode a three-byte length field
• • @ptr: Length encoded as per ACPI
• • @return decoded length, or -EINVAL on error
• */

+static int get_length(u8 *ptr) +{

•   if (!(*ptr & 0x80))
  

•           return -EINVAL;
  

•   return (*ptr & 0xf) | ptr[1] << 4 | ptr[2] << 12;
  

+}

+/* Test emitting a length */ +static int dm_test_acpi_len(struct unit_test_state *uts) +{

•   const int size = 0xc0000;
  

•   struct acpi_ctx *ctx;
  

•   u8 *ptr;
  

•   int i;
  

•   ut_assertok(alloc_context_size(&ctx, size));
  

•   ptr = acpigen_get_current(ctx);
  

•   /* Write a byte and a 3-byte length */
  

•   acpigen_write_len_f(ctx);
  

•   acpigen_emit_byte(ctx, 0x23);
  

•   acpigen_pop_len(ctx);
  

•   ut_asserteq(1 + 3, get_length(ptr));
  

•   /* Write 200 bytes so we need two length bytes */
  

•   ptr = ctx->current;
  

•   acpigen_write_len_f(ctx);
  

•   for (i = 0; i < 200; i++)
  

•           acpigen_emit_byte(ctx, 0x23);
  

•   acpigen_pop_len(ctx);
  

•   ut_asserteq(200 + 3, get_length(ptr));
  

•   /* Write 40KB so we need three length bytes */
  

•   ptr = ctx->current;
  

•   acpigen_write_len_f(ctx);
  

•   for (i = 0; i < 40000; i++)
  

•           acpigen_emit_byte(ctx, 0x23);
  

•   acpigen_pop_len(ctx);
  

•   ut_asserteq(40000 + 3, get_length(ptr));
  

•   free_context(&ctx);
  

•   return 0;
  

+} +DM_TEST(dm_test_acpi_len, 0);

Regards, Bin