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

[Abhishek Bhardwaj]

On Sun, Jul 5, 2020 at 8:57 AM Waiman Long <longman@xxxxxxxxxx> wrote:
>
> On 7/5/20 11:23 AM, Mike Rapoport wrote:
> >> 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?
> > Well, nothing prevents you from continuing to use the command line as
> > well;-)
> >
> > I can see why whould you want an ability to select compile time default
> > for an option, but I'm really not thrilled by the added ifdefery.
> >
>
> It turns out that CONFIG_KVM_VMENTRY_L1D_FLUSH values match the enum
> vmx_l1d_flush_state values. So one way to reduce the ifdefery is to do,
> for example,
>
> +#ifdef CONFIG_KVM_VMENTRY_L1D_FLUSH
> +#define VMENTER_L1D_FLUSH_DEFAULT CONFIG_KVM_VMENTRY_L1D_FLUSH
> +#else
> +#define VMENTER_L1D_FLUSH_DEFAULT      VMENTER_L1D_FLUSH_AUTO
> #endif
> -enum vmx_l1d_flush_state l1tf_vmx_mitigation = VMENTER_L1D_FLUSH_AUTO;
> +enum vmx_l1d_flush_state l1tf_vmx_mitigation = VMENTER_L1D_FLUSH_DEFAULT;
>
> Of course, we may need to add a comment on enum vmx_l1d_flush_state
> definition to highlight the dependency of CONFIG_KVM_VMENTRY_L1D_FLUSH
> on it to avoid future mismatch.

I explicitly wanted to avoid doing that for this very reason. In my
opinion this is brittle and bound to be missed
sooner or later.

I'd rather have the value assignment be explicit rather than some
clever hack. Notice, this wouldn't work if the enum
values were not contiguous. We just got lucky.

Do people feel strongly against this ifdef ladder ?

>
> Cheers,
> Longman
>



--
Abhishek