[PATCH v6 0/4] misc fixes to PV extensions code

[PATCH v6 0/4] misc fixes to PV extensions code

[Zhenzhong Duan]

Hi,

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, etc)
we want to disable PV extensions. We have xen_nopv for that purpose
but only for XEN. For a consistent admin experience a common command
line parameter set across all PV guest implementations is a better
choice.

To achieve this introduce a new 'nopv' parameter which is usable by
most of PV guest implementation. Due to the limitation of some PV
guests(XEN PV, XEN PVH and jailhouse), 'nopv' is ignored for XEN PV
, jailhouse and XEN PVH if booting via Xen-PVH boot entry. If booting
via normal boot entry(like grub2), PVH guest has to panic itself
currently.

While analyzing the PV guest code one bug were found and fixed.
(Patches 1). It can be applied independent of the functional
changes, but is kept in the series as the functional changes
depend on them.

For compatibility reason, "xen_nopv" is keeped and mapped to "nopv",
this way also avoids an issue with xen_nopv when booting PVH guest.

Build test passes with CONFIG_HYPERVISOR_GUEST enable and disabled.
I didn't get env to test with jailhouse and Hyperv, the others work
as expected.

v6:
PATCH3 add Reviewed-by
PATCH4 remove unnecessory xen_hvm_nopv_guest_late_init() per Boris

v5:
PATCH2:
update patch description per Boris
add declaration of nopv variable in arch/x86/include/asm/hypervisor.h
which will be used in PATCH3 and PATCH4

PATCH3 update xen_parse_nopv() per Boris
PATCH4 add nopv=false per Boris
Combine PATCH5 into PATCH3


v4:
PATCH5 a new patch to add 'xen_nopv' back per Boris

v3:
Remove some unrelated patches from patchset as suggested by Tglx

PATCH1 unchanged
PATCH2 add Reviewed-by
PATCH3 add Reviewed-by
PATCH4 rewrite the patch as Jgross found an issue in old patch,
description is also updated.



v2:
PATCH3 use 'ignore_nopv' for PVH/PV guest as suggested by Jgross.
PATCH5 new added one, specifically for HVM guest

Thanks
Zhenzhong

[Xen-devel] [PATCH v6 0/4] misc fixes to PV extensions code

[Zhenzhong Duan]

Hi,

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, etc)
we want to disable PV extensions. We have xen_nopv for that purpose
but only for XEN. For a consistent admin experience a common command
line parameter set across all PV guest implementations is a better
choice.

To achieve this introduce a new 'nopv' parameter which is usable by
most of PV guest implementation. Due to the limitation of some PV
guests(XEN PV, XEN PVH and jailhouse), 'nopv' is ignored for XEN PV
, jailhouse and XEN PVH if booting via Xen-PVH boot entry. If booting
via normal boot entry(like grub2), PVH guest has to panic itself
currently.

While analyzing the PV guest code one bug were found and fixed.
(Patches 1). It can be applied independent of the functional
changes, but is kept in the series as the functional changes
depend on them.

For compatibility reason, "xen_nopv" is keeped and mapped to "nopv",
this way also avoids an issue with xen_nopv when booting PVH guest.

Build test passes with CONFIG_HYPERVISOR_GUEST enable and disabled.
I didn't get env to test with jailhouse and Hyperv, the others work
as expected.

v6:
PATCH3 add Reviewed-by
PATCH4 remove unnecessory xen_hvm_nopv_guest_late_init() per Boris

v5:
PATCH2:
update patch description per Boris
add declaration of nopv variable in arch/x86/include/asm/hypervisor.h
which will be used in PATCH3 and PATCH4

PATCH3 update xen_parse_nopv() per Boris
PATCH4 add nopv=false per Boris
Combine PATCH5 into PATCH3


v4:
PATCH5 a new patch to add 'xen_nopv' back per Boris

v3:
Remove some unrelated patches from patchset as suggested by Tglx

PATCH1 unchanged
PATCH2 add Reviewed-by
PATCH3 add Reviewed-by
PATCH4 rewrite the patch as Jgross found an issue in old patch,
description is also updated.



v2:
PATCH3 use 'ignore_nopv' for PVH/PV guest as suggested by Jgross.
PATCH5 new added one, specifically for HVM guest

Thanks
Zhenzhong

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

[PATCH v6 1/4] x86/xen: Mark xen_hvm_need_lapic() and xen_x2apic_para_available() as __init

[Zhenzhong Duan]

.. as they are only called at early bootup stage. In fact, other
functions in x86_hyper_xen_hvm.init.* are all marked as __init.

Unexport xen_hvm_need_lapic as it's never used outside.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.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>
---
 arch/x86/include/asm/xen/hypervisor.h | 6 +++---
 arch/x86/xen/enlighten_hvm.c          | 3 +--
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/xen/hypervisor.h b/arch/x86/include/asm/xen/hypervisor.h
index 39171b3..42e1245 100644
--- a/arch/x86/include/asm/xen/hypervisor.h
+++ b/arch/x86/include/asm/xen/hypervisor.h
@@ -44,14 +44,14 @@ static inline uint32_t xen_cpuid_base(void)
 }
 
 #ifdef CONFIG_XEN
-extern bool xen_hvm_need_lapic(void);
+extern bool __init xen_hvm_need_lapic(void);
 
-static inline bool xen_x2apic_para_available(void)
+static inline bool __init xen_x2apic_para_available(void)
 {
 	return xen_hvm_need_lapic();
 }
 #else
-static inline bool xen_x2apic_para_available(void)
+static inline bool __init xen_x2apic_para_available(void)
 {
 	return (xen_cpuid_base() != 0);
 }
diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
index 0e75642..ac4943c 100644
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -218,7 +218,7 @@ static __init int xen_parse_nopv(char *arg)
 }
 early_param("xen_nopv", xen_parse_nopv);
 
-bool xen_hvm_need_lapic(void)
+bool __init xen_hvm_need_lapic(void)
 {
 	if (xen_nopv)
 		return false;
@@ -230,7 +230,6 @@ bool xen_hvm_need_lapic(void)
 		return false;
 	return true;
 }
-EXPORT_SYMBOL_GPL(xen_hvm_need_lapic);
 
 static uint32_t __init xen_platform_hvm(void)
 {
-- 
1.8.3.1

[Xen-devel] [PATCH v6 1/4] x86/xen: Mark xen_hvm_need_lapic() and xen_x2apic_para_available() as __init

[Zhenzhong Duan]

.. as they are only called at early bootup stage. In fact, other
functions in x86_hyper_xen_hvm.init.* are all marked as __init.

Unexport xen_hvm_need_lapic as it's never used outside.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.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>
---
 arch/x86/include/asm/xen/hypervisor.h | 6 +++---
 arch/x86/xen/enlighten_hvm.c          | 3 +--
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/xen/hypervisor.h b/arch/x86/include/asm/xen/hypervisor.h
index 39171b3..42e1245 100644
--- a/arch/x86/include/asm/xen/hypervisor.h
+++ b/arch/x86/include/asm/xen/hypervisor.h
@@ -44,14 +44,14 @@ static inline uint32_t xen_cpuid_base(void)
 }
 
 #ifdef CONFIG_XEN
-extern bool xen_hvm_need_lapic(void);
+extern bool __init xen_hvm_need_lapic(void);
 
-static inline bool xen_x2apic_para_available(void)
+static inline bool __init xen_x2apic_para_available(void)
 {
 	return xen_hvm_need_lapic();
 }
 #else
-static inline bool xen_x2apic_para_available(void)
+static inline bool __init xen_x2apic_para_available(void)
 {
 	return (xen_cpuid_base() != 0);
 }
diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
index 0e75642..ac4943c 100644
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -218,7 +218,7 @@ static __init int xen_parse_nopv(char *arg)
 }
 early_param("xen_nopv", xen_parse_nopv);
 
-bool xen_hvm_need_lapic(void)
+bool __init xen_hvm_need_lapic(void)
 {
 	if (xen_nopv)
 		return false;
@@ -230,7 +230,6 @@ bool xen_hvm_need_lapic(void)
 		return false;
 	return true;
 }
-EXPORT_SYMBOL_GPL(xen_hvm_need_lapic);
 
 static uint32_t __init xen_platform_hvm(void)
 {
-- 
1.8.3.1


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

[PATCH v6 2/4] 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. We have "xen_nopv" for that purpose
but only for XEN. For a consistent admin experience a common command
line parameter "nopv" set across all PV guest implementations is a
better choice.

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.

Suggested-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
Reviewed-by: Juergen Gross <jgross@suse.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>
---
 Documentation/admin-guide/kernel-parameters.txt |  5 +++++
 arch/x86/include/asm/hypervisor.h               |  4 ++++
 arch/x86/kernel/cpu/hypervisor.c                | 11 +++++++++++
 arch/x86/kernel/jailhouse.c                     |  1 +
 arch/x86/xen/enlighten_pv.c                     |  1 +
 5 files changed, 22 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..00240b0 100644
--- a/arch/x86/include/asm/hypervisor.h
+++ b/arch/x86/include/asm/hypervisor.h
@@ -52,8 +52,12 @@ struct hypervisor_x86 {
 
 	/* runtime callbacks */
 	struct x86_hyper_runtime runtime;
+
+	/* ignore nopv parameter */
+	bool ignore_nopv;
 };
 
+extern bool nopv;
 extern enum x86_hypervisor_type x86_hyper_type;
 extern void init_hypervisor_platform(void);
 static inline bool hypervisor_is_type(enum x86_hypervisor_type 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 1b2ee55..c52c4105 100644
--- a/arch/x86/kernel/jailhouse.c
+++ b/arch/x86/kernel/jailhouse.c
@@ -217,4 +217,5 @@ static bool 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] [PATCH v6 2/4] 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. We have "xen_nopv" for that purpose
but only for XEN. For a consistent admin experience a common command
line parameter "nopv" set across all PV guest implementations is a
better choice.

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.

Suggested-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
Reviewed-by: Juergen Gross <jgross@suse.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>
---
 Documentation/admin-guide/kernel-parameters.txt |  5 +++++
 arch/x86/include/asm/hypervisor.h               |  4 ++++
 arch/x86/kernel/cpu/hypervisor.c                | 11 +++++++++++
 arch/x86/kernel/jailhouse.c                     |  1 +
 arch/x86/xen/enlighten_pv.c                     |  1 +
 5 files changed, 22 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..00240b0 100644
--- a/arch/x86/include/asm/hypervisor.h
+++ b/arch/x86/include/asm/hypervisor.h
@@ -52,8 +52,12 @@ struct hypervisor_x86 {
 
 	/* runtime callbacks */
 	struct x86_hyper_runtime runtime;
+
+	/* ignore nopv parameter */
+	bool ignore_nopv;
 };
 
+extern bool nopv;
 extern enum x86_hypervisor_type x86_hyper_type;
 extern void init_hypervisor_platform(void);
 static inline bool hypervisor_is_type(enum x86_hypervisor_type 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 1b2ee55..c52c4105 100644
--- a/arch/x86/kernel/jailhouse.c
+++ b/arch/x86/kernel/jailhouse.c
@@ -217,4 +217,5 @@ static bool 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

[PATCH v6 3/4] xen: Map "xen_nopv" parameter to "nopv" and mark it obsolete

[Zhenzhong Duan]

Clean up unnecessory code after that operation.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
Reviewed-by: 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>
---
 Documentation/admin-guide/kernel-parameters.txt |  2 ++
 arch/x86/xen/enlighten_hvm.c                    | 12 ++++++------
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 21e08af..8ab34a1 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5254,6 +5254,8 @@
 	xen_nopv	[X86]
 			Disables the PV optimizations forcing the HVM guest to
 			run as generic HVM guest with no PV drivers.
+			This option is obsoleted by the "nopv" option, which
+			has equivalent effect for XEN platform.
 
 	xen_scrub_pages=	[XEN]
 			Boolean option to control scrubbing pages before giving them back
diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
index ac4943c..1756cf7 100644
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -210,18 +210,18 @@ 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;
+	pr_notice("\"xen_nopv\" is deprecated, please use \"nopv\" instead\n");
+
+	if (xen_cpuid_base())
+		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 +233,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] [PATCH v6 3/4] xen: Map "xen_nopv" parameter to "nopv" and mark it obsolete

[Zhenzhong Duan]

Clean up unnecessory code after that operation.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
Reviewed-by: 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>
---
 Documentation/admin-guide/kernel-parameters.txt |  2 ++
 arch/x86/xen/enlighten_hvm.c                    | 12 ++++++------
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 21e08af..8ab34a1 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5254,6 +5254,8 @@
 	xen_nopv	[X86]
 			Disables the PV optimizations forcing the HVM guest to
 			run as generic HVM guest with no PV drivers.
+			This option is obsoleted by the "nopv" option, which
+			has equivalent effect for XEN platform.
 
 	xen_scrub_pages=	[XEN]
 			Boolean option to control scrubbing pages before giving them back
diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
index ac4943c..1756cf7 100644
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -210,18 +210,18 @@ 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;
+	pr_notice("\"xen_nopv\" is deprecated, please use \"nopv\" instead\n");
+
+	if (xen_cpuid_base())
+		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 +233,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

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

[Zhenzhong Duan]

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

If PVH guest boots up via the Xen-PVH boot entry, xen_pvh is set early,
we know it's PVH guest and ignore "nopv" parameter directly.

If PVH guest boots up via the normal boot entry same as HVM guest, it's
hard to distinguish PVH and HVM guest at that time. In this case, we
have to panic early if PVH is detected and nopv is enabled to avoid a
worse situation later.

Move xen_platform_hvm() after xen_hvm_guest_late_init() to avoid compile
error.

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>
---
 arch/x86/xen/enlighten_hvm.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
index 1756cf7..7e1c75f 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,6 +254,26 @@ static __init void xen_hvm_guest_late_init(void)
 #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 */
+		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;
+	}
+	return xen_cpuid_base();
+}
+
 const __initconst struct hypervisor_x86 x86_hyper_xen_hvm = {
 	.name                   = "Xen HVM",
 	.detect                 = xen_platform_hvm,
@@ -268,4 +283,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] [PATCH v6 4/4] x86/xen: Add "nopv" support for HVM guest

[Zhenzhong Duan]

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

If PVH guest boots up via the Xen-PVH boot entry, xen_pvh is set early,
we know it's PVH guest and ignore "nopv" parameter directly.

If PVH guest boots up via the normal boot entry same as HVM guest, it's
hard to distinguish PVH and HVM guest at that time. In this case, we
have to panic early if PVH is detected and nopv is enabled to avoid a
worse situation later.

Move xen_platform_hvm() after xen_hvm_guest_late_init() to avoid compile
error.

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>
---
 arch/x86/xen/enlighten_hvm.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
index 1756cf7..7e1c75f 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,6 +254,26 @@ static __init void xen_hvm_guest_late_init(void)
 #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 */
+		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;
+	}
+	return xen_cpuid_base();
+}
+
 const __initconst struct hypervisor_x86 x86_hyper_xen_hvm = {
 	.name                   = "Xen HVM",
 	.detect                 = xen_platform_hvm,
@@ -268,4 +283,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: [PATCH v6 4/4] x86/xen: Add "nopv" support for HVM guest

[Boris Ostrovsky]

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.

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.


-boris


> +	}
> +	return xen_cpuid_base();
> +}
> +
>  const __initconst struct hypervisor_x86 x86_hyper_xen_hvm = {
>  	.name                   = "Xen HVM",
>  	.detect                 = xen_platform_hvm,
> @@ -268,4 +283,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,
>  };

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

[Boris Ostrovsky]

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.

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.


-boris


> +	}
> +	return xen_cpuid_base();
> +}
> +
>  const __initconst struct hypervisor_x86 x86_hyper_xen_hvm = {
>  	.name                   = "Xen HVM",
>  	.detect                 = xen_platform_hvm,
> @@ -268,4 +283,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: [PATCH v6 4/4] x86/xen: Add "nopv" support for HVM guest

[Zhenzhong Duan]

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

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

[Zhenzhong Duan]

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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

[Boris Ostrovsky]

On 7/9/19 12:20 AM, Zhenzhong Duan wrote:
>
> -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));


And this worked? I'd think it would fail since h points to RO section.


> +               memcpy(&x86_hyper_xen_hvm.runtime, (void
> *)&x86_platform.hyper,
> +                      sizeof(x86_platform.hyper));
> +               h->guest_late_init = xen_hvm_guest_late_init;


To me this still doesn't look right --- you are making assumptions about
x86_platform/x86_init.hyper and I don't think you can assume they have
not been set to point to another hypervisor, for example.

Would modifying all x86_hyper_xen_hvm's ops (except, I guess,
xen_hvm_guest_late_init()) to immediately return if nopv is set work?

Also question for Juergen --- is there anything perhaps in zeropage that
indicates we are booting PVH guest from grub?

-boris


> +       }
> +       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
>

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

[Boris Ostrovsky]

On 7/9/19 12:20 AM, Zhenzhong Duan wrote:
>
> -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));


And this worked? I'd think it would fail since h points to RO section.


> +               memcpy(&x86_hyper_xen_hvm.runtime, (void
> *)&x86_platform.hyper,
> +                      sizeof(x86_platform.hyper));
> +               h->guest_late_init = xen_hvm_guest_late_init;


To me this still doesn't look right --- you are making assumptions about
x86_platform/x86_init.hyper and I don't think you can assume they have
not been set to point to another hypervisor, for example.

Would modifying all x86_hyper_xen_hvm's ops (except, I guess,
xen_hvm_guest_late_init()) to immediately return if nopv is set work?

Also question for Juergen --- is there anything perhaps in zeropage that
indicates we are booting PVH guest from grub?

-boris


> +       }
> +       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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

[Zhenzhong Duan]

On 2019/7/9 22:54, Boris Ostrovsky wrote:
> On 7/9/19 12:20 AM, Zhenzhong Duan wrote:
>> -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));
>
> And this worked? I'd think it would fail since h points to RO section.
Yes, I have below changes in the patch.

-const __initconst struct hypervisor_x86 x86_hyper_xen_hvm = {
+struct hypervisor_x86 x86_hyper_xen_hvm __initdata = {

>
>
>> +               memcpy(&x86_hyper_xen_hvm.runtime, (void
>> *)&x86_platform.hyper,
>> +                      sizeof(x86_platform.hyper));
>> +               h->guest_late_init = xen_hvm_guest_late_init;
>
> To me this still doesn't look right --- you are making assumptions about
> x86_platform/x86_init.hyper and I don't think you can assume they have
> not been set to point to another hypervisor, for example.

You mean copy_array() calls in init_hypervisor_platform()? But that 
happens after

detect_hypervisor_vendor() shoose out the prefered hypervisor. In detect 
stage,

x86_platform/x86_init.hyper has default value for bare hardware, or I 
missed something?

Just realized I can use memset to zero instead of memcpy which looks 
more rational.

>
> Would modifying all x86_hyper_xen_hvm's ops (except, I guess,
> xen_hvm_guest_late_init()) to immediately return if nopv is set work?

I think so,  Let me try it.

Zhenzhong

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

[Zhenzhong Duan]

On 2019/7/9 22:54, Boris Ostrovsky wrote:
> On 7/9/19 12:20 AM, Zhenzhong Duan wrote:
>> -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));
>
> And this worked? I'd think it would fail since h points to RO section.
Yes, I have below changes in the patch.

-const __initconst struct hypervisor_x86 x86_hyper_xen_hvm = {
+struct hypervisor_x86 x86_hyper_xen_hvm __initdata = {

>
>
>> +               memcpy(&x86_hyper_xen_hvm.runtime, (void
>> *)&x86_platform.hyper,
>> +                      sizeof(x86_platform.hyper));
>> +               h->guest_late_init = xen_hvm_guest_late_init;
>
> To me this still doesn't look right --- you are making assumptions about
> x86_platform/x86_init.hyper and I don't think you can assume they have
> not been set to point to another hypervisor, for example.

You mean copy_array() calls in init_hypervisor_platform()? But that 
happens after

detect_hypervisor_vendor() shoose out the prefered hypervisor. In detect 
stage,

x86_platform/x86_init.hyper has default value for bare hardware, or I 
missed something?

Just realized I can use memset to zero instead of memcpy which looks 
more rational.

>
> Would modifying all x86_hyper_xen_hvm's ops (except, I guess,
> xen_hvm_guest_late_init()) to immediately return if nopv is set work?

I think so,  Let me try it.

Zhenzhong


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

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

[Boris Ostrovsky]

On 7/9/19 10:07 PM, Zhenzhong Duan wrote:
> On 2019/7/9 22:54, Boris Ostrovsky wrote:
>> On 7/9/19 12:20 AM, Zhenzhong Duan wrote:
>>> -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));
>>
>> And this worked? I'd think it would fail since h points to RO section.
> Yes, I have below changes in the patch.
>
> -const __initconst struct hypervisor_x86 x86_hyper_xen_hvm = {
> +struct hypervisor_x86 x86_hyper_xen_hvm __initdata = {


But hypervisors[] stays__initconst so that I thought could be a problem.
But apparently it's not.

>
>>
>>
>>> +               memcpy(&x86_hyper_xen_hvm.runtime, (void
>>> *)&x86_platform.hyper,
>>> +                      sizeof(x86_platform.hyper));
>>> +               h->guest_late_init = xen_hvm_guest_late_init;
>>
>> To me this still doesn't look right --- you are making assumptions about
>> x86_platform/x86_init.hyper and I don't think you can assume they have
>> not been set to point to another hypervisor, for example.
>
> You mean copy_array() calls in init_hypervisor_platform()? But that
> happens after
>
> detect_hypervisor_vendor() shoose out the prefered hypervisor. In
> detect stage,
>
> x86_platform/x86_init.hyper has default value for bare hardware, or I
> missed something?


Right. My point though is that this ordering is opaque to
hypervisor-specific code and can change. The same is true about making
assumptions about x86_platform/x86_init.hyper values.

-boris

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

[Boris Ostrovsky]

On 7/9/19 10:07 PM, Zhenzhong Duan wrote:
> On 2019/7/9 22:54, Boris Ostrovsky wrote:
>> On 7/9/19 12:20 AM, Zhenzhong Duan wrote:
>>> -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));
>>
>> And this worked? I'd think it would fail since h points to RO section.
> Yes, I have below changes in the patch.
>
> -const __initconst struct hypervisor_x86 x86_hyper_xen_hvm = {
> +struct hypervisor_x86 x86_hyper_xen_hvm __initdata = {


But hypervisors[] stays__initconst so that I thought could be a problem.
But apparently it's not.

>
>>
>>
>>> +               memcpy(&x86_hyper_xen_hvm.runtime, (void
>>> *)&x86_platform.hyper,
>>> +                      sizeof(x86_platform.hyper));
>>> +               h->guest_late_init = xen_hvm_guest_late_init;
>>
>> To me this still doesn't look right --- you are making assumptions about
>> x86_platform/x86_init.hyper and I don't think you can assume they have
>> not been set to point to another hypervisor, for example.
>
> You mean copy_array() calls in init_hypervisor_platform()? But that
> happens after
>
> detect_hypervisor_vendor() shoose out the prefered hypervisor. In
> detect stage,
>
> x86_platform/x86_init.hyper has default value for bare hardware, or I
> missed something?


Right. My point though is that this ordering is opaque to
hypervisor-specific code and can change. The same is true about making
assumptions about x86_platform/x86_init.hyper values.

-boris



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