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

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 2358 bytes --]

On Fri, 2023-03-10 at 09:52 +0100, Paolo Bonzini wrote:
> 
> I don't think this is abusing mc->kvm_type; that is the point where 
> startup code tells the machine "now you have your accelerator 
> configuration, do what you want with that info".  In fact I find using 
> xen_enabled() in mc->kvm_type to be less ugly than having a MachineClass 
> entry just for KVM. :)
> 
> But, if you want to move it to pc_basic_device_init() that is certainly 
> okay too.  It's not like people are going to add TCG/Xen support 
> tomorrow but it is a tiny step in the direction of less 
> accelerator-specific code for Xen emulation.

Yeah, I think I will.

We can't just leave it as it is; it needs *either* a coherent comment
explaining why the init calls happen from different locations, *or*
they need to be put together. Xiaoyao has triggered my "if it needs
documenting, fix it first and then explain what's left" instinct.

And it isn't even just a case of where we call the xen* functions from;
the xen_evtchn_connect_gsis() call *only* exists as a separate function
because we didn't have the GSIs available early enough to pass them to
xen_evtchn_create(). If we do it all in pc_basic_devices_init() then
that can be simplified too.

I cannot come up with a coherent comment explaining the current
situation, and thus I must fix it :)

Speaking of the 'fix it first' instinct... looking back at that mail
from December reminds me that I wanted to make '-M xenfv' work. Because
on the users' behalf, I *hate* what I had to write in
https://qemu-project.gitlab.io/qemu/system/i386/xen.html

qemu-system-x86_64 -M pc --accel kvm,xen-version=0x4000a,kernel-irqchip=split \
     -drive file=${GUEST_IMAGE},if=none,id=disk -device xen-disk,drive=disk,vdev=xvda

Why in the name of all that is holy can't that be

 qemu-system-x86_64 -M xenfv -drive file=${GUEST_IMAGE},if=xen-disk,xen-block.vdev=xvda

Now *maybe* I could live with them also having to add '-accel kvm'. If
we can genuinely make the argument that QEMU running on a Linux/KVM
host and not in a Xen dom0 couldn't possibly be expected to work that
out for itself. 

And in fact, why wouldn't it default to xen-disk for a Xen guest? Why
should the user have to specify that? And why *can't* it default to
xvda for the first (and only) present disk.... 

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]