X86_MPPARSE is only selectable when ACPI is enabled. However,
on Devicetree platforms where ACPI is disabled, it is always
enabled. Allow X86_MPPARSE to be selected by OF platforms as
well.
Signed-off-by: Saurabh Sengar <[email protected]>
---
arch/x86/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 53bab123a8ee..e60315397399 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -469,7 +469,7 @@ config X86_X2APIC
If you don't know what to do here, say N.
config X86_MPPARSE
- bool "Enable MPS table" if ACPI
+ bool "Enable MPS table" if ACPI || OF
default y
depends on X86_LOCAL_APIC
help
--
2.34.1
From: Saurabh Sengar <[email protected]> Sent: Thursday, May 11, 2023 7:55 AM
>
> X86_MPPARSE is only selectable when ACPI is enabled. However,
> on Devicetree platforms where ACPI is disabled, it is always
> enabled. Allow X86_MPPARSE to be selected by OF platforms as
> well.
>
> Signed-off-by: Saurabh Sengar <[email protected]>
> ---
> arch/x86/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 53bab123a8ee..e60315397399 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -469,7 +469,7 @@ config X86_X2APIC
> If you don't know what to do here, say N.
>
> config X86_MPPARSE
> - bool "Enable MPS table" if ACPI
> + bool "Enable MPS table" if ACPI || OF
> default y
> depends on X86_LOCAL_APIC
> help
> --
> 2.34.1
Reviewed-by: Michael Kelley <[email protected]>
On 5/11/23 07:54, Saurabh Sengar wrote:
> X86_MPPARSE is only selectable when ACPI is enabled. However,
> on Devicetree platforms where ACPI is disabled, it is always
> enabled. Allow X86_MPPARSE to be selected by OF platforms as
> well.
I'm finding this changelog really hard to read.
In Kconfig, you can "select FOO". But in this changelog, it means
something different. I think "selectable" here means that there's a
user prompt for the option.
Could you please rephrase this to be less confusing?
This is also one of those patches where I wonder: Why do _you_ care
about this? Are you just trying to be nice? Is this intended as some
kind of cleanup?
> -----Original Message-----
> From: Dave Hansen <[email protected]>
> Sent: Tuesday, May 23, 2023 11:23 PM
> To: Saurabh Sengar <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; KY Srinivasan <[email protected]>;
> Haiyang Zhang <[email protected]>; [email protected]; Dexuan Cui
> <[email protected]>; Michael Kelley (LINUX) <[email protected]>;
> [email protected]; [email protected]
> Cc: Saurabh Singh Sengar <[email protected]>
> Subject: [EXTERNAL] Re: [PATCH 1/2] x86/Kconfig: Allow
> CONFIG_X86_MPPARSE disable for OF platforms
>
> On 5/11/23 07:54, Saurabh Sengar wrote:
> > X86_MPPARSE is only selectable when ACPI is enabled. However, on
> > Devicetree platforms where ACPI is disabled, it is always enabled.
> > Allow X86_MPPARSE to be selected by OF platforms as well.
>
> I'm finding this changelog really hard to read.
>
> In Kconfig, you can "select FOO". But in this changelog, it means something
> different. I think "selectable" here means that there's a user prompt for the
> option.
>
> Could you please rephrase this to be less confusing?
Thanks for your review. Currently, in the absence of ACPI, it is impossible to
disable X86_MPPARSE. In the case of ACPI being enabled, one has the
option to either enable or disable X86_MPARSE. My intention is to permit
X86_MPPARSE=n for OF platforms where ACPI=n. To describe the capability
of choosing any desired value for MPPARSE, I used the term 'selectable.'
Perhaps 'configurable' would be a more appropriate word in this context?
I can fix this and include it in V2.
>
> This is also one of those patches where I wonder: Why do _you_ care about
> this? Are you just trying to be nice? Is this intended as some kind of cleanup?
It solves an issue for Hyper-V VBS setup, please refer to the 2/2 of this patch
series.
Regards,
Saurabh
On 5/24/23 09:23, Saurabh Singh Sengar wrote:
>> Could you please rephrase this to be less confusing?
>
> Thanks for your review. Currently, in the absence of ACPI, it is impossible to
> disable X86_MPPARSE. In the case of ACPI being enabled, one has the
> option to either enable or disable X86_MPARSE. My intention is to permit
> X86_MPPARSE=n for OF platforms where ACPI=n. To describe the capability
> of choosing any desired value for MPPARSE, I used the term 'selectable.'
> Perhaps 'configurable' would be a more appropriate word in this context?
> I can fix this and include it in V2.
OK, so this particular Hyper-V setup doesn't run normal normal distro
kernels? It requires a very specialized kernel? Or it's just _better_
that the kernel is configured this way?
>> This is also one of those patches where I wonder: Why do _you_ care about
>> this? Are you just trying to be nice? Is this intended as some kind of cleanup?
>
> It solves an issue for Hyper-V VBS setup, please refer to the 2/2 of this patch
> series.
Heh, that changelog is even more confusing than _this_ one. It doesn't
say that there's a problem, only that it removes "not required" code.
I'm still confused by this whole thing.
> -----Original Message-----
> From: Dave Hansen <[email protected]>
> Sent: Wednesday, May 24, 2023 11:33 PM
> To: Saurabh Singh Sengar <[email protected]>; Saurabh Sengar
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; KY Srinivasan <[email protected]>; Haiyang Zhang
> <[email protected]>; [email protected]; Dexuan Cui
> <[email protected]>; Michael Kelley (LINUX) <[email protected]>;
> [email protected]; [email protected]
> Subject: Re: [EXTERNAL] Re: [PATCH 1/2] x86/Kconfig: Allow
> CONFIG_X86_MPPARSE disable for OF platforms
>
> On 5/24/23 09:23, Saurabh Singh Sengar wrote:
> >> Could you please rephrase this to be less confusing?
> >
> > Thanks for your review. Currently, in the absence of ACPI, it is
> > impossible to disable X86_MPPARSE. In the case of ACPI being enabled,
> > one has the option to either enable or disable X86_MPARSE. My
> > intention is to permit X86_MPPARSE=n for OF platforms where ACPI=n. To
> > describe the capability of choosing any desired value for MPPARSE, I used
> the term 'selectable.'
> > Perhaps 'configurable' would be a more appropriate word in this context?
> > I can fix this and include it in V2.
>
> OK, so this particular Hyper-V setup doesn't run normal normal distro
> kernels? It requires a very specialized kernel? Or it's just _better_ that the
> kernel is configured this way?
This is a system where we provide isolation through VTL. More information
About VTL is here: https://learn.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/vsm#virtual-trust-level-vtl
In the context of VTL0, the system runs the normal kernel. However, for
higher VTLs, we enable the kernel with HYPERV_VTL_MODE. It's important
to note that the lower memory of the system is not included in the higher VTLs
It runs the normal kernel as part of VTL0 but for higher VTLs we run the kernel with
HYPERV_VTL_MODE enable. VTL0 is assigned to lower memory of system, the lower
memory not part of higher VTLs. However, MPARSE option disregard this and try to
scan VTL0 memory which causes issue. This will enable any system which
doesn't want lower memory to be scanned for MPPARSE for whatever reason.
>
> >> This is also one of those patches where I wonder: Why do _you_ care
> >> about this? Are you just trying to be nice? Is this intended as some kind of
> cleanup?
> >
> > It solves an issue for Hyper-V VBS setup, please refer to the 2/2 of
> > this patch series.
>
> Heh, that changelog is even more confusing than _this_ one. It doesn't say
> that there's a problem, only that it removes "not required" code.
>
> I'm still confused by this whole thing.
I agree the commit message could be better. And I commented below in that
thread to clarify. Hope it clarifies, please let me know if any more information is
required:
When CONFIG_X86_MPPARSE is enabled, the kernel will scan low memory,
looking for MP tables. In Hyper-V VBS setup, lower memory is assigned to VTL0.
This lower memory may contain the actual MPPARSE table for VTL0,
which can confuse the VTLx kernel and cause issues. (x > 0)
On 5/25/23 00:06, Saurabh Singh Sengar wrote:
> When CONFIG_X86_MPPARSE is enabled, the kernel will scan low memory,
> looking for MP tables. In Hyper-V VBS setup, lower memory is assigned to VTL0.
> This lower memory may contain the actual MPPARSE table for VTL0,
> which can confuse the VTLx kernel and cause issues. (x > 0)
This still just seems wrong.
If an action crashes the kernel because of changes, we don't just
compile it out and move on. We add enumeration for the new feature
that's causing it and turn off the action at runtime.
> -----Original Message-----
> From: Dave Hansen <[email protected]>
> Sent: Thursday, May 25, 2023 7:26 PM
> To: Saurabh Singh Sengar <[email protected]>; Saurabh Sengar
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; KY Srinivasan <[email protected]>; Haiyang Zhang
> <[email protected]>; [email protected]; Dexuan Cui
> <[email protected]>; Michael Kelley (LINUX) <[email protected]>;
> [email protected]; [email protected]
> Subject: Re: [EXTERNAL] Re: [PATCH 1/2] x86/Kconfig: Allow
> CONFIG_X86_MPPARSE disable for OF platforms
>
> On 5/25/23 00:06, Saurabh Singh Sengar wrote:
> > When CONFIG_X86_MPPARSE is enabled, the kernel will scan low memory,
> > looking for MP tables. In Hyper-V VBS setup, lower memory is assigned to
> VTL0.
> > This lower memory may contain the actual MPPARSE table for VTL0, which
> > can confuse the VTLx kernel and cause issues. (x > 0)
>
> This still just seems wrong.
>
> If an action crashes the kernel because of changes, we don't just compile it
> out and move on. We add enumeration for the new feature that's causing it
> and turn off the action at runtime.
>
Thanks, I greatly appreciate your suggestion and wanted to let you know that
I am actively working on upstreaming the new fix for the VTL driver to bypass
the need for the MP table scan.
Furthermore, I would like to learn about the rationale behind disallowing the
disablement of CONFIG_X86_MPPARSE when MP tables are not in use. Considering
that we compile out the features we don't support, wouldn't it be acceptable to
allow users to customize their configurations in this manner? Allowing the
disablement of CONFIG_X86_MPPARSE would provide greater flexibility and
efficiency for those who do not utilize MP tables.
On 6/2/23 05:22, Saurabh Singh Sengar wrote:
> Furthermore, I would like to learn about the rationale behind disallowing the
> disablement of CONFIG_X86_MPPARSE when MP tables are not in use. Considering
> that we compile out the features we don't support, wouldn't it be acceptable to
> allow users to customize their configurations in this manner? Allowing the
> disablement of CONFIG_X86_MPPARSE would provide greater flexibility and
> efficiency for those who do not utilize MP tables.
If someone sent a patch, I'd certainly look and figure out what
"flexibility" and "efficiency" it would provide. But, honestly, it
would just just be noise if it doesn't solve an _actual_ problem.
Would anyone care or notice the "flexibility" and "efficiency" it would
provide?
> -----Original Message-----
> From: Dave Hansen <[email protected]>
> Sent: Friday, June 2, 2023 7:54 PM
> To: Saurabh Singh Sengar <[email protected]>; Saurabh Sengar
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; KY Srinivasan <[email protected]>; Haiyang Zhang
> <[email protected]>; [email protected]; Dexuan Cui
> <[email protected]>; Michael Kelley (LINUX) <[email protected]>;
> [email protected]; [email protected]
> Subject: [EXTERNAL] Re: [PATCH 1/2] x86/Kconfig: Allow
> CONFIG_X86_MPPARSE disable for OF platforms
>
> On 6/2/23 05:22, Saurabh Singh Sengar wrote:
> > Furthermore, I would like to learn about the rationale behind
> > disallowing the disablement of CONFIG_X86_MPPARSE when MP tables are
> > not in use. Considering that we compile out the features we don't
> > support, wouldn't it be acceptable to allow users to customize their
> > configurations in this manner? Allowing the disablement of
> > CONFIG_X86_MPPARSE would provide greater flexibility and efficiency for
> those who do not utilize MP tables.
>
> If someone sent a patch, I'd certainly look and figure out what "flexibility" and
> "efficiency" it would provide. But, honestly, it would just just be noise if it
> doesn't solve an _actual_ problem.
>
> Would anyone care or notice the "flexibility" and "efficiency" it would
> provide?
After the new approach we have opted, I agree it's not blocking us from any functional
problem. Although one of the things we are trying to achieve is the minimal kernel
where we want to remove all the unnecessary options. Our system is not dependent
MPPARSE and we just don't want this code in our minimal kernel, and this config
option is preventing us from removing it.
Also, in past kernel has provided this flexibility for other options, which made me think
it is fine do it for OF as well.
https://lkml.indiana.edu/hypermail/linux/kernel/1210.3/01987.html
Happy to know your opinion and learn.