[PATCH v4 0/3] xen/arm: Allow Xen to boot with ACPI 5.1

[PATCH v4 0/3] xen/arm: Allow Xen to boot with ACPI 5.1

[Julien Grall]

From: Julien Grall <jgrall@amazon.com>

Hi all,

This series was originally called "xen/arm: Unbreak ACPI". It was
renamed because only the part to allow boot with ACPI 5.1 is not
merged.

After this series, it is possible to boot Xen on QEMU with ACPI 5.1.

Cheers,

Julien Grall (3):
  xen/arm: gic: acpi: Guard helpers to build the MADT with CONFIG_ACPI
  xen/arm: gic: acpi: Use the correct length for the GICC structure
  xen/arm: acpi: Allow Xen to boot with ACPI 5.1

 xen/arch/arm/acpi/boot.c  |  6 +++---
 xen/arch/arm/gic-v2.c     | 13 ++++++-------
 xen/arch/arm/gic-v3.c     | 17 +++++------------
 xen/arch/arm/gic.c        |  4 +++-
 xen/include/asm-arm/gic.h | 10 ++++++++--
 5 files changed, 25 insertions(+), 25 deletions(-)

-- 
2.17.1

[PATCH v4 1/3] xen/arm: gic: acpi: Guard helpers to build the MADT with CONFIG_ACPI

[Julien Grall]

From: Julien Grall <jgrall@amazon.com>

gic_make_hwdom_madt() and gic_get_hwdom_madt_size() are ACPI specific.

While they build fine today, this will change in a follow-up patch.
Rather than trying to fix the build on ACPI, it is best to avoid
compiling the helpers and the associated callbacks when CONFIG_ACPI=n.

Signed-off-by: Julien Grall <jgrall@amazon.com>

---
    Changes in v4:
        - Patch added
---
 xen/arch/arm/gic-v2.c     |  8 +++-----
 xen/arch/arm/gic-v3.c     | 11 ++---------
 xen/arch/arm/gic.c        |  2 ++
 xen/include/asm-arm/gic.h | 10 ++++++++--
 4 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 0f747538dbcd..581ea5ba6b2c 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -1114,12 +1114,12 @@ static int gicv2_iomem_deny_access(const struct domain *d)
     return iomem_deny_access(d, mfn, mfn + nr);
 }
 
+#ifdef CONFIG_ACPI
 static unsigned long gicv2_get_hwdom_extra_madt_size(const struct domain *d)
 {
     return 0;
 }
 
-#ifdef CONFIG_ACPI
 static int gicv2_make_hwdom_madt(const struct domain *d, u32 offset)
 {
     struct acpi_subtable_header *header;
@@ -1248,10 +1248,6 @@ static void __init gicv2_acpi_init(void)
 }
 #else
 static void __init gicv2_acpi_init(void) { }
-static int gicv2_make_hwdom_madt(const struct domain *d, u32 offset)
-{
-    return 0;
-}
 #endif
 
 static int __init gicv2_init(void)
@@ -1357,8 +1353,10 @@ const static struct gic_hw_operations gicv2_ops = {
     .read_apr            = gicv2_read_apr,
     .read_pending_state  = gicv2_read_pending_state,
     .make_hwdom_dt_node  = gicv2_make_hwdom_dt_node,
+#ifdef CONFIG_ACPI
     .make_hwdom_madt     = gicv2_make_hwdom_madt,
     .get_hwdom_extra_madt_size = gicv2_get_hwdom_extra_madt_size,
+#endif
     .map_hwdom_extra_mappings = gicv2_map_hwdown_extra_mappings,
     .iomem_deny_access   = gicv2_iomem_deny_access,
     .do_LPI              = gicv2_do_LPI,
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 0f6cbf6224e9..2a344393a0e4 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1735,15 +1735,6 @@ static void __init gicv3_acpi_init(void)
 }
 #else
 static void __init gicv3_acpi_init(void) { }
-static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
-{
-    return 0;
-}
-
-static unsigned long gicv3_get_hwdom_extra_madt_size(const struct domain *d)
-{
-    return 0;
-}
 #endif
 
 static bool gic_dist_supports_lpis(void)
@@ -1858,8 +1849,10 @@ static const struct gic_hw_operations gicv3_ops = {
     .read_pending_state  = gicv3_read_pending_state,
     .secondary_init      = gicv3_secondary_cpu_init,
     .make_hwdom_dt_node  = gicv3_make_hwdom_dt_node,
+#ifdef CONFIG_ACPI
     .make_hwdom_madt     = gicv3_make_hwdom_madt,
     .get_hwdom_extra_madt_size = gicv3_get_hwdom_extra_madt_size,
+#endif
     .iomem_deny_access   = gicv3_iomem_deny_access,
     .do_LPI              = gicv3_do_LPI,
 };
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index d623c57cb9fa..fe60619e99cf 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -443,6 +443,7 @@ int gic_make_hwdom_dt_node(const struct domain *d,
     return gic_hw_ops->make_hwdom_dt_node(d, gic, fdt);
 }
 
+#ifdef CONFIG_ACPI
 int gic_make_hwdom_madt(const struct domain *d, u32 offset)
 {
     return gic_hw_ops->make_hwdom_madt(d, offset);
@@ -459,6 +460,7 @@ unsigned long gic_get_hwdom_madt_size(const struct domain *d)
 
     return madt_size;
 }
+#endif
 
 int gic_iomem_deny_access(const struct domain *d)
 {
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index ba870523bb2a..ad0f7452d005 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -378,12 +378,14 @@ struct gic_hw_operations {
     /* Create GIC node for the hardware domain */
     int (*make_hwdom_dt_node)(const struct domain *d,
                               const struct dt_device_node *gic, void *fdt);
+#ifdef CONFIG_ACPI
     /* Create MADT table for the hardware domain */
     int (*make_hwdom_madt)(const struct domain *d, u32 offset);
-    /* Map extra GIC MMIO, irqs and other hw stuffs to the hardware domain. */
-    int (*map_hwdom_extra_mappings)(struct domain *d);
     /* Query the size of hardware domain madt table */
     unsigned long (*get_hwdom_extra_madt_size)(const struct domain *d);
+#endif
+    /* Map extra GIC MMIO, irqs and other hw stuffs to the hardware domain. */
+    int (*map_hwdom_extra_mappings)(struct domain *d);
     /* Deny access to GIC regions */
     int (*iomem_deny_access)(const struct domain *d);
     /* Handle LPIs, which require special handling */
@@ -435,8 +437,12 @@ void register_gic_ops(const struct gic_hw_operations *ops);
 int gic_make_hwdom_dt_node(const struct domain *d,
                            const struct dt_device_node *gic,
                            void *fdt);
+
+#ifdef CONFIG_ACPI
 int gic_make_hwdom_madt(const struct domain *d, u32 offset);
 unsigned long gic_get_hwdom_madt_size(const struct domain *d);
+#endif
+
 int gic_map_hwdom_extra_mappings(struct domain *d);
 int gic_iomem_deny_access(const struct domain *d);
 
-- 
2.17.1

Re: [PATCH v4 1/3] xen/arm: gic: acpi: Guard helpers to build the MADT with CONFIG_ACPI

[Bertrand Marquis]

Hi Julien,

> On 19 Nov 2020, at 17:08, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> gic_make_hwdom_madt() and gic_get_hwdom_madt_size() are ACPI specific.
> 
> While they build fine today, this will change in a follow-up patch.
> Rather than trying to fix the build on ACPI, it is best to avoid
> compiling the helpers and the associated callbacks when CONFIG_ACPI=n.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

I also tested the serie on FVP without ACPI and Xen is still booting properly Dom0.

Cheers
Bertrand

> 
> ---
>    Changes in v4:
>        - Patch added
> ---
> xen/arch/arm/gic-v2.c     |  8 +++-----
> xen/arch/arm/gic-v3.c     | 11 ++---------
> xen/arch/arm/gic.c        |  2 ++
> xen/include/asm-arm/gic.h | 10 ++++++++--
> 4 files changed, 15 insertions(+), 16 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index 0f747538dbcd..581ea5ba6b2c 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -1114,12 +1114,12 @@ static int gicv2_iomem_deny_access(const struct domain *d)
>     return iomem_deny_access(d, mfn, mfn + nr);
> }
> 
> +#ifdef CONFIG_ACPI
> static unsigned long gicv2_get_hwdom_extra_madt_size(const struct domain *d)
> {
>     return 0;
> }
> 
> -#ifdef CONFIG_ACPI
> static int gicv2_make_hwdom_madt(const struct domain *d, u32 offset)
> {
>     struct acpi_subtable_header *header;
> @@ -1248,10 +1248,6 @@ static void __init gicv2_acpi_init(void)
> }
> #else
> static void __init gicv2_acpi_init(void) { }
> -static int gicv2_make_hwdom_madt(const struct domain *d, u32 offset)
> -{
> -    return 0;
> -}
> #endif
> 
> static int __init gicv2_init(void)
> @@ -1357,8 +1353,10 @@ const static struct gic_hw_operations gicv2_ops = {
>     .read_apr            = gicv2_read_apr,
>     .read_pending_state  = gicv2_read_pending_state,
>     .make_hwdom_dt_node  = gicv2_make_hwdom_dt_node,
> +#ifdef CONFIG_ACPI
>     .make_hwdom_madt     = gicv2_make_hwdom_madt,
>     .get_hwdom_extra_madt_size = gicv2_get_hwdom_extra_madt_size,
> +#endif
>     .map_hwdom_extra_mappings = gicv2_map_hwdown_extra_mappings,
>     .iomem_deny_access   = gicv2_iomem_deny_access,
>     .do_LPI              = gicv2_do_LPI,
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 0f6cbf6224e9..2a344393a0e4 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1735,15 +1735,6 @@ static void __init gicv3_acpi_init(void)
> }
> #else
> static void __init gicv3_acpi_init(void) { }
> -static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
> -{
> -    return 0;
> -}
> -
> -static unsigned long gicv3_get_hwdom_extra_madt_size(const struct domain *d)
> -{
> -    return 0;
> -}
> #endif
> 
> static bool gic_dist_supports_lpis(void)
> @@ -1858,8 +1849,10 @@ static const struct gic_hw_operations gicv3_ops = {
>     .read_pending_state  = gicv3_read_pending_state,
>     .secondary_init      = gicv3_secondary_cpu_init,
>     .make_hwdom_dt_node  = gicv3_make_hwdom_dt_node,
> +#ifdef CONFIG_ACPI
>     .make_hwdom_madt     = gicv3_make_hwdom_madt,
>     .get_hwdom_extra_madt_size = gicv3_get_hwdom_extra_madt_size,
> +#endif
>     .iomem_deny_access   = gicv3_iomem_deny_access,
>     .do_LPI              = gicv3_do_LPI,
> };
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index d623c57cb9fa..fe60619e99cf 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -443,6 +443,7 @@ int gic_make_hwdom_dt_node(const struct domain *d,
>     return gic_hw_ops->make_hwdom_dt_node(d, gic, fdt);
> }
> 
> +#ifdef CONFIG_ACPI
> int gic_make_hwdom_madt(const struct domain *d, u32 offset)
> {
>     return gic_hw_ops->make_hwdom_madt(d, offset);
> @@ -459,6 +460,7 @@ unsigned long gic_get_hwdom_madt_size(const struct domain *d)
> 
>     return madt_size;
> }
> +#endif
> 
> int gic_iomem_deny_access(const struct domain *d)
> {
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index ba870523bb2a..ad0f7452d005 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -378,12 +378,14 @@ struct gic_hw_operations {
>     /* Create GIC node for the hardware domain */
>     int (*make_hwdom_dt_node)(const struct domain *d,
>                               const struct dt_device_node *gic, void *fdt);
> +#ifdef CONFIG_ACPI
>     /* Create MADT table for the hardware domain */
>     int (*make_hwdom_madt)(const struct domain *d, u32 offset);
> -    /* Map extra GIC MMIO, irqs and other hw stuffs to the hardware domain. */
> -    int (*map_hwdom_extra_mappings)(struct domain *d);
>     /* Query the size of hardware domain madt table */
>     unsigned long (*get_hwdom_extra_madt_size)(const struct domain *d);
> +#endif
> +    /* Map extra GIC MMIO, irqs and other hw stuffs to the hardware domain. */
> +    int (*map_hwdom_extra_mappings)(struct domain *d);
>     /* Deny access to GIC regions */
>     int (*iomem_deny_access)(const struct domain *d);
>     /* Handle LPIs, which require special handling */
> @@ -435,8 +437,12 @@ void register_gic_ops(const struct gic_hw_operations *ops);
> int gic_make_hwdom_dt_node(const struct domain *d,
>                            const struct dt_device_node *gic,
>                            void *fdt);
> +
> +#ifdef CONFIG_ACPI
> int gic_make_hwdom_madt(const struct domain *d, u32 offset);
> unsigned long gic_get_hwdom_madt_size(const struct domain *d);
> +#endif
> +
> int gic_map_hwdom_extra_mappings(struct domain *d);
> int gic_iomem_deny_access(const struct domain *d);
> 
> -- 
> 2.17.1
> 

Re: [PATCH v4 1/3] xen/arm: gic: acpi: Guard helpers to build the MADT with CONFIG_ACPI

[Stefano Stabellini]

On Thu, 19 Nov 2020, Bertrand Marquis wrote:
> Hi Julien,
> 
> > On 19 Nov 2020, at 17:08, Julien Grall <julien@xen.org> wrote:
> > 
> > From: Julien Grall <jgrall@amazon.com>
> > 
> > gic_make_hwdom_madt() and gic_get_hwdom_madt_size() are ACPI specific.
> > 
> > While they build fine today, this will change in a follow-up patch.
> > Rather than trying to fix the build on ACPI, it is best to avoid
> > compiling the helpers and the associated callbacks when CONFIG_ACPI=n.
> > 
> > Signed-off-by: Julien Grall <jgrall@amazon.com>
> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
> 
> I also tested the serie on FVP without ACPI and Xen is still booting properly Dom0.

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> > ---
> >    Changes in v4:
> >        - Patch added
> > ---
> > xen/arch/arm/gic-v2.c     |  8 +++-----
> > xen/arch/arm/gic-v3.c     | 11 ++---------
> > xen/arch/arm/gic.c        |  2 ++
> > xen/include/asm-arm/gic.h | 10 ++++++++--
> > 4 files changed, 15 insertions(+), 16 deletions(-)
> > 
> > diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> > index 0f747538dbcd..581ea5ba6b2c 100644
> > --- a/xen/arch/arm/gic-v2.c
> > +++ b/xen/arch/arm/gic-v2.c
> > @@ -1114,12 +1114,12 @@ static int gicv2_iomem_deny_access(const struct domain *d)
> >     return iomem_deny_access(d, mfn, mfn + nr);
> > }
> > 
> > +#ifdef CONFIG_ACPI
> > static unsigned long gicv2_get_hwdom_extra_madt_size(const struct domain *d)
> > {
> >     return 0;
> > }
> > 
> > -#ifdef CONFIG_ACPI
> > static int gicv2_make_hwdom_madt(const struct domain *d, u32 offset)
> > {
> >     struct acpi_subtable_header *header;
> > @@ -1248,10 +1248,6 @@ static void __init gicv2_acpi_init(void)
> > }
> > #else
> > static void __init gicv2_acpi_init(void) { }
> > -static int gicv2_make_hwdom_madt(const struct domain *d, u32 offset)
> > -{
> > -    return 0;
> > -}
> > #endif
> > 
> > static int __init gicv2_init(void)
> > @@ -1357,8 +1353,10 @@ const static struct gic_hw_operations gicv2_ops = {
> >     .read_apr            = gicv2_read_apr,
> >     .read_pending_state  = gicv2_read_pending_state,
> >     .make_hwdom_dt_node  = gicv2_make_hwdom_dt_node,
> > +#ifdef CONFIG_ACPI
> >     .make_hwdom_madt     = gicv2_make_hwdom_madt,
> >     .get_hwdom_extra_madt_size = gicv2_get_hwdom_extra_madt_size,
> > +#endif
> >     .map_hwdom_extra_mappings = gicv2_map_hwdown_extra_mappings,
> >     .iomem_deny_access   = gicv2_iomem_deny_access,
> >     .do_LPI              = gicv2_do_LPI,
> > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> > index 0f6cbf6224e9..2a344393a0e4 100644
> > --- a/xen/arch/arm/gic-v3.c
> > +++ b/xen/arch/arm/gic-v3.c
> > @@ -1735,15 +1735,6 @@ static void __init gicv3_acpi_init(void)
> > }
> > #else
> > static void __init gicv3_acpi_init(void) { }
> > -static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
> > -{
> > -    return 0;
> > -}
> > -
> > -static unsigned long gicv3_get_hwdom_extra_madt_size(const struct domain *d)
> > -{
> > -    return 0;
> > -}
> > #endif
> > 
> > static bool gic_dist_supports_lpis(void)
> > @@ -1858,8 +1849,10 @@ static const struct gic_hw_operations gicv3_ops = {
> >     .read_pending_state  = gicv3_read_pending_state,
> >     .secondary_init      = gicv3_secondary_cpu_init,
> >     .make_hwdom_dt_node  = gicv3_make_hwdom_dt_node,
> > +#ifdef CONFIG_ACPI
> >     .make_hwdom_madt     = gicv3_make_hwdom_madt,
> >     .get_hwdom_extra_madt_size = gicv3_get_hwdom_extra_madt_size,
> > +#endif
> >     .iomem_deny_access   = gicv3_iomem_deny_access,
> >     .do_LPI              = gicv3_do_LPI,
> > };
> > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> > index d623c57cb9fa..fe60619e99cf 100644
> > --- a/xen/arch/arm/gic.c
> > +++ b/xen/arch/arm/gic.c
> > @@ -443,6 +443,7 @@ int gic_make_hwdom_dt_node(const struct domain *d,
> >     return gic_hw_ops->make_hwdom_dt_node(d, gic, fdt);
> > }
> > 
> > +#ifdef CONFIG_ACPI
> > int gic_make_hwdom_madt(const struct domain *d, u32 offset)
> > {
> >     return gic_hw_ops->make_hwdom_madt(d, offset);
> > @@ -459,6 +460,7 @@ unsigned long gic_get_hwdom_madt_size(const struct domain *d)
> > 
> >     return madt_size;
> > }
> > +#endif
> > 
> > int gic_iomem_deny_access(const struct domain *d)
> > {
> > diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> > index ba870523bb2a..ad0f7452d005 100644
> > --- a/xen/include/asm-arm/gic.h
> > +++ b/xen/include/asm-arm/gic.h
> > @@ -378,12 +378,14 @@ struct gic_hw_operations {
> >     /* Create GIC node for the hardware domain */
> >     int (*make_hwdom_dt_node)(const struct domain *d,
> >                               const struct dt_device_node *gic, void *fdt);
> > +#ifdef CONFIG_ACPI
> >     /* Create MADT table for the hardware domain */
> >     int (*make_hwdom_madt)(const struct domain *d, u32 offset);
> > -    /* Map extra GIC MMIO, irqs and other hw stuffs to the hardware domain. */
> > -    int (*map_hwdom_extra_mappings)(struct domain *d);
> >     /* Query the size of hardware domain madt table */
> >     unsigned long (*get_hwdom_extra_madt_size)(const struct domain *d);
> > +#endif
> > +    /* Map extra GIC MMIO, irqs and other hw stuffs to the hardware domain. */
> > +    int (*map_hwdom_extra_mappings)(struct domain *d);
> >     /* Deny access to GIC regions */
> >     int (*iomem_deny_access)(const struct domain *d);
> >     /* Handle LPIs, which require special handling */
> > @@ -435,8 +437,12 @@ void register_gic_ops(const struct gic_hw_operations *ops);
> > int gic_make_hwdom_dt_node(const struct domain *d,
> >                            const struct dt_device_node *gic,
> >                            void *fdt);
> > +
> > +#ifdef CONFIG_ACPI
> > int gic_make_hwdom_madt(const struct domain *d, u32 offset);
> > unsigned long gic_get_hwdom_madt_size(const struct domain *d);
> > +#endif
> > +
> > int gic_map_hwdom_extra_mappings(struct domain *d);
> > int gic_iomem_deny_access(const struct domain *d);
> > 
> > -- 
> > 2.17.1
> > 
> 
> 

[PATCH v4 2/3] xen/arm: gic: acpi: Use the correct length for the GICC structure

[Julien Grall]

From: Julien Grall <julien.grall@arm.com>

The length of the GICC structure in the MADT ACPI table differs between
version 5.1 and 6.0, although there are no other relevant differences.

Use the BAD_MADT_GICC_ENTRY macro, which was specifically designed to
overcome this issue.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Julien Grall <jgrall@amazon.com>

---
    Changes in v3:
        - Update the commit title as we also modify GICv3 code
        - Use the correct length in more places

    Changes in v2:
        - Patch added
---
 xen/arch/arm/acpi/boot.c | 2 +-
 xen/arch/arm/gic-v2.c    | 5 +++--
 xen/arch/arm/gic-v3.c    | 6 +++---
 xen/arch/arm/gic.c       | 2 +-
 4 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
index 30e4bd1bc5a7..55c3e5cbc834 100644
--- a/xen/arch/arm/acpi/boot.c
+++ b/xen/arch/arm/acpi/boot.c
@@ -131,7 +131,7 @@ acpi_parse_gic_cpu_interface(struct acpi_subtable_header *header,
     struct acpi_madt_generic_interrupt *processor =
                container_of(header, struct acpi_madt_generic_interrupt, header);
 
-    if ( BAD_MADT_ENTRY(processor, end) )
+    if ( BAD_MADT_GICC_ENTRY(processor, end) )
         return -EINVAL;
 
     acpi_table_print_madt_entry(header);
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 581ea5ba6b2c..b2adc8ec9a64 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -1136,7 +1136,8 @@ static int gicv2_make_hwdom_madt(const struct domain *d, u32 offset)
 
     host_gicc = container_of(header, struct acpi_madt_generic_interrupt,
                              header);
-    size = sizeof(struct acpi_madt_generic_interrupt);
+
+    size = ACPI_MADT_GICC_LENGTH;
     /* Add Generic Interrupt */
     for ( i = 0; i < d->max_vcpus; i++ )
     {
@@ -1165,7 +1166,7 @@ gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
     struct acpi_madt_generic_interrupt *processor =
                container_of(header, struct acpi_madt_generic_interrupt, header);
 
-    if ( BAD_MADT_ENTRY(processor, end) )
+    if ( BAD_MADT_GICC_ENTRY(processor, end) )
         return -EINVAL;
 
     /* Read from APIC table and fill up the GIC variables */
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 2a344393a0e4..ac28013c1967 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1499,7 +1499,7 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
 
     host_gicc = container_of(header, struct acpi_madt_generic_interrupt,
                              header);
-    size = sizeof(struct acpi_madt_generic_interrupt);
+    size = ACPI_MADT_GICC_LENGTH;
     for ( i = 0; i < d->max_vcpus; i++ )
     {
         gicc = (struct acpi_madt_generic_interrupt *)(base_ptr + table_len);
@@ -1558,7 +1558,7 @@ gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
     struct acpi_madt_generic_interrupt *processor =
                container_of(header, struct acpi_madt_generic_interrupt, header);
 
-    if ( BAD_MADT_ENTRY(processor, end) )
+    if ( BAD_MADT_GICC_ENTRY(processor, end) )
         return -EINVAL;
 
     /* Read from APIC table and fill up the GIC variables */
@@ -1628,7 +1628,7 @@ gic_acpi_get_madt_cpu_num(struct acpi_subtable_header *header,
     struct acpi_madt_generic_interrupt *cpuif;
 
     cpuif = (struct acpi_madt_generic_interrupt *)header;
-    if ( BAD_MADT_ENTRY(cpuif, end) || !cpuif->gicr_base_address )
+    if ( BAD_MADT_GICC_ENTRY(cpuif, end) || !cpuif->gicr_base_address )
         return -EINVAL;
 
     return 0;
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index fe60619e99cf..3b0331b53830 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -454,7 +454,7 @@ unsigned long gic_get_hwdom_madt_size(const struct domain *d)
     unsigned long madt_size;
 
     madt_size = sizeof(struct acpi_table_madt)
-                + sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus
+                + ACPI_MADT_GICC_LENGTH * d->max_vcpus
                 + sizeof(struct acpi_madt_generic_distributor)
                 + gic_hw_ops->get_hwdom_extra_madt_size(d);
 
-- 
2.17.1

Re: [PATCH v4 2/3] xen/arm: gic: acpi: Use the correct length for the GICC structure

[Bertrand Marquis]

Hi,

> On 19 Nov 2020, at 17:08, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <julien.grall@arm.com>
> 
> The length of the GICC structure in the MADT ACPI table differs between
> version 5.1 and 6.0, although there are no other relevant differences.
> 
> Use the BAD_MADT_GICC_ENTRY macro, which was specifically designed to
> overcome this issue.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> 
> ---
>    Changes in v3:
>        - Update the commit title as we also modify GICv3 code
>        - Use the correct length in more places
> 
>    Changes in v2:
>        - Patch added
> ---
> xen/arch/arm/acpi/boot.c | 2 +-
> xen/arch/arm/gic-v2.c    | 5 +++--
> xen/arch/arm/gic-v3.c    | 6 +++---
> xen/arch/arm/gic.c       | 2 +-
> 4 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
> index 30e4bd1bc5a7..55c3e5cbc834 100644
> --- a/xen/arch/arm/acpi/boot.c
> +++ b/xen/arch/arm/acpi/boot.c
> @@ -131,7 +131,7 @@ acpi_parse_gic_cpu_interface(struct acpi_subtable_header *header,
>     struct acpi_madt_generic_interrupt *processor =
>                container_of(header, struct acpi_madt_generic_interrupt, header);
> 
> -    if ( BAD_MADT_ENTRY(processor, end) )
> +    if ( BAD_MADT_GICC_ENTRY(processor, end) )
>         return -EINVAL;
> 
>     acpi_table_print_madt_entry(header);
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index 581ea5ba6b2c..b2adc8ec9a64 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -1136,7 +1136,8 @@ static int gicv2_make_hwdom_madt(const struct domain *d, u32 offset)
> 
>     host_gicc = container_of(header, struct acpi_madt_generic_interrupt,
>                              header);
> -    size = sizeof(struct acpi_madt_generic_interrupt);
> +
> +    size = ACPI_MADT_GICC_LENGTH;
>     /* Add Generic Interrupt */
>     for ( i = 0; i < d->max_vcpus; i++ )
>     {
> @@ -1165,7 +1166,7 @@ gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
>     struct acpi_madt_generic_interrupt *processor =
>                container_of(header, struct acpi_madt_generic_interrupt, header);
> 
> -    if ( BAD_MADT_ENTRY(processor, end) )
> +    if ( BAD_MADT_GICC_ENTRY(processor, end) )
>         return -EINVAL;
> 
>     /* Read from APIC table and fill up the GIC variables */
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 2a344393a0e4..ac28013c1967 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1499,7 +1499,7 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
> 
>     host_gicc = container_of(header, struct acpi_madt_generic_interrupt,
>                              header);
> -    size = sizeof(struct acpi_madt_generic_interrupt);
> +    size = ACPI_MADT_GICC_LENGTH;
>     for ( i = 0; i < d->max_vcpus; i++ )
>     {
>         gicc = (struct acpi_madt_generic_interrupt *)(base_ptr + table_len);
> @@ -1558,7 +1558,7 @@ gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
>     struct acpi_madt_generic_interrupt *processor =
>                container_of(header, struct acpi_madt_generic_interrupt, header);
> 
> -    if ( BAD_MADT_ENTRY(processor, end) )
> +    if ( BAD_MADT_GICC_ENTRY(processor, end) )
>         return -EINVAL;
> 
>     /* Read from APIC table and fill up the GIC variables */
> @@ -1628,7 +1628,7 @@ gic_acpi_get_madt_cpu_num(struct acpi_subtable_header *header,
>     struct acpi_madt_generic_interrupt *cpuif;
> 
>     cpuif = (struct acpi_madt_generic_interrupt *)header;
> -    if ( BAD_MADT_ENTRY(cpuif, end) || !cpuif->gicr_base_address )
> +    if ( BAD_MADT_GICC_ENTRY(cpuif, end) || !cpuif->gicr_base_address )
>         return -EINVAL;
> 
>     return 0;
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index fe60619e99cf..3b0331b53830 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -454,7 +454,7 @@ unsigned long gic_get_hwdom_madt_size(const struct domain *d)
>     unsigned long madt_size;
> 
>     madt_size = sizeof(struct acpi_table_madt)
> -                + sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus
> +                + ACPI_MADT_GICC_LENGTH * d->max_vcpus
>                 + sizeof(struct acpi_madt_generic_distributor)
>                 + gic_hw_ops->get_hwdom_extra_madt_size(d);
> 
> -- 
> 2.17.1
> 

[PATCH v4 3/3] xen/arm: acpi: Allow Xen to boot with ACPI 5.1

[Julien Grall]

From: Julien Grall <julien.grall@arm.com>

At the moment Xen requires the FADT ACPI table to be at least version
6.0, apparently because of some reliance on other ACPI v6.0 features.

But actually this is overzealous, and Xen works now fine with ACPI v5.1.

Let's relax the version check for the FADT table to allow QEMU to
run the hypervisor with ACPI.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Julien Grall <jgrall@amazon.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>

---
    Changes in v3:
        - Add Stefano's acked-by

    Changes in v2:
        - Patch added
---
 xen/arch/arm/acpi/boot.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
index 55c3e5cbc834..7ea2990cb82c 100644
--- a/xen/arch/arm/acpi/boot.c
+++ b/xen/arch/arm/acpi/boot.c
@@ -181,8 +181,8 @@ static int __init acpi_parse_fadt(struct acpi_table_header *table)
      * we only deal with ACPI 6.0 or newer revision to get GIC and SMP
      * boot protocol configuration data, or we will disable ACPI.
      */
-    if ( table->revision > 6
-         || (table->revision == 6 && fadt->minor_revision >= 0) )
+    if ( table->revision > 5
+         || (table->revision == 5 && fadt->minor_revision >= 1) )
         return 0;
 
     printk("Unsupported FADT revision %d.%d, should be 6.0+, will disable ACPI\n",
-- 
2.17.1

Re: [PATCH v4 3/3] xen/arm: acpi: Allow Xen to boot with ACPI 5.1

[Bertrand Marquis]

Hi,

> On 19 Nov 2020, at 17:08, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <julien.grall@arm.com>
> 
> At the moment Xen requires the FADT ACPI table to be at least version
> 6.0, apparently because of some reliance on other ACPI v6.0 features.
> 
> But actually this is overzealous, and Xen works now fine with ACPI v5.1.
> 
> Let's relax the version check for the FADT table to allow QEMU to
> run the hypervisor with ACPI.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> 
> ---
>    Changes in v3:
>        - Add Stefano's acked-by
> 
>    Changes in v2:
>        - Patch added
> ---
> xen/arch/arm/acpi/boot.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
> index 55c3e5cbc834..7ea2990cb82c 100644
> --- a/xen/arch/arm/acpi/boot.c
> +++ b/xen/arch/arm/acpi/boot.c
> @@ -181,8 +181,8 @@ static int __init acpi_parse_fadt(struct acpi_table_header *table)
>      * we only deal with ACPI 6.0 or newer revision to get GIC and SMP
>      * boot protocol configuration data, or we will disable ACPI.
>      */
> -    if ( table->revision > 6
> -         || (table->revision == 6 && fadt->minor_revision >= 0) )
> +    if ( table->revision > 5
> +         || (table->revision == 5 && fadt->minor_revision >= 1) )
>         return 0;
> 
>     printk("Unsupported FADT revision %d.%d, should be 6.0+, will disable ACPI\n",
> -- 
> 2.17.1
>