2023-06-02 12:50:34

by Saurabh Singh Sengar

[permalink] [raw]
Subject: [PATCH] x86/hyperv: add noop functions to x86_init mpparse functions

In !ACPI system, there is no way to disable CONFIG_X86_MPPARSE.
When CONFIG_X86_MPPARSE is enabled for VTL2, the kernel will
scan low memory looking for MP tables. Don't allow this, because
low memory is controlled by VTL0 and may contain actual valid
tables for VTL0, which can confuse the VTL2 kernel.

Signed-off-by: Saurabh Sengar <[email protected]>
---
arch/x86/hyperv/hv_vtl.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
index 1ba5d3b99b16..ea21d897b5da 100644
--- a/arch/x86/hyperv/hv_vtl.c
+++ b/arch/x86/hyperv/hv_vtl.c
@@ -23,6 +23,10 @@ void __init hv_vtl_init_platform(void)
x86_init.irqs.pre_vector_init = x86_init_noop;
x86_init.timers.timer_init = x86_init_noop;

+ /* Avoid searching for BIOS MP tables */
+ x86_init.mpparse.find_smp_config = x86_init_noop;
+ x86_init.mpparse.get_smp_config = x86_init_uint_noop;
+
x86_platform.get_wallclock = get_rtc_noop;
x86_platform.set_wallclock = set_rtc_noop;
x86_platform.get_nmi_reason = hv_get_nmi_reason;
--
2.34.1



2023-06-02 15:52:17

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH] x86/hyperv: add noop functions to x86_init mpparse functions

On 6/2/23 05:41, Saurabh Sengar wrote:
> In !ACPI system, there is no way to disable CONFIG_X86_MPPARSE.
> When CONFIG_X86_MPPARSE is enabled for VTL2, the kernel will
> scan low memory looking for MP tables. Don't allow this, because
> low memory is controlled by VTL0 and may contain actual valid
> tables for VTL0, which can confuse the VTL2 kernel.

Do you folks have a writeup of this VTL* setup anywhere? I'm struggling
to grasp why VTL0 and VTL2 share the same address space and why they
would get confused by each other's data structures.

$ grep -r VTL[02] Documentation/
$

Either way, this is way better than the #ifdefs. But the changelog is
kinda just gibberish to me.

2023-06-02 16:23:56

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH] x86/hyperv: add noop functions to x86_init mpparse functions

On 6/2/23 09:17, Wei Liu wrote:
> On Fri, Jun 02, 2023 at 08:33:13AM -0700, Dave Hansen wrote:
>> On 6/2/23 05:41, Saurabh Sengar wrote:
>>> In !ACPI system, there is no way to disable CONFIG_X86_MPPARSE.
>>> When CONFIG_X86_MPPARSE is enabled for VTL2, the kernel will
>>> scan low memory looking for MP tables. Don't allow this, because
>>> low memory is controlled by VTL0 and may contain actual valid
>>> tables for VTL0, which can confuse the VTL2 kernel.
>> Do you folks have a writeup of this VTL* setup anywhere? I'm struggling
>> to grasp why VTL0 and VTL2 share the same address space and why they
>> would get confused by each other's data structures.
> Dave, here is some public information about Virtual Trust Level.
>
> https://learn.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/vsm#virtual-trust-level-vtl

Yeah, I can google too. ;)

Seriously, though. I don't need an architecture document. This is like
me asking how NMIs work in Linux and someone pointing me to the SDM
describing the hardware architecture.

I need to know about the Linux implementation of these VTL's, not the
overall VTL architecture.

2023-06-02 16:36:44

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH] x86/hyperv: add noop functions to x86_init mpparse functions

On Fri, Jun 02, 2023 at 08:33:13AM -0700, Dave Hansen wrote:
> On 6/2/23 05:41, Saurabh Sengar wrote:
> > In !ACPI system, there is no way to disable CONFIG_X86_MPPARSE.
> > When CONFIG_X86_MPPARSE is enabled for VTL2, the kernel will
> > scan low memory looking for MP tables. Don't allow this, because
> > low memory is controlled by VTL0 and may contain actual valid
> > tables for VTL0, which can confuse the VTL2 kernel.
>
> Do you folks have a writeup of this VTL* setup anywhere? I'm struggling
> to grasp why VTL0 and VTL2 share the same address space and why they
> would get confused by each other's data structures.

Dave, here is some public information about Virtual Trust Level.

https://learn.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/vsm#virtual-trust-level-vtl

I wished it could be more detailed.

With the proper configuration, VTL2 can see memory from VTL0, but not
the other way around.

Saurabh can probably give you more information for this particular setup.

Thanks,
Wei.

>
> $ grep -r VTL[02] Documentation/
> $
>
> Either way, this is way better than the #ifdefs. But the changelog is
> kinda just gibberish to me.

2023-06-02 16:40:11

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH] x86/hyperv: add noop functions to x86_init mpparse functions

On Fri, Jun 02, 2023 at 05:41:52AM -0700, Saurabh Sengar wrote:
> In !ACPI system, there is no way to disable CONFIG_X86_MPPARSE.
> When CONFIG_X86_MPPARSE is enabled for VTL2, the kernel will
> scan low memory looking for MP tables. Don't allow this, because
> low memory is controlled by VTL0 and may contain actual valid
> tables for VTL0, which can confuse the VTL2 kernel.
>
> Signed-off-by: Saurabh Sengar <[email protected]>
> ---
> arch/x86/hyperv/hv_vtl.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
> index 1ba5d3b99b16..ea21d897b5da 100644
> --- a/arch/x86/hyperv/hv_vtl.c
> +++ b/arch/x86/hyperv/hv_vtl.c
> @@ -23,6 +23,10 @@ void __init hv_vtl_init_platform(void)
> x86_init.irqs.pre_vector_init = x86_init_noop;
> x86_init.timers.timer_init = x86_init_noop;
>
> + /* Avoid searching for BIOS MP tables */
> + x86_init.mpparse.find_smp_config = x86_init_noop;
> + x86_init.mpparse.get_smp_config = x86_init_uint_noop;
> +

The code looks fine.

Can you expand the commit message a bit so that people who are not
familiar with VTL and your setup can understand what's going on?

Thanks,
Wei.

> x86_platform.get_wallclock = get_rtc_noop;
> x86_platform.set_wallclock = set_rtc_noop;
> x86_platform.get_nmi_reason = hv_get_nmi_reason;
> --
> 2.34.1
>

2023-06-02 16:40:57

by Saurabh Singh Sengar

[permalink] [raw]
Subject: RE: [EXTERNAL] Re: [PATCH] x86/hyperv: add noop functions to x86_init mpparse functions



> -----Original Message-----
> From: Wei Liu <[email protected]>
> Sent: Friday, June 2, 2023 9:53 PM
> To: Saurabh Sengar <[email protected]>
> Cc: KY Srinivasan <[email protected]>; Haiyang Zhang
> <[email protected]>; [email protected]; Dexuan Cui
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; Michael Kelley
> (LINUX) <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]
> Subject: [EXTERNAL] Re: [PATCH] x86/hyperv: add noop functions to x86_init
> mpparse functions
>
> On Fri, Jun 02, 2023 at 05:41:52AM -0700, Saurabh Sengar wrote:
> > In !ACPI system, there is no way to disable CONFIG_X86_MPPARSE.
> > When CONFIG_X86_MPPARSE is enabled for VTL2, the kernel will scan low
> > memory looking for MP tables. Don't allow this, because low memory is
> > controlled by VTL0 and may contain actual valid tables for VTL0, which
> > can confuse the VTL2 kernel.
> >
> > Signed-off-by: Saurabh Sengar <[email protected]>
> > ---
> > arch/x86/hyperv/hv_vtl.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c index
> > 1ba5d3b99b16..ea21d897b5da 100644
> > --- a/arch/x86/hyperv/hv_vtl.c
> > +++ b/arch/x86/hyperv/hv_vtl.c
> > @@ -23,6 +23,10 @@ void __init hv_vtl_init_platform(void)
> > x86_init.irqs.pre_vector_init = x86_init_noop;
> > x86_init.timers.timer_init = x86_init_noop;
> >
> > + /* Avoid searching for BIOS MP tables */
> > + x86_init.mpparse.find_smp_config = x86_init_noop;
> > + x86_init.mpparse.get_smp_config = x86_init_uint_noop;
> > +
>
> The code looks fine.
>
> Can you expand the commit message a bit so that people who are not
> familiar with VTL and your setup can understand what's going on?

Sure, I will add more details in commit message.

>
> Thanks,
> Wei.
>
> > x86_platform.get_wallclock = get_rtc_noop;
> > x86_platform.set_wallclock = set_rtc_noop;
> > x86_platform.get_nmi_reason = hv_get_nmi_reason;
> > --
> > 2.34.1
> >