diff mbox series

[14/14] EDAC/amd64: Add get_umc_error_info() into pvt->ops

Message ID 20220228161354.54923-15-nchatrad@amd.com (mailing list archive)
State New, archived
Headers show
Series EDAC/amd64: move platform specific routines to pvt->ops | expand

Commit Message

Naveen Krishna Chatradhi Feb. 28, 2022, 4:13 p.m. UTC
From: Muralidhara M K <muralimk@amd.com>

Add function pointer for get_umc_error_info() in pvt->ops and assign
family specific get_umc_error_info() definitions appropriately.

Signed-off-by: Muralidhara M K <muralimk@amd.com>
Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
---
This patch is created by splitting the 5/12th patch in series
[v7 5/12] https://patchwork.kernel.org/project/linux-edac/patch/20220203174942.31630-6-nchatrad@amd.com/

 drivers/edac/amd64_edac.c | 19 ++++++++++++++-----
 drivers/edac/amd64_edac.h |  1 +
 2 files changed, 15 insertions(+), 5 deletions(-)

Comments

Yazen Ghannam March 28, 2022, 5:13 p.m. UTC | #1
On Mon, Feb 28, 2022 at 09:43:54PM +0530, Naveen Krishna Chatradhi wrote:
> From: Muralidhara M K <muralimk@amd.com>
> 
> Add function pointer for get_umc_error_info() in pvt->ops and assign
> family specific get_umc_error_info() definitions appropriately.
>

Please include the "why".
 
> Signed-off-by: Muralidhara M K <muralimk@amd.com>
> Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
> ---
> This patch is created by splitting the 5/12th patch in series
> [v7 5/12] https://patchwork.kernel.org/project/linux-edac/patch/20220203174942.31630-6-nchatrad@amd.com/
> 
>  drivers/edac/amd64_edac.c | 19 ++++++++++++++-----
>  drivers/edac/amd64_edac.h |  1 +
>  2 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index 7a20f8a696de..ab4e16070a02 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -3056,10 +3056,13 @@ static inline void decode_bus_error(int node_id, struct mce *m)
>   * Currently, we can derive the channel number by looking at the 6th nibble in
>   * the instance_id. For example, instance_id=0xYXXXXX where Y is the channel
>   * number.
> + *
> + * csrow can be derived from the lower 3 bits of MCA_SYND value.

I think this comment can be expanded.

For DRAM ECC errors, the Chip Select number is given in bits [2:0] of
the MCA_SYND[ErrorInformation] field.

>   */
> -static int find_umc_channel(struct mce *m)
> +static void f17_umc_err_info(struct mce *m, struct err_info *err)
>  {
> -	return (m->ipid & GENMASK(31, 0)) >> 20;
> +	err->channel = (m->ipid & GENMASK(31, 0)) >> 20;
> +	err->csrow = m->synd & 0x7;
>  }
>  
>  static void decode_umc_error(int node_id, struct mce *m)
> @@ -3081,8 +3084,6 @@ static void decode_umc_error(int node_id, struct mce *m)
>  	if (m->status & MCI_STATUS_DEFERRED)
>  		ecc_type = 3;
>  
> -	err.channel = find_umc_channel(m);
> -
>  	if (!(m->status & MCI_STATUS_SYNDV)) {
>  		err.err_code = ERR_SYND;
>  		goto log_error;
> @@ -3097,7 +3098,7 @@ static void decode_umc_error(int node_id, struct mce *m)
>  			err.err_code = ERR_CHANNEL;
>  	}
>  
> -	err.csrow = m->synd & 0x7;
> +	pvt->ops->get_umc_error_info(m, &err);
>  
>  	if (umc_normaddr_to_sysaddr(m->addr, pvt->mc_node_id, err.channel, &sys_addr)) {
>  		err.err_code = ERR_NORM_ADDR;
> @@ -3927,6 +3928,7 @@ static int per_family_init(struct amd64_pvt *pvt)
>  		pvt->ops->populate_csrows		= init_csrows_df;
>  		pvt->ops->dump_misc_regs		= __dump_misc_regs_df;
>  		pvt->ops->get_cs_mode			= f17_get_cs_mode;
> +		pvt->ops->get_umc_error_info		= f17_umc_err_info;
>  
>  		if (pvt->fam == 0x18) {
>  			pvt->ctl_name			= "F18h";
> @@ -3974,6 +3976,7 @@ static int per_family_init(struct amd64_pvt *pvt)
>  		pvt->ops->populate_csrows		= init_csrows_df;
>  		pvt->ops->dump_misc_regs		= __dump_misc_regs_df;
>  		pvt->ops->get_cs_mode			= f17_get_cs_mode;
> +		pvt->ops->get_umc_error_info		= f17_umc_err_info;
>  		break;
>  
>  	default:
> @@ -3993,6 +3996,12 @@ static int per_family_init(struct amd64_pvt *pvt)
>  		return -EFAULT;
>  	}
>  
> +	/* ops required for families 17h and later */
> +	if (pvt->fam >= 0x17 && !pvt->ops->get_umc_error_info) {
> +		edac_dbg(1, "Platform specific helper routines not defined.\n");
> +		return -EFAULT;
> +	}
> +

I think this is a case where having the Family 17h+ ops as default would make
sense.

Thanks,
Yazen
diff mbox series

Patch

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 7a20f8a696de..ab4e16070a02 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -3056,10 +3056,13 @@  static inline void decode_bus_error(int node_id, struct mce *m)
  * Currently, we can derive the channel number by looking at the 6th nibble in
  * the instance_id. For example, instance_id=0xYXXXXX where Y is the channel
  * number.
+ *
+ * csrow can be derived from the lower 3 bits of MCA_SYND value.
  */
-static int find_umc_channel(struct mce *m)
+static void f17_umc_err_info(struct mce *m, struct err_info *err)
 {
-	return (m->ipid & GENMASK(31, 0)) >> 20;
+	err->channel = (m->ipid & GENMASK(31, 0)) >> 20;
+	err->csrow = m->synd & 0x7;
 }
 
 static void decode_umc_error(int node_id, struct mce *m)
@@ -3081,8 +3084,6 @@  static void decode_umc_error(int node_id, struct mce *m)
 	if (m->status & MCI_STATUS_DEFERRED)
 		ecc_type = 3;
 
-	err.channel = find_umc_channel(m);
-
 	if (!(m->status & MCI_STATUS_SYNDV)) {
 		err.err_code = ERR_SYND;
 		goto log_error;
@@ -3097,7 +3098,7 @@  static void decode_umc_error(int node_id, struct mce *m)
 			err.err_code = ERR_CHANNEL;
 	}
 
-	err.csrow = m->synd & 0x7;
+	pvt->ops->get_umc_error_info(m, &err);
 
 	if (umc_normaddr_to_sysaddr(m->addr, pvt->mc_node_id, err.channel, &sys_addr)) {
 		err.err_code = ERR_NORM_ADDR;
@@ -3927,6 +3928,7 @@  static int per_family_init(struct amd64_pvt *pvt)
 		pvt->ops->populate_csrows		= init_csrows_df;
 		pvt->ops->dump_misc_regs		= __dump_misc_regs_df;
 		pvt->ops->get_cs_mode			= f17_get_cs_mode;
+		pvt->ops->get_umc_error_info		= f17_umc_err_info;
 
 		if (pvt->fam == 0x18) {
 			pvt->ctl_name			= "F18h";
@@ -3974,6 +3976,7 @@  static int per_family_init(struct amd64_pvt *pvt)
 		pvt->ops->populate_csrows		= init_csrows_df;
 		pvt->ops->dump_misc_regs		= __dump_misc_regs_df;
 		pvt->ops->get_cs_mode			= f17_get_cs_mode;
+		pvt->ops->get_umc_error_info		= f17_umc_err_info;
 		break;
 
 	default:
@@ -3993,6 +3996,12 @@  static int per_family_init(struct amd64_pvt *pvt)
 		return -EFAULT;
 	}
 
+	/* ops required for families 17h and later */
+	if (pvt->fam >= 0x17 && !pvt->ops->get_umc_error_info) {
+		edac_dbg(1, "Platform specific helper routines not defined.\n");
+		return -EFAULT;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 2c93f8e0021a..43d9b11b826d 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -476,6 +476,7 @@  struct low_ops {
 	int  (*populate_csrows)(struct mem_ctl_info *mci);
 	void (*dump_misc_regs)(struct amd64_pvt *pvt);
 	int  (*get_cs_mode)(int dimm, u8 ctrl, struct amd64_pvt *pvt);
+	void (*get_umc_error_info)(struct mce *m, struct err_info *err);
 };
 
 int __amd64_read_pci_cfg_dword(struct pci_dev *pdev, int offset,