| Message ID | 20241111060428.44258-9-qiuxu.zhuo@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/mce: Clean up some x86/mce code | expand |
On 11/10/2024 10:04 PM, Qiuxu Zhuo wrote: > Fix typos in comments. > > Reviewed-by: Tony Luck <tony.luck@intel.com> > Reviewed-by: Nikolay Borisov <nik.borisov@suse.com> > Reviewed-by: Sohil Mehta <sohil.mehta@intel.com> > Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com> > --- It might have been fine to merge this with patch 6/8. They both touch the same file and the changes are mostly superficial. Probably consider it if you do another revision but not needed otherwise. > arch/x86/kernel/cpu/mce/core.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c > index 0f0c6e9d9183..6e194ccffc7c 100644 > --- a/arch/x86/kernel/cpu/mce/core.c > +++ b/arch/x86/kernel/cpu/mce/core.c > @@ -1144,7 +1144,7 @@ static noinstr int mce_start(int *no_way_out) > } else { > /* > * Subject: Now start the scanning loop one by one in > - * the original callin order. > + * the original calling order. > * This way when there are any shared banks it will be > * only seen by one CPU before cleared, avoiding duplicates. > */ > @@ -1917,7 +1917,7 @@ static void apply_quirks_amd(struct cpuinfo_x86 *c) > /* This should be disabled by the BIOS, but isn't always */ > if (c->x86 == 15 && this_cpu_read(mce_num_banks) > 4) { > /* > - * disable GART TBL walk error reporting, which > + * disable GART TLB walk error reporting, which > * trips off incorrectly with the IOMMU & 3ware > * & Cerberus: > */
On Mon, Nov 11, 2024 at 02:04:28PM +0800, Qiuxu Zhuo wrote: > Fix typos in comments. > > Reviewed-by: Tony Luck <tony.luck@intel.com> > Reviewed-by: Nikolay Borisov <nik.borisov@suse.com> > Reviewed-by: Sohil Mehta <sohil.mehta@intel.com> > Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com> > --- > Changes in v4: > - No changes. > > Changes in v3: > - Collect "Reviewed-by:" from Nikolay & Sohil. > - Remove the detail typos from the commit message (Sohil). > > Changes in v2: > - Collect "Reviewed-by:" from Tony. > > arch/x86/kernel/cpu/mce/core.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c > index 0f0c6e9d9183..6e194ccffc7c 100644 > --- a/arch/x86/kernel/cpu/mce/core.c > +++ b/arch/x86/kernel/cpu/mce/core.c > @@ -1144,7 +1144,7 @@ static noinstr int mce_start(int *no_way_out) > } else { > /* > * Subject: Now start the scanning loop one by one in > - * the original callin order. > + * the original calling order. I don't think this is a typo. It seems to refer to the mce_callin variable/idea. For example, each CPU "calls in" when ready. This is independent of when each CPU is "called" to do something. CPUs are called in this order 0, 1, 2. CPUs "call in" in this order 1, 0, 2. When a CPU is called can be different from when it responds. Maybe I'm reading too much into this. :/ > * This way when there are any shared banks it will be > * only seen by one CPU before cleared, avoiding duplicates. > */ > @@ -1917,7 +1917,7 @@ static void apply_quirks_amd(struct cpuinfo_x86 *c) > /* This should be disabled by the BIOS, but isn't always */ > if (c->x86 == 15 && this_cpu_read(mce_num_banks) > 4) { > /* > - * disable GART TBL walk error reporting, which > + * disable GART TLB walk error reporting, which This also is not a typo. TBL -> table From old AMD K8 BKDG document: 10 GartTblWkEn GART Table Walk Error Reporting Enable R/W 0 Thanks, Yazen
On 11/12/2024 7:38 AM, Yazen Ghannam wrote: > >> * This way when there are any shared banks it will be >> * only seen by one CPU before cleared, avoiding duplicates. >> */ >> @@ -1917,7 +1917,7 @@ static void apply_quirks_amd(struct cpuinfo_x86 *c) >> /* This should be disabled by the BIOS, but isn't always */ >> if (c->x86 == 15 && this_cpu_read(mce_num_banks) > 4) { >> /* >> - * disable GART TBL walk error reporting, which >> + * disable GART TLB walk error reporting, which > > This also is not a typo. TBL -> table > > From old AMD K8 BKDG document: > 10 GartTblWkEn GART Table Walk Error Reporting Enable R/W 0 > There is another comment in init_amd_gh() that seems to be related to the same thing and similarly worded. That seems to refer to TLB instead of TBL(table). /* * Disable GART TLB Walk Errors on Fam10h. We do this here because this * is always needed when GART is enabled, even in a kernel which has no * MCE support built in. BIOS should disable GartTlbWlk Errors already. * If it doesn't, we do it here as suggested by the BKDG. * * Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=33012 */ msr_set_bit(MSR_AMD64_MCx_MASK(4), 10);
On Tue, Nov 12, 2024 at 02:35:32PM -0800, Sohil Mehta wrote: > On 11/12/2024 7:38 AM, Yazen Ghannam wrote: > > > >> * This way when there are any shared banks it will be > >> * only seen by one CPU before cleared, avoiding duplicates. > >> */ > >> @@ -1917,7 +1917,7 @@ static void apply_quirks_amd(struct cpuinfo_x86 *c) > >> /* This should be disabled by the BIOS, but isn't always */ > >> if (c->x86 == 15 && this_cpu_read(mce_num_banks) > 4) { > >> /* > >> - * disable GART TBL walk error reporting, which > >> + * disable GART TLB walk error reporting, which > > > > This also is not a typo. TBL -> table > > > > From old AMD K8 BKDG document: > > 10 GartTblWkEn GART Table Walk Error Reporting Enable R/W 0 > > > > There is another comment in init_amd_gh() that seems to be related to > the same thing and similarly worded. That seems to refer to TLB instead > of TBL(table). > > /* > * Disable GART TLB Walk Errors on Fam10h. We do this here because this > * is always needed when GART is enabled, even in a kernel which has no > * MCE support built in. BIOS should disable GartTlbWlk Errors already. > * If it doesn't, we do it here as suggested by the BKDG. > * > * Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=33012 > */ > msr_set_bit(MSR_AMD64_MCx_MASK(4), 10); > > Now I think *that* is a typo, since it doesn't match the documentation. :) Thanks, Yazen
Hi Yazen, > From: Yazen Ghannam <yazen.ghannam@amd.com> > [...] > > @@ -1144,7 +1144,7 @@ static noinstr int mce_start(int *no_way_out) > > } else { > > /* > > * Subject: Now start the scanning loop one by one in > > - * the original callin order. > > + * the original calling order. > > I don't think this is a typo. It seems to refer to the mce_callin variable/idea. > > For example, each CPU "calls in" when ready. This is independent of when > each CPU is "called" to do something. > > CPUs are called in this order 0, 1, 2. > CPUs "call in" in this order 1, 0, 2. > > When a CPU is called can be different from when it responds. > > Maybe I'm reading too much into this. :/ Too finicky to me :( But I appreciate you sharing your reading and thoughts.
> From: Yazen Ghannam <yazen.ghannam@amd.com> > [...] > > >> - * disable GART TBL walk error reporting, which > > >> + * disable GART TLB walk error reporting, which > > > > > > This also is not a typo. TBL -> table > > > > > > From old AMD K8 BKDG document: > > > 10 GartTblWkEn GART Table Walk Error Reporting Enable R/W 0 > > > > > > > There is another comment in init_amd_gh() that seems to be related to > > the same thing and similarly worded. That seems to refer to TLB > > instead of TBL(table). > > > > /* > > * Disable GART TLB Walk Errors on Fam10h. We do this here because > > this > > * is always needed when GART is enabled, even in a kernel which has > > no > > * MCE support built in. BIOS should disable GartTlbWlk Errors already. > > * If it doesn't, we do it here as suggested by the BKDG. > > * > > * Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=33012 > > */ Thanks Sohil for this reference. > > msr_set_bit(MSR_AMD64_MCx_MASK(4), 10); > > > > > > Now I think *that* is a typo, since it doesn't match the documentation. > :) As Yazen has the documentation that says TBL (table) is the intended meaning. I'll drop this patch in next version. -Qiuxu
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c index 0f0c6e9d9183..6e194ccffc7c 100644 --- a/arch/x86/kernel/cpu/mce/core.c +++ b/arch/x86/kernel/cpu/mce/core.c @@ -1144,7 +1144,7 @@ static noinstr int mce_start(int *no_way_out) } else { /* * Subject: Now start the scanning loop one by one in - * the original callin order. + * the original calling order. * This way when there are any shared banks it will be * only seen by one CPU before cleared, avoiding duplicates. */ @@ -1917,7 +1917,7 @@ static void apply_quirks_amd(struct cpuinfo_x86 *c) /* This should be disabled by the BIOS, but isn't always */ if (c->x86 == 15 && this_cpu_read(mce_num_banks) > 4) { /* - * disable GART TBL walk error reporting, which + * disable GART TLB walk error reporting, which * trips off incorrectly with the IOMMU & 3ware * & Cerberus: */