Re: [Xen-devel] [PATCH v5 06/30] ARM: GICv3 ITS: allocate device and collection table

[Julien Grall <julien.grall@xxxxxxx>]

Hi Andre,

On 04/06/2017 12:18 AM, Andre Przywara wrote:
> Each ITS maps a pair of a DeviceID (for instance derived from a PCI
> b/d/f triplet) and an EventID (the MSI payload or interrupt ID) to a
> pair of LPI number and collection ID, which points to the target CPU.
> This mapping is stored in the device and collection tables, which software
> has to provide for the ITS to use.
> Allocate the required memory and hand it over to the ITS.
>
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> ---
>  xen/arch/arm/gic-v3-its.c        | 132 +++++++++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/gic_v3_its.h |  32 ++++++++++
>  2 files changed, 164 insertions(+)
>
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 0298866..eef2933 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -35,9 +35,109 @@ bool gicv3_its_host_has_its(void)
>      return !list_empty(&host_its_list);
>  }
>
> +#define BASER_ATTR_MASK                                           \
> +        ((0x3UL << GITS_BASER_SHAREABILITY_SHIFT)               | \
> +         (0x7UL << GITS_BASER_OUTER_CACHEABILITY_SHIFT)         | \
> +         (0x7UL << GITS_BASER_INNER_CACHEABILITY_SHIFT))
> +#define BASER_RO_MASK   (GENMASK_ULL(58, 56) | GENMASK_ULL(52, 48))
It is a bit odd that you have defined BASER_RO_MASK and ...

[...]

> diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
> index 295eb22..6e51b98 100644
> --- a/xen/include/asm-arm/gic_v3_its.h
> +++ b/xen/include/asm-arm/gic_v3_its.h
[...]

> +#define GITS_BASER_OUTER_CACHEABILITY_SHIFT        53
> +#define GITS_BASER_TYPE_NONE            0UL
> +#define GITS_BASER_TYPE_DEVICE          1UL
> +#define GITS_BASER_TYPE_VCPU            2UL
> +#define GITS_BASER_TYPE_CPU             3UL
> +#define GITS_BASER_TYPE_COLLECTION      4UL
> +#define GITS_BASER_TYPE_RESERVED5       5UL
> +#define GITS_BASER_TYPE_RESERVED6       6UL
> +#define GITS_BASER_TYPE_RESERVED7       7UL
> +#define GITS_BASER_ENTRY_SIZE_SHIFT     48
> +#define GITS_BASER_ENTRY_SIZE(reg)                                       \
> +                        (((reg >> GITS_BASER_ENTRY_SIZE_SHIFT) & 0x1f) + 1)
> +#define GITS_BASER_SHAREABILITY_SHIFT   10
> +#define GITS_BASER_PAGE_SIZE_SHIFT      8
> +#define GITS_BASER_RO_MASK              (GITS_BASER_TYPE_MASK | \
> +                                        (31UL << GITS_BASER_ENTRY_SIZE_SHIFT) 
> |\
> +                                        GITS_BASER_INDIRECT)
... GITS_BASER_RO_MASK which from my understand covers the same 
registers but for different purpose.
Looking more closely, this last definition should belong to vgic-v3-its.c

Cheers,

--
Julien Grall

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