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
