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

[André Przywara <andre.przywara@xxxxxxx>]

On 06/04/17 23:19, Julien Grall wrote:
> 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

Doh, good catch.

Cheers,
Andre


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