[Xen-devel] [PATCH v2 3/7] x86: Add nopv parameter to disable PV extensions

[Xen-devel] [PATCH v2 3/7] x86: Add nopv parameter to disable PV extensions

[Zhenzhong Duan]

In virtualization environment, PV extensions (drivers, interrupts,
timers, etc) are enabled in the majority of use cases which is the
best option.

However, in some cases (kexec not fully working, benchmarking)
we want to disable PV extensions. As such introduce the
'nopv' parameter that will do it.

There are guest types which just won't work without PV extensions,
like Xen PV, Xen PVH and jailhouse. add a "ignore_nopv" member to
struct hypervisor_x86 set to true for those guest types and call
the detect functions only if nopv is false or ignore_nopv is true.

There is already 'xen_nopv' parameter for XEN platform but not for
others. 'xen_nopv' can then be removed with this change.

Suggested-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: xen-devel@lists.xenproject.org
---
 Documentation/admin-guide/kernel-parameters.txt |  5 +++++
 arch/x86/include/asm/hypervisor.h               |  3 +++
 arch/x86/kernel/cpu/hypervisor.c                | 11 +++++++++++
 arch/x86/kernel/jailhouse.c                     |  1 +
 arch/x86/xen/enlighten_pv.c                     |  1 +
 5 files changed, 21 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 138f666..21e08af 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5268,6 +5268,11 @@
 			improve timer resolution at the expense of processing
 			more timer interrupts.
 
+	nopv=		[X86,XEN,KVM,HYPER_V,VMWARE]
+			Disables the PV optimizations forcing the guest to run
+			as generic guest with no PV drivers. Currently support
+			XEN HVM, KVM, HYPER_V and VMWARE guest.
+
 	xirc2ps_cs=	[NET,PCMCIA]
 			Format:
 			<irq>,<irq_mask>,<io>,<full_duplex>,<do_sound>,<lockup_hack>[,<irq2>[,<irq3>[,<irq4>]]]
diff --git a/arch/x86/include/asm/hypervisor.h b/arch/x86/include/asm/hypervisor.h
index 8c5aaba..d75d2ea 100644
--- a/arch/x86/include/asm/hypervisor.h
+++ b/arch/x86/include/asm/hypervisor.h
@@ -52,6 +52,9 @@ struct hypervisor_x86 {
 
 	/* runtime callbacks */
 	struct x86_hyper_runtime runtime;
+
+	/* ignore nopv parameter */
+	bool ignore_nopv;
 };
 
 extern enum x86_hypervisor_type x86_hyper_type;
diff --git a/arch/x86/kernel/cpu/hypervisor.c b/arch/x86/kernel/cpu/hypervisor.c
index 479ca47..337ff07 100644
--- a/arch/x86/kernel/cpu/hypervisor.c
+++ b/arch/x86/kernel/cpu/hypervisor.c
@@ -54,6 +54,14 @@
 enum x86_hypervisor_type x86_hyper_type;
 EXPORT_SYMBOL(x86_hyper_type);
 
+bool __initdata nopv;
+static __init int parse_nopv(char *arg)
+{
+	nopv = true;
+	return 0;
+}
+early_param("nopv", parse_nopv);
+
 static inline const struct hypervisor_x86 * __init
 detect_hypervisor_vendor(void)
 {
@@ -61,6 +69,9 @@
 	uint32_t pri, max_pri = 0;
 
 	for (p = hypervisors; p < hypervisors + ARRAY_SIZE(hypervisors); p++) {
+		if (unlikely(nopv) && !(*p)->ignore_nopv)
+			continue;
+
 		pri = (*p)->detect();
 		if (pri > max_pri) {
 			max_pri = pri;
diff --git a/arch/x86/kernel/jailhouse.c b/arch/x86/kernel/jailhouse.c
index d96d563..880329f 100644
--- a/arch/x86/kernel/jailhouse.c
+++ b/arch/x86/kernel/jailhouse.c
@@ -217,4 +217,5 @@ static bool __init jailhouse_x2apic_available(void)
 	.detect			= jailhouse_detect,
 	.init.init_platform	= jailhouse_init_platform,
 	.init.x2apic_available	= jailhouse_x2apic_available,
+	.ignore_nopv		= true,
 };
diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 4722ba2..5d16824 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -1463,4 +1463,5 @@ static uint32_t __init xen_platform_pv(void)
 	.detect                 = xen_platform_pv,
 	.type			= X86_HYPER_XEN_PV,
 	.runtime.pin_vcpu       = xen_pin_vcpu,
+	.ignore_nopv		= true,
 };
-- 
1.8.3.1


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

Re: [Xen-devel] [PATCH v2 3/7] x86: Add nopv parameter to disable PV extensions

[Juergen Gross]

On 24.06.19 14:02, Zhenzhong Duan wrote:
> In virtualization environment, PV extensions (drivers, interrupts,
> timers, etc) are enabled in the majority of use cases which is the
> best option.
> 
> However, in some cases (kexec not fully working, benchmarking)
> we want to disable PV extensions. As such introduce the
> 'nopv' parameter that will do it.
> 
> There are guest types which just won't work without PV extensions,
> like Xen PV, Xen PVH and jailhouse. add a "ignore_nopv" member to
> struct hypervisor_x86 set to true for those guest types and call
> the detect functions only if nopv is false or ignore_nopv is true.
> 
> There is already 'xen_nopv' parameter for XEN platform but not for
> others. 'xen_nopv' can then be removed with this change.
> 
> Suggested-by: Juergen Gross <jgross@suse.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: xen-devel@lists.xenproject.org

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

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

[Xen-devel] [PATCH v2 4/7] Revert "xen: Introduce 'xen_nopv' to disable PV extensions for HVM guests."

[Zhenzhong Duan]

This reverts commit 8d693b911bb9c57009c24cb1772d205b84c7985c.

Instead we use an unified parameter 'nopv' for all the hypervisor
platforms.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: xen-devel@lists.xenproject.org
---
 Documentation/admin-guide/kernel-parameters.txt |  4 ----
 arch/x86/xen/enlighten_hvm.c                    | 12 +-----------
 2 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 21e08af..d5c3dcc 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5251,10 +5251,6 @@
 			Disables the ticketlock slowpath using Xen PV
 			optimizations.
 
-	xen_nopv	[X86]
-			Disables the PV optimizations forcing the HVM guest to
-			run as generic HVM guest with no PV drivers.
-
 	xen_scrub_pages=	[XEN]
 			Boolean option to control scrubbing pages before giving them back
 			to Xen, for use by other domains. Can be also changed at runtime
diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
index ac4943c..7fcb4ea 100644
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -210,18 +210,8 @@ static void __init xen_hvm_guest_init(void)
 #endif
 }
 
-static bool xen_nopv;
-static __init int xen_parse_nopv(char *arg)
-{
-       xen_nopv = true;
-       return 0;
-}
-early_param("xen_nopv", xen_parse_nopv);
-
 bool __init xen_hvm_need_lapic(void)
 {
-	if (xen_nopv)
-		return false;
 	if (xen_pv_domain())
 		return false;
 	if (!xen_hvm_domain())
@@ -233,7 +223,7 @@ bool __init xen_hvm_need_lapic(void)
 
 static uint32_t __init xen_platform_hvm(void)
 {
-	if (xen_pv_domain() || xen_nopv)
+	if (xen_pv_domain())
 		return 0;
 
 	return xen_cpuid_base();
-- 
1.8.3.1


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

Re: [Xen-devel] [PATCH v2 4/7] Revert "xen: Introduce 'xen_nopv' to disable PV extensions for HVM guests."

[Juergen Gross]

On 24.06.19 14:02, Zhenzhong Duan wrote:
> This reverts commit 8d693b911bb9c57009c24cb1772d205b84c7985c.
> 
> Instead we use an unified parameter 'nopv' for all the hypervisor
> platforms.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: xen-devel@lists.xenproject.org

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

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

Re: [Xen-devel] [PATCH v2 4/7] Revert "xen: Introduce 'xen_nopv' to disable PV extensions for HVM guests."

[Thomas Gleixner]

On Wed, 26 Jun 2019, Juergen Gross wrote:
> On 24.06.19 14:02, Zhenzhong Duan wrote:
> > This reverts commit 8d693b911bb9c57009c24cb1772d205b84c7985c.
> > 
> > Instead we use an unified parameter 'nopv' for all the hypervisor
> > platforms.
> > 
> > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
> > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > Cc: Juergen Gross <jgross@suse.com>
> > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Borislav Petkov <bp@alien8.de>
> > Cc: xen-devel@lists.xenproject.org
> 
> Reviewed-by: Juergen Gross <jgross@suse.com>

You're really sure that you want to break existing setups which might use
that already?

Command line parameters are ABI. You can map xen_nopv to the new shiny nopv
implementation but removing it?

Your decision, but you've been told :)

Thanks,

	tglx

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

[Xen-devel] [PATCH v2 5/7] x86/xen: nopv parameter support for HVM guest

[Zhenzhong Duan]

PVH guest needs PV extentions to work, so nopv parameter is ignored
for PVH but not for HVM guest.

In order for nopv parameter to take effect for HVM guest, we need to
distinguish between PVH and HVM guest early in hypervisor detection
code. By moving the detection of PVH in xen_platform_hvm(),
xen_pvh_domain() could be used for that purpose.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: xen-devel@lists.xenproject.org
---
 arch/x86/xen/enlighten_hvm.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
index 7fcb4ea..26939e7 100644
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -25,6 +25,7 @@
 #include "mmu.h"
 #include "smp.h"
 
+extern bool nopv;
 static unsigned long shared_info_pfn;
 
 void xen_hvm_init_shared_info(void)
@@ -226,20 +227,24 @@ static uint32_t __init xen_platform_hvm(void)
 	if (xen_pv_domain())
 		return 0;
 
+#ifdef CONFIG_XEN_PVH
+	/* Test for PVH domain (PVH boot path taken overrides ACPI flags). */
+	if (!x86_platform.legacy.rtc && x86_platform.legacy.no_vga)
+		xen_pvh = true;
+#endif
+
+	if (!xen_pvh_domain() && nopv)
+		return 0;
+
 	return xen_cpuid_base();
 }
 
 static __init void xen_hvm_guest_late_init(void)
 {
 #ifdef CONFIG_XEN_PVH
-	/* Test for PVH domain (PVH boot path taken overrides ACPI flags). */
-	if (!xen_pvh &&
-	    (x86_platform.legacy.rtc || !x86_platform.legacy.no_vga))
+	if (!xen_pvh)
 		return;
 
-	/* PVH detected. */
-	xen_pvh = true;
-
 	/* 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;
@@ -258,4 +263,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,
 };
-- 
1.8.3.1


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

Re: [Xen-devel] [PATCH v2 5/7] x86/xen: nopv parameter support for HVM guest

[Juergen Gross]

On 24.06.19 14:02, Zhenzhong Duan wrote:
> PVH guest needs PV extentions to work, so nopv parameter is ignored
> for PVH but not for HVM guest.
> 
> In order for nopv parameter to take effect for HVM guest, we need to
> distinguish between PVH and HVM guest early in hypervisor detection
> code. By moving the detection of PVH in xen_platform_hvm(),
> xen_pvh_domain() could be used for that purpose.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: xen-devel@lists.xenproject.org
> ---
>   arch/x86/xen/enlighten_hvm.c | 18 ++++++++++++------
>   1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
> index 7fcb4ea..26939e7 100644
> --- a/arch/x86/xen/enlighten_hvm.c
> +++ b/arch/x86/xen/enlighten_hvm.c
> @@ -25,6 +25,7 @@
>   #include "mmu.h"
>   #include "smp.h"
>   
> +extern bool nopv;
>   static unsigned long shared_info_pfn;
>   
>   void xen_hvm_init_shared_info(void)
> @@ -226,20 +227,24 @@ static uint32_t __init xen_platform_hvm(void)
>   	if (xen_pv_domain())
>   		return 0;
>   
> +#ifdef CONFIG_XEN_PVH
> +	/* Test for PVH domain (PVH boot path taken overrides ACPI flags). */
> +	if (!x86_platform.legacy.rtc && x86_platform.legacy.no_vga)
> +		xen_pvh = true;

Sorry, this won't work, as ACPI tables are scanned only some time later.

You can test for xen_pvh being true here (for the case where the guest
has been booted via the Xen-PVH boot entry) and handle that case, but
the case of a PVH guest started via the normal boot entry (like via
grub2) and nopv specified is difficult. The only idea I have right now
would be to use another struct hypervisor_x86 for that case which will
only be used for Xen HVM/PVH _and_ nopv specified. It should be a copy
of the bare metal variant, but a special guest_late_init member issuing
a big fat warning in case PVH is being detected.


Juergen

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

Re: [Xen-devel] [PATCH v2 5/7] x86/xen: nopv parameter support for HVM guest

[Zhenzhong Duan]

On 2019/6/25 20:31, Juergen Gross wrote:
> On 24.06.19 14:02, Zhenzhong Duan wrote:
>> PVH guest needs PV extentions to work, so nopv parameter is ignored
>> for PVH but not for HVM guest.
>>
>> In order for nopv parameter to take effect for HVM guest, we need to
>> distinguish between PVH and HVM guest early in hypervisor detection
>> code. By moving the detection of PVH in xen_platform_hvm(),
>> xen_pvh_domain() could be used for that purpose.
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
>> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> Cc: Juergen Gross <jgross@suse.com>
>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Borislav Petkov <bp@alien8.de>
>> Cc: xen-devel@lists.xenproject.org
>> ---
>>   arch/x86/xen/enlighten_hvm.c | 18 ++++++++++++------
>>   1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
>> index 7fcb4ea..26939e7 100644
>> --- a/arch/x86/xen/enlighten_hvm.c
>> +++ b/arch/x86/xen/enlighten_hvm.c
>> @@ -25,6 +25,7 @@
>>   #include "mmu.h"
>>   #include "smp.h"
>>   +extern bool nopv;
>>   static unsigned long shared_info_pfn;
>>     void xen_hvm_init_shared_info(void)
>> @@ -226,20 +227,24 @@ static uint32_t __init xen_platform_hvm(void)
>>       if (xen_pv_domain())
>>           return 0;
>>   +#ifdef CONFIG_XEN_PVH
>> +    /* Test for PVH domain (PVH boot path taken overrides ACPI 
>> flags). */
>> +    if (!x86_platform.legacy.rtc && x86_platform.legacy.no_vga)
>> +        xen_pvh = true;
>
> Sorry, this won't work, as ACPI tables are scanned only some time later.
Hmm, right. Thanks for point out.
>
> You can test for xen_pvh being true here (for the case where the guest
> has been booted via the Xen-PVH boot entry) and handle that case, but
> the case of a PVH guest started via the normal boot entry (like via
> grub2) and nopv specified is difficult. The only idea I have right now
> would be to use another struct hypervisor_x86 for that case which will
> only be used for Xen HVM/PVH _and_ nopv specified. It should be a copy
> of the bare metal variant, but a special guest_late_init member issuing
> a big fat warning in case PVH is being detected.

After that warning, I guess PVH will run into hang finally? If it's 
true, BUG() is better?

Adding another hypervisor_x86 is a bit redundant, I think of below change.

I'll test it tomorrow. But appreciate your suggestion whether it's 
feasible. Thanks

--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -25,6 +25,7 @@
  #include "mmu.h"
  #include "smp.h"

+extern bool nopv;
  static unsigned long shared_info_pfn;

  void xen_hvm_init_shared_info(void)
@@ -221,11 +222,37 @@ bool __init xen_hvm_need_lapic(void)
         return true;
  }

+static __init void xen_hvm_nopv_guest_late_init(void)
+{
+#ifdef CONFIG_XEN_PVH
+       if (x86_platform.legacy.rtc || !x86_platform.legacy.no_vga)
+               return;
+
+       /* PVH detected. */
+       xen_pvh = true;
+
+       printk(KERN_CRIT "nopv parameter isn't supported in PVH guest\n");
+       BUG();
+#endif
+}
+
+
  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 */
+               printk(KERN_INFO "nopv parameter is ignored in PVH 
guest\n");
+       }
+       else if (nopv)
+       {
+       /* guest booting via normal boot entry (like via grub2) goes here */
+               x86_init.hyper.guest_late_init = 
xen_hvm_nopv_guest_late_init;
+               return 0;
+       }
         return xen_cpuid_base();
  }

@@ -258,4 +285,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,
  };


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

Re: [Xen-devel] [PATCH v2 5/7] x86/xen: nopv parameter support for HVM guest

[Juergen Gross]

On 26.06.19 10:56, Zhenzhong Duan wrote:
> 
> On 2019/6/25 20:31, Juergen Gross wrote:
>> On 24.06.19 14:02, Zhenzhong Duan wrote:
>>> PVH guest needs PV extentions to work, so nopv parameter is ignored
>>> for PVH but not for HVM guest.
>>>
>>> In order for nopv parameter to take effect for HVM guest, we need to
>>> distinguish between PVH and HVM guest early in hypervisor detection
>>> code. By moving the detection of PVH in xen_platform_hvm(),
>>> xen_pvh_domain() could be used for that purpose.
>>>
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
>>> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>>> Cc: Juergen Gross <jgross@suse.com>
>>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Cc: Ingo Molnar <mingo@redhat.com>
>>> Cc: Borislav Petkov <bp@alien8.de>
>>> Cc: xen-devel@lists.xenproject.org
>>> ---
>>>   arch/x86/xen/enlighten_hvm.c | 18 ++++++++++++------
>>>   1 file changed, 12 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
>>> index 7fcb4ea..26939e7 100644
>>> --- a/arch/x86/xen/enlighten_hvm.c
>>> +++ b/arch/x86/xen/enlighten_hvm.c
>>> @@ -25,6 +25,7 @@
>>>   #include "mmu.h"
>>>   #include "smp.h"
>>>   +extern bool nopv;
>>>   static unsigned long shared_info_pfn;
>>>     void xen_hvm_init_shared_info(void)
>>> @@ -226,20 +227,24 @@ static uint32_t __init xen_platform_hvm(void)
>>>       if (xen_pv_domain())
>>>           return 0;
>>>   +#ifdef CONFIG_XEN_PVH
>>> +    /* Test for PVH domain (PVH boot path taken overrides ACPI 
>>> flags). */
>>> +    if (!x86_platform.legacy.rtc && x86_platform.legacy.no_vga)
>>> +        xen_pvh = true;
>>
>> Sorry, this won't work, as ACPI tables are scanned only some time later.
> Hmm, right. Thanks for point out.
>>
>> You can test for xen_pvh being true here (for the case where the guest
>> has been booted via the Xen-PVH boot entry) and handle that case, but
>> the case of a PVH guest started via the normal boot entry (like via
>> grub2) and nopv specified is difficult. The only idea I have right now
>> would be to use another struct hypervisor_x86 for that case which will
>> only be used for Xen HVM/PVH _and_ nopv specified. It should be a copy
>> of the bare metal variant, but a special guest_late_init member issuing
>> a big fat warning in case PVH is being detected.
> 
> After that warning, I guess PVH will run into hang finally? If it's 
> true, BUG() is better?
> 
> Adding another hypervisor_x86 is a bit redundant, I think of below change.
> 
> I'll test it tomorrow. But appreciate your suggestion whether it's 
> feasible. Thanks

Yes, this seems to be a viable option.

> 
> --- a/arch/x86/xen/enlighten_hvm.c
> +++ b/arch/x86/xen/enlighten_hvm.c
> @@ -25,6 +25,7 @@
>   #include "mmu.h"
>   #include "smp.h"
> 
> +extern bool nopv;
>   static unsigned long shared_info_pfn;
> 
>   void xen_hvm_init_shared_info(void)
> @@ -221,11 +222,37 @@ bool __init xen_hvm_need_lapic(void)
>          return true;
>   }
> 
> +static __init void xen_hvm_nopv_guest_late_init(void)
> +{
> +#ifdef CONFIG_XEN_PVH
> +       if (x86_platform.legacy.rtc || !x86_platform.legacy.no_vga)
> +               return;
> +
> +       /* PVH detected. */
> +       xen_pvh = true;
> +
> +       printk(KERN_CRIT "nopv parameter isn't supported in PVH guest\n");
> +       BUG();
> +#endif
> +}
> +
> +
>   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 */

Mind adjusting indentation of that comment?

> +               printk(KERN_INFO "nopv parameter is ignored in PVH 
> guest\n");
> +       }
> +       else if (nopv)
> +       {
> +       /* guest booting via normal boot entry (like via grub2) goes 
> here */

Same again?

With those corrected and no other changes you can add my:

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

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