Re: [Xen-devel] [PATCH v2 1/2] xen/arm: GICv3: Parse ITS information from the firmware tables later on

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 24 Jan 2018, Julien Grall wrote:
> There are Device Tree (e.g for the Foundation Model) out that describes the
> ITS but LPIs is not supported by the platform. Booting with such DT will
> result to an early Data Abort. The same DT is booting fine with a
> baremetal  Linux because ITS will be initialized only when LPIs is
> supported.
> 
> While this is a bug in the DT, I think Xen should be boot with the same
> hardware level support (e.g ITS will not be used) as with a baremetal
> Linux.
> 
> The slight problem is Xen is relying on gicv3_its_host_has_its() to know
> if ITS can be used. The list is populated by gicv3_its_{dt,acpi}_init().
> It would theoritical be possible to gate those with a check of
           ^ theoretically


> GICD_TYPER.LPIS because we don't know yet whether the HW is an actual
> GICv3/GICv4.
> 
> Looking at the callers of gicv3_its_host_has_its(), they will only be
> done after gicv3_its_init() is called. Therefore move the parsing of ITS
> information from firmware tables later on.
> 
> Note that gicv3_its_init() has been moved at the end of the file to
> avoid forward declaration.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> 
> ---
> 
> I can move the code movement in a separate patch if necessary. It was
> small enough that I thought it would not be worth.

Yeah, that's OK, thanks for pointing it out

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


>     Changes in v2:
>         - Actually test compilation and boot of this patch...
> ---
>  xen/arch/arm/gic-v3-its.c        | 47 
> +++++++++++++++++++++++++---------------
>  xen/arch/arm/gic-v3.c            |  5 -----
>  xen/include/asm-arm/gic_v3_its.h | 12 ----------
>  3 files changed, 30 insertions(+), 34 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index e57ae05771..6127894d0b 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -515,21 +515,6 @@ static int gicv3_its_init_single_its(struct host_its 
> *hw_its)
>      return 0;
>  }
>  
> -int gicv3_its_init(void)
> -{
> -    struct host_its *hw_its;
> -    int ret;
> -
> -    list_for_each_entry(hw_its, &host_its_list, entry)
> -    {
> -        ret = gicv3_its_init_single_its(hw_its);
> -        if ( ret )
> -            return ret;
> -    }
> -
> -    return 0;
> -}
> -
>  /*
>   * TODO: Investigate the interaction when a guest removes a device while
>   * some LPIs are still in flight.
> @@ -1019,7 +1004,7 @@ static void add_to_host_its_list(paddr_t addr, paddr_t 
> size,
>  }
>  
>  /* Scan the DT for any ITS nodes and create a list of host ITSes out of it. 
> */
> -void gicv3_its_dt_init(const struct dt_device_node *node)
> +static void gicv3_its_dt_init(const struct dt_device_node *node)
>  {
>      const struct dt_device_node *its = NULL;
>  
> @@ -1056,7 +1041,7 @@ static int gicv3_its_acpi_probe(struct 
> acpi_subtable_header *header,
>      return 0;
>  }
>  
> -void gicv3_its_acpi_init(void)
> +static void gicv3_its_acpi_init(void)
>  {
>      /* Parse ITS information */
>      acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
> @@ -1081,8 +1066,36 @@ unsigned long gicv3_its_make_hwdom_madt(const struct 
> domain *d, void *base_ptr)
>  
>      return sizeof(struct acpi_madt_generic_translator) * 
> vgic_v3_its_count(d);
>  }
> +#else /* !CONFIG_ACPI */
> +
> +static void gicv3_its_acpi_init(void)
> +{
> +    ASSERT_UNREACHABLE();
> +}
> +
>  #endif
>  
> +int gicv3_its_init(void)
> +{
> +    struct host_its *hw_its;
> +    int ret;
> +
> +    if ( acpi_disabled )
> +        gicv3_its_dt_init(dt_interrupt_controller);
> +    else
> +        gicv3_its_acpi_init();
> +
> +    list_for_each_entry(hw_its, &host_its_list, entry)
> +    {
> +        ret = gicv3_its_init_single_its(hw_its);
> +        if ( ret )
> +            return ret;
> +    }
> +
> +    return 0;
> +}
> +
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index a0d290b55c..9f9cf59f82 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1314,9 +1314,6 @@ static void __init gicv3_dt_init(void)
>      if ( !res )
>          dt_device_get_address(node, 1 + gicv3.rdist_count + 2,
>                                &vbase, &vsize);
> -
> -    /* Check for ITS child nodes and build the host ITS list accordingly. */
> -    gicv3_its_dt_init(node);
>  }
>  
>  static int gicv3_iomem_deny_access(const struct domain *d)
> @@ -1608,8 +1605,6 @@ 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 40dffdc0fa..78a9bb14de 100644
> --- a/xen/include/asm-arm/gic_v3_its.h
> +++ b/xen/include/asm-arm/gic_v3_its.h
> @@ -133,11 +133,7 @@ struct host_its {
>  
>  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);
>  unsigned long gicv3_its_make_hwdom_madt(const struct domain *d,
>                                          void *base_ptr);
>  #endif
> @@ -202,15 +198,7 @@ void gicv3_lpi_update_host_entry(uint32_t host_lpi, int 
> domain_id,
>  
>  #else
>  
> -static inline void gicv3_its_dt_init(const struct dt_device_node *node)
> -{
> -}
> -
>  #ifdef CONFIG_ACPI
> -static inline void gicv3_its_acpi_init(void)
> -{
> -}
> -
>  static inline unsigned long gicv3_its_make_hwdom_madt(const struct domain *d,
>                                                        void *base_ptr)
>  {
> -- 
> 2.11.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel