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

From: Doug Anderson <dianders@google.com>
To: Mike Rapoport <rppt@linux.ibm.com>
Cc: Abhishek Bhardwaj <abhishekbh@google.com>,
	Anthony Steinhauser <asteinhauser@google.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Waiman Long <longman@redhat.com>, Borislav Petkov <bp@alien8.de>,
	"H. Peter Anvin" <hpa@zytor.com>, Ingo Molnar <mingo@redhat.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Mark Gross <mgross@linux.intel.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Pawan Gupta <pawan.kumar.gupta@linux.intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Sean Christopherson <sean.j.christopherson@intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Tony Luck <tony.luck@intel.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	kvm@vger.kernel.org, x86 <x86@kernel.org>
Subject: Re: [PATCH v3] x86/speculation/l1tf: Add KConfig for setting the L1D cache flush mode
Date: Fri, 3 Jul 2020 07:00:11 -0700	[thread overview]
Message-ID: <CAD=FV=XRbrFqSbR619h+9HXNyrYNbqfBF2e-+iUZco9qQ8Wokg@mail.gmail.com> (raw)
In-Reply-To: <20200703114037.GD2999146@linux.ibm.com>

Hi,

On Fri, Jul 3, 2020 at 4:40 AM Mike Rapoport <rppt@linux.ibm.com> wrote:
>
> On Thu, Jul 02, 2020 at 11:43:47PM -0700, Abhishek Bhardwaj wrote:
> > We have tried to steer away from kernel command line args for a few reasons.
> >
> > I am paraphrasing my colleague Doug's argument here (CC'ed him as well) -
> >
> > - The command line args are getting unwieldy. Kernel command line
> > parameters are not a scalable way to set kernel config. It's 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 kernel behavior.
>
> Why cannot you simply add this option to CONFIG_CMDLINE at your kernel build
> scripts?

At least in the past I've seen that 'CONFIG_CMDLINE' interacts badly
with the bootloader provided command line in some architectures.  In
days of yore I tried to post a patch to fix this, at least on ARM
targets, but it never seemed to go anywhere upstream.  I'm going to
assume this is still a problem because I still see an ANDROID tagged
patch in the Chrome OS 5.4 tree:

In any case, as per my previous arguments, stuffing lots of config
into the cmdline is a bit clunky and doesn't scale well.  You end up
with a really long run on command line and it's hard to tell where one
config option ends and the next one starts and if the same concept is
there more than one time it's hard to tell and something might cancel
out a previous config option or maybe it won't and by the time you end
up finishing this it's hard to tell where you started.  :-)

> > - Also, we know we want this setting from the start. This is a
> > definite smell that it deserves to be a compile time thing rather than
> > adding extra code + whatever miniscule time at runtime to pass an
> > extra arg.
>
> This might be a compile time thing in your environment, but not
> necessarily it must be the same in others. For instance, what option
> should distro kernels select?

Nothing prevents people from continuing to use the command line
options if they want, right?  This just allows a different default.
So if a distro is security focused and decided that it wanted a slower
/ more secure default then it could ship that way but individual users
could still override, right?

> > I think this was what CONFIGS were intended for. I'm happy to add all
> > this to the commit message once it's approved in spirit by the
> > maintainers.
> >
> > On Thu, Jul 2, 2020 at 8:18 PM Anthony Steinhauser
> > <asteinhauser@google.com> wrote:
> > >
> > > Yes, this probably requires an explanation why the change is necessary
> > > or useful. Without that it is difficult to give some meaningful
> > > feedback.
> >
> >
> >
> > --
> > Abhishek
>
> --
> Sincerely yours,
> Mike.