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

[David Woodhouse]

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)
 
 config XEN
     bool
     select FSDEV_9P if VIRTFS
+    select XENFV_MACHINE
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..066d74e4ff
--- /dev/null
+++ b/hw/xen/Kconfig
@@ -0,0 +1,2 @@
+config XENFV_MACHINE
+    bool
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..ceb6ddbf2a 100644
--- a/target/Kconfig
+++ b/target/Kconfig
@@ -18,3 +18,6 @@ source sh4/Kconfig
 source sparc/Kconfig
 source tricore/Kconfig
 source xtensa/Kconfig
+
+config XEN_EMU
+    select XENFV_MACHINE
-- 
2.25.1

smime.p7s
Description: S/MIME cryptographic signature