Re: [PATCH 1/6] x86/bugs: Create single parameter for VERW based mitigations

[Daniel Sneddon]

On 10/8/24 12:24, Kaplan, David wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
>> -----Original Message-----
>> From: Daniel Sneddon <daniel.sneddon@xxxxxxxxxxxxxxx>
>> Sent: Tuesday, September 24, 2024 5:32 PM
>> To: Jonathan Corbet <corbet@xxxxxxx>; Thomas Gleixner <tglx@xxxxxxxxxxxxx>;
>> Borislav Petkov <bp@xxxxxxxxx>; Peter Zijlstra <peterz@xxxxxxxxxxxxx>; Josh
>> Poimboeuf <jpoimboe@xxxxxxxxxx>; Ingo Molnar <mingo@xxxxxxxxxx>; Dave
>> Hansen <dave.hansen@xxxxxxxxxxxxxxx>; x86@xxxxxxxxxx
>> Cc: hpa@xxxxxxxxx; linux-doc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
>> pawan.kumar.gupta@xxxxxxxxxxxxxxx
>> Subject: [PATCH 1/6] x86/bugs: Create single parameter for VERW based
>> mitigations
>>
>> There are currently 4 mitigations that use VERW to flush different cpu buffers. This
>> can cause confusion when trying to disable all the different VERW mitigations.
>> Simplify enabling/disabling these mitigations by creating a single parameter for
>> controlling them.
> 
> Just curious, what is the use case for disabling the different VERW mitigations (but not other mitigations)?  Is that a testing/debugging use case or a production use case?
> 
>>
>> Future work will focus on combining similar code used in selecting these mitigations
>> to further simplify.
>>
>> Signed-off-by: Daniel Sneddon <daniel.sneddon@xxxxxxxxxxxxxxx>
>> ---
>>  .../admin-guide/kernel-parameters.txt         | 16 +++++++++
>>  arch/x86/kernel/cpu/bugs.c                    | 34 +++++++++++++++++++
>>  2 files changed, 50 insertions(+)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt
>> b/Documentation/admin-guide/kernel-parameters.txt
>> index 09126bb8cc9f..66b567c4dce5 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -628,6 +628,21 @@
>>       cio_ignore=     [S390]
>>                       See Documentation/arch/s390/common_io.rst for details.
>>
>> +     clear_cpu_buffers=
>> +                     [X86]
>> +                     Controls the mitigations that use
>> +                     X86_FEATURE_CLEAR_CPU_BUF, namely
>> +                     Micro-architectrual Data Sampling (MDS)
>> +                     MMIO Stale Data
>> +                     TSX Async Abort (TAA)
>> +                     Register File Data Sampling (RFDS)
>> +
>> +                     The options are:
>> +                     on              - Enable cpu buffer clearing
>> +                     on,nosmt        - Enable cpu buffer clearing and disable
>> +                                       SMT
>> +                     off             - Disables cpu buffer clearing
>> +
> 
> At the x86 uconf at LPC, someone asked me about if we should have command line options that were mitigation-focused rather than bug-focused (like to enable STIBP, IBRS, etc.).  The feedback I had there applies to this series too, which is that I'm concerned this makes things more difficult for users because users are reacting to bugs, they're not experts in mitigations.  A user wants to know how to mitigate CVE XYZ, and the bug-specific command line options support that.  It's an extra step to say that to mitigate MDS, you have to figure out that MDS requires clearing cpu buffers, and therefore you should set this mitigation-specific option.
> 
> My general concern with this series is it seems to tie X86_FEATURE_CLEAR_CPU_BUF directly to these 4 bugs.  What would happen if hypothetically there was a new bug that required X86_FEATURE_CLEAR_CPU_BUF and some other mitigation?  With the existing bug-specific options this is easy enough, as the new bug could force this feature and do whatever else it needed.  But with a mitigation-specific option like this one, it would seem to be harder as it might require multiple options to mitigate one bug.  And could create conflicts if you enable that new mitigation but disable clear_cpu_buffers.
> 

Any new bug that is using X86_FEATURE_CLEAR_CPU_BUF will be related to these
existing bugs regardless. We may need to add another option to this parameter,
similar to what we do with ",nosmt", but I would hope that would be sufficient.
With the existing bug-focused parameters we have the same potential for
conflicts. What if I say "reg_file_data_sampling=off mds=full"? Since mds is on
rfds will be on even though I requested it off. The intent of this parameter is
to remove such conflicts.

> I do get the point that these specific 4 bugs are all closely related.  Another idea to consider could be a single command line option for these 4 bugs, but is tied to the bugs themselves, not the mitigation.  That might be more future-proof as the scope remains only about these 4 bugs, not the mitigation itself.
> 

Are you suggesting a name change away from "clear_cpu_buffers" since it is
clearly about the mitigation rather than the bug? I'm not sure there is a good
common name for those 4 bugs that isn't about the mitigation, but I'm open to
any suggestions.

> --David Kaplan