Re: [RFC PATCH 10/21] i386/xen: handle guest hypercalls

[David Woodhouse]

On Tue, 2022-12-06 at 12:07 +0100, Philippe Mathieu-Daudé wrote:
> On 6/12/22 10:40, David Woodhouse wrote:
> > On Tue, 2022-12-06 at 09:16 +0100, Philippe Mathieu-Daudé wrote:
> > > +Thomas
> > > 
> > > On 6/12/22 02:10, David Woodhouse wrote:
> > > > On Mon, 2022-12-05 at 23:11 +0100, Philippe Mathieu-Daudé
> > > > wrote:
> > > > > On 5/12/22 18:31, David Woodhouse wrote:
> > > > > > +#ifdef CONFIG_XEN
> > > > > 
> > > > > CONFIG_XEN is set when the _host_ has Xen development files
> > > > > available.
> > > > > 
> > > > > IIUC here you want to check if Xen HVM guest support is
> > > > > enabled.
> > > > > 
> > > > > You might want to use a different CONFIG_XEN_xxx key, which
> > > > > itself
> > > > > depends on CONFIG_XEN.
> > > > 
> > > > Yeah, I'd be interested in opinions on that one.
> > > > 
> > > > Strictly, the only one that *needs* to be a configure option is
> > > > CONFIG_XEN for the Xen libraries, which is support for actually
> > > > running
> > > > on Xen.
> > > > 
> > > > Any time KVM is present, we *could* pull in the rest of the
> > > > xenfv
> > > > machine support unconditionally, since that's no longer
> > > > dependent on
> > > > true Xen.
> > > > 
> > > > But because there's a non-trivial amount of code in the event
> > > > channel
> > > > and grant table stuff, *perhaps* we want to make it optional? I
> > > > don't
> > > > really want to call that CONFIG_KVM_XEN since as noted, it's
> > > > theoretically possible to do it with TCG or other accelerators
> > > > too. So
> > > > we could call it CONFIG_XEN_EMULATION.
> > > 
> > > I concur CONFIG_KVM_XEN is confusing; CONFIG_XEN_EMULATION /
> > > CONFIG_XEN_EMU sounds better.
> > > 
> > > Is it useful to have the CONFIG_XEN_EMU code under target/i386/
> > > built
> > > without having the xenfv machine built in?
> > 
> > It isn't useful, no.
> > 
> > > I rather have hw/ and target/ features disentangled, so I'd use
> > > CONFIG_XEN_EMU under target/ and CONFIG_XENFV_MACHINE under hw/,
> > > eventually having CONFIG_XEN_EMU depending on
> > > CONFIG_XENFV_MACHINE
> > > and -- for now -- CONFIG_KVM.
> > 
> > Hm, I was thinking of XENFV_MACHINE as the parts which are needed
> > by
> > *both* XEN_EMU and real Xen. I think there are arch-independent
> > things
> > which want to go into hw/ like event channels and grant table
> > support;
> > you can think of those as an IRQ chip and an IOMMU respectively.
> > Since
> > those are emulation-only, they want to be conditional on XEN_EMU,
> > not
> > XENFV_MACHINE.
> > 
> > The core hypercall support lives in target/ and would call directly
> > to
> > gnttab_op/evtchn_op functions in hw/xen/ but I think that's OK. The
> > vCPU-specific things like timers and runstate can also stay in
> > target/.
> > 
> > Nothing in hw/ should explicitly mention KVM; the code in
> > target/i386/xen.c should wrap the KVM-specific implementations
> > unless
> > the pretence of future TCG support is really making it look awful.
> > 
> > Does that sound reasonable? Probably close enough, and we can take
> > an
> > other look at it once we see how it works out.
> > 
> > > > I don't think we'd make that depend on CONFIG_XEN though, since
> > > > none of
> > > > the actual Xen libraries would be needed once everything's
> > > > implemented
> > > > and cleaned up.
> > > 
> > > Agreed.
> > > 
> > > > So things like the xenfv machine code would then depend on
> > > > (CONFIG_XEN || CONFIG_XEN_EMULATION)... or we could make a new
> > > > automatic config symbol CONFIG_XEN_MACHINE which has the same
> > > > effect?
> > > 
> > > So per what you just cleared, not CONFIG_XEN but CONFIG_KVM.
> > 
> > I think it looks something like this...
> > 
> >  From 0a90999e37ec48b7fbd0467c243769b9bf726401 Mon Sep 17 00:00:00
> > 2001
> > From: David Woodhouse <
> > dwmw@amazon.co.uk
> > >
> > Date: Tue, 6 Dec 2022 09:03:48 +0000
> > Subject: [PATCH] Add CONFIG_XENFV_MACHINE and CONFIG_XEN_EMU
> > options for Xen
> >   emulation
> > 
> > The XEN_EMU option will cover core Xen support in target/, which
> > exists
> > only for x86 with KVM today but could theoretically also be
> > implemented
> > on Arm/Aarch64 and with TCG or other accelerators. It will also
> > cover
> > the support for architecture-independent grant table and event
> > channel
> > support which will be added in hw/xen/.
> > 
> > The XENFV_MACHINE option is for the xenfv platform support, which
> > will
> > now be used both by XEN_EMU and by real Xen.
> > 
> > The XEN option remains dependent on the Xen runtime libraries, and
> > covers
> > support for real Xen. Some code which currently resides under
> > CONFIG_XEN
> > will be moving to CONFIG_XENFV_MACHINE over time.
> > 
> > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> > ---
> >   accel/Kconfig  | 2 ++
> >   hw/Kconfig     | 1 +
> >   hw/xen/Kconfig | 2 ++
> >   meson.build    | 1 +
> >   target/Kconfig | 3 +++
> >   5 files changed, 9 insertions(+)
> >   create mode 100644 hw/xen/Kconfig
> > 
> > diff --git a/accel/Kconfig b/accel/Kconfig
> > index 8bdedb7d15..87d2880cad 100644
> > --- a/accel/Kconfig
> > +++ b/accel/Kconfig
> > @@ -15,7 +15,9 @@ config TCG
> >   
> >   config KVM
> >       bool
> > +    select XEN_EMU if (I386 || X86_64)
> 
> We certainly can build KVM without XEN_EMU...
> 
> You might want to s/select/imply/ here.

OK.

> >   config XEN
> >       bool
> >       select FSDEV_9P if VIRTFS
> > +    select XENFV_MACHINE
> 
> This is the XEN (host) accelerator switch... You said previously
> it is not related to the guest xenfv machine.

The xenfv platform definition, and various other things like the Xen
PCI platform device, are currently covered by CONFIG_XEN.

The idea is that we'd change that to CONFIG_XENFV_MACHINE so that they
can be present if *either* CONFIG_XEN or CONFIG_XEN_EMU are enabled.

So (given your later comment about target features not selecting a a
hw/machine component) I've made it:

config XENFV_MACHINE
    bool
    default y if (XEN || XEN_EMU)

How's this...

From eaff99027a888aac8a5df554ef22f37dc271be66 Mon Sep 17 00:00:00 2001
From: David Woodhouse <dwmw@amazon.co.uk>
Date: Tue, 6 Dec 2022 09:03:48 +0000
Subject: [PATCH] Add CONFIG_XENFV_MACHINE and CONFIG_XEN_EMU options for Xen
 emulation

The XEN_EMU option will cover core Xen support in target/, which exists
only for x86 with KVM today but could theoretically also be implemented
on Arm/Aarch64 and with TCG or other accelerators. It will also cover
the support for architecture-independent grant table and event channel
support which will be added in hw/xen/.

The XENFV_MACHINE option is for the xenfv platform support, which will
now be used both by XEN_EMU and by real Xen.

The XEN option remains dependent on the Xen runtime libraries, and covers
support for real Xen. Some code which currently resides under CONFIG_XEN
will be moving to CONFIG_XENFV_MACHINE over time.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 accel/Kconfig  | 1 +
 hw/Kconfig     | 1 +
 hw/xen/Kconfig | 3 +++
 meson.build    | 1 +
 target/Kconfig | 4 ++++
 5 files changed, 10 insertions(+)
 create mode 100644 hw/xen/Kconfig

diff --git a/accel/Kconfig b/accel/Kconfig
index 8bdedb7d15..41e089e610 100644
--- a/accel/Kconfig
+++ b/accel/Kconfig
@@ -15,6 +15,7 @@ config TCG
 
 config KVM
     bool
+    imply XEN_EMU if (I386 || X86_64)
 
 config XEN
     bool
diff --git a/hw/Kconfig b/hw/Kconfig
index 38233bbb0f..ba62ff6417 100644
--- a/hw/Kconfig
+++ b/hw/Kconfig
@@ -41,6 +41,7 @@ source tpm/Kconfig
 source usb/Kconfig
 source virtio/Kconfig
 source vfio/Kconfig
+source xen/Kconfig
 source watchdog/Kconfig
 
 # arch Kconfig
diff --git a/hw/xen/Kconfig b/hw/xen/Kconfig
new file mode 100644
index 0000000000..755c8b1faf
--- /dev/null
+++ b/hw/xen/Kconfig
@@ -0,0 +1,3 @@
+config XENFV_MACHINE
+    bool
+    default y if (XEN || XEN_EMU)
diff --git a/meson.build b/meson.build
index 5c6b5a1c75..9348cf572c 100644
--- a/meson.build
+++ b/meson.build
@@ -3828,6 +3828,7 @@ if have_system
   if xen.found()
     summary_info += {'xen ctrl version':  xen.version()}
   endif
+  summary_info += {'Xen emulation':     config_all.has_key('CONFIG_XEN_EMU')}
 endif
 summary_info += {'TCG support':       config_all.has_key('CONFIG_TCG')}
 if config_all.has_key('CONFIG_TCG')
diff --git a/target/Kconfig b/target/Kconfig
index 83da0bd293..e19c9d77b5 100644
--- a/target/Kconfig
+++ b/target/Kconfig
@@ -18,3 +18,7 @@ source sh4/Kconfig
 source sparc/Kconfig
 source tricore/Kconfig
 source xtensa/Kconfig
+
+config XEN_EMU
+    bool
+    depends on KVM && (I386 || X86_64)
-- 
2.25.1

smime.p7s
Description: S/MIME cryptographic signature