| Message ID | 20161010003235.4213-12-haozhong.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Oct 10, 2016 at 08:32:30AM +0800, Haozhong Zhang wrote: > ACPI tables built by the device model, whose signatures do not > conflict with tables built by Xen (except SSDT), are loaded after ACPI > tables built by Xen. > > ACPI namespace devices built by the device model, whose names do not > conflict with devices built by Xen, are assembled and placed in SSDTs > after ACPI tables built by Xen. > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > --- > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Wei Liu <wei.liu2@citrix.com> > --- > tools/firmware/hvmloader/util.c | 12 +++ > tools/libacpi/acpi2_0.h | 2 + > tools/libacpi/build.c | 216 ++++++++++++++++++++++++++++++++++++++++ > tools/libacpi/libacpi.h | 5 + > 4 files changed, 235 insertions(+) > > diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c > index dba954a..e6530cd 100644 > --- a/tools/firmware/hvmloader/util.c > +++ b/tools/firmware/hvmloader/util.c > @@ -998,6 +998,18 @@ void hvmloader_acpi_build_tables(struct acpi_config *config, > if ( !strncmp(xenstore_read("platform/acpi_s4", "1"), "1", 1) ) > config->table_flags |= ACPI_HAS_SSDT_S4; > > + s = xenstore_read(HVM_XS_DM_ACPI_ADDRESS, NULL); > + if ( s ) > + { > + config->dm.addr = strtoll(s, NULL, 0); > + > + s = xenstore_read(HVM_XS_DM_ACPI_LENGTH, NULL); > + if ( s ) > + config->dm.length = strtoll(s, NULL, 0); > + else > + config->dm.addr = 0; > + } > + > config->table_flags |= (ACPI_HAS_TCPA | ACPI_HAS_IOAPIC | ACPI_HAS_WAET); > > config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS; > diff --git a/tools/libacpi/acpi2_0.h b/tools/libacpi/acpi2_0.h > index 775eb7a..7414470 100644 > --- a/tools/libacpi/acpi2_0.h > +++ b/tools/libacpi/acpi2_0.h > @@ -430,6 +430,7 @@ struct acpi_20_slit { > #define ACPI_2_0_WAET_SIGNATURE ASCII32('W','A','E','T') > #define ACPI_2_0_SRAT_SIGNATURE ASCII32('S','R','A','T') > #define ACPI_2_0_SLIT_SIGNATURE ASCII32('S','L','I','T') > +#define ACPI_2_0_SSDT_SIGNATURE ASCII32('S','S','D','T') > > /* > * Table revision numbers. > @@ -445,6 +446,7 @@ struct acpi_20_slit { > #define ACPI_1_0_FADT_REVISION 0x01 > #define ACPI_2_0_SRAT_REVISION 0x01 > #define ACPI_2_0_SLIT_REVISION 0x01 > +#define ACPI_2_0_SSDT_REVISION 0x02 > > #pragma pack () > > diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c > index 47dae01..829a365 100644 > --- a/tools/libacpi/build.c > +++ b/tools/libacpi/build.c > @@ -20,6 +20,7 @@ > #include "ssdt_s4.h" > #include "ssdt_tpm.h" > #include "ssdt_pm.h" > +#include "aml_build.h" > #include <xen/hvm/hvm_info_table.h> > #include <xen/hvm/hvm_xs_strings.h> > #include <xen/hvm/params.h> > @@ -55,6 +56,34 @@ struct acpi_info { > uint64_t pci_hi_min, pci_hi_len; /* 24, 32 - PCI I/O hole boundaries */ > }; > > +#define DM_ACPI_BLOB_TYPE_TABLE 0 /* ACPI table */ > +#define DM_ACPI_BLOB_TYPE_NSDEV 1 /* AML definition of an ACPI namespace device */ > + > +/* ACPI tables of following signatures should not appear in DM ACPI */ It would be good to have some form of build check to check against this list.. > +static const uint64_t dm_acpi_signature_blacklist[] = { > + ACPI_2_0_RSDP_SIGNATURE, > + ACPI_2_0_FACS_SIGNATURE, > + ACPI_2_0_FADT_SIGNATURE, > + ACPI_2_0_MADT_SIGNATURE, > + ACPI_2_0_RSDT_SIGNATURE, > + ACPI_2_0_XSDT_SIGNATURE, > + ACPI_2_0_TCPA_SIGNATURE, > + ACPI_2_0_HPET_SIGNATURE, > + ACPI_2_0_WAET_SIGNATURE, > + ACPI_2_0_SRAT_SIGNATURE, > + ACPI_2_0_SLIT_SIGNATURE, > +}; > + > +/* ACPI namespace devices of following names should not appear in DM ACPI */ > +static const char *dm_acpi_devname_blacklist[] = { > + "MEM0", > + "PCI0", > + "AC", > + "BAT0", > + "BAT1", > + "TPM", .. and this one. But I am not even sure how one would do that? Perhaps add a big warning: "Make sure to add your table name if you this code (libacpi) is constructing it. "? Or maybe have some 'register_acpi_table' function which will expand this blacklist? > +}; > + > static void set_checksum( > void *table, uint32_t checksum_offset, uint32_t length) > { > @@ -339,6 +368,190 @@ static int construct_passthrough_tables(struct acpi_ctxt *ctxt, > return nr_added; > } > > +#define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0])) That may want to go libacpi.h ? > + > +static int check_signature_collision(uint64_t sig) bool > +{ > + int i; unsigned int > + for ( i = 0; i < ARRAY_SIZE(dm_acpi_signature_blacklist); i++ ) > + { > + if ( sig == dm_acpi_signature_blacklist[i] ) > + return 1; return true > + } > + return 0; > +} > + > +static int check_devname_collision(const char *name) bool > +{ > + int i; unsigned int > + for ( i = 0; i < ARRAY_SIZE(dm_acpi_devname_blacklist); i++ ) > + { > + if ( !strncmp(name, dm_acpi_devname_blacklist[i], 4) ) That 4 could be a #define > + return 1; > + } > + return 0; > +} > + > +static const char *xs_read_dm_acpi_blob_key(struct acpi_ctxt *ctxt, > + const char *name, const char *key) > +{ > +#define DM_ACPI_BLOB_PATH_MAX_LENGTH 30 > + char path[DM_ACPI_BLOB_PATH_MAX_LENGTH]; > + snprintf(path, DM_ACPI_BLOB_PATH_MAX_LENGTH, HVM_XS_DM_ACPI_ROOT"/%s/%s", > + name, key); > + return ctxt->xs_ops.read(ctxt, path); #undef DM_APCI_BLOB... but perhaps that should go in xen/include/public/hvm/hvm_xs_strings.h ? > +} > + > +static int construct_dm_table(struct acpi_ctxt *ctxt, bool > + unsigned long *table_ptrs, int nr_tables, unsigned int nr_tables > + const void *blob, uint32_t length) > +{ > + const struct acpi_header *header = blob; > + uint8_t *buffer; > + > + if ( check_signature_collision(header->signature) ) > + return 0; > + > + if ( header->length > length || header->length == 0 ) > + return 0; > + > + buffer = ctxt->mem_ops.alloc(ctxt, header->length, 16); > + if ( !buffer ) > + return 0; > + memcpy(buffer, header, header->length); > + > + /* some device models (e.g. QEMU) does not set checksum */ > + set_checksum(buffer, offsetof(struct acpi_header, checksum), > + header->length); > + > + table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, buffer); > + > + return 1; > +} > + > +static int construct_dm_nsdev(struct acpi_ctxt *ctxt, bool > + unsigned long *table_ptrs, int nr_tables, unsigned int > + const char *dev_name, > + const void *blob, uint32_t blob_length) > +{ > + struct acpi_header ssdt, *header; > + uint8_t *buffer; > + > + if ( check_devname_collision(dev_name) ) > + return 0; > + > + /* built ACPI namespace device from [name, blob] */ > + buffer = aml_build_begin(ctxt); > + aml_prepend_blob(buffer, blob, blob_length); > + aml_prepend_device(buffer, dev_name); > + aml_prepend_scope(buffer, "\\_SB"); > + > + /* build SSDT header */ > + ssdt.signature = ACPI_2_0_SSDT_SIGNATURE; > + ssdt.revision = ACPI_2_0_SSDT_REVISION; > + fixed_strcpy(ssdt.oem_id, ACPI_OEM_ID); > + fixed_strcpy(ssdt.oem_table_id, ACPI_OEM_TABLE_ID); > + ssdt.oem_revision = ACPI_OEM_REVISION; > + ssdt.creator_id = ACPI_CREATOR_ID; > + ssdt.creator_revision = ACPI_CREATOR_REVISION; > + > + /* prepend SSDT header to ACPI namespace device */ > + aml_prepend_blob(buffer, &ssdt, sizeof(ssdt)); > + header = (struct acpi_header *) buffer; > + header->length = aml_build_end(); > + > + /* calculate checksum of SSDT */ > + set_checksum(header, offsetof(struct acpi_header, checksum), > + header->length); > + > + table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, buffer); > + > + return 1; return true. > +} > + > +/* > + * All ACPI stuffs built by the device model are placed in the guest > + * buffer whose address and size are specified by config->dm.{addr, length}, > + * or XenStore keys HVM_XS_DM_ACPI_{ADDRESS, LENGTH}. This should be also provided in xen/include/public/hvm/hvm_xs_strings.h Especially as you are in effect adding new keys and attributes to it. > + * > + * The data layout within the buffer is further specified by XenStore > + * directories under HVM_XS_DM_ACPI_ROOT. Each directory specifies a Is each directory the name of the DSDT object? In which case you want to say a bit about the directory name and the limit (only four characters long), don't use the ones that are built-in, etc.. But it looks like it can be anything. You extract the name from the blob. But we should still say what the directory names ought to be. > + * data blob and contains following XenStore keys: > + * > + * - "type": > + * * DM_ACPI_BLOB_TYPE_TABLE > + * The data blob specified by this directory is an ACPI table. > + * * DM_ACPI_BLOB_TYPE_NSDEV > + * The data blob specified by this directory is an ACPI namespace device. > + * Its name is specified by the directory name, while the AML code of the > + * body of the AML device structure is in the data blob. Could those be strings on XenStore? Strings are nice. "table" or "nsdev"? > + * > + * - "length": the number of bytes in this data blob. > + * > + * - "offset": the offset in bytes of this data blob from the beginning of buffer > + */ > +static int construct_dm_tables(struct acpi_ctxt *ctxt, static unsigned int > + unsigned long *table_ptrs, > + int nr_tables, unsigned int nr_tables > + struct acpi_config *config) > +{ > + const char *s; > + char **dir; > + uint8_t type; > + void *blob; > + unsigned int num, length, offset, i; > + int nr_added = 0; unsigned int > + > + if ( !config->dm.addr ) > + return 0; > + > + dir = ctxt->xs_ops.directory(ctxt, HVM_XS_DM_ACPI_ROOT, &num); > + if ( !dir || !num ) > + return 0; > + > + if ( num > ACPI_MAX_SECONDARY_TABLES - nr_tables ) > + return 0; > + > + for ( i = 0; i < num; i++, dir++ ) > + { You probably want to check that *dir is not NULL. Just in case. > + s = xs_read_dm_acpi_blob_key(ctxt, *dir, "type"); > + if ( !s ) > + continue; > + type = (uint8_t)strtoll(s, NULL, 0); > + > + s = xs_read_dm_acpi_blob_key(ctxt, *dir, "length"); > + if ( !s ) > + continue; > + length = (uint32_t)strtoll(s, NULL, 0); > + > + s = xs_read_dm_acpi_blob_key(ctxt, *dir, "offset"); > + if ( !s ) > + continue; > + offset = (uint32_t)strtoll(s, NULL, 0); > + > + blob = ctxt->mem_ops.p2v(ctxt, config->dm.addr + offset); > + > + switch ( type ) > + { > + case DM_ACPI_BLOB_TYPE_TABLE: > + nr_added += construct_dm_table(ctxt, > + table_ptrs, nr_tables + nr_added, > + blob, length); > + break; > + case DM_ACPI_BLOB_TYPE_NSDEV: > + nr_added += construct_dm_nsdev(ctxt, > + table_ptrs, nr_tables + nr_added, > + *dir, blob, length); > + break; > + default: > + /* skip blobs of unknown types */ > + continue; > + } > + } > + > + return nr_added; > +} > + > static int construct_secondary_tables(struct acpi_ctxt *ctxt, > unsigned long *table_ptrs, > struct acpi_config *config, > @@ -461,6 +674,9 @@ static int construct_secondary_tables(struct acpi_ctxt *ctxt, > nr_tables += construct_passthrough_tables(ctxt, table_ptrs, > nr_tables, config); > > + /* Load any additional tables passed from device model (e.g. QEMU) */ Perhaps an period at the end of the sentence? > + nr_tables += construct_dm_tables(ctxt, table_ptrs, nr_tables, config); > + > table_ptrs[nr_tables] = 0; > return nr_tables; > } > diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h > index 12cafd8..684502d 100644 > --- a/tools/libacpi/libacpi.h > +++ b/tools/libacpi/libacpi.h > @@ -82,6 +82,11 @@ struct acpi_config { > uint32_t length; > } pt; > > + struct { > + uint32_t addr; > + uint32_t length; > + } dm; > + > struct acpi_numa numa; > const struct hvm_info_table *hvminfo; > > -- > 2.10.1 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > https://lists.xen.org/xen-devel
On 01/27/17 16:40 -0500, Konrad Rzeszutek Wilk wrote: >On Mon, Oct 10, 2016 at 08:32:30AM +0800, Haozhong Zhang wrote: >> ACPI tables built by the device model, whose signatures do not >> conflict with tables built by Xen (except SSDT), are loaded after ACPI >> tables built by Xen. >> >> ACPI namespace devices built by the device model, whose names do not >> conflict with devices built by Xen, are assembled and placed in SSDTs >> after ACPI tables built by Xen. >> >> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> >> --- >> Cc: Jan Beulich <jbeulich@suse.com> >> Cc: Andrew Cooper <andrew.cooper3@citrix.com> >> Cc: Ian Jackson <ian.jackson@eu.citrix.com> >> Cc: Wei Liu <wei.liu2@citrix.com> >> --- >> tools/firmware/hvmloader/util.c | 12 +++ >> tools/libacpi/acpi2_0.h | 2 + >> tools/libacpi/build.c | 216 ++++++++++++++++++++++++++++++++++++++++ >> tools/libacpi/libacpi.h | 5 + >> 4 files changed, 235 insertions(+) >> >> diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c >> index dba954a..e6530cd 100644 >> --- a/tools/firmware/hvmloader/util.c >> +++ b/tools/firmware/hvmloader/util.c >> @@ -998,6 +998,18 @@ void hvmloader_acpi_build_tables(struct acpi_config *config, >> if ( !strncmp(xenstore_read("platform/acpi_s4", "1"), "1", 1) ) >> config->table_flags |= ACPI_HAS_SSDT_S4; >> >> + s = xenstore_read(HVM_XS_DM_ACPI_ADDRESS, NULL); >> + if ( s ) >> + { >> + config->dm.addr = strtoll(s, NULL, 0); >> + >> + s = xenstore_read(HVM_XS_DM_ACPI_LENGTH, NULL); >> + if ( s ) >> + config->dm.length = strtoll(s, NULL, 0); >> + else >> + config->dm.addr = 0; >> + } >> + >> config->table_flags |= (ACPI_HAS_TCPA | ACPI_HAS_IOAPIC | ACPI_HAS_WAET); >> >> config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS; >> diff --git a/tools/libacpi/acpi2_0.h b/tools/libacpi/acpi2_0.h >> index 775eb7a..7414470 100644 >> --- a/tools/libacpi/acpi2_0.h >> +++ b/tools/libacpi/acpi2_0.h >> @@ -430,6 +430,7 @@ struct acpi_20_slit { >> #define ACPI_2_0_WAET_SIGNATURE ASCII32('W','A','E','T') >> #define ACPI_2_0_SRAT_SIGNATURE ASCII32('S','R','A','T') >> #define ACPI_2_0_SLIT_SIGNATURE ASCII32('S','L','I','T') >> +#define ACPI_2_0_SSDT_SIGNATURE ASCII32('S','S','D','T') >> >> /* >> * Table revision numbers. >> @@ -445,6 +446,7 @@ struct acpi_20_slit { >> #define ACPI_1_0_FADT_REVISION 0x01 >> #define ACPI_2_0_SRAT_REVISION 0x01 >> #define ACPI_2_0_SLIT_REVISION 0x01 >> +#define ACPI_2_0_SSDT_REVISION 0x02 >> >> #pragma pack () >> >> diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c >> index 47dae01..829a365 100644 >> --- a/tools/libacpi/build.c >> +++ b/tools/libacpi/build.c >> @@ -20,6 +20,7 @@ >> #include "ssdt_s4.h" >> #include "ssdt_tpm.h" >> #include "ssdt_pm.h" >> +#include "aml_build.h" >> #include <xen/hvm/hvm_info_table.h> >> #include <xen/hvm/hvm_xs_strings.h> >> #include <xen/hvm/params.h> >> @@ -55,6 +56,34 @@ struct acpi_info { >> uint64_t pci_hi_min, pci_hi_len; /* 24, 32 - PCI I/O hole boundaries */ >> }; >> >> +#define DM_ACPI_BLOB_TYPE_TABLE 0 /* ACPI table */ >> +#define DM_ACPI_BLOB_TYPE_NSDEV 1 /* AML definition of an ACPI namespace device */ >> + >> +/* ACPI tables of following signatures should not appear in DM ACPI */ > >It would be good to have some form of build check to check against >this list.. >> +static const uint64_t dm_acpi_signature_blacklist[] = { >> + ACPI_2_0_RSDP_SIGNATURE, >> + ACPI_2_0_FACS_SIGNATURE, >> + ACPI_2_0_FADT_SIGNATURE, >> + ACPI_2_0_MADT_SIGNATURE, >> + ACPI_2_0_RSDT_SIGNATURE, >> + ACPI_2_0_XSDT_SIGNATURE, >> + ACPI_2_0_TCPA_SIGNATURE, >> + ACPI_2_0_HPET_SIGNATURE, >> + ACPI_2_0_WAET_SIGNATURE, >> + ACPI_2_0_SRAT_SIGNATURE, >> + ACPI_2_0_SLIT_SIGNATURE, >> +}; >> + >> +/* ACPI namespace devices of following names should not appear in DM ACPI */ >> +static const char *dm_acpi_devname_blacklist[] = { >> + "MEM0", >> + "PCI0", >> + "AC", >> + "BAT0", >> + "BAT1", >> + "TPM", > >.. and this one. > >But I am not even sure how one would do that? > >Perhaps add a big warning: > >"Make sure to add your table name if you this code (libacpi) is >constructing it. "? > >Or maybe have some 'register_acpi_table' function which will expand >this blacklist? > I think the later is better. >> +}; >> + >> static void set_checksum( >> void *table, uint32_t checksum_offset, uint32_t length) >> { >> @@ -339,6 +368,190 @@ static int construct_passthrough_tables(struct acpi_ctxt *ctxt, >> return nr_added; >> } >> >> +#define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0])) > >That may want to go libacpi.h ? will move to libacpi.h, so others can use it in future. >> + >> +static int check_signature_collision(uint64_t sig) > >bool will change here and below >> +{ >> + int i; > >unsigned int ditto > >> + for ( i = 0; i < ARRAY_SIZE(dm_acpi_signature_blacklist); i++ ) >> + { >> + if ( sig == dm_acpi_signature_blacklist[i] ) >> + return 1; > >return true > ditto >> + } >> + return 0; >> +} >> + >> +static int check_devname_collision(const char *name) > >bool >> +{ >> + int i; > >unsigned int > >> + for ( i = 0; i < ARRAY_SIZE(dm_acpi_devname_blacklist); i++ ) >> + { >> + if ( !strncmp(name, dm_acpi_devname_blacklist[i], 4) ) > >That 4 could be a #define yes >> + return 1; >> + } >> + return 0; >> +} >> + >> +static const char *xs_read_dm_acpi_blob_key(struct acpi_ctxt *ctxt, >> + const char *name, const char *key) >> +{ >> +#define DM_ACPI_BLOB_PATH_MAX_LENGTH 30 >> + char path[DM_ACPI_BLOB_PATH_MAX_LENGTH]; >> + snprintf(path, DM_ACPI_BLOB_PATH_MAX_LENGTH, HVM_XS_DM_ACPI_ROOT"/%s/%s", >> + name, key); >> + return ctxt->xs_ops.read(ctxt, path); > >#undef DM_APCI_BLOB... but perhaps that should go in >xen/include/public/hvm/hvm_xs_strings.h ? > will move to the header file >> +} >> + >> +static int construct_dm_table(struct acpi_ctxt *ctxt, > >bool >> + unsigned long *table_ptrs, int nr_tables, > >unsigned int nr_tables >> + const void *blob, uint32_t length) >> +{ >> + const struct acpi_header *header = blob; >> + uint8_t *buffer; >> + >> + if ( check_signature_collision(header->signature) ) >> + return 0; >> + >> + if ( header->length > length || header->length == 0 ) >> + return 0; >> + >> + buffer = ctxt->mem_ops.alloc(ctxt, header->length, 16); >> + if ( !buffer ) >> + return 0; >> + memcpy(buffer, header, header->length); >> + >> + /* some device models (e.g. QEMU) does not set checksum */ >> + set_checksum(buffer, offsetof(struct acpi_header, checksum), >> + header->length); >> + >> + table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, buffer); >> + >> + return 1; >> +} >> + >> +static int construct_dm_nsdev(struct acpi_ctxt *ctxt, > >bool >> + unsigned long *table_ptrs, int nr_tables, > >unsigned int >> + const char *dev_name, >> + const void *blob, uint32_t blob_length) >> +{ >> + struct acpi_header ssdt, *header; >> + uint8_t *buffer; >> + >> + if ( check_devname_collision(dev_name) ) >> + return 0; >> + >> + /* built ACPI namespace device from [name, blob] */ >> + buffer = aml_build_begin(ctxt); >> + aml_prepend_blob(buffer, blob, blob_length); >> + aml_prepend_device(buffer, dev_name); >> + aml_prepend_scope(buffer, "\\_SB"); >> + >> + /* build SSDT header */ >> + ssdt.signature = ACPI_2_0_SSDT_SIGNATURE; >> + ssdt.revision = ACPI_2_0_SSDT_REVISION; >> + fixed_strcpy(ssdt.oem_id, ACPI_OEM_ID); >> + fixed_strcpy(ssdt.oem_table_id, ACPI_OEM_TABLE_ID); >> + ssdt.oem_revision = ACPI_OEM_REVISION; >> + ssdt.creator_id = ACPI_CREATOR_ID; >> + ssdt.creator_revision = ACPI_CREATOR_REVISION; >> + >> + /* prepend SSDT header to ACPI namespace device */ >> + aml_prepend_blob(buffer, &ssdt, sizeof(ssdt)); >> + header = (struct acpi_header *) buffer; >> + header->length = aml_build_end(); >> + >> + /* calculate checksum of SSDT */ >> + set_checksum(header, offsetof(struct acpi_header, checksum), >> + header->length); >> + >> + table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, buffer); >> + >> + return 1; > >return true. >> +} >> + >> +/* >> + * All ACPI stuffs built by the device model are placed in the guest >> + * buffer whose address and size are specified by config->dm.{addr, length}, >> + * or XenStore keys HVM_XS_DM_ACPI_{ADDRESS, LENGTH}. > >This should be also provided in >xen/include/public/hvm/hvm_xs_strings.h > >Especially as you are in effect adding new keys and attributes to it. > yes, will move to the header file. >> + * >> + * The data layout within the buffer is further specified by XenStore >> + * directories under HVM_XS_DM_ACPI_ROOT. Each directory specifies a > >Is each directory the name of the DSDT object? In which case you >want to say a bit about the directory name and the limit (only four >characters long), don't use the ones that are built-in, etc.. > If the type is DM_ACPI_BLOB_TYPE_TABLE, the directory name makes little sense as long as it's different than other names. If the type is DM_ACPI_BLOB_TYPE_NSDEV, the directory name is the name of namespace device (see the comment in the patch about DM_ACPI_BLOB_TYPE_NSDEV below), such as "NVDR" below Scope (\_SB) { Device (NVDR) { ... } } I'll add comments about the limitation of the name. >But it looks like it can be anything. You extract the name from >the blob. > >But we should still say what the directory names ought to be. > >> + * data blob and contains following XenStore keys: >> + * >> + * - "type": >> + * * DM_ACPI_BLOB_TYPE_TABLE >> + * The data blob specified by this directory is an ACPI table. >> + * * DM_ACPI_BLOB_TYPE_NSDEV >> + * The data blob specified by this directory is an ACPI namespace device. >> + * Its name is specified by the directory name, while the AML code of the >> + * body of the AML device structure is in the data blob. > >Could those be strings on XenStore? Strings are nice. "table" or >"nsdev"? I'm not object to use a string, but isn't an integer easier for programs to parse? Or you are considering it should also be human readable? >> + * >> + * - "length": the number of bytes in this data blob. >> + * >> + * - "offset": the offset in bytes of this data blob from the beginning of buffer >> + */ >> +static int construct_dm_tables(struct acpi_ctxt *ctxt, > >static unsigned int >> + unsigned long *table_ptrs, >> + int nr_tables, > >unsigned int nr_tables OK. As long as ACPI_MAX_SECONDARY_TABLES (i.e. the max value of nr_tables) is smaller than INT_MAX, the signed -> unsigned type will not cause trouble to its caller. >> + struct acpi_config *config) >> +{ >> + const char *s; >> + char **dir; >> + uint8_t type; >> + void *blob; >> + unsigned int num, length, offset, i; >> + int nr_added = 0; > >unsigned int >> + >> + if ( !config->dm.addr ) >> + return 0; >> + >> + dir = ctxt->xs_ops.directory(ctxt, HVM_XS_DM_ACPI_ROOT, &num); >> + if ( !dir || !num ) >> + return 0; >> + >> + if ( num > ACPI_MAX_SECONDARY_TABLES - nr_tables ) >> + return 0; >> + >> + for ( i = 0; i < num; i++, dir++ ) >> + { > >You probably want to check that *dir is not NULL. Just in case. will do > >> + s = xs_read_dm_acpi_blob_key(ctxt, *dir, "type"); >> + if ( !s ) >> + continue; >> + type = (uint8_t)strtoll(s, NULL, 0); >> + >> + s = xs_read_dm_acpi_blob_key(ctxt, *dir, "length"); >> + if ( !s ) >> + continue; >> + length = (uint32_t)strtoll(s, NULL, 0); >> + >> + s = xs_read_dm_acpi_blob_key(ctxt, *dir, "offset"); >> + if ( !s ) >> + continue; >> + offset = (uint32_t)strtoll(s, NULL, 0); >> + >> + blob = ctxt->mem_ops.p2v(ctxt, config->dm.addr + offset); >> + >> + switch ( type ) >> + { >> + case DM_ACPI_BLOB_TYPE_TABLE: >> + nr_added += construct_dm_table(ctxt, >> + table_ptrs, nr_tables + nr_added, >> + blob, length); >> + break; >> + case DM_ACPI_BLOB_TYPE_NSDEV: >> + nr_added += construct_dm_nsdev(ctxt, >> + table_ptrs, nr_tables + nr_added, >> + *dir, blob, length); >> + break; >> + default: >> + /* skip blobs of unknown types */ >> + continue; >> + } >> + } >> + >> + return nr_added; >> +} >> + >> static int construct_secondary_tables(struct acpi_ctxt *ctxt, >> unsigned long *table_ptrs, >> struct acpi_config *config, >> @@ -461,6 +674,9 @@ static int construct_secondary_tables(struct acpi_ctxt *ctxt, >> nr_tables += construct_passthrough_tables(ctxt, table_ptrs, >> nr_tables, config); >> >> + /* Load any additional tables passed from device model (e.g. QEMU) */ > >Perhaps an period at the end of the sentence? Yes. Thanks, Haozhong > >> + nr_tables += construct_dm_tables(ctxt, table_ptrs, nr_tables, config); >> + >> table_ptrs[nr_tables] = 0; >> return nr_tables; >> } >> diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h >> index 12cafd8..684502d 100644 >> --- a/tools/libacpi/libacpi.h >> +++ b/tools/libacpi/libacpi.h >> @@ -82,6 +82,11 @@ struct acpi_config { >> uint32_t length; >> } pt; >> >> + struct { >> + uint32_t addr; >> + uint32_t length; >> + } dm; >> + >> struct acpi_numa numa; >> const struct hvm_info_table *hvminfo; >> >> -- >> 2.10.1 >> >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xen.org >> https://lists.xen.org/xen-devel
.giant snip.. > > > + * - "type": > > > + * * DM_ACPI_BLOB_TYPE_TABLE > > > + * The data blob specified by this directory is an ACPI table. > > > + * * DM_ACPI_BLOB_TYPE_NSDEV > > > + * The data blob specified by this directory is an ACPI namespace device. > > > + * Its name is specified by the directory name, while the AML code of the > > > + * body of the AML device structure is in the data blob. > > > > Could those be strings on XenStore? Strings are nice. "table" or > > "nsdev"? > > I'm not object to use a string, but isn't an integer easier for > programs to parse? Or you are considering it should also be human > readable? Integers are easier. But it needs to be defined in the public header files.
diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c index dba954a..e6530cd 100644 --- a/tools/firmware/hvmloader/util.c +++ b/tools/firmware/hvmloader/util.c @@ -998,6 +998,18 @@ void hvmloader_acpi_build_tables(struct acpi_config *config, if ( !strncmp(xenstore_read("platform/acpi_s4", "1"), "1", 1) ) config->table_flags |= ACPI_HAS_SSDT_S4; + s = xenstore_read(HVM_XS_DM_ACPI_ADDRESS, NULL); + if ( s ) + { + config->dm.addr = strtoll(s, NULL, 0); + + s = xenstore_read(HVM_XS_DM_ACPI_LENGTH, NULL); + if ( s ) + config->dm.length = strtoll(s, NULL, 0); + else + config->dm.addr = 0; + } + config->table_flags |= (ACPI_HAS_TCPA | ACPI_HAS_IOAPIC | ACPI_HAS_WAET); config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS; diff --git a/tools/libacpi/acpi2_0.h b/tools/libacpi/acpi2_0.h index 775eb7a..7414470 100644 --- a/tools/libacpi/acpi2_0.h +++ b/tools/libacpi/acpi2_0.h @@ -430,6 +430,7 @@ struct acpi_20_slit { #define ACPI_2_0_WAET_SIGNATURE ASCII32('W','A','E','T') #define ACPI_2_0_SRAT_SIGNATURE ASCII32('S','R','A','T') #define ACPI_2_0_SLIT_SIGNATURE ASCII32('S','L','I','T') +#define ACPI_2_0_SSDT_SIGNATURE ASCII32('S','S','D','T') /* * Table revision numbers. @@ -445,6 +446,7 @@ struct acpi_20_slit { #define ACPI_1_0_FADT_REVISION 0x01 #define ACPI_2_0_SRAT_REVISION 0x01 #define ACPI_2_0_SLIT_REVISION 0x01 +#define ACPI_2_0_SSDT_REVISION 0x02 #pragma pack () diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c index 47dae01..829a365 100644 --- a/tools/libacpi/build.c +++ b/tools/libacpi/build.c @@ -20,6 +20,7 @@ #include "ssdt_s4.h" #include "ssdt_tpm.h" #include "ssdt_pm.h" +#include "aml_build.h" #include <xen/hvm/hvm_info_table.h> #include <xen/hvm/hvm_xs_strings.h> #include <xen/hvm/params.h> @@ -55,6 +56,34 @@ struct acpi_info { uint64_t pci_hi_min, pci_hi_len; /* 24, 32 - PCI I/O hole boundaries */ }; +#define DM_ACPI_BLOB_TYPE_TABLE 0 /* ACPI table */ +#define DM_ACPI_BLOB_TYPE_NSDEV 1 /* AML definition of an ACPI namespace device */ + +/* ACPI tables of following signatures should not appear in DM ACPI */ +static const uint64_t dm_acpi_signature_blacklist[] = { + ACPI_2_0_RSDP_SIGNATURE, + ACPI_2_0_FACS_SIGNATURE, + ACPI_2_0_FADT_SIGNATURE, + ACPI_2_0_MADT_SIGNATURE, + ACPI_2_0_RSDT_SIGNATURE, + ACPI_2_0_XSDT_SIGNATURE, + ACPI_2_0_TCPA_SIGNATURE, + ACPI_2_0_HPET_SIGNATURE, + ACPI_2_0_WAET_SIGNATURE, + ACPI_2_0_SRAT_SIGNATURE, + ACPI_2_0_SLIT_SIGNATURE, +}; + +/* ACPI namespace devices of following names should not appear in DM ACPI */ +static const char *dm_acpi_devname_blacklist[] = { + "MEM0", + "PCI0", + "AC", + "BAT0", + "BAT1", + "TPM", +}; + static void set_checksum( void *table, uint32_t checksum_offset, uint32_t length) { @@ -339,6 +368,190 @@ static int construct_passthrough_tables(struct acpi_ctxt *ctxt, return nr_added; } +#define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0])) + +static int check_signature_collision(uint64_t sig) +{ + int i; + for ( i = 0; i < ARRAY_SIZE(dm_acpi_signature_blacklist); i++ ) + { + if ( sig == dm_acpi_signature_blacklist[i] ) + return 1; + } + return 0; +} + +static int check_devname_collision(const char *name) +{ + int i; + for ( i = 0; i < ARRAY_SIZE(dm_acpi_devname_blacklist); i++ ) + { + if ( !strncmp(name, dm_acpi_devname_blacklist[i], 4) ) + return 1; + } + return 0; +} + +static const char *xs_read_dm_acpi_blob_key(struct acpi_ctxt *ctxt, + const char *name, const char *key) +{ +#define DM_ACPI_BLOB_PATH_MAX_LENGTH 30 + char path[DM_ACPI_BLOB_PATH_MAX_LENGTH]; + snprintf(path, DM_ACPI_BLOB_PATH_MAX_LENGTH, HVM_XS_DM_ACPI_ROOT"/%s/%s", + name, key); + return ctxt->xs_ops.read(ctxt, path); +} + +static int construct_dm_table(struct acpi_ctxt *ctxt, + unsigned long *table_ptrs, int nr_tables, + const void *blob, uint32_t length) +{ + const struct acpi_header *header = blob; + uint8_t *buffer; + + if ( check_signature_collision(header->signature) ) + return 0; + + if ( header->length > length || header->length == 0 ) + return 0; + + buffer = ctxt->mem_ops.alloc(ctxt, header->length, 16); + if ( !buffer ) + return 0; + memcpy(buffer, header, header->length); + + /* some device models (e.g. QEMU) does not set checksum */ + set_checksum(buffer, offsetof(struct acpi_header, checksum), + header->length); + + table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, buffer); + + return 1; +} + +static int construct_dm_nsdev(struct acpi_ctxt *ctxt, + unsigned long *table_ptrs, int nr_tables, + const char *dev_name, + const void *blob, uint32_t blob_length) +{ + struct acpi_header ssdt, *header; + uint8_t *buffer; + + if ( check_devname_collision(dev_name) ) + return 0; + + /* built ACPI namespace device from [name, blob] */ + buffer = aml_build_begin(ctxt); + aml_prepend_blob(buffer, blob, blob_length); + aml_prepend_device(buffer, dev_name); + aml_prepend_scope(buffer, "\\_SB"); + + /* build SSDT header */ + ssdt.signature = ACPI_2_0_SSDT_SIGNATURE; + ssdt.revision = ACPI_2_0_SSDT_REVISION; + fixed_strcpy(ssdt.oem_id, ACPI_OEM_ID); + fixed_strcpy(ssdt.oem_table_id, ACPI_OEM_TABLE_ID); + ssdt.oem_revision = ACPI_OEM_REVISION; + ssdt.creator_id = ACPI_CREATOR_ID; + ssdt.creator_revision = ACPI_CREATOR_REVISION; + + /* prepend SSDT header to ACPI namespace device */ + aml_prepend_blob(buffer, &ssdt, sizeof(ssdt)); + header = (struct acpi_header *) buffer; + header->length = aml_build_end(); + + /* calculate checksum of SSDT */ + set_checksum(header, offsetof(struct acpi_header, checksum), + header->length); + + table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, buffer); + + return 1; +} + +/* + * All ACPI stuffs built by the device model are placed in the guest + * buffer whose address and size are specified by config->dm.{addr, length}, + * or XenStore keys HVM_XS_DM_ACPI_{ADDRESS, LENGTH}. + * + * The data layout within the buffer is further specified by XenStore + * directories under HVM_XS_DM_ACPI_ROOT. Each directory specifies a + * data blob and contains following XenStore keys: + * + * - "type": + * * DM_ACPI_BLOB_TYPE_TABLE + * The data blob specified by this directory is an ACPI table. + * * DM_ACPI_BLOB_TYPE_NSDEV + * The data blob specified by this directory is an ACPI namespace device. + * Its name is specified by the directory name, while the AML code of the + * body of the AML device structure is in the data blob. + * + * - "length": the number of bytes in this data blob. + * + * - "offset": the offset in bytes of this data blob from the beginning of buffer + */ +static int construct_dm_tables(struct acpi_ctxt *ctxt, + unsigned long *table_ptrs, + int nr_tables, + struct acpi_config *config) +{ + const char *s; + char **dir; + uint8_t type; + void *blob; + unsigned int num, length, offset, i; + int nr_added = 0; + + if ( !config->dm.addr ) + return 0; + + dir = ctxt->xs_ops.directory(ctxt, HVM_XS_DM_ACPI_ROOT, &num); + if ( !dir || !num ) + return 0; + + if ( num > ACPI_MAX_SECONDARY_TABLES - nr_tables ) + return 0; + + for ( i = 0; i < num; i++, dir++ ) + { + s = xs_read_dm_acpi_blob_key(ctxt, *dir, "type"); + if ( !s ) + continue; + type = (uint8_t)strtoll(s, NULL, 0); + + s = xs_read_dm_acpi_blob_key(ctxt, *dir, "length"); + if ( !s ) + continue; + length = (uint32_t)strtoll(s, NULL, 0); + + s = xs_read_dm_acpi_blob_key(ctxt, *dir, "offset"); + if ( !s ) + continue; + offset = (uint32_t)strtoll(s, NULL, 0); + + blob = ctxt->mem_ops.p2v(ctxt, config->dm.addr + offset); + + switch ( type ) + { + case DM_ACPI_BLOB_TYPE_TABLE: + nr_added += construct_dm_table(ctxt, + table_ptrs, nr_tables + nr_added, + blob, length); + break; + case DM_ACPI_BLOB_TYPE_NSDEV: + nr_added += construct_dm_nsdev(ctxt, + table_ptrs, nr_tables + nr_added, + *dir, blob, length); + break; + default: + /* skip blobs of unknown types */ + continue; + } + } + + return nr_added; +} + static int construct_secondary_tables(struct acpi_ctxt *ctxt, unsigned long *table_ptrs, struct acpi_config *config, @@ -461,6 +674,9 @@ static int construct_secondary_tables(struct acpi_ctxt *ctxt, nr_tables += construct_passthrough_tables(ctxt, table_ptrs, nr_tables, config); + /* Load any additional tables passed from device model (e.g. QEMU) */ + nr_tables += construct_dm_tables(ctxt, table_ptrs, nr_tables, config); + table_ptrs[nr_tables] = 0; return nr_tables; } diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h index 12cafd8..684502d 100644 --- a/tools/libacpi/libacpi.h +++ b/tools/libacpi/libacpi.h @@ -82,6 +82,11 @@ struct acpi_config { uint32_t length; } pt; + struct { + uint32_t addr; + uint32_t length; + } dm; + struct acpi_numa numa; const struct hvm_info_table *hvminfo;
ACPI tables built by the device model, whose signatures do not conflict with tables built by Xen (except SSDT), are loaded after ACPI tables built by Xen. ACPI namespace devices built by the device model, whose names do not conflict with devices built by Xen, are assembled and placed in SSDTs after ACPI tables built by Xen. Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> --- Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> --- tools/firmware/hvmloader/util.c | 12 +++ tools/libacpi/acpi2_0.h | 2 + tools/libacpi/build.c | 216 ++++++++++++++++++++++++++++++++++++++++ tools/libacpi/libacpi.h | 5 + 4 files changed, 235 insertions(+)