Re: [Qemu-devel] [PATCH v2 42/47] acpi: add acpi_def_block() term

[Shannon Zhao]

On 2015/1/29 16:45, Igor Mammedov wrote:
> On Thu, 29 Jan 2015 16:02:47 +0800
> Shannon Zhao <address@hidden> wrote:
> 
>> > On 2015/1/22 22:50, Igor Mammedov wrote:
>>> > > Signed-off-by: Igor Mammedov <address@hidden>
>>> > > ---
>>> > >  hw/acpi/acpi-build-utils.c         | 47 
>>> > > ++++++++++++++++++++++++++++++++++++++
>>> > >  hw/i386/acpi-build.c               |  1 -
>>> > >  include/hw/acpi/acpi-build-utils.h |  8 +++++++
>>> > >  3 files changed, 55 insertions(+), 1 deletion(-)
>>> > > 
>>> > > diff --git a/hw/acpi/acpi-build-utils.c b/hw/acpi/acpi-build-utils.c
>>> > > index b19d370..58f88cd 100644
>>> > > --- a/hw/acpi/acpi-build-utils.c
>>> > > +++ b/hw/acpi/acpi-build-utils.c
>>> > > @@ -26,6 +26,7 @@
>>> > >  #include <string.h>
>>> > >  #include "hw/acpi/acpi-build-utils.h"
>>> > >  #include "qemu/bswap.h"
>>> > > +#include "hw/acpi/bios-linker-loader.h"
>>> > >  
>>> > >  GArray *build_alloc_array(void)
>>> > >  {
>>> > > @@ -312,6 +313,21 @@ void aml_append(AcpiAml *parent_ctx, AcpiAml child)
>>> > >          build_prepend_int(child.buf, child.buf->len);
>>> > >          build_package(child.buf, child.op);
>>> > >          break;
>>> > > +    case DEF_BLOCK: {
>>> > > +        uint8_t *start = (uint8_t *)parent_ctx->buf->data +
>>> > > +                         parent_ctx->buf->len;
>>> > > +        uint32_t le32_len = cpu_to_le32(child.buf->len);
>>> > > +
>>> > > +        /* create linker entry for the DefinitionBlock */
>>> > > +        bios_linker_loader_add_checksum(parent_ctx->linker,
>>> > > +            ACPI_BUILD_TABLE_FILE,
>>> > > +            parent_ctx->buf->data,
>>> > > +            start, child.buf->len, start + 9 /* checksum offset */);
>>> > > +
>>> > > +        /* set DefinitionBlock length at TableLength offset*/
>>> > > +        memcpy(child.buf->data + 4, &le32_len, sizeof le32_len);
>>> > > +        break;
>>> > > +    }
>>> > >      default:
>>> > >          break;
>>> > >      }
>>> > > @@ -843,3 +859,34 @@ AcpiAml acpi_qword_memory(acpiDecode dec, 
>>> > > acpiMinFixed min_fixed,
>>> > >                                dec, addr_gran, addr_min, addr_max,
>>> > >                                addr_trans, len, flags);
>>> > >  }
>>> > > +
>>> > > +/* ACPI 5.0: 20.2.1 Table and Table Header Encoding */
>>> > > +AcpiAml acpi_def_block(const char *signature, uint8_t revision,
>>> > > +                       const char *oem_id, const char *oem_table_id,
>>> > > +                       uint32_t oem_revision)
>>> > > +{
>>> > > +    int len;
>>> > > +    AcpiAml var = aml_allocate_internal(0, DEF_BLOCK);
>>> > > +
>>> > > +    assert(strlen(signature) == 4);
>>> > > +    g_array_append_vals(var.buf, signature, 4);
>>> > > +    build_append_value(var.buf, 0, 4); /* Length place holder */
>>> > > +    build_append_byte(var.buf, revision);
>>> > > +    build_append_byte(var.buf, 0); /* place holder for Checksum */
>>> > > +
>>> > > +    len = strlen(oem_id);
>>> > > +    assert(len <= 6);
>>> > > +    g_array_append_vals(var.buf, oem_id, len);
>>> > > +    g_array_append_vals(var.buf, "\0\0\0\0\0\0\0\0", 6 - len);
>>> > > +
>>> > > +    len = strlen(oem_table_id);
>>> > > +    assert(len <= 8);
>>> > > +    g_array_append_vals(var.buf, oem_table_id, len);
>>> > > +    g_array_append_vals(var.buf, "\0\0\0\0\0\0\0\0", 8 - len);
>>> > > +
>>> > > +    build_append_value(var.buf, oem_revision, 4);
>>> > > +    g_array_append_vals(var.buf, ACPI_BUILD_APPNAME4, 4); /* 
>>> > > asl_compiler_id */
>>> > > +    build_append_value(var.buf, 1, 4); /* asl_compiler_revision */
>>> > > +
>>> > > +    return var;
>>> > > +}
>> > 
>> > This function is similar with build_header() in hw/i386/acpi-build.c
>> > But the format of omt_id, oem_table_id, asl_compiler_id are not exactly 
>> > same.
>> > Maybe we should make it consistent.
> I've tried to follow APCI spec for definition block here,
> which should be identical to build_header().
> What exactly is not the same?
> 

Sorry, I don make myself clear.
I mean that the values of omt_id, oem_table_id, asl_compiler_id are different 
between
using build_header and acpi_def_block.
E.g. using acpi_def_block the asl_compiler_id would be "BXPC" while 
build_header is blank.

I think we should make the omt_id, oem_table_id, asl_compiler_id as parameters 
of the
functions, then users assign the value.

Thanks,
Shannon