Re: [Xen-devel] [PATCH v3 4/5] tools/libacpi: update FADT layout to support version 5

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.12.16 at 16:04, <roger.pau@xxxxxxxxxx> wrote:
> --- a/tools/firmware/hvmloader/util.c
> +++ b/tools/firmware/hvmloader/util.c
> @@ -949,7 +949,7 @@ void hvmloader_acpi_build_tables(struct acpi_config 
> *config,
>          config->table_flags |= ACPI_HAS_SSDT_S4;
>  
>      config->table_flags |= (ACPI_HAS_TCPA | ACPI_HAS_IOAPIC | ACPI_HAS_WAET |
> -                            ACPI_HAS_VGA | ACPI_HAS_8042);
> +                            ACPI_HAS_VGA | ACPI_HAS_8042 | ACPI_FADT_4);

It shouldn't be an explicit FADT version that gets requested, but
a specific ACPI version (and I don't think a boolean flag is suitable
for this).

> --- a/tools/libacpi/acpi2_0.h
> +++ b/tools/libacpi/acpi2_0.h
> @@ -169,7 +169,7 @@ struct acpi_10_fadt {
>  /*
>   * Fixed ACPI Description Table Structure (FADT).
>   */
> -struct acpi_20_fadt {
> +struct acpi_50_fadt {

I think it would be better to drop the version number from this
name and ...

> @@ -222,6 +222,8 @@ struct acpi_20_fadt {
>      struct acpi_20_generic_address x_pm_tmr_blk;
>      struct acpi_20_generic_address x_gpe0_blk;
>      struct acpi_20_generic_address x_gpe1_blk;
> +    struct acpi_20_generic_address sleep_control;
> +    struct acpi_20_generic_address sleep_status;
>  };

... add version comments to the new fields.

> --- a/tools/libacpi/build.c
> +++ b/tools/libacpi/build.c
> @@ -503,12 +503,13 @@ int acpi_build_tables(struct acpi_ctxt *ctxt, struct 
> acpi_config *config)
>      struct acpi_20_rsdp *rsdp;
>      struct acpi_20_rsdt *rsdt;
>      struct acpi_20_xsdt *xsdt;
> -    struct acpi_20_fadt *fadt;
> +    struct acpi_50_fadt *fadt;
>      struct acpi_10_fadt *fadt_10;
>      struct acpi_20_facs *facs;
>      unsigned char       *dsdt;
>      unsigned long        secondary_tables[ACPI_MAX_SECONDARY_TABLES];
>      int                  nr_secondaries, i;
> +    unsigned long        fadt_size;

unsigned int would suffice here, wouldn't it? Otherwise it should be
size_t.

> --- a/tools/libacpi/static_tables.c
> +++ b/tools/libacpi/static_tables.c
> @@ -38,11 +38,11 @@ struct acpi_20_facs Facs = {
>  #define ACPI_PM_TMR_BLK_BIT_WIDTH           0x20
>  #define ACPI_PM_TMR_BLK_BIT_OFFSET          0x00
>  
> -struct acpi_20_fadt Fadt = {
> +struct acpi_50_fadt Fadt = {
>      .header = {
> -        .signature    = ACPI_2_0_FADT_SIGNATURE,
> -        .length       = sizeof(struct acpi_20_fadt),
> -        .revision     = ACPI_2_0_FADT_REVISION,
> +        .signature    = ACPI_FADT_SIGNATURE,
> +        .length       = sizeof(struct acpi_50_fadt),
> +        .revision     = ACPI_5_0_FADT_REVISION,

Let's please not pre-initialize either value, but set both fields uniformly
at runtime.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel