Messages in this thread | | | Subject | Re: [v2 1/1] x86/cpufeatures: Implement Predictive Store Forwarding control. | From | "Saripalli, RK" <> | Date | Mon, 26 Apr 2021 11:40:40 -0500 |
| |
On 4/26/2021 11:28 AM, Reiji Watanabe wrote: >> --- a/arch/x86/kernel/cpu/amd.c >> +++ b/arch/x86/kernel/cpu/amd.c >> @@ -1170,3 +1170,22 @@ void set_dr_addr_mask(unsigned long mask, int dr) >> break; >> } >> } >> + >> +static int __init psf_cmdline(char *str) >> +{ >> + if (!boot_cpu_has(X86_FEATURE_PSFD)) >> + return 0; >> + >> + if (!str) >> + return -EINVAL; >> + >> + if (!strcmp(str, "off")) { >> + x86_spec_ctrl_base |= SPEC_CTRL_PSFD; >> + wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base); >> + setup_clear_cpu_cap(X86_FEATURE_PSFD); > > Shouldn't X86_FEATURE_MSR_SPEC_CTRL be set if the CPU supports PSF ? > x86_spec_ctrl_setup_ap(), which is called on non-boot CPUs, doesn't > update MSR_IA32_SPEC_CTRL with x86_spec_ctrl_base not having > X86_FEATURE_MSR_SPEC_CTRL (i.e. if a CPU supports PSF but no other > existing feature that makes the kernel set X86_FEATURE_MSR_SPEC_CTRL).
Reiji, that is a good catch. This patch unconditionally sets to bit 7 even on machines where bit 7 is undefined thereby risking a kernel oops. I will fix this.
I did test this code on a Milan machine and verified that with the setting to off, the SPEC_CTRL MSR was showing bit 7 set to 1 and with on setting SPEC_CTRL MSR bit 7 was cleared. I tested this with both spec_ctrl_bypass_disable kernel parameter and without. I verified with {sudo modprobe msr; rdmsr -a 72 which dumps on all CPUs. But I may not have tested this patch on a non-Milan machine with both settings. I will make sure to test it on non-Milan machines before sending the next version out.
> > Also, since check_bugs() reads the SPEC_CTRL MSR to account for reserved > bits which may have unknown bits to set x86_spec_ctrl_base > (if X86_FEATURE_MSR_SPEC_CTRL is set), > I'm wondering if psf_cmdline(), which is called earlier > than check_bugs(), should do the same instead of overwriting > it with x86_spec_ctrl_base | SPEC_CTRL_PSFD. > Ok. I did not think of that path. I will go through that code and fix up the patch. > > Thanks, > Reiji >
Thanks, RK
| |