On 5/15/24 02:32, Jan Beulich wrote:
> On 14.05.2024 22:31, Stewart Hildebrand wrote:
>> Here's what the patch ("arm/vpci: honor access size when returning an
>> error") now looks like based on staging:
>>
>> diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c
>> index 3bc4bb55082a..31e9e1d20751 100644
>> --- a/xen/arch/arm/vpci.c
>> +++ b/xen/arch/arm/vpci.c
>> @@ -29,6 +29,8 @@ static int vpci_mmio_read(struct vcpu *v, mmio_info_t
>> *info,
>> {
>> struct pci_host_bridge *bridge = p;
>> pci_sbdf_t sbdf = vpci_sbdf_from_gpa(bridge, info->gpa);
>> + const uint8_t access_size = (1U << info->dabt.size) * 8;
>
> And why exactly uint8_t here, rather than unsigned int? See ./CODING_STYLE.
I'll change to unsigned int.
>
>> + const uint64_t invalid = GENMASK_ULL(access_size - 1, 0);
>
> I'm not entirely convinced of uint64_t here either, but I'd view this as
> more borderline than the uint8_t above. As per ...
>
>> @@ -39,7 +41,7 @@ static int vpci_mmio_read(struct vcpu *v, mmio_info_t
>> *info,
>> return 1;
>> }
>>
>> - *r = ~0ul;
>> + *r = invalid;
>
> ... the original rhs here, unsigned long (or perhaps register_t) would seem
> more appropriate, but I have no idea whether on Arm32 info->dabt.size can
> end up being 3.
Well, it depends which spec we look at. In the ARMv7 ARM, 3 is reserved.
But in the ARMv8 ARM, in the aarch32 section, 3 appears to be valid...
Anyway, since the value returned in r can only be as wide as register_t
due to the way our mmio handlers are implemented, I'll change to
register_t for now to match the lhs.
