|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [RFC XEN PATCH 11/16] tools/libacpi: load ACPI built by the device model
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@xxxxxxxxx>
> ---
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxx
> https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |