Re: [PATCH v5 01/16] x86/bugs: Restructure MDS mitigation

Borislav Petkov
2025-04-18T20:43:06Z

On Fri, Apr 18, 2025 at 11:17:06AM -0500, David Kaplan wrote:
> @@ -284,6 +314,9 @@ enum rfds_mitigations {
> static enum rfds_mitigations rfds_mitigation __ro_after_init =
> IS_ENABLED(CONFIG_MITIGATION_RFDS) ? RFDS_MITIGATION_AUTO : RFDS_MITIGATION_OFF;
>
> +/* Set if any of MDS/TAA/MMIO/RFDS are going to enable VERW. */
> +static bool verw_mitigation_selected __ro_after_init;
> +

Yeah, pls pull that one up - see diff at the end.

> static void __init mds_select_mitigation(void)
> {
> if (!boot_cpu_has_bug(X86_BUG_MDS) || cpu_mitigations_off()) {
> @@ -294,12 +327,34 @@ static void __init mds_select_mitigation(void)
> if (mds_mitigation == MDS_MITIGATION_AUTO)
> mds_mitigation = MDS_MITIGATION_FULL;
>
> + if (mds_mitigation == MDS_MITIGATION_OFF)
> + return;
> +
> + verw_mitigation_selected = true;
> +}
> +
> +static void __init mds_update_mitigation(void)
> +{
> + if (!boot_cpu_has_bug(X86_BUG_MDS) || cpu_mitigations_off())
> + return;

Can we simply do

if (mds_mitigation == MDS_MITIGATION_OFF)
return;

here?

We already checked the X86_BUG and cpu_mitigations_off() in the select
function.

> +
> + /* If TAA, MMIO, or RFDS are being mitigated, MDS gets mitigated too. */

A version of that comment is already over verw_mitigation_selected's
definition.

> + if (verw_mitigation_selected)
> + mds_mitigation = MDS_MITIGATION_FULL;

So we have this here now:

if (mds_mitigation == MDS_MITIGATION_OFF)
return;

if (verw_mitigation_selected)
mds_mitigation = MDS_MITIGATION_FULL;

or what you have:

if (!boot_cpu_has_bug(X86_BUG_MDS) || cpu_mitigations_off())
return;

/* If TAA, MMIO, or RFDS are being mitigated, MDS gets mitigated too. */
if (verw_mitigation_selected)
mds_mitigation = MDS_MITIGATION_FULL;

Now, if the CPU is not affected by MDS, this second branch won't ever get set
because we will return earlier.

Which then means that "If TAA, MMIO, or RFDS are being mitigated, MDS gets
mitigated too" is not really true.

IOW, I'm wondering if this would be the more fitting order:

static void __init mds_update_mitigation(void)
{
if (verw_mitigation_selected)
mds_mitigation = MDS_MITIGATION_FULL;

if (mds_mitigation == MDS_MITIGATION_OFF)
return;

I.e., if *any* mitigation did set verw_mitigation_selected, even if the CPU is
not affected by MDS, it must set mds_mitigation to FULL too.

Hmmm?

---

All of the changes ontop:

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 4295502ea082..61b9aaea8d09 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -87,6 +87,9 @@ static DEFINE_MUTEX(spec_ctrl_mutex);

void (*x86_return_thunk)(void) __ro_after_init = __x86_return_thunk;

+/* Set if any of MDS/TAA/MMIO/RFDS are going to enable VERW. */
+static bool verw_mitigation_selected __ro_after_init;
+
/* Update SPEC_CTRL MSR and its cached copy unconditionally */
static void update_spec_ctrl(u64 val)
{
@@ -314,9 +317,6 @@ enum rfds_mitigations {
static enum rfds_mitigations rfds_mitigation __ro_after_init =
IS_ENABLED(CONFIG_MITIGATION_RFDS) ? RFDS_MITIGATION_AUTO : RFDS_MITIGATION_OFF;

-/* Set if any of MDS/TAA/MMIO/RFDS are going to enable VERW. */
-static bool verw_mitigation_selected __ro_after_init;
-
static void __init mds_select_mitigation(void)
{
if (!boot_cpu_has_bug(X86_BUG_MDS) || cpu_mitigations_off()) {
@@ -324,24 +324,23 @@ static void __init mds_select_mitigation(void)
return;
}

- if (mds_mitigation == MDS_MITIGATION_AUTO)
- mds_mitigation = MDS_MITIGATION_FULL;
-
if (mds_mitigation == MDS_MITIGATION_OFF)
return;

+ if (mds_mitigation == MDS_MITIGATION_AUTO)
+ mds_mitigation = MDS_MITIGATION_FULL;
+
verw_mitigation_selected = true;
}

static void __init mds_update_mitigation(void)
{
- if (!boot_cpu_has_bug(X86_BUG_MDS) || cpu_mitigations_off())
- return;
-
- /* If TAA, MMIO, or RFDS are being mitigated, MDS gets mitigated too. */
if (verw_mitigation_selected)
mds_mitigation = MDS_MITIGATION_FULL;

+ if (mds_mitigation == MDS_MITIGATION_OFF)
+ return;
+
if (mds_mitigation == MDS_MITIGATION_FULL) {
if (!boot_cpu_has(X86_FEATURE_MD_CLEAR))
mds_mitigation = MDS_MITIGATION_VMWERV;

--
Regards/Gruss,
Boris.