[PATCH v5] x86/speculation/l1tf: Add KConfig for setting the L1D cache flush mode

[PATCH v5] x86/speculation/l1tf: Add KConfig for setting the L1D cache flush mode

[Abhishek Bhardwaj]

This change adds a new kernel configuration that sets the l1d cache
flush setting at compile time rather than at run time.

The reasons for this change are as follows -

 - Kernel command line arguments are getting unwieldy. These parameters
 are not a scalable way to set the kernel config. They're intended as a
 super limited way for the bootloader to pass info to the kernel and
 also as a way for end users who are not compiling the kernel themselves
 to tweak the kernel behavior.

 - Also, if a user wants this setting from the start. It's a definite
 smell that it deserves to be a compile time thing rather than adding
 extra code plus whatever miniscule time at runtime to pass an
 extra argument.

 - Finally, it doesn't preclude the runtime / kernel command line way.
 Users are free to use those as well.

Signed-off-by: Abhishek Bhardwaj <abhishekbh@google.com>

---

Changes in v5:
- Fix indentation of the help text in the KConfig.

Changes in v4:
- Add motivation for the change in the commit message.

Changes in v3:
- Change depends on to only x86_64.
- Remove copy paste errors at the end of the KConfig.

Changes in v2:
- Fix typo in the help of the new KConfig.

 arch/x86/kernel/cpu/bugs.c |  8 ++++++++
 arch/x86/kvm/Kconfig       | 13 +++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 0b71970d2d3d2..1dcc875cf5547 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -1406,7 +1406,15 @@ enum l1tf_mitigations l1tf_mitigation __ro_after_init = L1TF_MITIGATION_FLUSH;
 #if IS_ENABLED(CONFIG_KVM_INTEL)
 EXPORT_SYMBOL_GPL(l1tf_mitigation);
 #endif
+#if (CONFIG_KVM_VMENTRY_L1D_FLUSH == 1)
+enum vmx_l1d_flush_state l1tf_vmx_mitigation = VMENTER_L1D_FLUSH_NEVER;
+#elif (CONFIG_KVM_VMENTRY_L1D_FLUSH == 2)
+enum vmx_l1d_flush_state l1tf_vmx_mitigation = VMENTER_L1D_FLUSH_COND;
+#elif (CONFIG_KVM_VMENTRY_L1D_FLUSH == 3)
+enum vmx_l1d_flush_state l1tf_vmx_mitigation = VMENTER_L1D_FLUSH_ALWAYS;
+#else
 enum vmx_l1d_flush_state l1tf_vmx_mitigation = VMENTER_L1D_FLUSH_AUTO;
+#endif
 EXPORT_SYMBOL_GPL(l1tf_vmx_mitigation);
 
 /*
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index b277a2db62676..cb2639dfb6da1 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -107,4 +107,17 @@ config KVM_MMU_AUDIT
 	 This option adds a R/W kVM module parameter 'mmu_audit', which allows
 	 auditing of KVM MMU events at runtime.
 
+config KVM_VMENTRY_L1D_FLUSH
+	int "L1D cache flush settings (1-3)"
+	range 1 3
+	default "2"
+	depends on KVM && X86_64
+	help
+	  This setting determines the L1D cache flush behavior before a VMENTER.
+	  This is similar to setting the option / parameter to
+	  kvm-intel.vmentry_l1d_flush.
+	  1 - Never flush.
+	  2 - Conditionally flush.
+	  3 - Always flush.
+
 endif # VIRTUALIZATION
-- 
2.27.0.383.g050319c2ae-goog

Re: [PATCH v5] x86/speculation/l1tf: Add KConfig for setting the L1D cache flush mode

[Thomas Gleixner]

Abhishek Bhardwaj <abhishekbh@google.com> writes:
> This change adds a new kernel configuration that sets the l1d cache
> flush setting at compile time rather than at run time.
>
> The reasons for this change are as follows -
>
>  - Kernel command line arguments are getting unwieldy. These parameters
>  are not a scalable way to set the kernel config. They're intended as a
>  super limited way for the bootloader to pass info to the kernel and
>  also as a way for end users who are not compiling the kernel themselves
>  to tweak the kernel behavior.
>
>  - Also, if a user wants this setting from the start. It's a definite
>  smell that it deserves to be a compile time thing rather than adding
>  extra code plus whatever miniscule time at runtime to pass an
>  extra argument.
>
>  - Finally, it doesn't preclude the runtime / kernel command line way.
>  Users are free to use those as well.

TBH, I don't see why this is a good idea.

 1) I'm not following your argumentation that the command line option is
    a poor Kconfig replacement. The L1TF mode is a boot time (module
    load time) decision and the command line parameter is there to
    override the carefully chosen and sensible default behaviour.

 2) You can add the desired mode to the compiled in (partial) kernel
    command line today.

 3) Boot loaders are well capable of handling large kernel command lines
    and the extra time spend for reading the parameter does not matter
    at all.

 4) It's just a tiny part of the whole speculation maze. If we go there
    for L1TF then we open the flood gates for a gazillion other config
    options.

 5) It's completely useless for distro kernels.

 6) The implementation is horrible. We have proper choice selectors
    which allow to add parseable information instead of random numbers
    and a help text.

Sorry, you need to find better arguments than 'unwieldy and smell' to
make this palatable.

Thanks,

        tglx

Re: [PATCH v5] x86/speculation/l1tf: Add KConfig for setting the L1D cache flush mode

[Doug Anderson]

Hi,

On Thu, Jul 9, 2020 at 3:51 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Abhishek Bhardwaj <abhishekbh@google.com> writes:
> > This change adds a new kernel configuration that sets the l1d cache
> > flush setting at compile time rather than at run time.
> >
> > The reasons for this change are as follows -
> >
> >  - Kernel command line arguments are getting unwieldy. These parameters
> >  are not a scalable way to set the kernel config. They're intended as a
> >  super limited way for the bootloader to pass info to the kernel and
> >  also as a way for end users who are not compiling the kernel themselves
> >  to tweak the kernel behavior.
> >
> >  - Also, if a user wants this setting from the start. It's a definite
> >  smell that it deserves to be a compile time thing rather than adding
> >  extra code plus whatever miniscule time at runtime to pass an
> >  extra argument.
> >
> >  - Finally, it doesn't preclude the runtime / kernel command line way.
> >  Users are free to use those as well.
>
> TBH, I don't see why this is a good idea.
>
>  1) I'm not following your argumentation that the command line option is
>     a poor Kconfig replacement. The L1TF mode is a boot time (module
>     load time) decision and the command line parameter is there to
>     override the carefully chosen and sensible default behaviour.

When you say that the default behavior is carefully chosen and
sensible, are you saying that (in your opinion) there would never be a
good reason for someone distributing a kernel to others to change the
default?  Certainly I agree that having the kernel command line
parameter is nice to allow someone to override whatever the person
building the kernel chose, but IMO it's not a good way to change the
default built-in to the kernel.

The current plan (as I understand it) is that we'd like to ship
Chromebook kernels with this option changed from the default that's
there now.  In your opinion, is that a sane thing to do?


>  2) You can add the desired mode to the compiled in (partial) kernel
>     command line today.

This might be easier on x86 than it is on ARM.  ARM (and ARM64)
kernels only have two modes: kernel provides cmdline and bootloader
provides cmdline.  There are out-of-mainline ANDROID patches to
address this but nothing in mainline.

The patch we're discussing now is x86-only so it's not such a huge
deal, but the fact that combining the kernel and bootloader
commandline never landed in mainline for arm/arm64 means that this
isn't a super common/expected thing to do.


>  3) Boot loaders are well capable of handling large kernel command lines
>     and the extra time spend for reading the parameter does not matter
>     at all.

Long command lines can still be a bit of a chore for humans to deal
with.  Many times I've needed to look at "/proc/cmdline" and make
sense of it.  The longer the command line is and the more cruft
stuffed into it the more of a chore it is.  Yes, this is just one
thing to put in the command line, but if 10 different drivers all have
their "one thing" to put there it gets really long.  If 100 different
drivers all want their one config option there it gets really really
long.  IMO the command line should be a last resort place to put
things and should just contain:

1. Legacy things that _have_ to be in the command line because they've
always been there.

2. Things that the bootloader/BIOS needs to communicate to the kernel
and has no better way to communicate.

3. Cases where the person running the kernel needs to override a
default set by the person compiling the kernel.


>  4) It's just a tiny part of the whole speculation maze. If we go there
>     for L1TF then we open the flood gates for a gazillion other config
>     options.

It seems like the only options that we'd need CONFIG option for would
be the ones where it would be sane to change the default compiled into
the kernel.  Hopefully that's not too many things?


>  5) It's completely useless for distro kernels.
>
>  6) The implementation is horrible. We have proper choice selectors
>     which allow to add parseable information instead of random numbers
>     and a help text.

If my other arguments make sense and Abhishek could just fix #6, would
that work?


Obviously, like many design choices, the above is all subjective.
It's really your call and if these arguments don't convince you it
sounds like the way forward is just to use "CONFIG_CMDLINE" and take
advantage of the fact that on x86 this will get merged with the
bootloader's command line.


-Doug

Re: [PATCH v5] x86/speculation/l1tf: Add KConfig for setting the L1D cache flush mode

[mark gross]

On Thu, Jul 09, 2020 at 12:42:57PM -0700, Doug Anderson wrote:
> Hi,
> 
> On Thu, Jul 9, 2020 at 3:51 AM Thomas Gleixner <tglx@linutronix.de> wrote:
> >
> > Abhishek Bhardwaj <abhishekbh@google.com> writes:
> > > This change adds a new kernel configuration that sets the l1d cache
> > > flush setting at compile time rather than at run time.
> > >
> > > The reasons for this change are as follows -
> > >
> > >  - Kernel command line arguments are getting unwieldy. These parameters
> > >  are not a scalable way to set the kernel config. They're intended as a
> > >  super limited way for the bootloader to pass info to the kernel and
> > >  also as a way for end users who are not compiling the kernel themselves
> > >  to tweak the kernel behavior.
> > >
> > >  - Also, if a user wants this setting from the start. It's a definite
> > >  smell that it deserves to be a compile time thing rather than adding
> > >  extra code plus whatever miniscule time at runtime to pass an
> > >  extra argument.
> > >
> > >  - Finally, it doesn't preclude the runtime / kernel command line way.
> > >  Users are free to use those as well.
> >
> > TBH, I don't see why this is a good idea.
> >
> >  1) I'm not following your argumentation that the command line option is
> >     a poor Kconfig replacement. The L1TF mode is a boot time (module
> >     load time) decision and the command line parameter is there to
> >     override the carefully chosen and sensible default behaviour.
> 
> When you say that the default behavior is carefully chosen and
> sensible, are you saying that (in your opinion) there would never be a
> good reason for someone distributing a kernel to others to change the
> default?  Certainly I agree that having the kernel command line
> parameter is nice to allow someone to override whatever the person
> building the kernel chose, but IMO it's not a good way to change the
> default built-in to the kernel.
> 
> The current plan (as I understand it) is that we'd like to ship
> Chromebook kernels with this option changed from the default that's
> there now.  In your opinion, is that a sane thing to do?
> 
> 
> >  2) You can add the desired mode to the compiled in (partial) kernel
> >     command line today.
> 
> This might be easier on x86 than it is on ARM.  ARM (and ARM64)
> kernels only have two modes: kernel provides cmdline and bootloader
> provides cmdline.  There are out-of-mainline ANDROID patches to
> address this but nothing in mainline.
> 
> The patch we're discussing now is x86-only so it's not such a huge
> deal, but the fact that combining the kernel and bootloader
> commandline never landed in mainline for arm/arm64 means that this
> isn't a super common/expected thing to do.
> 
> 
> >  3) Boot loaders are well capable of handling large kernel command lines
> >     and the extra time spend for reading the parameter does not matter
> >     at all.
> 
> Long command lines can still be a bit of a chore for humans to deal
> with.  Many times I've needed to look at "/proc/cmdline" and make
> sense of it.  The longer the command line is and the more cruft
> stuffed into it the more of a chore it is.  Yes, this is just one
> thing to put in the command line, but if 10 different drivers all have
> their "one thing" to put there it gets really long.  If 100 different
> drivers all want their one config option there it gets really really
> long.  IMO the command line should be a last resort place to put
> things and should just contain:

This takes me back to my years doing android kernel work for Intel, I'm glad
those are over.  Yes, the android kernel command lines got hideous, I think we
even had patches to make the cmdline buffer bigger than the default was.

From a practical point of view the command line was part of the boot image and
cryptography protected so it was a handy way to securely communicate parameters
from the platform to the kernel, drivers and even just user mode.  It got
pretty ugly but, it worked (mostly).

What I don't get is why pick on l1tf in isolation?  There are a bunch of
command line parameters similar to l1tf.  Would a more general option make
sense?

Anyway, I think there is a higher level issue you are poking at that might be
better addressed by talking about it directly.

--mark

Re: [PATCH v5] x86/speculation/l1tf: Add KConfig for setting the L1D cache flush mode

[Doug Anderson]

Hi,

On Thu, Jul 9, 2020 at 4:05 PM Abhishek Bhardwaj <abhishekbh@google.com> wrote:
>
>
>
> On Thu, Jul 9, 2020 at 3:43 PM mark gross <mgross@linux.intel.com> wrote:
>>
>> On Thu, Jul 09, 2020 at 12:42:57PM -0700, Doug Anderson wrote:
>> > Hi,
>> >
>> > On Thu, Jul 9, 2020 at 3:51 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>> > >
>> > > Abhishek Bhardwaj <abhishekbh@google.com> writes:
>> > > > This change adds a new kernel configuration that sets the l1d cache
>> > > > flush setting at compile time rather than at run time.
>> > > >
>> > > > The reasons for this change are as follows -
>> > > >
>> > > >  - Kernel command line arguments are getting unwieldy. These parameters
>> > > >  are not a scalable way to set the kernel config. They're intended as a
>> > > >  super limited way for the bootloader to pass info to the kernel and
>> > > >  also as a way for end users who are not compiling the kernel themselves
>> > > >  to tweak the kernel behavior.
>> > > >
>> > > >  - Also, if a user wants this setting from the start. It's a definite
>> > > >  smell that it deserves to be a compile time thing rather than adding
>> > > >  extra code plus whatever miniscule time at runtime to pass an
>> > > >  extra argument.
>> > > >
>> > > >  - Finally, it doesn't preclude the runtime / kernel command line way.
>> > > >  Users are free to use those as well.
>> > >
>> > > TBH, I don't see why this is a good idea.
>> > >
>> > >  1) I'm not following your argumentation that the command line option is
>> > >     a poor Kconfig replacement. The L1TF mode is a boot time (module
>> > >     load time) decision and the command line parameter is there to
>> > >     override the carefully chosen and sensible default behaviour.
>> >
>> > When you say that the default behavior is carefully chosen and
>> > sensible, are you saying that (in your opinion) there would never be a
>> > good reason for someone distributing a kernel to others to change the
>> > default?  Certainly I agree that having the kernel command line
>> > parameter is nice to allow someone to override whatever the person
>> > building the kernel chose, but IMO it's not a good way to change the
>> > default built-in to the kernel.
>> >
>> > The current plan (as I understand it) is that we'd like to ship
>> > Chromebook kernels with this option changed from the default that's
>> > there now.  In your opinion, is that a sane thing to do?
>> >
>> >
>> > >  2) You can add the desired mode to the compiled in (partial) kernel
>> > >     command line today.
>> >
>> > This might be easier on x86 than it is on ARM.  ARM (and ARM64)
>> > kernels only have two modes: kernel provides cmdline and bootloader
>> > provides cmdline.  There are out-of-mainline ANDROID patches to
>> > address this but nothing in mainline.
>> >
>> > The patch we're discussing now is x86-only so it's not such a huge
>> > deal, but the fact that combining the kernel and bootloader
>> > commandline never landed in mainline for arm/arm64 means that this
>> > isn't a super common/expected thing to do.
>> >
>> >
>> > >  3) Boot loaders are well capable of handling large kernel command lines
>> > >     and the extra time spend for reading the parameter does not matter
>> > >     at all.
>> >
>> > Long command lines can still be a bit of a chore for humans to deal
>> > with.  Many times I've needed to look at "/proc/cmdline" and make
>> > sense of it.  The longer the command line is and the more cruft
>> > stuffed into it the more of a chore it is.  Yes, this is just one
>> > thing to put in the command line, but if 10 different drivers all have
>> > their "one thing" to put there it gets really long.  If 100 different
>> > drivers all want their one config option there it gets really really
>> > long.  IMO the command line should be a last resort place to put
>> > things and should just contain:
>>
>> This takes me back to my years doing android kernel work for Intel, I'm glad
>> those are over.  Yes, the android kernel command lines got hideous, I think we
>> even had patches to make the cmdline buffer bigger than the default was.
>>
>> From a practical point of view the command line was part of the boot image and
>> cryptography protected so it was a handy way to securely communicate parameters
>> from the platform to the kernel, drivers and even just user mode.  It got
>> pretty ugly but, it worked (mostly).
>>
>> What I don't get is why pick on l1tf in isolation?
>
> Because this is what we needed given the state of our projects.

Right.  Basically the flow is:

1. Someone comes up with a tweak that they think some people might
want to try out but it's not expected to be the default for anyone:
add a module parameter.

2. Someone comes around and says: I think it's sensible to change the
default for a whole class of devices, but another class of devices can
still use the old default: add a config option in addition to the
kernel parameter.


If #2 never happens then it can stay a module parameter forever.  If
#2 happens that's fine--it doesn't really hurt to add a config option
for it.


>> There are a bunch of
>> command line parameters similar to l1tf.  Would a more general option make
>> sense?
>
> Maybe. Given how much resistance this CONFIG has met, I can only imagine the resistance
> upon introducing a more reaching configuration. I also think there's a fine line between YAGNI (You Ain't
> Gonna Need It) vs planning for the future. I don't want to introduce a big CONFIG that won't be used for
> anything else. I'd rather we cross that bridge when someone else feels the same way about other options.

What are you envisioning?  I guess the "generic" solution is to add
cmdline extension (being able to combine kernel and bootloader
cmdline) to all platforms in a somewhat uniform way without breaking
any backward compatibility.  You'd still end up with a really ugly
"/proc/cmdline" if you had too many options there, but I guess it
wouldn't be the end of the world.

Getting this done on all platforms doesn't seem like it'd be an easy
task, though.


>> Anyway, I think there is a higher level issue you are poking at that might be
>> better addressed by talking about it directly.
>
>
> I am new to this process. I don't know who to "convince" or talk about these issues. Who is the final authority on
> this ?. I don't think appeasing every concern is going to be productive for anyone. I've offered to change the implementation,
> if that is what's required. However, if the final authority is against a CONFIG, I don't really know how to proceed.
>
>>
>>
>> --mark
>>
>>
>
>
> --
> Abhishek

Re: [PATCH v5] x86/speculation/l1tf: Add KConfig for setting the L1D cache flush mode

[Thomas Gleixner]

Doug,

Doug Anderson <dianders@chromium.org> writes:
> On Thu, Jul 9, 2020 at 3:51 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>> TBH, I don't see why this is a good idea.
>>
>>  1) I'm not following your argumentation that the command line option is
>>     a poor Kconfig replacement. The L1TF mode is a boot time (module
>>     load time) decision and the command line parameter is there to
>>     override the carefully chosen and sensible default behaviour.
>
> When you say that the default behavior is carefully chosen and
> sensible, are you saying that (in your opinion) there would never be a
> good reason for someone distributing a kernel to others to change the
> default?  Certainly I agree that having the kernel command line
> parameter is nice to allow someone to override whatever the person
> building the kernel chose, but IMO it's not a good way to change the
> default built-in to the kernel.

The problem is that you have to be careful about what you stick into
Kconfig. It's L1TF on x86 today and tomorrow it's MDS and whatever and
then you do the same thing on the other architectures as well. And we
still need the command line options so that generic builds can be
customized at boot time.

> The current plan (as I understand it) is that we'd like to ship
> Chromebook kernels with this option changed from the default that's
> there now.  In your opinion, is that a sane thing to do?

If it's sane for you folks, then feel free to do so. Distros & al patch
the kernel do death anyway, but that does not mean that mainline has to
have everything these people chose to do.

>>  2) You can add the desired mode to the compiled in (partial) kernel
>>     command line today.
>
> This might be easier on x86 than it is on ARM.  ARM (and ARM64)
> kernels only have two modes: kernel provides cmdline and bootloader
> provides cmdline.  There are out-of-mainline ANDROID patches to
> address this but nothing in mainline.
>
> The patch we're discussing now is x86-only so it's not such a huge
> deal, but the fact that combining the kernel and bootloader
> commandline never landed in mainline for arm/arm64 means that this
> isn't a super common/expected thing to do.

Did you try to get that merged for arm/arm64?

>>  3) Boot loaders are well capable of handling large kernel command lines
>>     and the extra time spend for reading the parameter does not matter
>>     at all.
>
> Long command lines can still be a bit of a chore for humans to deal
> with.  Many times I've needed to look at "/proc/cmdline" and make
> sense of it.  The longer the command line is and the more cruft
> stuffed into it the more of a chore it is.  Yes, this is just one
> thing to put in the command line, but if 10 different drivers all have
> their "one thing" to put there it gets really long.  If 100 different
> drivers all want their one config option there it gets really really
> long.

This will not go away when you want to support a gazillion of systems
which need tweaks left and right due to creative hardware/BIOS with a
generic kernel. And come on, parsing a long command line is not rocket
science and it's not something you do every two minutes.

> IMO the command line should be a last resort place to put
> things and should just contain:
>
> 1. Legacy things that _have_ to be in the command line because they've
> always been there.
>
> 2. Things that the bootloader/BIOS needs to communicate to the kernel
> and has no better way to communicate.
>
> 3. Cases where the person running the kernel needs to override a
> default set by the person compiling the kernel.

Which is the case for a lot of things and it's widely used exactly for
that reason.

>>  4) It's just a tiny part of the whole speculation maze. If we go there
>>     for L1TF then we open the flood gates for a gazillion other config
>>     options.
>
> It seems like the only options that we'd need CONFIG option for would
> be the ones where it would be sane to change the default compiled into
> the kernel.  Hopefully that's not too many things?

That's what _you_ need. But accepting this we set a precedence and how
do I argue that L1TF makes sense, but other things not? This stuff is
horrible enough already, no need to add more ifdefs and options and
whatever to it.

> Obviously, like many design choices, the above is all subjective.
> It's really your call and if these arguments don't convince you it
> sounds like the way forward is just to use "CONFIG_CMDLINE" and take
> advantage of the fact that on x86 this will get merged with the
> bootloader's command line.

I rather see the support for command line merging extended to arm/arm64
because that's of general usefulness beyond the problem at hand.

Thanks,

        tglx

Re: [PATCH v5] x86/speculation/l1tf: Add KConfig for setting the L1D cache flush mode

[Doug Anderson]

Hi,

On Fri, Jul 17, 2020 at 9:22 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Doug,
>
> Doug Anderson <dianders@chromium.org> writes:
> > On Thu, Jul 9, 2020 at 3:51 AM Thomas Gleixner <tglx@linutronix.de> wrote:
> >> TBH, I don't see why this is a good idea.
> >>
> >>  1) I'm not following your argumentation that the command line option is
> >>     a poor Kconfig replacement. The L1TF mode is a boot time (module
> >>     load time) decision and the command line parameter is there to
> >>     override the carefully chosen and sensible default behaviour.
> >
> > When you say that the default behavior is carefully chosen and
> > sensible, are you saying that (in your opinion) there would never be a
> > good reason for someone distributing a kernel to others to change the
> > default?  Certainly I agree that having the kernel command line
> > parameter is nice to allow someone to override whatever the person
> > building the kernel chose, but IMO it's not a good way to change the
> > default built-in to the kernel.
>
> The problem is that you have to be careful about what you stick into
> Kconfig. It's L1TF on x86 today and tomorrow it's MDS and whatever and
> then you do the same thing on the other architectures as well. And we
> still need the command line options so that generic builds can be
> customized at boot time.
>
> > The current plan (as I understand it) is that we'd like to ship
> > Chromebook kernels with this option changed from the default that's
> > there now.  In your opinion, is that a sane thing to do?
>
> If it's sane for you folks, then feel free to do so. Distros & al patch
> the kernel do death anyway, but that does not mean that mainline has to
> have everything these people chose to do.

Sure, we could carry a local patch.  ...but Chrome OS tries not to be
like all other distros.  We try very hard (and keep trying harder each
year) to _not_ carry local patches.  Sure, we fail sometimes, but we
will still try.  In general it seems like the upstream Linux community
pushes distros and other Linux users to be upstream first so it's a
little discouraging to hear the advice of "just carry a local patch".

In this case, though, I should close the loop.  Abhishek can correct
me if I'm wrong, but I think the answer here was that it _wasn't_ sane
for us to change this option via KConfig or even via kernel command
line.  I believe that Abhishek found that it was better to (in
userspace) look at the sysfs nodes and configure things how he wanted
it at runtime.


> >>  2) You can add the desired mode to the compiled in (partial) kernel
> >>     command line today.
> >
> > This might be easier on x86 than it is on ARM.  ARM (and ARM64)
> > kernels only have two modes: kernel provides cmdline and bootloader
> > provides cmdline.  There are out-of-mainline ANDROID patches to
> > address this but nothing in mainline.
> >
> > The patch we're discussing now is x86-only so it's not such a huge
> > deal, but the fact that combining the kernel and bootloader
> > commandline never landed in mainline for arm/arm64 means that this
> > isn't a super common/expected thing to do.
>
> Did you try to get that merged for arm/arm64?

Yes, years ago.  References:

https://lore.kernel.org/r/1326492948-26160-1-git-send-email-dianders@chromium.org
https://lore.kernel.org/r/1328223508-1228-1-git-send-email-dianders@chromium.org

I didn't keep poking it because I got advice (not on the mailing list)
that it was better to add a KConfig option rather than extending the
command line.  ;-)  At the time it seemed a reasonable argument to me
so I didn't keep pushing that patch.


> >>  3) Boot loaders are well capable of handling large kernel command lines
> >>     and the extra time spend for reading the parameter does not matter
> >>     at all.
> >
> > Long command lines can still be a bit of a chore for humans to deal
> > with.  Many times I've needed to look at "/proc/cmdline" and make
> > sense of it.  The longer the command line is and the more cruft
> > stuffed into it the more of a chore it is.  Yes, this is just one
> > thing to put in the command line, but if 10 different drivers all have
> > their "one thing" to put there it gets really long.  If 100 different
> > drivers all want their one config option there it gets really really
> > long.
>
> This will not go away when you want to support a gazillion of systems
> which need tweaks left and right due to creative hardware/BIOS with a
> generic kernel. And come on, parsing a long command line is not rocket
> science and it's not something you do every two minutes.

It's funny.  Your argument is the same as mine but the opposite.  I
say: "but so much crap will be in the command line and it will be
crazy" and you say "but so much crap will be in the KConfig and it
will be crazy".  ;-)  I guess it depends on where you want to dump
your trash.


> > IMO the command line should be a last resort place to put
> > things and should just contain:
> >
> > 1. Legacy things that _have_ to be in the command line because they've
> > always been there.
> >
> > 2. Things that the bootloader/BIOS needs to communicate to the kernel
> > and has no better way to communicate.
> >
> > 3. Cases where the person running the kernel needs to override a
> > default set by the person compiling the kernel.
>
> Which is the case for a lot of things and it's widely used exactly for
> that reason.

Right.  To be clear, I definitely wasn't suggesting the option be
_removed_ from the command line.


> >>  4) It's just a tiny part of the whole speculation maze. If we go there
> >>     for L1TF then we open the flood gates for a gazillion other config
> >>     options.
> >
> > It seems like the only options that we'd need CONFIG option for would
> > be the ones where it would be sane to change the default compiled into
> > the kernel.  Hopefully that's not too many things?
>
> That's what _you_ need. But accepting this we set a precedence and how
> do I argue that L1TF makes sense, but other things not? This stuff is
> horrible enough already, no need to add more ifdefs and options and
> whatever to it.

Again, I think it gets into a matter of opinion.  I'm also objecting
to setting a precedence, but in the opposite direction.  Today in
Chrome OS we don't need to use the command line extending for this
type of thing at all and (IMO) it's really nice.  Your config options
are in one place, not mixed halfway between the command line and the
KConfig.  Allowing this one config option to be configured in the
command line opens the flood gates for having two places to look for
the base config.  ;-)

In any case, it doesn't sound like I'm going to convince you.  Also,
since Abhishek has found another way to move forward it sounds like
the right thing is to consider his patch abandoned anyway.


> > Obviously, like many design choices, the above is all subjective.
> > It's really your call and if these arguments don't convince you it
> > sounds like the way forward is just to use "CONFIG_CMDLINE" and take
> > advantage of the fact that on x86 this will get merged with the
> > bootloader's command line.
>
> I rather see the support for command line merging extended to arm/arm64
> because that's of general usefulness beyond the problem at hand.

I have no objections to someone upstreaming those patches, for sure.
Maybe one day the Android team will do it since they seem to have been
carrying it as a local patch forever.  If nothing else, it gives us
more flexibility.


-Doug

Re: [PATCH v5] x86/speculation/l1tf: Add KConfig for setting the L1D cache flush mode

[Borislav Petkov]

+ Masami.

On Thu, Jul 09, 2020 at 12:51:34PM +0200, Thomas Gleixner wrote:
> Abhishek Bhardwaj <abhishekbh@google.com> writes:
> > This change adds a new kernel configuration that sets the l1d cache
> > flush setting at compile time rather than at run time.
> >
> > The reasons for this change are as follows -
> >
> >  - Kernel command line arguments are getting unwieldy. These parameters
> >  are not a scalable way to set the kernel config. They're intended as a
> >  super limited way for the bootloader to pass info to the kernel and
> >  also as a way for end users who are not compiling the kernel themselves
> >  to tweak the kernel behavior.
> >
> >  - Also, if a user wants this setting from the start. It's a definite
> >  smell that it deserves to be a compile time thing rather than adding
> >  extra code plus whatever miniscule time at runtime to pass an
> >  extra argument.
> >
> >  - Finally, it doesn't preclude the runtime / kernel command line way.
> >  Users are free to use those as well.
> 
> TBH, I don't see why this is a good idea.
> 
>  1) I'm not following your argumentation that the command line option is
>     a poor Kconfig replacement. The L1TF mode is a boot time (module
>     load time) decision and the command line parameter is there to
>     override the carefully chosen and sensible default behaviour.
> 
>  2) You can add the desired mode to the compiled in (partial) kernel
>     command line today.
> 
>  3) Boot loaders are well capable of handling large kernel command lines
>     and the extra time spend for reading the parameter does not matter
>     at all.

Also, there's Documentation/admin-guide/bootconfig.rst which extends
cmdline options handling even more and allows for passing options in a
file. Maybe that'll help in this case too.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette