diff mbox series

[SRU,Artful] x86/mce/AMD: Allow any CPU to initialize the smca_banks array

Message ID 20171117114016.26805-1-tjaalton@ubuntu.com
State New
Headers show
Series [SRU,Artful] x86/mce/AMD: Allow any CPU to initialize the smca_banks array | expand

Commit Message

Timo Aaltonen Nov. 17, 2017, 11:40 a.m. UTC 
From: Yazen Ghannam <yazen.ghannam@amd.com>

BugLink: http://bugs.launchpad.net/bugs/1732894

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.

CPU 0 seemed like a good candidate to do the populating. However, it's
possible that CPU 0 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 CPU 0 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.

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Acked-by: Jack Miller <jack@codezen.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tony Luck <tony.luck@intel.com>
Cc: linux-edac <linux-edac@vger.kernel.org>
Link: http://lkml.kernel.org/r/20170724101228.17326-4-bp@alien8.de
Signed-off-by: Ingo Molnar <mingo@kernel.org>
(cherry picked from commit 9662d43f523dfc0dc242ec29c2921c43898d6ae5)
Signed-off-by: Timo Aaltonen <tjaalton@debian.org>
---
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

Comments

Stefan Bader Nov. 17, 2017, 2:17 p.m. UTC | #1 
On 17.11.2017 12:40, Timo Aaltonen wrote:
> From: Yazen Ghannam <yazen.ghannam@amd.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1732894
> 
> 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.
> 
> CPU 0 seemed like a good candidate to do the populating. However, it's
> possible that CPU 0 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 CPU 0 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.
> 
> Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Acked-by: Jack Miller <jack@codezen.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: linux-edac <linux-edac@vger.kernel.org>
> Link: http://lkml.kernel.org/r/20170724101228.17326-4-bp@alien8.de
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> (cherry picked from commit 9662d43f523dfc0dc242ec29c2921c43898d6ae5)
> Signed-off-by: Timo Aaltonen <tjaalton@debian.org>
Acked-by: Stefan Bader <stefan.bader@canonical.com>

> ---
>  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 9e314bcf67cc..5ce1a5689162 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
> @@ -201,8 +201,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), &low, &high)) {
> @@ -216,11 +216,6 @@ static void smca_configure(unsigned int bank, unsigned int cpu)
>  	for (i = 0; i < ARRAY_SIZE(smca_hwid_mcatypes); i++) {
>  		s_hwid = &smca_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++;
>
Timo Aaltonen Nov. 17, 2017, 9:11 p.m. UTC | #2 
On 17.11.2017 13:40, Timo Aaltonen wrote:
> From: Yazen Ghannam <yazen.ghannam@amd.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1732894
> 
> 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.
> 
> CPU 0 seemed like a good candidate to do the populating. However, it's
> possible that CPU 0 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 CPU 0 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.
> 
> Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Acked-by: Jack Miller <jack@codezen.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: linux-edac <linux-edac@vger.kernel.org>
> Link: http://lkml.kernel.org/r/20170724101228.17326-4-bp@alien8.de
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> (cherry picked from commit 9662d43f523dfc0dc242ec29c2921c43898d6ae5)
> Signed-off-by: Timo Aaltonen <tjaalton@debian.org>

Oops, don't normally git-send from my laptop, so fix that to:

Signed-off-by: Timo Aaltonen <timo.aaltonen@canonical.com>

> ---
>  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 9e314bcf67cc..5ce1a5689162 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
> @@ -201,8 +201,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), &low, &high)) {
> @@ -216,11 +216,6 @@ static void smca_configure(unsigned int bank, unsigned int cpu)
>  	for (i = 0; i < ARRAY_SIZE(smca_hwid_mcatypes); i++) {
>  		s_hwid = &smca_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++;
>
Colin Ian King Nov. 20, 2017, 11:30 a.m. UTC | #3 
On 17/11/17 11:40, Timo Aaltonen wrote:
> From: Yazen Ghannam <yazen.ghannam@amd.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1732894
> 
> 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.
> 
> CPU 0 seemed like a good candidate to do the populating. However, it's
> possible that CPU 0 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 CPU 0 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.
> 
> Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Acked-by: Jack Miller <jack@codezen.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: linux-edac <linux-edac@vger.kernel.org>
> Link: http://lkml.kernel.org/r/20170724101228.17326-4-bp@alien8.de
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> (cherry picked from commit 9662d43f523dfc0dc242ec29c2921c43898d6ae5)
> Signed-off-by: Timo Aaltonen <tjaalton@debian.org>
> ---
>  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 9e314bcf67cc..5ce1a5689162 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
> @@ -201,8 +201,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), &low, &high)) {
> @@ -216,11 +216,6 @@ static void smca_configure(unsigned int bank, unsigned int cpu)
>  	for (i = 0; i < ARRAY_SIZE(smca_hwid_mcatypes); i++) {
>  		s_hwid = &smca_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++;
> 

Clean upstream cherry pick.

Acked-by: Colin Ian King <colin.king@canonical.com>
Stefan Bader Nov. 20, 2017, 2:03 p.m. UTC | #4 
On 17.11.2017 12:40, Timo Aaltonen wrote:
> From: Yazen Ghannam <yazen.ghannam@amd.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1732894
> 
> 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.
> 
> CPU 0 seemed like a good candidate to do the populating. However, it's
> possible that CPU 0 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 CPU 0 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.
> 
> Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Acked-by: Jack Miller <jack@codezen.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: linux-edac <linux-edac@vger.kernel.org>
> Link: http://lkml.kernel.org/r/20170724101228.17326-4-bp@alien8.de
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> (cherry picked from commit 9662d43f523dfc0dc242ec29c2921c43898d6ae5)
> Signed-off-by: Timo Aaltonen <tjaalton@debian.org>
> ---
>  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 9e314bcf67cc..5ce1a5689162 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
> @@ -201,8 +201,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), &low, &high)) {
> @@ -216,11 +216,6 @@ static void smca_configure(unsigned int bank, unsigned int cpu)
>  	for (i = 0; i < ARRAY_SIZE(smca_hwid_mcatypes); i++) {
>  		s_hwid = &smca_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++;
> 
Applied to Artful master-next. Thanks (sob updated)
diff mbox series

Patch

diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index 9e314bcf67cc..5ce1a5689162 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -201,8 +201,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), &low, &high)) {
@@ -216,11 +216,6 @@  static void smca_configure(unsigned int bank, unsigned int cpu)
 	for (i = 0; i < ARRAY_SIZE(smca_hwid_mcatypes); i++) {
 		s_hwid = &smca_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++;