[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 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? 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? Attachment:
smime.p7s
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |