On Mon, 2022-12-05 at 23:06 +0100, Philippe Mathieu-Daudé wrote:
> On 5/12/22 18:31, David Woodhouse wrote:
> > From: Joao Martins <joao.m.martins@oracle.com>
> >
> > This allows -machine xenfv to work with Xen emulated guests.
> >
> > Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> > ---
> > hw/i386/pc_piix.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index 3dcac2f4b6..d1127adde0 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -404,8 +404,8 @@ static void pc_xen_hvm_init(MachineState *machine)
> > {
> > PCMachineState *pcms = PC_MACHINE(machine);
> >
> > - if (!xen_enabled()) {
> > - error_report("xenfv machine requires the xen accelerator");
> > + if (!xen_enabled() && (xen_mode != XEN_EMULATE)) {
> > + error_report("xenfv machine requires the xen or kvm accelerator");
> > exit(1);
> > }
>
> What about the XEN_EMULATE case? Shouldn't this be:
>
> if (!xen_enabled()) {
> if (xen_mode == XEN_EMULATE) {
> error_report("xenfv machine requires the xen accelerator");
> } else {
> error_report("xenfv machine requires the xen or kvm accelerator");
> }
> exit(1);
> }
>
> ?
Erm... that one I cherry-picked directly from the original and I
confess I haven't yet done much thinking about it.
There are two sane cases.
If xen_mode == XEN_ATTACH, then xen_enabled() should be true.
If xen_mode == XEN_EMULATED, then we'd better be using KVM with the Xen
support (which could *theoretically* be added to TCG if anyone really
wanted to).
So this check is working because it's allowing *either* xen_enabled()
*or* xen_mode==XEN_ATTACH to satisfy it. But it's too lax. I think it
should *require* KVM in the case of XEN_EMULATE.
That ought to be sufficient since it's going to set the xen-version
machine property, and that would cause KVM itself to bail out if the
required support isn't present in the kernel.
(I'm assuming the existing XEN_EMULATE mode is long dead along with
Xenner? Even on true Xen we run PV guests in a shim these days, and
that shim works without modification under KVM and will eventually be
one of my test cases as I get this all working under qemu)