Re: [RFC PATCH 01/16] x86/xen: add xenhost_t interface

[Juergen Gross]

On 11.06.19 09:16, Ankur Arora wrote:
> On 2019-06-07 8:04 a.m., Juergen Gross wrote:
> > On 09.05.19 19:25, Ankur Arora wrote:
> > > Add xenhost_t which will serve as an abstraction over Xen interfaces.
> > > It co-exists with the PV/HVM/PVH abstractions (x86_init, hypervisor_x86,
> > > pv_ops etc) and is meant to capture mechanisms for communication with
> > > Xen so we could have different types of underlying Xen: regular, local,
> > > and nested.
> > >
> > > Also add xenhost_register() and stub registration in the various guest
> > > types.
> > >
> > > Signed-off-by: Ankur Arora <ankur.a.arora@xxxxxxxxxx>
> > > ---
> > > Â arch/x86/xen/MakefileÂÂÂÂÂÂÂ |Â 1 +
> > > Â arch/x86/xen/enlighten_hvm.c | 13 +++++
> > >  arch/x86/xen/enlighten_pv.c | 16 ++++++
> > > Â arch/x86/xen/enlighten_pvh.c | 12 +++++
> > > Â arch/x86/xen/xenhost.cÂÂÂÂÂÂ | 75 ++++++++++++++++++++++++++++
> > > Â include/xen/xen.hÂÂÂÂÂÂÂÂÂÂÂ |Â 3 ++
> > > Â include/xen/xenhost.hÂÂÂÂÂÂÂ | 95 ++++++++++++++++++++++++++++++++++++
> > > Â 7 files changed, 215 insertions(+)
> > > Â create mode 100644 arch/x86/xen/xenhost.c
> > > Â create mode 100644 include/xen/xenhost.h
> > >
> > > diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
> > > index 084de77a109e..564b4dddbc15 100644
> > > --- a/arch/x86/xen/Makefile
> > > +++ b/arch/x86/xen/Makefile
> > > @@ -18,6 +18,7 @@ obj-yÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ += mmu.o
> > > Â obj-yÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ += time.o
> > > Â obj-yÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ += grant-table.o
> > > Â obj-yÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ += suspend.o
> > > +obj-yÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ += xenhost.o
> > > Â obj-$(CONFIG_XEN_PVHVM)ÂÂÂÂÂÂÂ += enlighten_hvm.o
> > > Â obj-$(CONFIG_XEN_PVHVM)ÂÂÂÂÂÂÂ += mmu_hvm.o
> > > diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
> > > index 0e75642d42a3..100452f4f44c 100644
> > > --- a/arch/x86/xen/enlighten_hvm.c
> > > +++ b/arch/x86/xen/enlighten_hvm.c
> > > @@ -5,6 +5,7 @@
> > > Â #include <linux/kexec.h>
> > > Â #include <linux/memblock.h>
> > > +#include <xen/xenhost.h>
> > > Â #include <xen/features.h>
> > > Â #include <xen/events.h>
> > > Â #include <xen/interface/memory.h>
> > > @@ -82,6 +83,12 @@ static void __init xen_hvm_init_mem_mapping(void)
> > > ÂÂÂÂÂ xen_vcpu_info_reset(0);
> > > Â }
> > > +xenhost_ops_t xh_hvm_ops = {
> > > +};
> > > +
> > > +xenhost_ops_t xh_hvm_nested_ops = {
> > > +};
> > > +
> > > Â static void __init init_hvm_pv_info(void)
> > > Â {
> > > ÂÂÂÂÂ int major, minor;
> > > @@ -179,6 +186,12 @@ static void __init xen_hvm_guest_init(void)
> > > Â {
> > > ÂÂÂÂÂ if (xen_pv_domain())
> > > ÂÂÂÂÂÂÂÂÂ return;
> > > +ÂÂÂ /*
> > > +ÂÂÂÂ * We need only xenhost_r1 for HVM guests since they cannot be
> > > +ÂÂÂÂ * driver domain (?) or dom0.
> >
> > I think even HVM guests could (in theory) be driver domains.
> >
> > > +ÂÂÂÂ */
> > > +ÂÂÂ if (!xen_pvh_domain())
> > > +ÂÂÂÂÂÂÂ xenhost_register(xenhost_r1, &xh_hvm_ops);
> > > ÂÂÂÂÂ init_hvm_pv_info();
> > > diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
> > > index c54a493e139a..bb6e811c1525 100644
> > > --- a/arch/x86/xen/enlighten_pv.c
> > > +++ b/arch/x86/xen/enlighten_pv.c
> > > @@ -36,6 +36,7 @@
> > > Â #include <xen/xen.h>
> > > Â #include <xen/events.h>
> > > +#include <xen/xenhost.h>
> > > Â #include <xen/interface/xen.h>
> > > Â #include <xen/interface/version.h>
> > > Â #include <xen/interface/physdev.h>
> > > @@ -1188,6 +1189,12 @@ static void __init 
> > > xen_dom0_set_legacy_features(void)
> > > ÂÂÂÂÂ x86_platform.legacy.rtc = 1;
> > > Â }
> > > +xenhost_ops_t xh_pv_ops = {
> > > +};
> > > +
> > > +xenhost_ops_t xh_pv_nested_ops = {
> > > +};
> > > +
> > > Â /* First C function to be called on Xen boot */
> > > Â asmlinkage __visible void __init xen_start_kernel(void)
> > > Â {
> > > @@ -1198,6 +1205,15 @@ asmlinkage __visible void __init 
> > > xen_start_kernel(void)
> > > ÂÂÂÂÂ if (!xen_start_info)
> > > ÂÂÂÂÂÂÂÂÂ return;
> > > +ÂÂÂ xenhost_register(xenhost_r1, &xh_pv_ops);
> > > +
> > > +ÂÂÂ /*
> > > +ÂÂÂÂ * Detect in some implementation defined manner whether this is
> > > +ÂÂÂÂ * nested or not.
> > > +ÂÂÂÂ */
> > > +ÂÂÂ if (xen_driver_domain() && xen_nested())
> > > +ÂÂÂÂÂÂÂ xenhost_register(xenhost_r2, &xh_pv_nested_ops);
> >
> > I don't think a driver domain other than dom0 "knows" this in the
> > beginning. It will need to register xenhost_r2
> Right. No point in needlessly registrating as xenhost_r2 without
> needing to handle any xenhost_r2 devices.
>
> > Âin case it learns about a pv device from L0 hypervisor.
> What's the mechanism you are thinking of, for this?
> I'm guessing this PV device notification could arrive at an
> arbitrary point in time after the system has booted.

I'm not sure yet how this should be handled.

Maybe an easy solution would be the presence of a Xen PCI device
passed through from L1 hypervisor to L1 dom0. OTOH this would
preclude nested Xen for L1 hypervisor running in PVH mode. And for
L1 driver domains this would need either a shared PCI device or
multiple Xen PCI devices or something new.

There is a design session planned for this topic at the Xen developer
summit in July.


Juergen