Re: [PATCH] x86/mce/AMD: Allow any CPU to initialize smca_banks array

[Borislav Petkov]

On Thu, Jun 29, 2017 at 01:08:28PM -0500, Yazen Ghannam wrote:
> From: Yazen Ghannam 
> 
> Current SMCA implementations have the same banks on each CPU with the
> non-core banks only visible to a "master thread" on each Die. Practically,
> this means the smca_banks array, which describes the banks, only needs to
> be populated once by a single master thread.

...

> If the first CPU up is not a master thread, then it will populate the array
> with all core banks. The first CPU afterwards that is a master thread will
> skip populating the core banks and continue populating the non-core banks.
> Every CPU afterwards will then return early.
> 
> Signed-off-by: Yazen Ghannam 
> ---
>  arch/x86/kernel/cpu/mcheck/mce_amd.c | 9 ++---
>  1 file changed, 2 insertions(+), 7 deletions(-)

Applied, thanks.

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 

Re: [PATCH] x86/mce/AMD: Allow any CPU to initialize smca_banks array

[Borislav Petkov]

On Thu, Jun 29, 2017 at 01:08:28PM -0500, Yazen Ghannam wrote:
> From: Yazen Ghannam 
> 
> Current SMCA implementations have the same banks on each CPU with the
> non-core banks only visible to a "master thread" on each Die. Practically,
> this means the smca_banks array, which describes the banks, only needs to
> be populated once by a single master thread.

...

> If the first CPU up is not a master thread, then it will populate the array
> with all core banks. The first CPU afterwards that is a master thread will
> skip populating the core banks and continue populating the non-core banks.
> Every CPU afterwards will then return early.
> 
> Signed-off-by: Yazen Ghannam 
> ---
>  arch/x86/kernel/cpu/mcheck/mce_amd.c | 9 ++---
>  1 file changed, 2 insertions(+), 7 deletions(-)

Applied, thanks.

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 

Re: [PATCH] x86/mce/AMD: Allow any CPU to initialize smca_banks array

[Jack Miller]

This patch works for me, thanks!

Jack

On Thu, Jun 29, 2017 at 1:08 PM, Yazen Ghannam  wrote:
> From: Yazen Ghannam 
>
> Current SMCA implementations have the same banks on each CPU with the
> non-core banks only visible to a "master thread" on each Die. Practically,
> this means the smca_banks array, which describes the banks, only needs to
> be populated once by a single master thread.
>
> CPU0 seemed like a good candidate to do the populating. However, it's
> possible that CPU0 is not enabled in which case the smca_banks array won't
> be populated.
>
> Rather than try to figure out another master thread to do the populating,
> we should just allow any CPU to populate the array.
>
> Drop the CPU0 check and return early if the bank was already initialized.
> Also, drop the WARNing about an already initialized bank, since this will
> be a common, expected occurrence.
>
> The smca_banks array is only populated at boot time and CPUs are brought
> online sequentially. So there's no need for locking around the array.
>
> If the first CPU up is a master thread, then it will populate the array
> with all banks, core and non-core. Every CPU afterwards will return early.
>
> If the first CPU up is not a master thread, then it will populate the array
> with all core banks. The first CPU afterwards that is a master thread will
> skip populating the core banks and continue populating the non-core banks.
> Every CPU afterwards will then return early.
>
> Signed-off-by: Yazen Ghannam 
> ---
>  arch/x86/kernel/cpu/mcheck/mce_amd.c | 9 ++---
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c 
> b/arch/x86/kernel/cpu/mcheck/mce_amd.c
> index b41466723f71..37dc43f820b4 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
> @@ -214,8 +214,8 @@ static void smca_configure(unsigned int bank, unsigned 
> int cpu)
> wrmsr(smca_config, low, high);
> }
>
> -   /* Collect bank_info using CPU 0 for now. */
> -   if (cpu)
> +   /* Return early if this bank was already initialized. */
> +   if (smca_banks[bank].hwid)
> return;
>
> if (rdmsr_safe_on_cpu(cpu, MSR_AMD64_SMCA_MCx_IPID(bank), , 
> )) {
> @@ -229,11 +229,6 @@ static void smca_configure(unsigned int bank, unsigned 
> int cpu)
> for (i = 0; i < ARRAY_SIZE(smca_hwid_mcatypes); i++) {
> s_hwid = _hwid_mcatypes[i];
> if (hwid_mcatype == s_hwid->hwid_mcatype) {
> -
> -   WARN(smca_banks[bank].hwid,
> -"Bank %s already initialized!\n",
> -smca_get_name(s_hwid->bank_type));
> -
> smca_banks[bank].hwid = s_hwid;
> smca_banks[bank].id = low;
> smca_banks[bank].sysfs_id = s_hwid->count++;
> --
> 2.7.4
>

Re: [PATCH] x86/mce/AMD: Allow any CPU to initialize smca_banks array

[Jack Miller]

This patch works for me, thanks!

Jack

On Thu, Jun 29, 2017 at 1:08 PM, Yazen Ghannam  wrote:
> From: Yazen Ghannam 
>
> Current SMCA implementations have the same banks on each CPU with the
> non-core banks only visible to a "master thread" on each Die. Practically,
> this means the smca_banks array, which describes the banks, only needs to
> be populated once by a single master thread.
>
> CPU0 seemed like a good candidate to do the populating. However, it's
> possible that CPU0 is not enabled in which case the smca_banks array won't
> be populated.
>
> Rather than try to figure out another master thread to do the populating,
> we should just allow any CPU to populate the array.
>
> Drop the CPU0 check and return early if the bank was already initialized.
> Also, drop the WARNing about an already initialized bank, since this will
> be a common, expected occurrence.
>
> The smca_banks array is only populated at boot time and CPUs are brought
> online sequentially. So there's no need for locking around the array.
>
> If the first CPU up is a master thread, then it will populate the array
> with all banks, core and non-core. Every CPU afterwards will return early.
>
> If the first CPU up is not a master thread, then it will populate the array
> with all core banks. The first CPU afterwards that is a master thread will
> skip populating the core banks and continue populating the non-core banks.
> Every CPU afterwards will then return early.
>
> Signed-off-by: Yazen Ghannam 
> ---
>  arch/x86/kernel/cpu/mcheck/mce_amd.c | 9 ++---
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c 
> b/arch/x86/kernel/cpu/mcheck/mce_amd.c
> index b41466723f71..37dc43f820b4 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
> @@ -214,8 +214,8 @@ static void smca_configure(unsigned int bank, unsigned 
> int cpu)
> wrmsr(smca_config, low, high);
> }
>
> -   /* Collect bank_info using CPU 0 for now. */
> -   if (cpu)
> +   /* Return early if this bank was already initialized. */
> +   if (smca_banks[bank].hwid)
> return;
>
> if (rdmsr_safe_on_cpu(cpu, MSR_AMD64_SMCA_MCx_IPID(bank), , 
> )) {
> @@ -229,11 +229,6 @@ static void smca_configure(unsigned int bank, unsigned 
> int cpu)
> for (i = 0; i < ARRAY_SIZE(smca_hwid_mcatypes); i++) {
> s_hwid = _hwid_mcatypes[i];
> if (hwid_mcatype == s_hwid->hwid_mcatype) {
> -
> -   WARN(smca_banks[bank].hwid,
> -"Bank %s already initialized!\n",
> -smca_get_name(s_hwid->bank_type));
> -
> smca_banks[bank].hwid = s_hwid;
> smca_banks[bank].id = low;
> smca_banks[bank].sysfs_id = s_hwid->count++;
> --
> 2.7.4
>

[PATCH] x86/mce/AMD: Allow any CPU to initialize smca_banks array

[Yazen Ghannam]

From: Yazen Ghannam 

Current SMCA implementations have the same banks on each CPU with the
non-core banks only visible to a "master thread" on each Die. Practically,
this means the smca_banks array, which describes the banks, only needs to
be populated once by a single master thread.

CPU0 seemed like a good candidate to do the populating. However, it's
possible that CPU0 is not enabled in which case the smca_banks array won't
be populated.

Rather than try to figure out another master thread to do the populating,
we should just allow any CPU to populate the array.

Drop the CPU0 check and return early if the bank was already initialized.
Also, drop the WARNing about an already initialized bank, since this will
be a common, expected occurrence.

The smca_banks array is only populated at boot time and CPUs are brought
online sequentially. So there's no need for locking around the array.

If the first CPU up is a master thread, then it will populate the array
with all banks, core and non-core. Every CPU afterwards will return early.

If the first CPU up is not a master thread, then it will populate the array
with all core banks. The first CPU afterwards that is a master thread will
skip populating the core banks and continue populating the non-core banks.
Every CPU afterwards will then return early.

Signed-off-by: Yazen Ghannam 
---
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c 
b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index b41466723f71..37dc43f820b4 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -214,8 +214,8 @@ static void smca_configure(unsigned int bank, unsigned int 
cpu)
wrmsr(smca_config, low, high);
}
 
-   /* Collect bank_info using CPU 0 for now. */
-   if (cpu)
+   /* Return early if this bank was already initialized. */
+   if (smca_banks[bank].hwid)
return;
 
if (rdmsr_safe_on_cpu(cpu, MSR_AMD64_SMCA_MCx_IPID(bank), , )) 
{
@@ -229,11 +229,6 @@ static void smca_configure(unsigned int bank, unsigned int 
cpu)
for (i = 0; i < ARRAY_SIZE(smca_hwid_mcatypes); i++) {
s_hwid = _hwid_mcatypes[i];
if (hwid_mcatype == s_hwid->hwid_mcatype) {
-
-   WARN(smca_banks[bank].hwid,
-"Bank %s already initialized!\n",
-smca_get_name(s_hwid->bank_type));
-
smca_banks[bank].hwid = s_hwid;
smca_banks[bank].id = low;
smca_banks[bank].sysfs_id = s_hwid->count++;
-- 
2.7.4

[PATCH] x86/mce/AMD: Allow any CPU to initialize smca_banks array

[Yazen Ghannam]

From: Yazen Ghannam 

Current SMCA implementations have the same banks on each CPU with the
non-core banks only visible to a "master thread" on each Die. Practically,
this means the smca_banks array, which describes the banks, only needs to
be populated once by a single master thread.

CPU0 seemed like a good candidate to do the populating. However, it's
possible that CPU0 is not enabled in which case the smca_banks array won't
be populated.

Rather than try to figure out another master thread to do the populating,
we should just allow any CPU to populate the array.

Drop the CPU0 check and return early if the bank was already initialized.
Also, drop the WARNing about an already initialized bank, since this will
be a common, expected occurrence.

The smca_banks array is only populated at boot time and CPUs are brought
online sequentially. So there's no need for locking around the array.

If the first CPU up is a master thread, then it will populate the array
with all banks, core and non-core. Every CPU afterwards will return early.

If the first CPU up is not a master thread, then it will populate the array
with all core banks. The first CPU afterwards that is a master thread will
skip populating the core banks and continue populating the non-core banks.
Every CPU afterwards will then return early.

Signed-off-by: Yazen Ghannam 
---
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c 
b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index b41466723f71..37dc43f820b4 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -214,8 +214,8 @@ static void smca_configure(unsigned int bank, unsigned int 
cpu)
wrmsr(smca_config, low, high);
}
 
-   /* Collect bank_info using CPU 0 for now. */
-   if (cpu)
+   /* Return early if this bank was already initialized. */
+   if (smca_banks[bank].hwid)
return;
 
if (rdmsr_safe_on_cpu(cpu, MSR_AMD64_SMCA_MCx_IPID(bank), , )) 
{
@@ -229,11 +229,6 @@ static void smca_configure(unsigned int bank, unsigned int 
cpu)
for (i = 0; i < ARRAY_SIZE(smca_hwid_mcatypes); i++) {
s_hwid = _hwid_mcatypes[i];
if (hwid_mcatype == s_hwid->hwid_mcatype) {
-
-   WARN(smca_banks[bank].hwid,
-"Bank %s already initialized!\n",
-smca_get_name(s_hwid->bank_type));
-
smca_banks[bank].hwid = s_hwid;
smca_banks[bank].id = low;
smca_banks[bank].sysfs_id = s_hwid->count++;
-- 
2.7.4