[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH-for-9.0 04/10] hw/xen: Factor xen_arch_align_ioreq_data() out of handle_ioreq()
On 13/11/23 18:11, David Woodhouse wrote: On Mon, 2023-11-13 at 17:09 +0100, Philippe Mathieu-Daudé wrote: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.Heh, mostly the guest just crashes in the cases I've seen so far. See commit a1c1082908d ("hw/xen: use correct default protocol for xen- block on x86").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.Where it checks dataplane->protocol and does different things for BLKIF_PROTOCOL_NATIVE/BLKIF_PROTOCOL_X86_32/BLKIF_PROTOCOL_X86_64, the *structures* it uses are intended to be using the correct ABI. I think the structs for BLKIF_PROTOCOL_NATIVE may actually be *different* according to the target, in theory? OK I see what you mean, blkif_back_rings_t union in hw/block/xen_blkif.h These structures shouldn't differ between targets, this is the point of an ABI :) And if they were, they wouldn't compile as target agnostic. I don't know that they are *correct* right now, if the host is different from the target. But that's just a bug (that only matters if we ever want to support Xen-compatible guests using TCG).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 :)I think that if we ever do support Xen-compatible guests using TCG, we'll have to fix that bug and use the right target-specific structures... and then perhaps we'll want the affected files to actually become target-specfic again? I think this series makes it look like target-agnostic support *should* work... but it doesn't really? For testing we have: aarch64: tests/avocado/boot_xen.py x86_64: tests/avocado/kvm_xen_guest.py No combination with i386 is tested, Xen within aarch64 KVM is not tested (not sure it works).
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |