2019-07-12 12:06:13

by Zhenzhong Duan

[permalink] [raw]
Subject: [PATCH v7 0/5] misc fixes to PV extensions code

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.

v7:
PATCH4 a new added patch prerequite for PATCH5(previously PATCH4)
PATCH5 rewrite the code based on Boris's suggestion. I compare the one
to update interface function one-by-one and the one to modify all
x86_hyper_xen_hvm's ops to immediately return if nopv is set, both
have same effect and the first looks smarter, so choose the 1st one.

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


2019-07-12 12:07:55

by Zhenzhong Duan

[permalink] [raw]
Subject: [PATCH v7 2/5] x86: Add "nopv" parameter to disable PV extensions

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 <[email protected]>
Signed-off-by: Zhenzhong Duan <[email protected]>
Reviewed-by: Juergen Gross <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Jan Kiszka <[email protected]>
Cc: Boris Ostrovsky <[email protected]>
Cc: Stefano Stabellini <[email protected]>
---
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 f1c433d..dbfe9c2 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5271,6 +5271,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 50a30f6..f7b4c53 100644
--- a/arch/x86/include/asm/hypervisor.h
+++ b/arch/x86/include/asm/hypervisor.h
@@ -53,8 +53,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 87e39ad..7eaad41 100644
--- a/arch/x86/kernel/cpu/hypervisor.c
+++ b/arch/x86/kernel/cpu/hypervisor.c
@@ -58,6 +58,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)
{
@@ -65,6 +73,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 6857b45..3ad34f0 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

2019-07-12 12:08:11

by Zhenzhong Duan

[permalink] [raw]
Subject: [PATCH v7 3/5] xen: Map "xen_nopv" parameter to "nopv" and mark it obsolete

Clean up unnecessory code after that operation.

Signed-off-by: Zhenzhong Duan <[email protected]>
Reviewed-by: Boris Ostrovsky <[email protected]>
Cc: Juergen Gross <[email protected]>
Cc: Stefano Stabellini <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
---
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 dbfe9c2..c3f3e01 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5257,6 +5257,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

2019-07-12 12:08:47

by Zhenzhong Duan

[permalink] [raw]
Subject: [PATCH v7 1/5] x86/xen: Mark xen_hvm_need_lapic() and xen_x2apic_para_available() as __init

.. 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 <[email protected]>
Reviewed-by: Juergen Gross <[email protected]>
Cc: Boris Ostrovsky <[email protected]>
Cc: Stefano Stabellini <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
---
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

2019-07-12 12:09:48

by Zhenzhong Duan

[permalink] [raw]
Subject: [PATCH v7 5/5] x86/xen: Add "nopv" support for HVM guest

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 <[email protected]>
Cc: Boris Ostrovsky <[email protected]>
Cc: Juergen Gross <[email protected]>
Cc: Stefano Stabellini <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
---
arch/x86/include/asm/x86_init.h | 2 ++
arch/x86/kernel/x86_init.c | 4 ++--
arch/x86/xen/enlighten_hvm.c | 45 ++++++++++++++++++++++++++++++++---------
3 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index b85a7c5..ac09341 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -301,6 +301,8 @@ struct x86_apic_ops {
extern void x86_early_init_platform_quirks(void);
extern void x86_init_noop(void);
extern void x86_init_uint_noop(unsigned int unused);
+extern bool bool_x86_init_noop(void);
+extern void x86_op_int_noop(int cpu);
extern bool x86_pnpbios_disabled(void);

#endif
diff --git a/arch/x86/kernel/x86_init.c b/arch/x86/kernel/x86_init.c
index 50a2b49..1bef687 100644
--- a/arch/x86/kernel/x86_init.c
+++ b/arch/x86/kernel/x86_init.c
@@ -29,8 +29,8 @@ void x86_init_noop(void) { }
void __init x86_init_uint_noop(unsigned int unused) { }
static int __init iommu_init_noop(void) { return 0; }
static void iommu_shutdown_noop(void) { }
-static bool __init bool_x86_init_noop(void) { return false; }
-static void x86_op_int_noop(int cpu) { }
+bool __init bool_x86_init_noop(void) { return false; }
+void x86_op_int_noop(int cpu) { }

/*
* The platform setup functions are preset with the default functions
diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
index 1756cf7..e138f7d 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,38 @@ 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.
+ */
+ h->init_platform = x86_init_noop;
+ h->x2apic_available = bool_x86_init_noop;
+ h->init_mem_mapping = x86_init_noop;
+ h->init_after_bootmem = x86_init_noop;
+ h->guest_late_init = xen_hvm_guest_late_init;
+ x86_hyper_xen_hvm.runtime.pin_vcpu = x86_op_int_noop;
+ }
+ 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 +294,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

2019-07-15 15:19:44

by Boris Ostrovsky

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] x86/xen: Add "nopv" support for HVM guest

On 7/11/19 8:02 AM, Zhenzhong Duan wrote:
> 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 <[email protected]>
> Cc: Boris Ostrovsky <[email protected]>
> Cc: Juergen Gross <[email protected]>
> Cc: Stefano Stabellini <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Borislav Petkov <[email protected]>


Reviewed-by: Boris Ostrovsky <[email protected]>