Re: [PATCH 15/14] x86/gds: Lock GDS mitigation when keylocker feature is present

Chang S. Bae
2024-04-22T07:35:50Z

On 4/19/2024 10:47 AM, Pawan Gupta wrote:
>
> /*
> @@ -840,6 +843,11 @@ static void __init gds_select_mitigation(void)
> gds_mitigation = GDS_MITIGATION_FULL_LOCKED;
> }
>
> + /* Keylocker can only be enabled when GDS mitigation is locked */
> + if (boot_cpu_has(X86_FEATURE_KEYLOCKER) &&
> + gds_mitigation == GDS_MITIGATION_FULL)
> + gds_mitigation = GDS_MITIGATION_FULL_LOCKED;
> +

I'm having trouble understanding this change:

gds_select_mitigation()
{
...
if (gds_mitigation == GDS_MITIGATION_FORCE)
gds_mitigation = GDS_MITIGATION_FULL;

rdmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl);
if (mcu_ctrl & GDS_MITG_LOCKED) {
...
gds_mitigation = GDS_MITIGATION_FULL_LOCKED;
}

if (boot_cpu_has(X86_FEATURE_KEYLOCKER) &&
gds_mitigation == GDS_MITIGATION_FULL)
gds_mitigation = GDS_MITIGATION_FULL_LOCKED;

As I understand it, gds_mitigation is set to GDS_MITIGATION_FULL only if
the gds force option is enabled but IA32_MCU_OPT_CTRL[GDS_MITG_LOCK] is
not set.

Then, if the CPU has Key Locker, this code sets gds_mitigation to
GDS_MITIGATION_FULL_LOCKED, which seems contradictory. I'm not sure why
this change is necessary.

I'm also not convinced that the Key Locker series needs to modify this
function. The Key Locker setup code should simply check the current
mitigation status and enable the feature only if proper mitigation is in
place. Am I missing something here?

Thanks,
Chang