Re: [PATCH v15 15/60] i386/xen: add pc_machine_kvm_type to initialize XEN_EMULATE mode

[David Woodhouse]

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 <dwmw@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-0554166802da@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