On Fri, 2023-03-10 at 11:15 +0800, Xiaoyao Li wrote: > On 3/1/2023 9:51 PM, David Woodhouse wrote: > > From: David Woodhouse <d...@amazon.co.uk> > > > > The xen_overlay device (and later similar devices for event channels and > > grant tables) need to be instantiated. Do this from a kvm_type method on > > the PC machine derivatives, since KVM is only way to support Xen emulation > > for now. > > Just curious, isn't there any more reasonable place to add machine > specific initialization? > > abusing the mc->kvm_type() looks bad to me.
Hm, good question. Off the top of my head I have no better answer than "Paolo made me do it": https://lore.kernel.org/qemu-devel/8495140d-3301-7693-b804-055416680...@redhat.com/ But I have gained a bit more clue since December, and reading that message again I'll put a lot more focus on the fact that he said "during mc->kvm_type OR AFTERWARDS". That's the *earliest* we can depend on xen_mode being set to XEN_EMULATE, but now the dust has settled I don't think we actually *need* to do it that early. I'll have a play with moving it to pc_basic_device_init() where there's some other XEN_EMULATE initialisation anyway: --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1319,6 +1319,10 @@ void pc_basic_device_init(struct PCMachineState *pcms, #ifdef CONFIG_XEN_EMU if (xen_mode == XEN_EMULATE) { + xen_overlay_create(); + xen_evtchn_create(); + xen_gnttab_create(); + xen_xenstore_create(); xen_evtchn_connect_gsis(gsi); if (pcms->bus) { pci_create_simple(pcms->bus, -1, "xen-platform"); @@ -1868,14 +1872,6 @@ static void pc_machine_initfn(Object *obj) int pc_machine_kvm_type(MachineState *machine, const char *kvm_type) { -#ifdef CONFIG_XEN_EMU - if (xen_mode == XEN_EMULATE) { - xen_overlay_create(); - xen_evtchn_create(); - xen_gnttab_create(); - xen_xenstore_create(); - } -#endif return 0; } I have *also* gained some Avocado tests since December, which exercise Xen guests (with PV disk) in a bunch of different interrupt-delivery modes. And pass when I do the above. So I'll let the morning coffee take effect and give it a bit more thought and testing, and probably submit it. Shall I leave pc_machine_kvm_type() present but empty, given that you're about to come along and put something back there? Thanks.
smime.p7s
Description: S/MIME cryptographic signature
