[PATCH] EDAC, amd64: Add Family 17h Model 11h support.

[PATCH] EDAC, amd64: Add Family 17h Model 11h support.

[Michael Jin]

Add support for ECC error decoding for F17h M11h (Great Horned Owl) processors.

Signed-off-by: Michael Jin <mikhail.jin@gmail.com>
---
 drivers/edac/amd64_edac.c | 14 ++++++++++++++
 drivers/edac/amd64_edac.h |  3 +++
 2 files changed, 17 insertions(+)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 18aeabb1d5ee..de077c1f5ae3 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -2200,6 +2200,15 @@ static struct amd64_family_type family_types[] = {
 			.dbam_to_cs		= f17_base_addr_to_cs_size,
 		}
 	},
+	[F17_M11H_CPUS] = {
+		.ctl_name = "F17h_M11h",
+		.f0_id = PCI_DEVICE_ID_AMD_17H_M11H_DF_F0,
+		.f6_id = PCI_DEVICE_ID_AMD_17H_M11H_DF_F6,
+		.ops = {
+			.early_channel_count	= f17_early_channel_count,
+			.dbam_to_cs		= f17_base_addr_to_cs_size,
+		}
+	},
 };
 
 /*
@@ -3188,6 +3197,11 @@ static struct amd64_family_type *per_family_init(struct amd64_pvt *pvt)
 		break;
 
 	case 0x17:
+		if (pvt->model == 0x11) {
+			fam_type = &family_types[F17_M11H_CPUS];
+			pvt->ops = &family_types[F17_M11H_CPUS].ops;
+			break;
+		}
 		fam_type	= &family_types[F17_CPUS];
 		pvt->ops	= &family_types[F17_CPUS].ops;
 		break;
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 1d4b74e9a037..e50226cd53c6 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -115,6 +115,8 @@
 #define PCI_DEVICE_ID_AMD_16H_M30H_NB_F2 0x1582
 #define PCI_DEVICE_ID_AMD_17H_DF_F0	0x1460
 #define PCI_DEVICE_ID_AMD_17H_DF_F6	0x1466
+#define PCI_DEVICE_ID_AMD_17H_M11H_DF_F0 0x15e8
+#define PCI_DEVICE_ID_AMD_17H_M11H_DF_F6 0x15ee
 
 /*
  * Function 1 - Address Map
@@ -281,6 +283,7 @@ enum amd_families {
 	F16_CPUS,
 	F16_M30H_CPUS,
 	F17_CPUS,
+	F17_M11H_CPUS,
 	NUM_FAMILIES,
 };
 
-- 
2.14.3 (Apple Git-98)

RE: [PATCH] EDAC, amd64: Add Family 17h Model 11h support.

[Ghannam, Yazen]

> -----Original Message-----
> From: Michael Jin <mikhail.jin@gmail.com>
> Sent: Friday, August 10, 2018 2:36 PM
> To: Borislav Petkov <bp@suse.de>; Ghannam, Yazen
> <Yazen.Ghannam@amd.com>; Mauro Carvalho Chehab
> <mchehab@kernel.org>
> Cc: linux-edac@vger.kernel.org; linux-kernel@vger.kernel.org; Michael Jin
> <mikhail.jin@gmail.com>
> Subject: [PATCH] EDAC, amd64: Add Family 17h Model 11h support.
> 
> Add support for ECC error decoding for F17h M11h (Great Horned Owl)
> processors.
> 
> Signed-off-by: Michael Jin <mikhail.jin@gmail.com>
> ---
>  drivers/edac/amd64_edac.c | 14 ++++++++++++++
>  drivers/edac/amd64_edac.h |  3 +++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index 18aeabb1d5ee..de077c1f5ae3 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -2200,6 +2200,15 @@ static struct amd64_family_type family_types[] = {
>  			.dbam_to_cs		= f17_base_addr_to_cs_size,
>  		}
>  	},
> +	[F17_M11H_CPUS] = {
> +		.ctl_name = "F17h_M11h",
> +		.f0_id = PCI_DEVICE_ID_AMD_17H_M11H_DF_F0,
> +		.f6_id = PCI_DEVICE_ID_AMD_17H_M11H_DF_F6,
> +		.ops = {
> +			.early_channel_count	= f17_early_channel_count,
> +			.dbam_to_cs		= f17_base_addr_to_cs_size,
> +		}
> +	},
>  };
> 
>  /*
> @@ -3188,6 +3197,11 @@ static struct amd64_family_type
> *per_family_init(struct amd64_pvt *pvt)
>  		break;
> 
>  	case 0x17:
> +		if (pvt->model == 0x11) {

I think we can expand this to include more models. Please see comment
below.

There may be some differences between models, but things should generally
work. It's just a matter of whether or not the Platform enables certain things
like DRAM ECC, etc.

Does the amd64_edac_mod module load on your platform with just this patch?

> +			fam_type = &family_types[F17_M11H_CPUS];
> +			pvt->ops = &family_types[F17_M11H_CPUS].ops;
> +			break;
> +		}
>  		fam_type	= &family_types[F17_CPUS];
>  		pvt->ops	= &family_types[F17_CPUS].ops;
>  		break;
> diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
> index 1d4b74e9a037..e50226cd53c6 100644
> --- a/drivers/edac/amd64_edac.h
> +++ b/drivers/edac/amd64_edac.h
> @@ -115,6 +115,8 @@
>  #define PCI_DEVICE_ID_AMD_16H_M30H_NB_F2 0x1582
>  #define PCI_DEVICE_ID_AMD_17H_DF_F0	0x1460
>  #define PCI_DEVICE_ID_AMD_17H_DF_F6	0x1466
> +#define PCI_DEVICE_ID_AMD_17H_M11H_DF_F0 0x15e8
> +#define PCI_DEVICE_ID_AMD_17H_M11H_DF_F6 0x15ee
> 

These IDs are used for Fam17h Models 10h-2Fh. Can you please change
the names here and in the rest of this patch?

The format is to use the first supported model in the name, e.g. M11H -> M10H.

>  /*
>   * Function 1 - Address Map
> @@ -281,6 +283,7 @@ enum amd_families {
>  	F16_CPUS,
>  	F16_M30H_CPUS,
>  	F17_CPUS,
> +	F17_M11H_CPUS,
>  	NUM_FAMILIES,
>  };
> 

Thanks,
Yazen

Re: [PATCH] EDAC, amd64: Add Family 17h Model 11h support.

[Michael Jin]

On Tue, Aug 14, 2018 at 4:26 PM, Ghannam, Yazen <Yazen.Ghannam@amd.com> wrote:
>
> > -----Original Message-----
> > From: Michael Jin <mikhail.jin@gmail.com>
> > Sent: Friday, August 10, 2018 2:36 PM
> > To: Borislav Petkov <bp@suse.de>; Ghannam, Yazen
> > <Yazen.Ghannam@amd.com>; Mauro Carvalho Chehab
> > <mchehab@kernel.org>
> > Cc: linux-edac@vger.kernel.org; linux-kernel@vger.kernel.org; Michael Jin
> > <mikhail.jin@gmail.com>
> There may be some differences between models, but things should generally
> work. It's just a matter of whether or not the Platform enables certain things
> like DRAM ECC, etc.
>
> Does the amd64_edac_mod module load on your platform with just this patch?
>
> > +                     fam_type = &family_types[F17_M11H_CPUS];
> > +                     pvt->ops = &family_types[F17_M11H_CPUS].ops;
> > +                     break;
> > +             }
> >               fam_type        = &family_types[F17_CPUS];
> >               pvt->ops        = &family_types[F17_CPUS].ops;
> >               break;

Yes, I was able to load amd64_edac_mod on my AMD Ryzen Embedded V1807B
(further details on my blog -
https://ndimcomputing.io/fs-fp5v_ecc_linux.html).

> > diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
> > index 1d4b74e9a037..e50226cd53c6 100644
> > --- a/drivers/edac/amd64_edac.h
> > +++ b/drivers/edac/amd64_edac.h
> > @@ -115,6 +115,8 @@
> >  #define PCI_DEVICE_ID_AMD_16H_M30H_NB_F2 0x1582
> >  #define PCI_DEVICE_ID_AMD_17H_DF_F0  0x1460
> >  #define PCI_DEVICE_ID_AMD_17H_DF_F6  0x1466
> > +#define PCI_DEVICE_ID_AMD_17H_M11H_DF_F0 0x15e8
> > +#define PCI_DEVICE_ID_AMD_17H_M11H_DF_F6 0x15ee
> >
>
> These IDs are used for Fam17h Models 10h-2Fh. Can you please change
> the names here and in the rest of this patch?
>
> The format is to use the first supported model in the name, e.g. M11H -> M10H.
>
> >  /*
> >   * Function 1 - Address Map
> > @@ -281,6 +283,7 @@ enum amd_families {
> >       F16_CPUS,
> >       F16_M30H_CPUS,
> >       F17_CPUS,
> > +     F17_M11H_CPUS,
> >       NUM_FAMILIES,
> >  };
> >

According to https://en.wikichip.org/wiki/amd/cpuid, family 17h model
10h is not publicly known.

Therefore, I would like you to confirm that model 10h uses 0x15e8
(device F0) and 0x15ee (device F6) as I can not find any documentation
or test whether ECC works.

Raven Ridge BIOS motherboards do not enable ECC, but share the same
CPUID (0081_0F10h
http://www.cpu-world.com/CPUs/Zen/AMD-Ryzen%205%202400G.html) as AMD
Ryzen Embedded V1000 Processor Family.

This patch was written due to the fact that amd64_edac_mod erroneously
sets the wrong device ids for the AMD Ryzen Embedded V1000 Family
(device F0 and F6 fail to load, as they do not share 0x1460 and 0x1466
with the other family 17h processors).

Michael

Re: [PATCH] EDAC, amd64: Add Family 17h Model 11h support.

[Borislav Petkov]

On Tue, Aug 14, 2018 at 08:14:47PM -0400, Michael Jin wrote:
> Therefore, I would like you to confirm that model 10h uses 0x15e8 (device
> F0) and 0x15ee (device F6) as I can not find any documentation or test
> whether ECC works.

He just asked you to make the change for models 0x10-0x2f. How much more
confirmation do you need?

-- 
Regards/Gruss,
    Boris.

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