Date:   Thu, 9 Jul 2020 15:43:19 -0700
From:   mark gross <mgross@...ux.intel.com>
To:     Doug Anderson <dianders@...omium.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Abhishek Bhardwaj <abhishekbh@...gle.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Anthony Steinhauser <asteinhauser@...gle.com>,
        Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Sean Christopherson <sean.j.christopherson@...el.com>,
        Tony Luck <tony.luck@...el.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Waiman Long <longman@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>, kvm@...r.kernel.org,
        x86 <x86@...nel.org>
Subject: Re: [PATCH v5] x86/speculation/l1tf: Add KConfig for setting the L1D
 cache flush mode

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@...utronix.de> wrote:
> >
> > Abhishek Bhardwaj <abhishekbh@...gle.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

Powered by blists - more mailing lists