Re: [Xen-devel] [PATCH v6 4/4] x86/xen: Add "nopv" support for HVM guest

[Zhenzhong Duan <zhenzhong.duan@xxxxxxxxxx>]

On 2019/7/8 21:46, Boris Ostrovsky wrote:
> On 7/7/19 5:15 AM, Zhenzhong Duan wrote:
> >   
> > +static uint32_t __init xen_platform_hvm(void)
> > +{
> > +       if (xen_pv_domain())
> > +               return 0;
> > +
> > +       if (xen_pvh_domain() && nopv) {
> > +               /* Guest booting via the Xen-PVH boot entry goes here */
> > +               pr_info("\"nopv\" parameter is ignored in PVH guest\n");
> > +               nopv = false;
> > +       } else if (nopv) {
> > +               /*
> > +                * Guest booting via normal boot entry (like via grub2) goes
> > +                * here.
> > +                */
> > +               x86_init.hyper.guest_late_init = xen_hvm_guest_late_init;
> > +               return 0;
> After staring at this some more I don't think we can do this.
> Hypervisor-specific code should not muck with with x86_init.hyper, it's
> layering violation. That's what init_hypervisor_platform() is for.
Ok, I see.
> So we may have to create another hypervisor_x86 ops structure for Xen
> nopv (which I don't find very appealing TBH). Or update
> x86_hyper_xen_hvm and return xen_cpuid_base() instead of zero (but then
> you'd need to update all these structs from __initconst to _init or some
> such). Or something else.
I choose the second. I made below changes, not clear if it's also a 
layering violation
as use x86_init.hyper as source for memcpy. I choose memcpy instead of 
updating
functions pointers one-by-one because if x86_hyper_init interface 
functions extends
in the future we need no changes. Let me know if you prefer updating 
pointers directly.
This isn't a formal patch for review, just want to get answer of above 
question.
diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c

index 1756cf7..8416640d 100644
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -231,14 +231,6 @@ bool __init xen_hvm_need_lapic(void)
        return true;
 }

-static uint32_t __init xen_platform_hvm(void)
-{
-       if (xen_pv_domain())
-               return 0;
-
-       return xen_cpuid_base();
-}
-
 static __init void xen_hvm_guest_late_init(void)
 {
 #ifdef CONFIG_XEN_PVH
@@ -250,6 +242,9 @@ static __init void xen_hvm_guest_late_init(void)
        /* PVH detected. */
        xen_pvh = true;

+       if (nopv)
+               panic("\"nopv\" and \"xen_nopv\" parameters are 
unsupported in PVH guest.");
+
        /* Make sure we don't fall back to (default) ACPI_IRQ_MODEL_PIC. */
        if (!nr_ioapics && acpi_irq_model == ACPI_IRQ_MODEL_PIC)
                acpi_irq_model = ACPI_IRQ_MODEL_PLATFORM;
@@ -259,7 +254,36 @@ static __init void xen_hvm_guest_late_init(void)
 #endif
 }

-const __initconst struct hypervisor_x86 x86_hyper_xen_hvm = {
+static uint32_t __init xen_platform_hvm(void)
+{
+       uint32_t xen_domain = xen_cpuid_base();
+       struct x86_hyper_init *h = &x86_hyper_xen_hvm.init;
+
+       if (xen_pv_domain())
+               return 0;
+
+       if (xen_pvh_domain() && nopv) {
+               /* Guest booting via the Xen-PVH boot entry goes here */
+               pr_info("\"nopv\" parameter is ignored in PVH guest\n");
+               nopv = false;
+       } else if (nopv && xen_domain) {
+               /*
+                * Guest booting via normal boot entry (like via grub2) goes
+                * here.
+                *
+                * Use interface functions for bare hardware if nopv,
+                * xen_hvm_guest_late_init is an exception as we need to
+                * detect PVH and panic there.
+                */
+               memcpy(h, (void *)&x86_init.hyper, sizeof(x86_init.hyper));
+               memcpy(&x86_hyper_xen_hvm.runtime, (void 
*)&x86_platform.hyper,
+                      sizeof(x86_platform.hyper));
+               h->guest_late_init = xen_hvm_guest_late_init;
+       }
+       return xen_domain;
+}
+
+struct hypervisor_x86 x86_hyper_xen_hvm __initdata = {
        .name                   = "Xen HVM",
        .detect                 = xen_platform_hvm,
        .type                   = X86_HYPER_XEN_HVM,
@@ -268,4 +292,5 @@ static __init void xen_hvm_guest_late_init(void)
        .init.init_mem_mapping  = xen_hvm_init_mem_mapping,
        .init.guest_late_init   = xen_hvm_guest_late_init,
        .runtime.pin_vcpu       = xen_pin_vcpu,
+       .ignore_nopv            = true,
 };


Thanks

Zhenzhong


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel