Re: [PATCH-for-9.0 04/10] hw/xen: Factor xen_arch_align_ioreq_data() out of handle_ioreq()

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 1747 bytes --]

On Mon, 2023-11-13 at 16:21 +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.
> 
> In order to compile this file once for all targets, factor the
> target-specific code out of handle_ioreq() as a per-target handler
> called xen_arch_align_ioreq_data().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

I prefer commits like this to explicitly state 'No function change
intended', and on that basis:

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>

But...



> --- a/hw/i386/xen/xen-hvm.c
> +++ b/hw/i386/xen/xen-hvm.c
> @@ -699,6 +699,14 @@ void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
>      }
>  }
>  
> +void xen_arch_align_ioreq_data(ioreq_t *req)
> +{
> +    if (!req->data_is_ptr && (req->dir == IOREQ_WRITE)
> +            && (req->size < sizeof(target_ulong))) {
> +        req->data &= ((target_ulong) 1 << (8 * req->size)) - 1;
> +    }
> +}
> +

If a 64-bit Xen host is running a 32-bit guest, what is target_ulong,
and what is the actual alignment? I think we are actually communicating
with the 64-bit Xen and it's 64 bits, although the *guest* is 32?

I guess the only time when this would matter is when using
qemu-system-i386 as the device model on 64-bit Xen? And that's not
going to work for various reasons including this?

(I should clarify that I'm not objecting to your patch series, but I
just to understand just what the situation is, before you make it
*look* saner than it is... :)

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]