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. > + 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. Jan