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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 19 Nov 2020, Bertrand Marquis wrote:
> Hi Julien,
> 
> > On 19 Nov 2020, at 17:08, Julien Grall <julien@xxxxxxx> wrote:
> > 
> > From: Julien Grall <jgrall@xxxxxxxxxx>
> > 
> > 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@xxxxxxxxxx>
> Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> 
> I also tested the serie on FVP without ACPI and Xen is still booting properly 
> Dom0.

Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


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