On Mon, 2022-12-12 at 18:30 +0100, Paolo Bonzini wrote: > On 12/9/22 10:55, David Woodhouse wrote: > > - m->default_machine_opts = "accel=xen,suppress-vmdesc=on"; > > + if (xen_enabled()) > > + m->default_machine_opts = "accel=xen,suppress-vmdesc=on"; > > + else > > + m->default_machine_opts = "accel=kvm,xen-version=0x30001"; > > Please do not modify pc_xen_hvm_init(). > > "-M xenfv" should be the same as "-M pc-i440fx-...,suppress-vmdesc=on > -accel xen -device xen-platform". It must work *without* "-accel xen", > while here you're basically requiring it. For now, please make > KVM-emulated Xen use "-M pc -device xen-platform".
I did that (well, you also need -accel kvm,xen_version=xxx). > We can figure out "-M xenfv" later. I don't know that we care about doing this. Sure, I could change the if (xen_enabled()) to something which isn't recursive, and works out which of Xen or KVM mode is *possible* right now. But having moved the xen-version from a machine property to a kvm-accel property, using "accel=kvm,xen-version=xxx" doesn't even work; not without the hackery to 'forward' that from the machine to the accelerator, like we do in qemu_apply_legacy_machine_options() for properties like kvm-shadow-mem. I don't think we want that. So I think I'm most inclined to rename the CONFIG_XENFV_MACHINE option to CONFIG_XEN_BUS, since that's basically what it ended up being once the rest of the patches took shape on top of the basic platform support. And drop the idea of making '-M xenfv' work altogether. I'll add some documentation on how to launch it using -accel kvm,xen-version=. > You can instead have: > > - a check in xen_init() that checks that xen_mode is XEN_ATTACH. If > not, fail. > > - an extra enum value for xen_mode, XEN_DISABLED, which is the default > instead of XEN_EMULATE; > > - an accelerator property "-accel kvm,xen-version=...", added in > kvm_accel_class_init() instead of the machine property. The property, > when set to a nonzero value, flips xen_mode from XEN_DISABLED to > XEN_EMULATE. > > The Xen overlay device can be created using the mc->kvm_type function > (which you can set in DEFINE_PC_MACHINE); at that point, xen_mode has > switched from XEN_DISABLED to XEN_EMULATE. Those xen_enabled() checks > that apply to KVM then become xen_mode != XEN_DISABLED, as long as they > run during mc->kvm_type or afterwards. > > The platform device can be created either in mc->kvm_type or manually > (not sure if it makes sense to have a "XenVMMXenVMM" CPUID + emulated > hypercalls but no platform device---would it still use pvclock for > example?). Yeah, I think fairly much everything *can* work without the platform device. It's only really the hook for the legacy event channel interrupt, and the unplug protocol. Linux does use its BAR to map grant tables over, but that's just a crutch to find a free bit of guest physical address space. But still, I think it makes sense to add it unconditionally as you suggest. In mc->kvm_type, pcms->bus isn't set yet but I can do it like this: --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1313,6 +1313,9 @@ void pc_basic_device_init(struct PCMachineState *pcms, #ifdef CONFIG_XEN_EMU if (xen_mode == XEN_EMULATE) { xen_evtchn_connect_gsis(gsi); + if (pcms->bus) { + pci_create_simple(pcms->bus, -1, "xen-platform"); + } } #endif
smime.p7s
Description: S/MIME cryptographic signature