2019-07-17 04:23:45

by Zhenzhong Duan

[permalink] [raw]
Subject: [PATCH v8 4/5] x86/paravirt: Remove const mark from x86_hyper_xen_hvm variable

.. as "nopv" support needs it to be changeable at boot up stage.

Checkpatch reports warning, so move variable declarations from
hypervisor.c to hypervisor.h

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/hypervisor.h | 8 ++++++++
arch/x86/kernel/cpu/hypervisor.c | 8 --------
arch/x86/xen/enlighten_hvm.c | 2 +-
3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/hypervisor.h b/arch/x86/include/asm/hypervisor.h
index f7b4c53..e41cbf2 100644
--- a/arch/x86/include/asm/hypervisor.h
+++ b/arch/x86/include/asm/hypervisor.h
@@ -58,6 +58,14 @@ struct hypervisor_x86 {
bool ignore_nopv;
};

+extern const struct hypervisor_x86 x86_hyper_vmware;
+extern const struct hypervisor_x86 x86_hyper_ms_hyperv;
+extern const struct hypervisor_x86 x86_hyper_xen_pv;
+extern const struct hypervisor_x86 x86_hyper_kvm;
+extern const struct hypervisor_x86 x86_hyper_jailhouse;
+extern const struct hypervisor_x86 x86_hyper_acrn;
+extern struct hypervisor_x86 x86_hyper_xen_hvm;
+
extern bool nopv;
extern enum x86_hypervisor_type x86_hyper_type;
extern void init_hypervisor_platform(void);
diff --git a/arch/x86/kernel/cpu/hypervisor.c b/arch/x86/kernel/cpu/hypervisor.c
index 7eaad41..553bfbf 100644
--- a/arch/x86/kernel/cpu/hypervisor.c
+++ b/arch/x86/kernel/cpu/hypervisor.c
@@ -26,14 +26,6 @@
#include <asm/processor.h>
#include <asm/hypervisor.h>

-extern const struct hypervisor_x86 x86_hyper_vmware;
-extern const struct hypervisor_x86 x86_hyper_ms_hyperv;
-extern const struct hypervisor_x86 x86_hyper_xen_pv;
-extern const struct hypervisor_x86 x86_hyper_xen_hvm;
-extern const struct hypervisor_x86 x86_hyper_kvm;
-extern const struct hypervisor_x86 x86_hyper_jailhouse;
-extern const struct hypervisor_x86 x86_hyper_acrn;
-
static const __initconst struct hypervisor_x86 * const hypervisors[] =
{
#ifdef CONFIG_XEN_PV
diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
index 1756cf7..b671983 100644
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -259,7 +259,7 @@ static __init void xen_hvm_guest_late_init(void)
#endif
}

-const __initconst struct hypervisor_x86 x86_hyper_xen_hvm = {
+struct hypervisor_x86 x86_hyper_xen_hvm __initdata = {
.name = "Xen HVM",
.detect = xen_platform_hvm,
.type = X86_HYPER_XEN_HVM,
--
1.8.3.1


2019-07-17 04:25:10

by Zhenzhong Duan

[permalink] [raw]
Subject: [PATCH v8 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.

Remove static from bool_x86_init_noop/x86_op_int_noop so they could be
used globally. Move xen_platform_hvm() after xen_hvm_guest_late_init()
to avoid compile error.

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]>
---
arch/x86/include/asm/x86_init.h | 2 ++
arch/x86/kernel/x86_init.c | 4 ++--
arch/x86/xen/enlighten_hvm.c | 43 +++++++++++++++++++++++++++++++++--------
3 files changed, 39 insertions(+), 10 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 b671983..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,6 +254,37 @@ static __init void xen_hvm_guest_late_init(void)
#endif
}

+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,
@@ -268,4 +294,5 @@ struct hypervisor_x86 x86_hyper_xen_hvm __initdata = {
.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-17 04:59:35

by Jürgen Groß

[permalink] [raw]
Subject: Re: [PATCH v8 4/5] x86/paravirt: Remove const mark from x86_hyper_xen_hvm variable

On 16.07.19 06:26, Zhenzhong Duan wrote:
> .. as "nopv" support needs it to be changeable at boot up stage.
>
> Checkpatch reports warning, so move variable declarations from
> hypervisor.c to hypervisor.h
>
> Signed-off-by: Zhenzhong Duan <[email protected]>

Reviewed-by: Juergen Gross <[email protected]>

... and complete series applied to xen/tip.git for-linus-5.3a


Juergen

2019-07-17 06:47:25

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH v8 4/5] x86/paravirt: Remove const mark from x86_hyper_xen_hvm variable

On Tue, 2019-07-16 at 12:26 +0800, Zhenzhong Duan wrote:
> .. as "nopv" support needs it to be changeable at boot up stage.
>
> Checkpatch reports warning, so move variable declarations from
> hypervisor.c to hypervisor.h
[]
> diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
[]
> @@ -259,7 +259,7 @@ static __init void xen_hvm_guest_late_init(void)
> #endif
> }
>
> -const __initconst struct hypervisor_x86 x86_hyper_xen_hvm = {
> +struct hypervisor_x86 x86_hyper_xen_hvm __initdata = {

static?


2019-07-17 06:50:04

by Jürgen Groß

[permalink] [raw]
Subject: Re: [PATCH v8 4/5] x86/paravirt: Remove const mark from x86_hyper_xen_hvm variable

On 17.07.19 08:46, Joe Perches wrote:
> On Tue, 2019-07-16 at 12:26 +0800, Zhenzhong Duan wrote:
>> .. as "nopv" support needs it to be changeable at boot up stage.
>>
>> Checkpatch reports warning, so move variable declarations from
>> hypervisor.c to hypervisor.h
> []
>> diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
> []
>> @@ -259,7 +259,7 @@ static __init void xen_hvm_guest_late_init(void)
>> #endif
>> }
>>
>> -const __initconst struct hypervisor_x86 x86_hyper_xen_hvm = {
>> +struct hypervisor_x86 x86_hyper_xen_hvm __initdata = {
>
> static?

It is being referenced from arch/x86/kernel/cpu/hypervisor.c


Juergen

2019-07-17 17:24:55

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH v8 4/5] x86/paravirt: Remove const mark from x86_hyper_xen_hvm variable

On Wed, 2019-07-17 at 08:49 +0200, Juergen Gross wrote:
> On 17.07.19 08:46, Joe Perches wrote:
> > On Tue, 2019-07-16 at 12:26 +0800, Zhenzhong Duan wrote:
> > > .. as "nopv" support needs it to be changeable at boot up stage.
> > >
> > > Checkpatch reports warning, so move variable declarations from
> > > hypervisor.c to hypervisor.h
> > []
> > > diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
> > []
> > > @@ -259,7 +259,7 @@ static __init void xen_hvm_guest_late_init(void)
> > > #endif
> > > }
> > >
> > > -const __initconst struct hypervisor_x86 x86_hyper_xen_hvm = {
> > > +struct hypervisor_x86 x86_hyper_xen_hvm __initdata = {
> >
> > static?
>
> It is being referenced from arch/x86/kernel/cpu/hypervisor.c

But wasn't it also removed from the list of externs?

Rereading the .h file, no it wasn't. I missed that.

Perhaps the extern list could be reordered to move this
x86_hyper_xen_hvm to be next to x86_hyper_type.

I also suggest that "extern bool nopv" might be a bit
non-specific and could use a longer identifier.


2019-07-18 03:52:09

by Jürgen Groß

[permalink] [raw]
Subject: Re: [PATCH v8 4/5] x86/paravirt: Remove const mark from x86_hyper_xen_hvm variable

On 17.07.19 19:22, Joe Perches wrote:
> On Wed, 2019-07-17 at 08:49 +0200, Juergen Gross wrote:
>> On 17.07.19 08:46, Joe Perches wrote:
>>> On Tue, 2019-07-16 at 12:26 +0800, Zhenzhong Duan wrote:
>>>> .. as "nopv" support needs it to be changeable at boot up stage.
>>>>
>>>> Checkpatch reports warning, so move variable declarations from
>>>> hypervisor.c to hypervisor.h
>>> []
>>>> diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
>>> []
>>>> @@ -259,7 +259,7 @@ static __init void xen_hvm_guest_late_init(void)
>>>> #endif
>>>> }
>>>>
>>>> -const __initconst struct hypervisor_x86 x86_hyper_xen_hvm = {
>>>> +struct hypervisor_x86 x86_hyper_xen_hvm __initdata = {
>>>
>>> static?
>>
>> It is being referenced from arch/x86/kernel/cpu/hypervisor.c
>
> But wasn't it also removed from the list of externs?
>
> Rereading the .h file, no it wasn't. I missed that.
>
> Perhaps the extern list could be reordered to move this
> x86_hyper_xen_hvm to be next to x86_hyper_type.
>
> I also suggest that "extern bool nopv" might be a bit
> non-specific and could use a longer identifier.

You are a little bit late. It has been this way since V5 of the series
posted on July 3rd.

I have pushed the series to my tree already and I'm about to send the
pull request.

Followup patches welcome. :-)


Juergen