x86/hyperv: add noop functions to x86_init mpparse functions

Message ID 1685709712-13752-1-git-send-email-ssengar@linux.microsoft.com
State New
Headers
Series x86/hyperv: add noop functions to x86_init mpparse functions |

Commit Message

Saurabh Singh Sengar June 2, 2023, 12:41 p.m. UTC
  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 <ssengar@linux.microsoft.com>
---
 arch/x86/hyperv/hv_vtl.c | 4 ++++
 1 file changed, 4 insertions(+)
  

Comments

Dave Hansen June 2, 2023, 3:33 p.m. UTC | #1
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.
  
Wei Liu June 2, 2023, 4:17 p.m. UTC | #2
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.
  
Dave Hansen June 2, 2023, 4:21 p.m. UTC | #3
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.
  
Wei Liu June 2, 2023, 4:22 p.m. UTC | #4
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 <ssengar@linux.microsoft.com>
> ---
>  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
>
  
Saurabh Singh Sengar June 2, 2023, 4:27 p.m. UTC | #5
> -----Original Message-----
> From: Wei Liu <wei.liu@kernel.org>
> Sent: Friday, June 2, 2023 9:53 PM
> To: Saurabh Sengar <ssengar@linux.microsoft.com>
> Cc: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; wei.liu@kernel.org; Dexuan Cui
> <decui@microsoft.com>; tglx@linutronix.de; mingo@redhat.com;
> bp@alien8.de; dave.hansen@linux.intel.com; x86@kernel.org; Michael Kelley
> (LINUX) <mikelley@microsoft.com>; linux-kernel@vger.kernel.org; linux-
> hyperv@vger.kernel.org; hpa@zytor.com
> 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 <ssengar@linux.microsoft.com>
> > ---
> >  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
> >
  

Patch

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;