Linux EDAC development
 help / color / mirror / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: "Ghannam, Yazen" <Yazen.Ghannam@amd.com>
Cc: "linux-edac@vger.kernel.org" <linux-edac@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"tony.luck@intel.com" <tony.luck@intel.com>,
	"x86@kernel.org" <x86@kernel.org>
Subject: [v2,3/6] x86/MCE/AMD: Don't cache block addresses on SMCA systems
Date: Tue, 16 Apr 2019 15:44:07 +0200	[thread overview]
Message-ID: <20190416134407.GF31772@zn.tnic> (raw)

On Thu, Apr 11, 2019 at 08:18:02PM +0000, Ghannam, Yazen wrote:
> From: Yazen Ghannam <yazen.ghannam@amd.com>
> 
> On legacy systems, the addresses of the MCA_MISC* registers need to be
> recursively discovered based on a Block Pointer field in the registers.
> 
> On Scalable MCA systems, the register space is fixed, and particular
> addresses can be derived by regular offsets for bank and register type.
> This fixed address space includes the MCA_MISC* registers.
> 
> MCA_MISC0 is always available for each MCA bank. MCA_MISC1 through
> MCA_MISC4 are considered available if MCA_MISC0[BlkPtr]=1.
> 
> Cache the value of MCA_MISC0[BlkPtr] for each bank and per CPU. This
> needs to be done only during init. The values should be saved per CPU
> to accommodate heterogeneous SMCA systems.
> 
> Redo smca_get_block_address() to directly return the block addresses.
> Use the cached Block Pointer value to decide if the MCA_MISC1-4
> addresses should be returned.
> 
> Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
> ---
> Link:
> https://lkml.kernel.org/r/20190408141205.12376-4-Yazen.Ghannam@amd.com
> 
> v1->v2:
> * No change.
> 
>  arch/x86/kernel/cpu/mce/amd.c | 71 +++++++++++++++++------------------
>  1 file changed, 35 insertions(+), 36 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c
> index e64de5149e50..f0644b59848d 100644
> --- a/arch/x86/kernel/cpu/mce/amd.c
> +++ b/arch/x86/kernel/cpu/mce/amd.c
> @@ -101,11 +101,6 @@ static struct smca_bank_name smca_names[] = {
>  	[SMCA_PCIE]	= { "pcie",		"PCI Express Unit" },
>  };
>  
> -static u32 smca_bank_addrs[MAX_NR_BANKS][NR_BLOCKS] __ro_after_init =
> -{
> -	[0 ... MAX_NR_BANKS - 1] = { [0 ... NR_BLOCKS - 1] = -1 }
> -};
> -
>  static const char *smca_get_name(enum smca_bank_types t)
>  {
>  	if (t >= N_SMCA_BANK_TYPES)
> @@ -198,6 +193,7 @@ static char buf_mcatype[MAX_MCATYPE_NAME_LEN];
>  
>  static DEFINE_PER_CPU(struct threshold_bank **, threshold_banks);
>  static DEFINE_PER_CPU(unsigned int, bank_map);	/* see which banks are on */
> +static DEFINE_PER_CPU(u32, smca_blkptr_map); /* see which banks use BlkPtr */

smca_misc_banks_map I guess. BlkPtr means something only to you, PPR
writers and me. And a couple others... :-)

>  static void amd_threshold_interrupt(void);
>  static void amd_deferred_error_interrupt(void);
> @@ -208,6 +204,28 @@ static void default_deferred_error_interrupt(void)
>  }
>  void (*deferred_error_int_vector)(void) = default_deferred_error_interrupt;
>  
> +static void smca_set_blkptr_map(unsigned int bank, unsigned int cpu)

Accordingly...

> +{
> +	u32 low, high;
> +
> +	/*
> +	 * For SMCA enabled processors, BLKPTR field of the first MISC register
> +	 * (MCx_MISC0) indicates presence of additional MISC regs set (MISC1-4).
> +	 */
> +	if (rdmsr_safe(MSR_AMD64_SMCA_MCx_CONFIG(bank), &low, &high))
> +		return;
> +
> +	if (!(low & MCI_CONFIG_MCAX))
> +		return;
> +
> +	if (rdmsr_safe(MSR_AMD64_SMCA_MCx_MISC(bank), &low, &high))
> +		return;
> +
> +	if (low & MASK_BLKPTR_LO)
> +		per_cpu(smca_blkptr_map, cpu) |= 1 << bank;

BIT(bank)

> +
> +}
> +
>  static void smca_configure(unsigned int bank, unsigned int cpu)
>  {
>  	unsigned int i, hwid_mcatype;
> @@ -245,6 +263,8 @@ static void smca_configure(unsigned int bank, unsigned int cpu)
>  		wrmsr(smca_config, low, high);
>  	}
>  
> +	smca_set_blkptr_map(bank, cpu);
> +
>  	/* Return early if this bank was already initialized. */
>  	if (smca_banks[bank].hwid)
>  		return;
> @@ -455,42 +475,21 @@ static void deferred_error_interrupt_enable(struct cpuinfo_x86 *c)
>  	wrmsr(MSR_CU_DEF_ERR, low, high);
>  }
>  
> -static u32 smca_get_block_address(unsigned int bank, unsigned int block)
> +static u32 smca_get_block_address(unsigned int bank, unsigned int block,
> +				  unsigned int cpu)
>  {
> -	u32 low, high;
> -	u32 addr = 0;
> -
> -	if (smca_get_bank_type(bank) == SMCA_RESERVED)
> -		return addr;
> -
>  	if (!block)
>  		return MSR_AMD64_SMCA_MCx_MISC(bank);
>  
> -	/* Check our cache first: */
> -	if (smca_bank_addrs[bank][block] != -1)
> -		return smca_bank_addrs[bank][block];
> -
> -	/*
> -	 * For SMCA enabled processors, BLKPTR field of the first MISC register
> -	 * (MCx_MISC0) indicates presence of additional MISC regs set (MISC1-4).
> -	 */
> -	if (rdmsr_safe(MSR_AMD64_SMCA_MCx_CONFIG(bank), &low, &high))
> -		goto out;
> -
> -	if (!(low & MCI_CONFIG_MCAX))
> -		goto out;
> -
> -	if (!rdmsr_safe(MSR_AMD64_SMCA_MCx_MISC(bank), &low, &high) &&
> -	    (low & MASK_BLKPTR_LO))
> -		addr = MSR_AMD64_SMCA_MCx_MISCy(bank, block - 1);
> +	if (!(per_cpu(smca_blkptr_map, cpu) & (1 << bank)))

BIT(bank)

> +		return 0;
>  
> -out:
> -	smca_bank_addrs[bank][block] = addr;
> -	return addr;
> +	return MSR_AMD64_SMCA_MCx_MISCy(bank, block - 1);
>  }

WARNING: multiple messages have this Message-ID (diff)
From: Borislav Petkov <bp@alien8.de>
To: "Ghannam, Yazen" <Yazen.Ghannam@amd.com>
Cc: "linux-edac@vger.kernel.org" <linux-edac@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"tony.luck@intel.com" <tony.luck@intel.com>,
	"x86@kernel.org" <x86@kernel.org>
Subject: Re: [PATCH v2 3/6] x86/MCE/AMD: Don't cache block addresses on SMCA systems
Date: Tue, 16 Apr 2019 15:44:07 +0200	[thread overview]
Message-ID: <20190416134407.GF31772@zn.tnic> (raw)
Message-ID: <20190416134407.X2eHprTsRAYQ9pvwxcQpqk2Gvn8bQqtIs6rwGUUoMjw@z> (raw)
In-Reply-To: <20190411201743.43195-4-Yazen.Ghannam@amd.com>

On Thu, Apr 11, 2019 at 08:18:02PM +0000, Ghannam, Yazen wrote:
> From: Yazen Ghannam <yazen.ghannam@amd.com>
> 
> On legacy systems, the addresses of the MCA_MISC* registers need to be
> recursively discovered based on a Block Pointer field in the registers.
> 
> On Scalable MCA systems, the register space is fixed, and particular
> addresses can be derived by regular offsets for bank and register type.
> This fixed address space includes the MCA_MISC* registers.
> 
> MCA_MISC0 is always available for each MCA bank. MCA_MISC1 through
> MCA_MISC4 are considered available if MCA_MISC0[BlkPtr]=1.
> 
> Cache the value of MCA_MISC0[BlkPtr] for each bank and per CPU. This
> needs to be done only during init. The values should be saved per CPU
> to accommodate heterogeneous SMCA systems.
> 
> Redo smca_get_block_address() to directly return the block addresses.
> Use the cached Block Pointer value to decide if the MCA_MISC1-4
> addresses should be returned.
> 
> Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
> ---
> Link:
> https://lkml.kernel.org/r/20190408141205.12376-4-Yazen.Ghannam@amd.com
> 
> v1->v2:
> * No change.
> 
>  arch/x86/kernel/cpu/mce/amd.c | 71 +++++++++++++++++------------------
>  1 file changed, 35 insertions(+), 36 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c
> index e64de5149e50..f0644b59848d 100644
> --- a/arch/x86/kernel/cpu/mce/amd.c
> +++ b/arch/x86/kernel/cpu/mce/amd.c
> @@ -101,11 +101,6 @@ static struct smca_bank_name smca_names[] = {
>  	[SMCA_PCIE]	= { "pcie",		"PCI Express Unit" },
>  };
>  
> -static u32 smca_bank_addrs[MAX_NR_BANKS][NR_BLOCKS] __ro_after_init =
> -{
> -	[0 ... MAX_NR_BANKS - 1] = { [0 ... NR_BLOCKS - 1] = -1 }
> -};
> -
>  static const char *smca_get_name(enum smca_bank_types t)
>  {
>  	if (t >= N_SMCA_BANK_TYPES)
> @@ -198,6 +193,7 @@ static char buf_mcatype[MAX_MCATYPE_NAME_LEN];
>  
>  static DEFINE_PER_CPU(struct threshold_bank **, threshold_banks);
>  static DEFINE_PER_CPU(unsigned int, bank_map);	/* see which banks are on */
> +static DEFINE_PER_CPU(u32, smca_blkptr_map); /* see which banks use BlkPtr */

smca_misc_banks_map I guess. BlkPtr means something only to you, PPR
writers and me. And a couple others... :-)

>  static void amd_threshold_interrupt(void);
>  static void amd_deferred_error_interrupt(void);
> @@ -208,6 +204,28 @@ static void default_deferred_error_interrupt(void)
>  }
>  void (*deferred_error_int_vector)(void) = default_deferred_error_interrupt;
>  
> +static void smca_set_blkptr_map(unsigned int bank, unsigned int cpu)

Accordingly...

> +{
> +	u32 low, high;
> +
> +	/*
> +	 * For SMCA enabled processors, BLKPTR field of the first MISC register
> +	 * (MCx_MISC0) indicates presence of additional MISC regs set (MISC1-4).
> +	 */
> +	if (rdmsr_safe(MSR_AMD64_SMCA_MCx_CONFIG(bank), &low, &high))
> +		return;
> +
> +	if (!(low & MCI_CONFIG_MCAX))
> +		return;
> +
> +	if (rdmsr_safe(MSR_AMD64_SMCA_MCx_MISC(bank), &low, &high))
> +		return;
> +
> +	if (low & MASK_BLKPTR_LO)
> +		per_cpu(smca_blkptr_map, cpu) |= 1 << bank;

BIT(bank)

> +
> +}
> +
>  static void smca_configure(unsigned int bank, unsigned int cpu)
>  {
>  	unsigned int i, hwid_mcatype;
> @@ -245,6 +263,8 @@ static void smca_configure(unsigned int bank, unsigned int cpu)
>  		wrmsr(smca_config, low, high);
>  	}
>  
> +	smca_set_blkptr_map(bank, cpu);
> +
>  	/* Return early if this bank was already initialized. */
>  	if (smca_banks[bank].hwid)
>  		return;
> @@ -455,42 +475,21 @@ static void deferred_error_interrupt_enable(struct cpuinfo_x86 *c)
>  	wrmsr(MSR_CU_DEF_ERR, low, high);
>  }
>  
> -static u32 smca_get_block_address(unsigned int bank, unsigned int block)
> +static u32 smca_get_block_address(unsigned int bank, unsigned int block,
> +				  unsigned int cpu)
>  {
> -	u32 low, high;
> -	u32 addr = 0;
> -
> -	if (smca_get_bank_type(bank) == SMCA_RESERVED)
> -		return addr;
> -
>  	if (!block)
>  		return MSR_AMD64_SMCA_MCx_MISC(bank);
>  
> -	/* Check our cache first: */
> -	if (smca_bank_addrs[bank][block] != -1)
> -		return smca_bank_addrs[bank][block];
> -
> -	/*
> -	 * For SMCA enabled processors, BLKPTR field of the first MISC register
> -	 * (MCx_MISC0) indicates presence of additional MISC regs set (MISC1-4).
> -	 */
> -	if (rdmsr_safe(MSR_AMD64_SMCA_MCx_CONFIG(bank), &low, &high))
> -		goto out;
> -
> -	if (!(low & MCI_CONFIG_MCAX))
> -		goto out;
> -
> -	if (!rdmsr_safe(MSR_AMD64_SMCA_MCx_MISC(bank), &low, &high) &&
> -	    (low & MASK_BLKPTR_LO))
> -		addr = MSR_AMD64_SMCA_MCx_MISCy(bank, block - 1);
> +	if (!(per_cpu(smca_blkptr_map, cpu) & (1 << bank)))

BIT(bank)

> +		return 0;
>  
> -out:
> -	smca_bank_addrs[bank][block] = addr;
> -	return addr;
> +	return MSR_AMD64_SMCA_MCx_MISCy(bank, block - 1);
>  }

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

         reply	other threads:[~2019-04-16 13:44 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-11 20:18 [PATCH v2 0/6] Handle MCA banks in a per_cpu way Ghannam, Yazen
2019-04-11 20:18 ` [v2,1/6] x86/MCE: Make struct mce_banks[] static Yazen Ghannam
2019-04-11 20:18   ` [PATCH v2 1/6] " Ghannam, Yazen
2019-04-11 20:18 ` [v2,2/6] x86/MCE: Handle MCA controls in a per_cpu way Yazen Ghannam
2019-04-11 20:18   ` [PATCH v2 2/6] " Ghannam, Yazen
2019-04-16 10:21   ` [v2,2/6] " Borislav Petkov
2019-04-16 10:21     ` [PATCH v2 2/6] " Borislav Petkov
2019-04-11 20:18 ` [v2,3/6] x86/MCE/AMD: Don't cache block addresses on SMCA systems Yazen Ghannam
2019-04-11 20:18   ` [PATCH v2 3/6] " Ghannam, Yazen
2019-04-16 13:44   ` Borislav Petkov [this message]
2019-04-16 13:44     ` Borislav Petkov
2019-04-11 20:18 ` [v2,4/6] x86/MCE: Make number of MCA banks per_cpu Yazen Ghannam
2019-04-11 20:18   ` [PATCH v2 4/6] " Ghannam, Yazen
2019-04-16 13:55   ` [v2,4/6] " Borislav Petkov
2019-04-16 13:55     ` [PATCH v2 4/6] " Borislav Petkov
2019-04-16 17:36   ` [v2,4/6] " Borislav Petkov
2019-04-16 17:36     ` [PATCH v2 4/6] " Borislav Petkov
2019-04-11 20:18 ` [v2,6/6] x86/MCE: Treat MCE bank as initialized if control bits set in hardware Yazen Ghannam
2019-04-11 20:18   ` [PATCH v2 6/6] " Ghannam, Yazen
2019-04-11 20:18 ` [v2,5/6] x86/MCE: Save MCA control bits that get " Yazen Ghannam
2019-04-11 20:18   ` [PATCH v2 5/6] " Ghannam, Yazen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190416134407.GF31772@zn.tnic \
    --to=bp@alien8.de \
    --cc=Yazen.Ghannam@amd.com \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox