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

[Julien Grall <julien.grall@xxxxxxx>]

Hi Manish,

On 21/09/17 14:17, 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        | 24 ++++++++++++++++++++++++
>   xen/arch/arm/gic-v3.c            |  2 ++
>   xen/include/asm-arm/gic_v3_its.h | 10 ++++++++++
>   3 files changed, 36 insertions(+)
>
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 0610991..0f662cf 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -18,6 +18,7 @@
>    * along with this program; If not, see <http://www.gnu.org/licenses/>.
>    */
>   
> +#include <xen/acpi.h>
>   #include <xen/lib.h>
>   #include <xen/delay.h>
>   #include <xen/libfdt/libfdt.h>
> @@ -1018,6 +1019,29 @@ void gicv3_its_dt_init(const struct dt_device_node *node)
>       }
>   }
>   
> +#ifdef CONFIG_ACPI
> +static int gicv3_its_acpi_probe(struct acpi_subtable_header *header,
> +                                const unsigned long end)
> +{
> +    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, GICV3_ITS_SIZE, NULL);
After the comment from Andre, I was expecting some rework to avoid store 
the size of the ITS in host_its. So what's the plan for that?
> +
> +    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);
The indentation still looks wrong here.

> +}
> +#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..e1be33c 100644
> --- a/xen/include/asm-arm/gic_v3_its.h
> +++ b/xen/include/asm-arm/gic_v3_its.h
> @@ -20,6 +20,7 @@
>   #ifndef __ASM_ARM_ITS_H__
>   #define __ASM_ARM_ITS_H__
>   
> +#define GICV3_ITS_SIZE                  SZ_128K
A less random place for this is close to the ITS_DOORBELL_OFFSET definition.

>   #define GITS_CTLR                       0x000
>   #define GITS_IIDR                       0x004
>   #define GITS_TYPER                      0x008
> @@ -135,6 +136,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 +200,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;
Cheers,

--
Julien Grall

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