[PATCH v3 0/5] tools/libacpi: fix boot flags passed to PVHv2 guests

[PATCH v3 0/5] tools/libacpi: fix boot flags passed to PVHv2 guests

[Roger Pau Monne]

Hello,

There are a couple of boot flags that should be passed to PVHv2 guests, that
report the lack of VGA and CMOS RTC, and we shouldn't also pass the 8042 flag,
because PVHv2 guests don't have access to such controller. The no CMOS RTC flags
requires that the FADT table is bumped to version 5 at least for PVHv2 guests.

Thanks, Roger.


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

[PATCH v3 1/5] tools/libacpi: add _FADT_ to the FADT boot flags definitions

[Roger Pau Monne]

Signed-off-by: Roger Pau Monné <roger.pau@citrix.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>
Cc: boris.ostrovsky@oracle.com
Cc: konrad.wilk@oracle.com
---
Changes since v2:
 - Fix usage of ACPI_8042 in the static boot tables.
---
 tools/libacpi/acpi2_0.h       | 4 ++--
 tools/libacpi/static_tables.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/libacpi/acpi2_0.h b/tools/libacpi/acpi2_0.h
index 775eb7a..03c5dcb 100644
--- a/tools/libacpi/acpi2_0.h
+++ b/tools/libacpi/acpi2_0.h
@@ -227,8 +227,8 @@ struct acpi_20_fadt {
 /*
  * FADT Boot Architecture Flags.
  */
-#define ACPI_LEGACY_DEVICES (1 << 0)
-#define ACPI_8042           (1 << 1)
+#define ACPI_FADT_LEGACY_DEVICES    (1 << 0)
+#define ACPI_FADT_8042              (1 << 1)
 
 /*
  * FADT Fixed Feature Flags.
diff --git a/tools/libacpi/static_tables.c b/tools/libacpi/static_tables.c
index 617bf68..2659153 100644
--- a/tools/libacpi/static_tables.c
+++ b/tools/libacpi/static_tables.c
@@ -63,7 +63,7 @@ struct acpi_20_fadt Fadt = {
 
     .p_lvl2_lat = 0x0fff, /* >100,  means we do not support C2 state */
     .p_lvl3_lat = 0x0fff, /* >1000, means we do not support C3 state */
-    .iapc_boot_arch = ACPI_8042,
+    .iapc_boot_arch = ACPI_FADT_8042,
     .flags = (ACPI_PROC_C1 |
               ACPI_WBINVD |
               ACPI_FIX_RTC | ACPI_TMR_VAL_EXT |
-- 
2.9.3 (Apple Git-75)


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

Re: [PATCH v3 1/5] tools/libacpi: add _FADT_ to the FADT boot flags definitions

[Andrew Cooper]

On 05/12/16 15:04, Roger Pau Monne wrote:
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Re: [PATCH v3 1/5] tools/libacpi: add _FADT_ to the FADT boot flags definitions

[Wei Liu]

On Mon, Dec 05, 2016 at 03:04:53PM +0000, Roger Pau Monne wrote:
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

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

[PATCH v3 2/5] tools/libacpi: set FADT boot flag to notify lack of VGA for PVHv2 guests

[Roger Pau Monne]

PVHv2 guests don't have any VGA card, and as so it must be notified in the FADT.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.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>
Cc: boris.ostrovsky@oracle.com
Cc: konrad.wilk@oracle.com
---
 tools/firmware/hvmloader/util.c | 3 ++-
 tools/libacpi/acpi2_0.h         | 1 +
 tools/libacpi/build.c           | 2 ++
 tools/libacpi/libacpi.h         | 1 +
 4 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
index 6e0cfe7..3e192bf 100644
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -948,7 +948,8 @@ 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;
 
-    config->table_flags |= (ACPI_HAS_TCPA | ACPI_HAS_IOAPIC | ACPI_HAS_WAET);
+    config->table_flags |= (ACPI_HAS_TCPA | ACPI_HAS_IOAPIC | ACPI_HAS_WAET |
+                            ACPI_HAS_VGA);
 
     config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS;
 
diff --git a/tools/libacpi/acpi2_0.h b/tools/libacpi/acpi2_0.h
index 03c5dcb..500f95e 100644
--- a/tools/libacpi/acpi2_0.h
+++ b/tools/libacpi/acpi2_0.h
@@ -229,6 +229,7 @@ struct acpi_20_fadt {
  */
 #define ACPI_FADT_LEGACY_DEVICES    (1 << 0)
 #define ACPI_FADT_8042              (1 << 1)
+#define ACPI_FADT_NO_VGA            (1 << 2)
 
 /*
  * FADT Fixed Feature Flags.
diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
index 47dae01..8799e2c 100644
--- a/tools/libacpi/build.c
+++ b/tools/libacpi/build.c
@@ -579,6 +579,8 @@ int acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config)
     fadt->x_dsdt = ctxt->mem_ops.v2p(ctxt, dsdt);
     fadt->firmware_ctrl   = ctxt->mem_ops.v2p(ctxt, facs);
     fadt->x_firmware_ctrl = ctxt->mem_ops.v2p(ctxt, facs);
+    if ( !(config->table_flags & ACPI_HAS_VGA) )
+        fadt->iapc_boot_arch |= ACPI_FADT_NO_VGA;
     set_checksum(fadt,
                  offsetof(struct acpi_header, checksum),
                  sizeof(struct acpi_20_fadt));
diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h
index 1d388f9..d7ea6e1 100644
--- a/tools/libacpi/libacpi.h
+++ b/tools/libacpi/libacpi.h
@@ -30,6 +30,7 @@
 #define ACPI_HAS_TCPA        (1<<7)
 #define ACPI_HAS_IOAPIC      (1<<8)
 #define ACPI_HAS_WAET        (1<<9)
+#define ACPI_HAS_VGA         (1<<10)
 
 struct xen_vmemrange;
 struct acpi_numa {
-- 
2.9.3 (Apple Git-75)


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

[PATCH v3 3/5] tools/libacpi: don't announce a 8042 controller in the FADT for PVHv2 guests

[Roger Pau Monne]

There's no such controler available for PVHv2 guests.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.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>
Cc: boris.ostrovsky@oracle.com
Cc: konrad.wilk@oracle.com
---
 tools/firmware/hvmloader/util.c | 2 +-
 tools/libacpi/build.c           | 2 ++
 tools/libacpi/libacpi.h         | 1 +
 tools/libacpi/static_tables.c   | 1 -
 4 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
index 3e192bf..3f86c88 100644
--- 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_VGA | ACPI_HAS_8042);
 
     config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS;
 
diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
index 8799e2c..844dea1 100644
--- a/tools/libacpi/build.c
+++ b/tools/libacpi/build.c
@@ -581,6 +581,8 @@ int acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config)
     fadt->x_firmware_ctrl = ctxt->mem_ops.v2p(ctxt, facs);
     if ( !(config->table_flags & ACPI_HAS_VGA) )
         fadt->iapc_boot_arch |= ACPI_FADT_NO_VGA;
+    if ( config->table_flags & ACPI_HAS_8042 )
+        fadt->iapc_boot_arch |= ACPI_FADT_8042;
     set_checksum(fadt,
                  offsetof(struct acpi_header, checksum),
                  sizeof(struct acpi_20_fadt));
diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h
index d7ea6e1..e0f1537 100644
--- a/tools/libacpi/libacpi.h
+++ b/tools/libacpi/libacpi.h
@@ -31,6 +31,7 @@
 #define ACPI_HAS_IOAPIC      (1<<8)
 #define ACPI_HAS_WAET        (1<<9)
 #define ACPI_HAS_VGA         (1<<10)
+#define ACPI_HAS_8042        (1<<11)
 
 struct xen_vmemrange;
 struct acpi_numa {
diff --git a/tools/libacpi/static_tables.c b/tools/libacpi/static_tables.c
index 2659153..1f6247d 100644
--- a/tools/libacpi/static_tables.c
+++ b/tools/libacpi/static_tables.c
@@ -63,7 +63,6 @@ struct acpi_20_fadt Fadt = {
 
     .p_lvl2_lat = 0x0fff, /* >100,  means we do not support C2 state */
     .p_lvl3_lat = 0x0fff, /* >1000, means we do not support C3 state */
-    .iapc_boot_arch = ACPI_FADT_8042,
     .flags = (ACPI_PROC_C1 |
               ACPI_WBINVD |
               ACPI_FIX_RTC | ACPI_TMR_VAL_EXT |
-- 
2.9.3 (Apple Git-75)


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

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

[Roger Pau Monne]

Update the structure of the FADT table to version 5, and use that version for
PVHv2 guests. Note that HVM guests will continue to use FADT 4.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.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>
Cc: boris.ostrovsky@oracle.com
Cc: konrad.wilk@oracle.com
---
 tools/firmware/hvmloader/util.c |  2 +-
 tools/libacpi/acpi2_0.h         |  9 ++++++---
 tools/libacpi/build.c           | 24 ++++++++++++++++++------
 tools/libacpi/libacpi.h         |  1 +
 tools/libacpi/static_tables.c   |  8 ++++----
 5 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
index 3f86c88..5dbb448 100644
--- 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);
 
     config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS;
 
diff --git a/tools/libacpi/acpi2_0.h b/tools/libacpi/acpi2_0.h
index 500f95e..b8ef777 100644
--- 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 {
     struct acpi_header header;
     uint32_t firmware_ctrl;
     uint32_t dsdt;
@@ -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;
 };
 
 /*
@@ -422,7 +424,7 @@ struct acpi_20_slit {
  */
 #define ACPI_2_0_RSDP_SIGNATURE ASCII64('R','S','D',' ','P','T','R',' ')
 #define ACPI_2_0_FACS_SIGNATURE ASCII32('F','A','C','S')
-#define ACPI_2_0_FADT_SIGNATURE ASCII32('F','A','C','P')
+#define ACPI_FADT_SIGNATURE     ASCII32('F','A','C','P')
 #define ACPI_2_0_MADT_SIGNATURE ASCII32('A','P','I','C')
 #define ACPI_2_0_RSDT_SIGNATURE ASCII32('R','S','D','T')
 #define ACPI_2_0_XSDT_SIGNATURE ASCII32('X','S','D','T')
@@ -436,7 +438,8 @@ struct acpi_20_slit {
  * Table revision numbers.
  */
 #define ACPI_2_0_RSDP_REVISION 0x02
-#define ACPI_2_0_FADT_REVISION 0x04
+#define ACPI_4_0_FADT_REVISION 0x04
+#define ACPI_5_0_FADT_REVISION 0x05
 #define ACPI_2_0_MADT_REVISION 0x02
 #define ACPI_2_0_RSDT_REVISION 0x01
 #define ACPI_2_0_XSDT_REVISION 0x01
diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
index 844dea1..eb40989 100644
--- 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;
 
     acpi_info = (struct acpi_info *)config->infop;
     memset(acpi_info, 0, sizeof(*acpi_info));
@@ -572,9 +573,16 @@ int acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config)
                  offsetof(struct acpi_header, checksum),
                  sizeof(struct acpi_10_fadt));
 
-    fadt = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_fadt), 16);
+    /*
+     * NB: we can use offsetof because there's no padding between
+     * x_gpe1_blk and sleep_control.
+     */
+    fadt_size = config->table_flags & ACPI_FADT_4 ?
+                    offsetof(struct acpi_50_fadt, sleep_control) :
+                    sizeof(struct acpi_50_fadt);
+    fadt = ctxt->mem_ops.alloc(ctxt, fadt_size, 16);
     if (!fadt) goto oom;
-    memcpy(fadt, &Fadt, sizeof(struct acpi_20_fadt));
+    memcpy(fadt, &Fadt, fadt_size);
     fadt->dsdt   = ctxt->mem_ops.v2p(ctxt, dsdt);
     fadt->x_dsdt = ctxt->mem_ops.v2p(ctxt, dsdt);
     fadt->firmware_ctrl   = ctxt->mem_ops.v2p(ctxt, facs);
@@ -583,9 +591,13 @@ int acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config)
         fadt->iapc_boot_arch |= ACPI_FADT_NO_VGA;
     if ( config->table_flags & ACPI_HAS_8042 )
         fadt->iapc_boot_arch |= ACPI_FADT_8042;
-    set_checksum(fadt,
-                 offsetof(struct acpi_header, checksum),
-                 sizeof(struct acpi_20_fadt));
+
+    if ( config->table_flags & ACPI_FADT_4 )
+    {
+        fadt->header.revision = ACPI_4_0_FADT_REVISION;
+        fadt->header.length = fadt_size;
+    }
+    set_checksum(fadt, offsetof(struct acpi_header, checksum), fadt_size);
 
     nr_secondaries = construct_secondary_tables(ctxt, secondary_tables,
                  config, acpi_info);
diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h
index e0f1537..89bfcc6 100644
--- a/tools/libacpi/libacpi.h
+++ b/tools/libacpi/libacpi.h
@@ -32,6 +32,7 @@
 #define ACPI_HAS_WAET        (1<<9)
 #define ACPI_HAS_VGA         (1<<10)
 #define ACPI_HAS_8042        (1<<11)
+#define ACPI_FADT_4          (1<<12)
 
 struct xen_vmemrange;
 struct acpi_numa {
diff --git a/tools/libacpi/static_tables.c b/tools/libacpi/static_tables.c
index 1f6247d..ab7fa3b 100644
--- 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,
         .oem_id       = ACPI_OEM_ID, 
         .oem_table_id = ACPI_OEM_TABLE_ID,
         .oem_revision = ACPI_OEM_REVISION,
-- 
2.9.3 (Apple Git-75)


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

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

[Jan Beulich]

>>> On 05.12.16 at 16:04, <roger.pau@citrix.com> 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@lists.xen.org
https://lists.xen.org/xen-devel

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

[Roger Pau Monne]

On Wed, Dec 07, 2016 at 09:04:14AM -0700, Jan Beulich wrote:
> >>> On 05.12.16 at 16:04, <roger.pau@citrix.com> 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).

Hm, I'm not opposed to this, but maybe then there are other tables to be bumped 
if we really want to request a specific ACPI version?
 
> > --- 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.

Ack.

> > --- 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.

Yes, unsigned int should be fine.

> > --- 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.

Ack.

Roger.

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

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

[Jan Beulich]

>>> On 12.12.16 at 11:45, <roger.pau@citrix.com> wrote:
> On Wed, Dec 07, 2016 at 09:04:14AM -0700, Jan Beulich wrote:
>> >>> On 05.12.16 at 16:04, <roger.pau@citrix.com> 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).
> 
> Hm, I'm not opposed to this, but maybe then there are other tables to be bumped 
> if we really want to request a specific ACPI version?

Indeed. But I can't reasonably expect you to do the entire switch
within this series.

Jan


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

[PATCH v3 5/5] tools/libacpi: announce that PVHv2 has no CMOS RTC in FADT

[Roger Pau Monne]

At the moment this flag is unconditionally set for PVHv2 domains. Note that
using this boot flag requires that the FADT table revision is at least 5 (or any
later version), so bump the current FADT version from 4 to 5 and add two new
fields that will be unused.

Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.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>
Cc: boris.ostrovsky@oracle.com
Cc: konrad.wilk@oracle.com
---
Ideally this should be somehow fetched from the emulation_flags set of flags,
but sadly there's no way for hvmloader (which runs in guest context) to fetch
this information. If at some point a rtc option is added to libxl, we should see
about passing it through to init_acpi_config in libxl_x86_acpi.c so that we can
then appropriately set this flag for PVHv2 guests.
---
 tools/firmware/hvmloader/util.c | 3 ++-
 tools/libacpi/acpi2_0.h         | 1 +
 tools/libacpi/build.c           | 9 +++++++++
 tools/libacpi/libacpi.h         | 1 +
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
index 5dbb448..df04324 100644
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -949,7 +949,8 @@ 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_FADT_4);
+                            ACPI_HAS_VGA | ACPI_HAS_8042 | ACPI_FADT_4 |
+                            ACPI_HAS_CMOS_RTC);
 
     config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS;
 
diff --git a/tools/libacpi/acpi2_0.h b/tools/libacpi/acpi2_0.h
index b8ef777..eee13ae 100644
--- a/tools/libacpi/acpi2_0.h
+++ b/tools/libacpi/acpi2_0.h
@@ -232,6 +232,7 @@ struct acpi_50_fadt {
 #define ACPI_FADT_LEGACY_DEVICES    (1 << 0)
 #define ACPI_FADT_8042              (1 << 1)
 #define ACPI_FADT_NO_VGA            (1 << 2)
+#define ACPI_FADT_NO_CMOS_RTC       (1 << 5)
 
 /*
  * FADT Fixed Feature Flags.
diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
index eb40989..cd76ac3 100644
--- a/tools/libacpi/build.c
+++ b/tools/libacpi/build.c
@@ -591,6 +591,15 @@ int acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config)
         fadt->iapc_boot_arch |= ACPI_FADT_NO_VGA;
     if ( config->table_flags & ACPI_HAS_8042 )
         fadt->iapc_boot_arch |= ACPI_FADT_8042;
+    if ( !(config->table_flags & ACPI_HAS_CMOS_RTC) )
+    {
+        if ( config->table_flags & ACPI_FADT_4 )
+        {
+            printf("ACPI_FADT_NO_CMOS_RTC is not available on FADT revision 4\n");
+            return -1;
+        }
+        fadt->iapc_boot_arch |= ACPI_FADT_NO_CMOS_RTC;
+    }
 
     if ( config->table_flags & ACPI_FADT_4 )
     {
diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h
index 89bfcc6..8881687 100644
--- a/tools/libacpi/libacpi.h
+++ b/tools/libacpi/libacpi.h
@@ -33,6 +33,7 @@
 #define ACPI_HAS_VGA         (1<<10)
 #define ACPI_HAS_8042        (1<<11)
 #define ACPI_FADT_4          (1<<12)
+#define ACPI_HAS_CMOS_RTC    (1<<13)
 
 struct xen_vmemrange;
 struct acpi_numa {
-- 
2.9.3 (Apple Git-75)


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

Re: [PATCH v3 5/5] tools/libacpi: announce that PVHv2 has no CMOS RTC in FADT

[Jan Beulich]

>>> On 05.12.16 at 16:04, <roger.pau@citrix.com> wrote:
> At the moment this flag is unconditionally set for PVHv2 domains. Note that
> using this boot flag requires that the FADT table revision is at least 5 (or any
> later version), so bump the current FADT version from 4 to 5 and add two new
> fields that will be unused.

This version bumping part appears to be stale now. The patch itself
looks okay, but will likely need some re-basing over changes to the
previous one.

Jan


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