Re: [Xen-devel] [PATCH v3 2/5] ARM: ITS: Populate host_its_list from ACPI MADT Table

[Andre Przywara <andre.przywara@xxxxxxx>]

Hi,

On 05/09/17 18:14, mjaggi@xxxxxxxxxxxxxxxxxx wrote:
> From: Manish Jaggi <mjaggi@xxxxxxxxxx>
> 
> Added gicv3_its_acpi_init to update host_its_list from MADT table.
> For ACPI, host_its structure  stores dt_node as NULL.
> 
> Signed-off-by: Manish Jaggi <mjaggi@xxxxxxxxxx>
> ---
>  xen/arch/arm/gic-v3-its.c        | 26 ++++++++++++++++++++++++++
>  xen/arch/arm/gic-v3.c            |  2 ++
>  xen/include/asm-arm/gic_v3_its.h |  9 +++++++++
>  3 files changed, 37 insertions(+)
> 
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 61a6452..536b48d 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -25,6 +25,7 @@
>  #include <xen/rbtree.h>
>  #include <xen/sched.h>
>  #include <xen/sizes.h>
> +#include <xen/acpi.h>
>  #include <asm/gic.h>
>  #include <asm/gic_v3_defs.h>
>  #include <asm/gic_v3_its.h>
> @@ -32,6 +33,7 @@
>  #include <asm/page.h>
>  
>  #define ITS_CMD_QUEUE_SZ                SZ_1M
> +#define ACPI_GICV3_ITS_MEM_SIZE         SZ_64K

Although this is used for ACPI only, this size is really the architected
size for the ITS register frame and thus should be named like this,
possibly GUEST_GICV3_ITS_SIZE or so (in xen/include/public/arch-arm.h).
Which actually makes me wonder why we would need to store this size in
the data structure in the first place ...

>  /*
>   * No lock here, as this list gets only populated upon boot while scanning
> @@ -1018,6 +1020,30 @@ void gicv3_its_dt_init(const struct dt_device_node 
> *node)
>      }
>  }
>  
> +#ifdef CONFIG_ACPI
> +int gicv3_its_acpi_probe(struct acpi_subtable_header *header,
> +                        const unsigned long end)

                          w/s?
> +{
> +    struct acpi_madt_generic_translator *its;
> +
> +    its = (struct acpi_madt_generic_translator *)header;
> +    if ( BAD_MADT_ENTRY(its, end) )
> +        return -EINVAL;
> +
> +    add_to_host_its_list(its->base_address,
> +                        ACPI_GICV3_ITS_MEM_SIZE, NULL);

                          w/s?

> +
> +    return 0;
> +}
> +
> +void gicv3_its_acpi_init(void)
> +{
> +    /* Parse ITS information */
> +    acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
> +                                  gicv3_its_acpi_probe, 0);

                            w/s?

Cheers,
Andre.

> +}
> +#endif
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index f990eae..6f562f4 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1567,6 +1567,8 @@ static void __init gicv3_acpi_init(void)
>  
>      gicv3.rdist_stride = 0;
>  
> +    gicv3_its_acpi_init();
> +
>      /*
>       * In ACPI, 0 is considered as the invalid address. However the rest
>       * of the initialization rely on the invalid address to be
> diff --git a/xen/include/asm-arm/gic_v3_its.h 
> b/xen/include/asm-arm/gic_v3_its.h
> index 1fac1c7..993819a 100644
> --- a/xen/include/asm-arm/gic_v3_its.h
> +++ b/xen/include/asm-arm/gic_v3_its.h
> @@ -135,6 +135,9 @@ extern struct list_head host_its_list;
>  /* Parse the host DT and pick up all host ITSes. */
>  void gicv3_its_dt_init(const struct dt_device_node *node);
>  
> +#ifdef CONFIG_ACPI
> +void gicv3_its_acpi_init(void);
> +#endif
>  bool gicv3_its_host_has_its(void);
>  
>  unsigned int vgic_v3_its_count(const struct domain *d);
> @@ -196,6 +199,12 @@ static inline void gicv3_its_dt_init(const struct 
> dt_device_node *node)
>  {
>  }
>  
> +#ifdef CONFIG_ACPI
> +static inline void gicv3_its_acpi_init(void)
> +{
> +}
> +#endif
> +
>  static inline bool gicv3_its_host_has_its(void)
>  {
>      return false;
> 

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