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

[Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>]

On 13/11/23 16:58, Woodhouse, David wrote:
> On 13 Nov 2023 10:22, Philippe Mathieu-Daudé <philmd@xxxxxxxxxx> 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@xxxxxxxxxx>
>     ---
>     Should we have a 'unsigned qemu_target_long_bits();' helper
>     such qemu_target_page_foo() API and target_words_bigendian()?
>
>
> It can be more fun than that though. What about 
> qemu_target_alignof_uint64() for example, which differs between i386 and 
> x86_64 and causes even structs with *explicitly* sized fields to differ 
> because of padding.
> I'd *love* to see this series as a step towards my fantasy of being able 
> to support Xen under TCG. After all, without that what's the point in 
> being target-agnostic?
Another win is we are building all these files once instead of one for
each i386/x86_64/aarch64 targets, so we save CI time and Amazon trees.

> However, I am mildly concerned that some of these files are accidentally 
> using the host ELF ABI, perhaps with explicit management of 32-bit 
> compatibility, and the target-agnosticity is purely an illusion?
> See the "protocol" handling and the three ABIs for the ring in 
> xen-block, for example.
If so I'd expect build failures or violent runtime assertions.

Reviewing quickly hw/block/dataplane/xen-block.c, this code doesn't
seem target specific at all IMHO. Otherwise I'd really expect it to
fail compiling. But I don't know much about Xen, so I'll let block &
xen experts to have a look.

> Can we be explicit about what's expected to work here and what's not in 
> scope?
What do you mean? Everything is expected to work like without this
series applied :)

Regards,

Phil.