On Tue, Nov 14, 2023 at 03:38:09PM +0100, Philippe Mathieu-Daudé wrote:
> Per commit f17068c1c7 ("xen-hvm: reorganize xen-hvm and move common
> function to xen-hvm-common"), handle_ioreq() is expected to be
> target-agnostic. However it uses 'target_ulong', which is a target
> specific definition.
>
> Per xen/include/public/hvm/ioreq.h header:
>
> struct ioreq {
> uint64_t addr; /* physical address */
> uint64_t data; /* data (or paddr of data) */
> uint32_t count; /* for rep prefixes */
> uint32_t size; /* size in bytes */
> uint32_t vp_eport; /* evtchn for notifications to/from device model */
> uint16_t _pad0;
> uint8_t state:4;
> uint8_t data_is_ptr:1; /* if 1, data above is the guest paddr
> * of the real data to use. */
> uint8_t dir:1; /* 1=read, 0=write */
> uint8_t df:1;
> uint8_t _pad1:1;
> uint8_t type; /* I/O type */
> };
> typedef struct ioreq ioreq_t;
>
> If 'data' is not a pointer, it is a u64.
>
> - In PIO / VMWARE_PORT modes, only 32-bit are used.
>
> - In MMIO COPY mode, memory is accessed by chunks of 64-bit
Looks like it could also be 8, 16, or 32 as well, depending on
req->size.
> - In PCI_CONFIG mode, access is u8 or u16 or u32.
>
> - None of TIMEOFFSET / INVALIDATE use 'req'.
>
> - Fallback is only used in x86 for VMWARE_PORT.
>
> Masking the upper bits of 'data' to keep 'req->size' low bits
> is irrelevant of the target word size. Remove the word size
> check and always extract the relevant bits.
When building QEMU for Xen, we tend to build the target "i386-softmmu",
which looks like to have target_ulong == uint32_t. So the `data`
clamping would only apply to size 8 and 16. The clamping with
target_ulong was introduce long time ago, here:
https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=b4a663b87df3954557434a2d31bff7f6b2706ec1
and they were more IOREQ types.
So my guess is it isn't relevant anymore, but extending the clamping to
32-bits request should be fine, when using qemu-system-i386 that is, as
it is already be done if one use qemu-system-x86_64.
So I think the patch is fine, and the tests I've ran so far worked fine.
> Signed-off-by: Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>
Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
Thanks,
--
Anthony PERARD
