| Message ID | 20211207193028.9389-1-yazen.ghannam@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/MCE/AMD: Allow thresholding interface updates after init | expand |
On Tue, Dec 07, 2021 at 07:30:28PM +0000, Yazen Ghannam wrote: > Changes to the AMD Thresholding sysfs code prevents sysfs writes from > updating the underlying registers once CPU init is completed, i.e. > "threshold_banks" is set. > > Allow the registers to be updated if the thresholding interface is > already initialized or if in the init path. Use the "set_lvt_off" value > to indicate if running in the init path, since this value is only set > during init. > > Fixes: a037f3ca0ea0 ("x86/mce/amd: Make threshold bank setting hotplug robust") > Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com> We want that in stable, right? > --- > arch/x86/kernel/cpu/mce/amd.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c > index 2eadc7c4c902..408c9546ea0b 100644 > --- a/arch/x86/kernel/cpu/mce/amd.c > +++ b/arch/x86/kernel/cpu/mce/amd.c > @@ -400,8 +400,12 @@ static void threshold_restart_bank(void *_tr) > struct thresh_restart *tr = _tr; > u32 hi, lo; > > - /* sysfs write might race against an offline operation */ > - if (this_cpu_read(threshold_banks)) > + /* > + * sysfs write might race against an offline operation. > + * Prevent register writes if threshold_banks are not set and this is > + * not called from the init path as indicated by "set_lvt_off". > + */ So if you convert this text into code, it would read like: if (!this_cpu_read(threshold_banks) && !tr->set_lvt_off) which is equivalent to: > + if (!(this_cpu_read(threshold_banks) || tr->set_lvt_off)) but easier to follow, methinks.
On Wed, Dec 29, 2021 at 06:42:18PM +0100, Borislav Petkov wrote: > On Tue, Dec 07, 2021 at 07:30:28PM +0000, Yazen Ghannam wrote: > > Changes to the AMD Thresholding sysfs code prevents sysfs writes from > > updating the underlying registers once CPU init is completed, i.e. > > "threshold_banks" is set. > > > > Allow the registers to be updated if the thresholding interface is > > already initialized or if in the init path. Use the "set_lvt_off" value > > to indicate if running in the init path, since this value is only set > > during init. > > > > Fixes: a037f3ca0ea0 ("x86/mce/amd: Make threshold bank setting hotplug robust") > > Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com> > > We want that in stable, right? > Yes, thanks for catching that. > > --- > > arch/x86/kernel/cpu/mce/amd.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c > > index 2eadc7c4c902..408c9546ea0b 100644 > > --- a/arch/x86/kernel/cpu/mce/amd.c > > +++ b/arch/x86/kernel/cpu/mce/amd.c > > @@ -400,8 +400,12 @@ static void threshold_restart_bank(void *_tr) > > struct thresh_restart *tr = _tr; > > u32 hi, lo; > > > > - /* sysfs write might race against an offline operation */ > > - if (this_cpu_read(threshold_banks)) > > + /* > > + * sysfs write might race against an offline operation. > > + * Prevent register writes if threshold_banks are not set and this is > > + * not called from the init path as indicated by "set_lvt_off". > > + */ > > So if you convert this text into code, it would read like: > > if (!this_cpu_read(threshold_banks) && !tr->set_lvt_off) > > which is equivalent to: > > > + if (!(this_cpu_read(threshold_banks) || tr->set_lvt_off)) > > but easier to follow, methinks. > Sure, I'll make the change and send another version. Thanks, Yazen
diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c index 2eadc7c4c902..408c9546ea0b 100644 --- a/arch/x86/kernel/cpu/mce/amd.c +++ b/arch/x86/kernel/cpu/mce/amd.c @@ -400,8 +400,12 @@ static void threshold_restart_bank(void *_tr) struct thresh_restart *tr = _tr; u32 hi, lo; - /* sysfs write might race against an offline operation */ - if (this_cpu_read(threshold_banks)) + /* + * sysfs write might race against an offline operation. + * Prevent register writes if threshold_banks are not set and this is + * not called from the init path as indicated by "set_lvt_off". + */ + if (!(this_cpu_read(threshold_banks) || tr->set_lvt_off)) return; rdmsr(tr->b->address, lo, hi);
Changes to the AMD Thresholding sysfs code prevents sysfs writes from updating the underlying registers once CPU init is completed, i.e. "threshold_banks" is set. Allow the registers to be updated if the thresholding interface is already initialized or if in the init path. Use the "set_lvt_off" value to indicate if running in the init path, since this value is only set during init. Fixes: a037f3ca0ea0 ("x86/mce/amd: Make threshold bank setting hotplug robust") Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com> --- arch/x86/kernel/cpu/mce/amd.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)