Message ID | 1686640391-13001-1-git-send-email-ssengar@linux.microsoft.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp362515vqr; Tue, 13 Jun 2023 00:31:50 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6elUDoJ4+BU1k9o3UUj8sLpz9P3Q6vG9W1tTshftYxG7aoeGhCqBhazUyy0pT8iQvoKMUB X-Received: by 2002:a05:6a20:7d96:b0:103:7b36:f21 with SMTP id v22-20020a056a207d9600b001037b360f21mr18313457pzj.21.1686641510619; Tue, 13 Jun 2023 00:31:50 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1686641510; cv=none; d=google.com; s=arc-20160816; b=LPubLqsSJRIo4TmU13E4h/gWc/1R9ow4qLHEbpRVnSoIgINYFbY1utWeTiBch7qs4L Xb8RwlgxfkV6IG4QDDWiVTxnvJp4siqsWTPMXnHgHnp/agNidzxMOTQe0HCgCqxykiIY aNlm7srsopjTCmOkTifVPJifMCq6iYEXeMKkkl0D+4HRatihR98e2VLC9xEqJYJQIA2i YE2BQsxA/ywKgyn+tKpRx/b+PEOA7b3eIR+ndyKRc4+F9uKOTdl8Haz4xp2U25keeR9o hMXknNlupTU8D3h9EVHWsfVcIl99uY7dal2sx1DZTQorYb4ogO+NenukMLXlOLwyBI+W DWIg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:message-id:date:subject:to:from:dkim-signature :dkim-filter; bh=/PT81JOy5+bQnTFFKIvi5pPXC/tbbHKChswSnu/pguk=; b=OkqUUI+qyicK2xR0tt8Su/ePmdA3ghGH9II3SwvC/SAPAfB7OF6odEV6MgapmjwXzV vnIwW7/Vwg7d5MEmsk5ADxUX/l0XLMiz36hU/7O3cMh5eyxA8muaqnJ4Y5RyGZ6vuhsO kT+yx3koi3qniAmBWY41w0xTupvp1LuifuSeSp4kMh+Zp7thVK+I+PbSU00bUshyKxHM 4AJldgTFVLZX8IXlmUtMljwCPz1kq3WXCYTBsgtIKUjvWbcLm8NbefJtTNcQe3RaYj6M XVL/uwrpdgNwpuctuPBM4fS/+SKiVvWjXqVkPNZuYoiOdWaNDenZJjO53LFk5s4X8Xe7 ZIWQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=V0Db+28u; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linux.microsoft.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id h28-20020a63385c000000b0053ef51d3fa8si8117911pgn.401.2023.06.13.00.31.37; Tue, 13 Jun 2023 00:31:50 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=V0Db+28u; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linux.microsoft.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238607AbjFMHNX (ORCPT <rfc822;rust.linux@gmail.com> + 99 others); Tue, 13 Jun 2023 03:13:23 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55506 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238669AbjFMHNV (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 13 Jun 2023 03:13:21 -0400 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 4CF0ECE; Tue, 13 Jun 2023 00:13:19 -0700 (PDT) Received: from linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net (linux.microsoft.com [13.77.154.182]) by linux.microsoft.com (Postfix) with ESMTPSA id AF6D320FE88D; Tue, 13 Jun 2023 00:13:18 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com AF6D320FE88D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1686640398; bh=/PT81JOy5+bQnTFFKIvi5pPXC/tbbHKChswSnu/pguk=; h=From:To:Subject:Date:From; b=V0Db+28uycwdF4LyY20ikpS4q2z84hTbVu/y2Alw28k2dkzMg2Q1YQnUHdjrk98ry 0pUJQZh+OXbB+VMThRoCMf69fz/KXJ/NSBt/Ax/a++xb93ROR30TTIKXmIDGDYYlUE MFwpCm1c25Ul/GVzqCnmYWc9GoP+owhi/j9ncVpM= From: Saurabh Sengar <ssengar@linux.microsoft.com> To: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org, decui@microsoft.com, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org, mikelley@microsoft.com, linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org, hpa@zytor.com Subject: [PATCH v2] x86/hyperv: add noop functions to x86_init mpparse functions Date: Tue, 13 Jun 2023 00:13:11 -0700 Message-Id: <1686640391-13001-1-git-send-email-ssengar@linux.microsoft.com> X-Mailer: git-send-email 1.8.3.1 X-Spam-Status: No, score=-19.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,ENV_AND_HDR_SPF_MATCH,RCVD_IN_DNSWL_MED, SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE,USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1768571808443790182?= X-GMAIL-MSGID: =?utf-8?q?1768571808443790182?= |
Series |
[v2] x86/hyperv: add noop functions to x86_init mpparse functions
|
|
Commit Message
Saurabh Singh Sengar
June 13, 2023, 7:13 a.m. UTC
In certain configurations, VTL0 and VTL2 can share the same VM
partition and hence share the same memory address space. In such
systems VTL2 has visibility of all of the VTL0 memory space.
When CONFIG_X86_MPPARSE is enabled for VTL2, the kernel performs
a scan of low memory to search for MP tables. However, in systems
where VTL0 controls the low memory and may contain valid tables
specific to VTL0, this scanning process can lead to confusion
within the VTL2 kernel.
In !ACPI system, there is no way to disable CONFIG_X86_MPPARSE
hence add the noop function instead.
Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
---
[V2]: Improve commit message
arch/x86/hyperv/hv_vtl.c | 4 ++++
1 file changed, 4 insertions(+)
Comments
On 6/13/23 00:13, Saurabh Sengar wrote: > In certain configurations, VTL0 and VTL2 can share the same VM > partition and hence share the same memory address space. In such > systems VTL2 has visibility of all of the VTL0 memory space. FWIW, this is pretty much gibberish to me. The way I suggest avoiding producing gibberish is avoiding acronyms: Hyper-V can run VMs at different privilege "levels". Sometimes, it chooses to run two different VMs at different levels but they share some of their address space. This <insert reason that someone might want to do this>. That's not gibberish. > When CONFIG_X86_MPPARSE is enabled for VTL2, the kernel performs > a scan of low memory to search for MP tables. However, in systems > where VTL0 controls the low memory and may contain valid tables > specific to VTL0, this scanning process can lead to confusion > within the VTL2 kernel. What is the end-user-visible effect of this "confusion"? A crash? A warning? An error message? What is the impact on end users? This information will help the maintainers decide how to disposition your patch. Should we send it upstream immediately because it's impacting millions of users? Or can we do it in a bit more leisurely fashion because nobody cares? > In !ACPI system, there is no way to disable CONFIG_X86_MPPARSE > hence add the noop function instead. This makes zero sense to me. Like I told you before, we don't compile things out just because they don't work on one weirdo system. If we did that, we'd have a billion incompatible x86 kernel images that can't boot across systems.
> -----Original Message----- > From: Dave Hansen <dave.hansen@intel.com> > Sent: Tuesday, June 13, 2023 11:03 PM > To: Saurabh Sengar <ssengar@linux.microsoft.com>; 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 v2] x86/hyperv: add noop functions to > x86_init mpparse functions > > On 6/13/23 00:13, Saurabh Sengar wrote: > > In certain configurations, VTL0 and VTL2 can share the same VM > > partition and hence share the same memory address space. In such > > systems VTL2 has visibility of all of the VTL0 memory space. > > FWIW, this is pretty much gibberish to me. The way I suggest avoiding > producing gibberish is avoiding acronyms: > > Hyper-V can run VMs at different privilege "levels". Sometimes, > it chooses to run two different VMs at different levels but > they share some of their address space. This <insert reason > that someone might want to do this>. > > That's not gibberish. Thanks for your suggestion I can add this in v3. > > > When CONFIG_X86_MPPARSE is enabled for VTL2, the kernel performs a > > scan of low memory to search for MP tables. However, in systems where > > VTL0 controls the low memory and may contain valid tables specific to > > VTL0, this scanning process can lead to confusion within the VTL2 > > kernel. > > What is the end-user-visible effect of this "confusion"? A crash? A warning? > An error message? What is the impact on end users? The VTL2 kernel is currently scanning the VTL0 MP table and incorporating that information, which is incorrect because VTL2 doesn't support MP tables and instead, is booted with DT. While I don't have an immediate crash or error message to present, this situation could potentially result in incorrect behaviour. > > This information will help the maintainers decide how to disposition your > patch. Should we send it upstream immediately because it's impacting > millions of users? Or can we do it in a bit more leisurely fashion because > nobody cares? I understand, I have provided all the information I have please consider what is appropriate in this case. > > > In !ACPI system, there is no way to disable CONFIG_X86_MPPARSE hence > > add the noop function instead. > > This makes zero sense to me. My intention was to tell that this fix is required because we are in !ACPI system. If it was ACPI system we could have simply disable this CONFIG_X86_MPPARSE Option. But as you suggested I am fine removing these 2 lines. > > Like I told you before, we don't compile things out just because they don't > work on one weirdo system. If we did that, we'd have a billion incompatible > x86 kernel images that can't boot across systems. > Understood, thank you. I was just considering the option of keeping the default setting for CONFIG_X86_MPPARSE as 'Y' and allowing the choice to change it to 'N' if someone specifically desires to do so. I also considered that nowadays, not many kernels are likely to utilize MP tables for booting x86 machines, which is why I thought this change wouldn't have a significant impact. Moreover, there is a potential benefit in terms of reducing the kernel's size. However, I completely respect and am open to whatever you decide, having better visibility of wider kernel community needs. - Saurabh
> -----Original Message----- > From: Saurabh Singh Sengar <ssengar@microsoft.com> > Sent: Wednesday, June 14, 2023 9:36 AM > To: Dave Hansen <dave.hansen@intel.com>; Saurabh Sengar > <ssengar@linux.microsoft.com>; 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: RE: [EXTERNAL] Re: [PATCH v2] x86/hyperv: add noop functions to > x86_init mpparse functions > > > > > -----Original Message----- > > From: Dave Hansen <dave.hansen@intel.com> > > Sent: Tuesday, June 13, 2023 11:03 PM > > To: Saurabh Sengar <ssengar@linux.microsoft.com>; 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 v2] x86/hyperv: add noop functions to > > x86_init mpparse functions > > > > On 6/13/23 00:13, Saurabh Sengar wrote: > > > In certain configurations, VTL0 and VTL2 can share the same VM > > > partition and hence share the same memory address space. In such > > > systems VTL2 has visibility of all of the VTL0 memory space. > > > > FWIW, this is pretty much gibberish to me. The way I suggest avoiding > > producing gibberish is avoiding acronyms: > > > > Hyper-V can run VMs at different privilege "levels". Sometimes, > > it chooses to run two different VMs at different levels but > > they share some of their address space. This <insert reason > > that someone might want to do this>. > > > > That's not gibberish. > > Thanks for your suggestion I can add this in v3. > > > > > > When CONFIG_X86_MPPARSE is enabled for VTL2, the kernel performs a > > > scan of low memory to search for MP tables. However, in systems > > > where > > > VTL0 controls the low memory and may contain valid tables specific > > > to VTL0, this scanning process can lead to confusion within the VTL2 > > > kernel. > > > > What is the end-user-visible effect of this "confusion"? A crash? A > warning? > > An error message? What is the impact on end users? > > The VTL2 kernel is currently scanning the VTL0 MP table and incorporating > that information, which is incorrect because VTL2 doesn't support MP tables > and instead, is booted with DT. While I don't have an immediate crash or > error message to present, this situation could potentially result in incorrect > behaviour. > > > > > This information will help the maintainers decide how to disposition > > your patch. Should we send it upstream immediately because it's > > impacting millions of users? Or can we do it in a bit more leisurely > > fashion because nobody cares? > > I understand, I have provided all the information I have please consider what > is appropriate in this case. > > > > > > In !ACPI system, there is no way to disable CONFIG_X86_MPPARSE hence > > > add the noop function instead. > > > > This makes zero sense to me. > > My intention was to tell that this fix is required because we are in !ACPI > system. > If it was ACPI system we could have simply disable this > CONFIG_X86_MPPARSE Option. But as you suggested I am fine removing > these 2 lines. > > > > > Like I told you before, we don't compile things out just because they > > don't work on one weirdo system. If we did that, we'd have a billion > > incompatible > > x86 kernel images that can't boot across systems. > > > > Understood, thank you. I was just considering the option of keeping the > default setting for CONFIG_X86_MPPARSE as 'Y' and allowing the choice to > change it to 'N' if someone specifically desires to do so. I also considered that > nowadays, not many kernels are likely to utilize MP tables for booting x86 > machines, which is why I thought this change wouldn't have a significant > impact. Moreover, there is a potential benefit in terms of reducing the > kernel's size. However, I completely respect and am open to whatever you > decide, having better visibility of wider kernel community needs. > > - Saurabh Below is the updated commit message, if there are no more concerns I will send the V3 with it. Hyper-V can run VMs at different privilege "levels" known as Virtual Trust Levels (VTL). Sometimes, it chooses to run two different VMs at different levels but they share some of their address space. In such setups VTL2 (higher level VM) has visibility of all of the VTL0 (level 0) memory space. When the CONFIG_X86_MPPARSE is enabled for VTL2, the VTL2 kernel performs a search within the low memory to locate MP tables. However, in systems where VTL0 manages the low memory and may contain valid tables, this scanning can result in incorrect MP table information being provided to the VTL2 kernel, mistakenly considering VTL0's MP table as its own Add noop functions to avoid MP parse scan by VTL2. - Saurabh
diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c index 85d38b9f3586..db5d2ea39fc0 100644 --- a/arch/x86/hyperv/hv_vtl.c +++ b/arch/x86/hyperv/hv_vtl.c @@ -25,6 +25,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;