[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 05/52] acpi: add aml_def_block() term
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH v3 05/52] acpi: add aml_def_block() term |
Date: |
Tue, 17 Feb 2015 20:03:52 +0100 |
On Tue, Feb 17, 2015 at 05:47:26PM +0100, Igor Mammedov wrote:
> On Tue, 17 Feb 2015 17:35:55 +0100
> "Michael S. Tsirkin" <address@hidden> wrote:
>
> > On Mon, Feb 09, 2015 at 10:53:27AM +0000, Igor Mammedov wrote:
> > > Signed-off-by: Igor Mammedov <address@hidden>
> > > ---
> > > hw/acpi/aml-build.c | 38 ++++++++++++++++++++++++++++++++++++++
> > > hw/i386/acpi-build.c | 4 ----
> > > include/hw/acpi/aml-build.h | 10 ++++++++++
> > > 3 files changed, 48 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > > index 67d1371..cb1a1bd 100644
> > > --- a/hw/acpi/aml-build.c
> > > +++ b/hw/acpi/aml-build.c
> > > @@ -335,3 +335,41 @@ void aml_append(Aml *parent_ctx, Aml *child)
> > > }
> > > build_append_array(parent_ctx->buf, child->buf);
> > > }
> > > +
> > > +/*
> > > + * ACPI 1.0b: 16.2.1 Top Level AML
> > > + * 5.2.3 System Description Table Header
> > > + *
> > > + * ACPI 5.0: 20.2.1 Table and Table Header Encoding
> > > + */
> > > +Aml *aml_def_block(const char *signature, uint8_t revision,
> > > + const char *oem_id, const char *oem_table_id,
> > > + uint32_t oem_revision, uint32_t creator_id,
> > > + uint32_t creator_revision)
> > > +{
> > > + int len;
> > > + Aml *var = aml_alloc();
> > > + var->block_flags = AML_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);
> > > + build_append_value(var->buf, creator_id, 4);
> > > + build_append_value(var->buf, creator_revision, 4);
> > > +
> > > + return var;
> >
> >
> > Please don't do this.
> > First, this is not a definition block encoding, so name
> > is wrong.
> >
> > Second, we already have functions to fill in headers.
> > So just call them.
> >
> > Filling structures byte by byte is unreadable -
> > I know ACPI spec often makes us do this but
> > when it doesn't, we should not do it.
> it does, in ACPI 5.1,
>
> 20.2.1 Table and Table Header Encoding
> AMLCode := DefBlockHeader TermList
> DefBlockHeader := TableSignature TableLength SpecCompliance CheckSum OemID
> OemTableID OemRevision CreatorID CreatorRevision
Sorry I'm not merging this in this form, we should use structures
where possible, this is where C shines, just use the language
properly.
> >
> > Pls keep using build_header for now.
> >
> >
> > > +}
> > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > index a1bf450..553c86b 100644
> > > --- a/hw/i386/acpi-build.c
> > > +++ b/hw/i386/acpi-build.c
> > > @@ -254,10 +254,6 @@ static void acpi_get_pci_info(PcPciInfo *info)
> > > NULL);
> > > }
> > >
> > > -#define ACPI_BUILD_APPNAME "Bochs"
> > > -#define ACPI_BUILD_APPNAME6 "BOCHS "
> > > -#define ACPI_BUILD_APPNAME4 "BXPC"
> > > -
> > > #define ACPI_BUILD_RSDP_FILE "etc/acpi/rsdp"
> > > #define ACPI_BUILD_TPMLOG_FILE "etc/tpm/log"
> > >
> > > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> > > index 4033796..2610336 100644
> > > --- a/include/hw/acpi/aml-build.h
> > > +++ b/include/hw/acpi/aml-build.h
> > > @@ -6,6 +6,10 @@
> > > #include "qemu/compiler.h"
> > >
> > > #define ACPI_BUILD_TABLE_FILE "etc/acpi/tables"
> > > +#define ACPI_BUILD_APPNAME "Bochs"
> > > +#define ACPI_BUILD_APPNAME6 "BOCHS "
> > > +#define ACPI_BUILD_APPNAME4 "BXPC"
> > > +#define ACPI_BUILD_APPNAME4_HEX 0x43505842
> > >
> > > typedef enum {
> > > AML_HELPER = 0,
> > > @@ -65,6 +69,12 @@ void free_aml_allocator(void);
> > > */
> > > void aml_append(Aml *parent_ctx, Aml *child);
> > >
> > > +/* Block AML object primitives */
> > > +Aml *aml_def_block(const char *signature, uint8_t revision,
> > > + const char *oem_id, const char *oem_table_id,
> > > + uint32_t oem_revision, uint32_t creator_id,
> > > + uint32_t creator_revision);
> > > +
> > > /* other helpers */
> > > GArray *build_alloc_array(void);
> > > void build_free_array(GArray *array);
> > > --
> > > 1.8.3.1
- [Qemu-devel] [PATCH v3 10/52] acpi: add aml_method() term, (continued)
- [Qemu-devel] [PATCH v3 07/52] acpi: add aml_scope() term, Igor Mammedov, 2015/02/09
- [Qemu-devel] [PATCH v3 05/52] acpi: add aml_def_block() term, Igor Mammedov, 2015/02/09
- Re: [Qemu-devel] [PATCH v3 05/52] acpi: add aml_def_block() term, Michael S. Tsirkin, 2015/02/17
- Re: [Qemu-devel] [PATCH v3 05/52] acpi: add aml_def_block() term, Igor Mammedov, 2015/02/18
- Re: [Qemu-devel] [PATCH v3 05/52] acpi: add aml_def_block() term, Michael S. Tsirkin, 2015/02/18
[Qemu-devel] [PATCH v3 04/52] acpi: factor out ACPI const int packing out of build_append_value(), Igor Mammedov, 2015/02/09
[Qemu-devel] [PATCH v3 11/52] acpi: add aml_if() term, Igor Mammedov, 2015/02/09