Re: [????] Re: [PATCH] x86/bugs: Fix GDS mitigation check for CPUs without ARCH_CAP_GDS_CTRL

[Pawan Gupta <pawan.kumar.gupta@linux.intel.com>]

On Fri, Aug 15, 2025 at 05:28:18AM +0000, Li,Rongqing wrote:
> 
> 
> > -----Original Message-----
> > From: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> > Sent: 2025年8月15日 13:09
> > To: Li,Rongqing <lirongqing@baidu.com>
> > Cc: tglx@linutronix.de; bp@alien8.de; peterz@infradead.org;
> > jpoimboe@kernel.org; mingo@redhat.com; dave.hansen@linux.intel.com;
> > x86@kernel.org; hpa@zytor.com; david.kaplan@amd.com;
> > linux-kernel@vger.kernel.org
> > Subject: [????] Re: [PATCH] x86/bugs: Fix GDS mitigation check for CPUs without
> > ARCH_CAP_GDS_CTRL
> > 
> > On Fri, Aug 15, 2025 at 11:53:34AM +0800, lirongqing wrote:
> > > From: Li RongQing <lirongqing@baidu.com>
> > >
> > > The commit 8c7261abcb7ad("x86/bugs: Add attack vector controls for
> > > GDS") caused call traces during secondary CPU initialization because
> > > it didn't properly handle CPUs that lack the ARCH_CAP_GDS_CTRL capability.
> > >
> > > For CPUs without ARCH_CAP_GDS_CTRL support, we should set the
> > > mitigation to GDS_MITIGATION_UCODE_NEEDED rather than
> > > GDS_MITIGATION_OFF, as these CPUs may still be vulnerable but cannot
> > disable mitigation.
> > >
> > > Add the missing check for ARCH_CAP_GDS_CTRL to properly determine the
> > > mitigation state for affected CPUs.
> > >
> > > [    2.809147] unchecked MSR access error: RDMSR from 0x123 at rIP:
> > 0xffffffffb3452807 (update_gds_msr+0x87/0xe0)
> > > (update_gds_msr+0x87/0xe0)
> > > [    2.809147] Call Trace:
> > > [    2.809147]  <TASK>
> > > [    2.809147]  identify_secondary_cpu+0x72/0x90
> > > [    2.809147]  start_secondary+0x7a/0x140
> > > [    2.809147]  common_startup_64+0x13e/0x141
> > > [    2.809147]  </TASK>
> > > [    2.809147] unchecked MSR access error: WRMSR to 0x123 (tried to write
> > 0x0000000000000010) at rIP: 0xffffffffb34527b8
> > > (update_gds_msr+0x38/0xe0)
> > > [    2.809147] Call Trace:
> > > [    2.809147]  <TASK>
> > > [    2.809147]  identify_secondary_cpu+0x72/0x90
> > > [    2.809147]  start_secondary+0x7a/0x140
> > > [    2.809147]  common_startup_64+0x13e/0x141
> > > [    2.809147]  </TASK>
> > > [    2.809147] ------------[ cut here ]------------
> > > [    2.809147] WARNING: CPU: 1 PID: 0 at arch/x86/kernel/cpu/bugs.c:1053
> > update_gds_msr+0x9b/0xe0
> > >
> > > Fixes: 8c7261abcb7ad ("x86/bugs: Add attack vector controls for GDS")
> > > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > > ---
> > >  arch/x86/kernel/cpu/bugs.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> > > index b74bf93..3af911c 100644
> > > --- a/arch/x86/kernel/cpu/bugs.c
> > > +++ b/arch/x86/kernel/cpu/bugs.c
> > > @@ -1071,6 +1071,8 @@ static void __init gds_select_mitigation(void)
> > >  			gds_mitigation = GDS_MITIGATION_FULL;
> > >  		else {
> > >  			gds_mitigation = GDS_MITIGATION_OFF;
> > > +			if (!(x86_arch_cap_msr & ARCH_CAP_GDS_CTRL))
> > 
> > This check is already present few lines below.
> > 
> > > +				gds_mitigation = GDS_MITIGATION_UCODE_NEEDED;
> > >  			return;
> > 
> > To avoid duplicating, a better fix could be to not return here, and let the next
> > block DTRT:
> 
> But if cpu has ARCH_CAP_GDS_CTRL, the next block will be skipped, and the
> codes after checking ARCH_CAP_GDS_CTRL block will be run, this is not
> expected

How is that a problem? That is how it was originally implemented.

Infact, the following checks are required for the correct behavior:

         if (mcu_ctrl & GDS_MITG_LOCKED) {
                 if (gds_mitigation == GDS_MITIGATION_OFF)
                         pr_warn("Mitigation locked. Disable failed.\n");
	...
                 gds_mitigation = GDS_MITIGATION_FULL_LOCKED;
         }

If the GDS microcode mitigation is locked before the kernel boot, MSR write
for OFF will not take effect anyways. And you report OFF when the
mitigation is locked to ON. While also triggering below WARN_ON_ONCE():

update_gds_msr()
{
...
         /*
          * Check to make sure that the WRMSR value was not ignored. Writes to
          * GDS_MITG_DIS will be ignored if this processor is locked but the boot
          * processor was not.
          */
         rdmsrq(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl_after);
         WARN_ON_ONCE(mcu_ctrl != mcu_ctrl_after);