2023-09-29 22:53:27

by Joseph, Jithu

[permalink] [raw]
Subject: [PATCH v3 9/9] platform/x86/intel/ifs: ARRAY BIST for Sierra Forest

Array BIST MSR addresses, bit definition and semantics are different for
Sierra Forest. Branch into a separate Array BIST flow on Sierra Forest
when user invokes Array Test.

Signed-off-by: Jithu Joseph <[email protected]>
Reviewed-by: Tony Luck <[email protected]>
Tested-by: Pengfei Xu <[email protected]>
---
drivers/platform/x86/intel/ifs/ifs.h | 4 +++
drivers/platform/x86/intel/ifs/core.c | 15 +++++-----
drivers/platform/x86/intel/ifs/runtest.c | 37 +++++++++++++++++++++++-
3 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
index f0dd849b3400..b183aba3ffdf 100644
--- a/drivers/platform/x86/intel/ifs/ifs.h
+++ b/drivers/platform/x86/intel/ifs/ifs.h
@@ -137,6 +137,8 @@
#define MSR_CHUNKS_AUTHENTICATION_STATUS 0x000002c5
#define MSR_ACTIVATE_SCAN 0x000002c6
#define MSR_SCAN_STATUS 0x000002c7
+#define MSR_ARRAY_TRIGGER 0x000002d6
+#define MSR_ARRAY_STATUS 0x000002d7
#define MSR_SAF_CTRL 0x000004f0

#define SCAN_NOT_TESTED 0
@@ -272,6 +274,7 @@ struct ifs_test_caps {
* @cur_batch: number indicating the currently loaded test file
* @generation: IFS test generation enumerated by hardware
* @chunk_size: size of a test chunk
+ * @array_gen: test generation of array test
*/
struct ifs_data {
int loaded_version;
@@ -283,6 +286,7 @@ struct ifs_data {
u32 cur_batch;
u32 generation;
u32 chunk_size;
+ u32 array_gen;
};

struct ifs_work {
diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
index 0c8927916373..934eaf348f9d 100644
--- a/drivers/platform/x86/intel/ifs/core.c
+++ b/drivers/platform/x86/intel/ifs/core.c
@@ -11,16 +11,16 @@

#include "ifs.h"

-#define X86_MATCH(model) \
+#define X86_MATCH(model, array_gen) \
X86_MATCH_VENDOR_FAM_MODEL_FEATURE(INTEL, 6, \
- INTEL_FAM6_##model, X86_FEATURE_CORE_CAPABILITIES, NULL)
+ INTEL_FAM6_##model, X86_FEATURE_CORE_CAPABILITIES, array_gen)

static const struct x86_cpu_id ifs_cpu_ids[] __initconst = {
- X86_MATCH(SAPPHIRERAPIDS_X),
- X86_MATCH(EMERALDRAPIDS_X),
- X86_MATCH(GRANITERAPIDS_X),
- X86_MATCH(GRANITERAPIDS_D),
- X86_MATCH(ATOM_CRESTMONT_X),
+ X86_MATCH(SAPPHIRERAPIDS_X, 0),
+ X86_MATCH(EMERALDRAPIDS_X, 0),
+ X86_MATCH(GRANITERAPIDS_X, 0),
+ X86_MATCH(GRANITERAPIDS_D, 0),
+ X86_MATCH(ATOM_CRESTMONT_X, 1),
{}
};
MODULE_DEVICE_TABLE(x86cpu, ifs_cpu_ids);
@@ -100,6 +100,7 @@ static int __init ifs_init(void)
continue;
ifs_devices[i].rw_data.generation = FIELD_GET(MSR_INTEGRITY_CAPS_SAF_GEN_MASK,
msrval);
+ ifs_devices[i].rw_data.array_gen = (u32)m->driver_data;
ret = misc_register(&ifs_devices[i].misc);
if (ret)
goto err_exit;
diff --git a/drivers/platform/x86/intel/ifs/runtest.c b/drivers/platform/x86/intel/ifs/runtest.c
index 4fe544d79946..a54cd97920c4 100644
--- a/drivers/platform/x86/intel/ifs/runtest.c
+++ b/drivers/platform/x86/intel/ifs/runtest.c
@@ -329,6 +329,38 @@ static void ifs_array_test_core(int cpu, struct device *dev)
ifsd->status = SCAN_TEST_PASS;
}

+#define ARRAY_GEN1_TEST_ALL_ARRAYS 0x0ULL
+#define ARRAY_GEN1_STATUS_FAIL 0x1ULL
+
+static int do_array_test_gen1(void *status)
+{
+ int cpu = smp_processor_id();
+ int first;
+
+ first = cpumask_first(cpu_smt_mask(cpu));
+
+ if (cpu == first) {
+ wrmsrl(MSR_ARRAY_TRIGGER, ARRAY_GEN1_TEST_ALL_ARRAYS);
+ rdmsrl(MSR_ARRAY_STATUS, *((u64 *)status));
+ }
+
+ return 0;
+}
+
+static void ifs_array_test_gen1(int cpu, struct device *dev)
+{
+ struct ifs_data *ifsd = ifs_get_data(dev);
+ u64 status = 0;
+
+ stop_core_cpuslocked(cpu, do_array_test_gen1, &status);
+ ifsd->scan_details = status;
+
+ if (status & ARRAY_GEN1_STATUS_FAIL)
+ ifsd->status = SCAN_TEST_FAIL;
+ else
+ ifsd->status = SCAN_TEST_PASS;
+}
+
/*
* Initiate per core test. It wakes up work queue threads on the target cpu and
* its sibling cpu. Once all sibling threads wake up, the scan test gets executed and
@@ -356,7 +388,10 @@ int do_core_test(int cpu, struct device *dev)
ifs_test_core(cpu, dev);
break;
case IFS_TYPE_ARRAY_BIST:
- ifs_array_test_core(cpu, dev);
+ if (ifsd->array_gen == 0)
+ ifs_array_test_core(cpu, dev);
+ else
+ ifs_array_test_gen1(cpu, dev);
break;
default:
return -EINVAL;
--
2.25.1


2023-10-02 12:01:33

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH v3 9/9] platform/x86/intel/ifs: ARRAY BIST for Sierra Forest

On Fri, 29 Sep 2023, Jithu Joseph wrote:

> Array BIST MSR addresses, bit definition and semantics are different for
> Sierra Forest. Branch into a separate Array BIST flow on Sierra Forest
> when user invokes Array Test.
>
> Signed-off-by: Jithu Joseph <[email protected]>
> Reviewed-by: Tony Luck <[email protected]>
> Tested-by: Pengfei Xu <[email protected]>
> ---
> drivers/platform/x86/intel/ifs/ifs.h | 4 +++
> drivers/platform/x86/intel/ifs/core.c | 15 +++++-----
> drivers/platform/x86/intel/ifs/runtest.c | 37 +++++++++++++++++++++++-
> 3 files changed, 48 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
> index f0dd849b3400..b183aba3ffdf 100644
> --- a/drivers/platform/x86/intel/ifs/ifs.h
> +++ b/drivers/platform/x86/intel/ifs/ifs.h
> @@ -137,6 +137,8 @@
> #define MSR_CHUNKS_AUTHENTICATION_STATUS 0x000002c5
> #define MSR_ACTIVATE_SCAN 0x000002c6
> #define MSR_SCAN_STATUS 0x000002c7
> +#define MSR_ARRAY_TRIGGER 0x000002d6
> +#define MSR_ARRAY_STATUS 0x000002d7
> #define MSR_SAF_CTRL 0x000004f0
>
> #define SCAN_NOT_TESTED 0
> @@ -272,6 +274,7 @@ struct ifs_test_caps {
> * @cur_batch: number indicating the currently loaded test file
> * @generation: IFS test generation enumerated by hardware
> * @chunk_size: size of a test chunk
> + * @array_gen: test generation of array test
> */
> struct ifs_data {
> int loaded_version;
> @@ -283,6 +286,7 @@ struct ifs_data {
> u32 cur_batch;
> u32 generation;
> u32 chunk_size;
> + u32 array_gen;
> };
>
> struct ifs_work {
> diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
> index 0c8927916373..934eaf348f9d 100644
> --- a/drivers/platform/x86/intel/ifs/core.c
> +++ b/drivers/platform/x86/intel/ifs/core.c
> @@ -11,16 +11,16 @@
>
> #include "ifs.h"
>
> -#define X86_MATCH(model) \
> +#define X86_MATCH(model, array_gen) \
> X86_MATCH_VENDOR_FAM_MODEL_FEATURE(INTEL, 6, \
> - INTEL_FAM6_##model, X86_FEATURE_CORE_CAPABILITIES, NULL)
> + INTEL_FAM6_##model, X86_FEATURE_CORE_CAPABILITIES, array_gen)
>
> static const struct x86_cpu_id ifs_cpu_ids[] __initconst = {
> - X86_MATCH(SAPPHIRERAPIDS_X),
> - X86_MATCH(EMERALDRAPIDS_X),
> - X86_MATCH(GRANITERAPIDS_X),
> - X86_MATCH(GRANITERAPIDS_D),
> - X86_MATCH(ATOM_CRESTMONT_X),
> + X86_MATCH(SAPPHIRERAPIDS_X, 0),
> + X86_MATCH(EMERALDRAPIDS_X, 0),
> + X86_MATCH(GRANITERAPIDS_X, 0),
> + X86_MATCH(GRANITERAPIDS_D, 0),
> + X86_MATCH(ATOM_CRESTMONT_X, 1),

Just a suggestion that would IMO make these easier to understand, you
could name these array generations with defines so that one does not need
to look what's defined in X86_MATCH() to understand the purpose of the
second parameter. But it's up to you.

--
i.

> {}
> };
> MODULE_DEVICE_TABLE(x86cpu, ifs_cpu_ids);
> @@ -100,6 +100,7 @@ static int __init ifs_init(void)
> continue;
> ifs_devices[i].rw_data.generation = FIELD_GET(MSR_INTEGRITY_CAPS_SAF_GEN_MASK,
> msrval);
> + ifs_devices[i].rw_data.array_gen = (u32)m->driver_data;
> ret = misc_register(&ifs_devices[i].misc);
> if (ret)
> goto err_exit;
> diff --git a/drivers/platform/x86/intel/ifs/runtest.c b/drivers/platform/x86/intel/ifs/runtest.c
> index 4fe544d79946..a54cd97920c4 100644
> --- a/drivers/platform/x86/intel/ifs/runtest.c
> +++ b/drivers/platform/x86/intel/ifs/runtest.c
> @@ -329,6 +329,38 @@ static void ifs_array_test_core(int cpu, struct device *dev)
> ifsd->status = SCAN_TEST_PASS;
> }
>
> +#define ARRAY_GEN1_TEST_ALL_ARRAYS 0x0ULL
> +#define ARRAY_GEN1_STATUS_FAIL 0x1ULL
> +
> +static int do_array_test_gen1(void *status)
> +{
> + int cpu = smp_processor_id();
> + int first;
> +
> + first = cpumask_first(cpu_smt_mask(cpu));
> +
> + if (cpu == first) {
> + wrmsrl(MSR_ARRAY_TRIGGER, ARRAY_GEN1_TEST_ALL_ARRAYS);
> + rdmsrl(MSR_ARRAY_STATUS, *((u64 *)status));
> + }
> +
> + return 0;
> +}
> +
> +static void ifs_array_test_gen1(int cpu, struct device *dev)
> +{
> + struct ifs_data *ifsd = ifs_get_data(dev);
> + u64 status = 0;
> +
> + stop_core_cpuslocked(cpu, do_array_test_gen1, &status);
> + ifsd->scan_details = status;
> +
> + if (status & ARRAY_GEN1_STATUS_FAIL)
> + ifsd->status = SCAN_TEST_FAIL;
> + else
> + ifsd->status = SCAN_TEST_PASS;
> +}
> +
> /*
> * Initiate per core test. It wakes up work queue threads on the target cpu and
> * its sibling cpu. Once all sibling threads wake up, the scan test gets executed and
> @@ -356,7 +388,10 @@ int do_core_test(int cpu, struct device *dev)
> ifs_test_core(cpu, dev);
> break;
> case IFS_TYPE_ARRAY_BIST:
> - ifs_array_test_core(cpu, dev);
> + if (ifsd->array_gen == 0)
> + ifs_array_test_core(cpu, dev);
> + else
> + ifs_array_test_gen1(cpu, dev);
> break;
> default:
> return -EINVAL;
>

2023-10-02 23:01:34

by Joseph, Jithu

[permalink] [raw]
Subject: Re: [PATCH v3 9/9] platform/x86/intel/ifs: ARRAY BIST for Sierra Forest



On 10/2/2023 4:59 AM, Ilpo Järvinen wrote:

>
> Just a suggestion that would IMO make these easier to understand, you
> could name these array generations with defines so that one does not need
> to look what's defined in X86_MATCH() to understand the purpose of the
> second parameter. But it's up to you.
>

I can make this change ... Thanks for the review

Jithu

2023-10-04 19:14:43

by Joseph, Jithu

[permalink] [raw]
Subject: [PATCH v3 9/9] platform/x86/intel/ifs: ARRAY BIST for Sierra Forest

Array BIST MSR addresses, bit definition and semantics are different for
Sierra Forest. Branch into a separate Array BIST flow on Sierra Forest
when user invokes Array Test.

Signed-off-by: Jithu Joseph <[email protected]>
Reviewed-by: Tony Luck <[email protected]>
Tested-by: Pengfei Xu <[email protected]>
---
- Add #define for Array generation (Ilpo Järvinen)

drivers/platform/x86/intel/ifs/ifs.h | 7 +++++
drivers/platform/x86/intel/ifs/core.c | 15 +++++-----
drivers/platform/x86/intel/ifs/runtest.c | 37 +++++++++++++++++++++++-
3 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
index f0dd849b3400..2dd5e3406dac 100644
--- a/drivers/platform/x86/intel/ifs/ifs.h
+++ b/drivers/platform/x86/intel/ifs/ifs.h
@@ -137,6 +137,8 @@
#define MSR_CHUNKS_AUTHENTICATION_STATUS 0x000002c5
#define MSR_ACTIVATE_SCAN 0x000002c6
#define MSR_SCAN_STATUS 0x000002c7
+#define MSR_ARRAY_TRIGGER 0x000002d6
+#define MSR_ARRAY_STATUS 0x000002d7
#define MSR_SAF_CTRL 0x000004f0

#define SCAN_NOT_TESTED 0
@@ -146,6 +148,9 @@
#define IFS_TYPE_SAF 0
#define IFS_TYPE_ARRAY_BIST 1

+#define ARRAY_GEN_0 0
+#define ARRAY_GEN_1 1
+
/* MSR_SCAN_HASHES_STATUS bit fields */
union ifs_scan_hashes_status {
u64 data;
@@ -272,6 +277,7 @@ struct ifs_test_caps {
* @cur_batch: number indicating the currently loaded test file
* @generation: IFS test generation enumerated by hardware
* @chunk_size: size of a test chunk
+ * @array_gen: test generation of array test
*/
struct ifs_data {
int loaded_version;
@@ -283,6 +289,7 @@ struct ifs_data {
u32 cur_batch;
u32 generation;
u32 chunk_size;
+ u32 array_gen;
};

struct ifs_work {
diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
index 0c8927916373..0a872b874af8 100644
--- a/drivers/platform/x86/intel/ifs/core.c
+++ b/drivers/platform/x86/intel/ifs/core.c
@@ -11,16 +11,16 @@

#include "ifs.h"

-#define X86_MATCH(model) \
+#define X86_MATCH(model, array_gen) \
X86_MATCH_VENDOR_FAM_MODEL_FEATURE(INTEL, 6, \
- INTEL_FAM6_##model, X86_FEATURE_CORE_CAPABILITIES, NULL)
+ INTEL_FAM6_##model, X86_FEATURE_CORE_CAPABILITIES, array_gen)

static const struct x86_cpu_id ifs_cpu_ids[] __initconst = {
- X86_MATCH(SAPPHIRERAPIDS_X),
- X86_MATCH(EMERALDRAPIDS_X),
- X86_MATCH(GRANITERAPIDS_X),
- X86_MATCH(GRANITERAPIDS_D),
- X86_MATCH(ATOM_CRESTMONT_X),
+ X86_MATCH(SAPPHIRERAPIDS_X, ARRAY_GEN_0),
+ X86_MATCH(EMERALDRAPIDS_X, ARRAY_GEN_0),
+ X86_MATCH(GRANITERAPIDS_X, ARRAY_GEN_0),
+ X86_MATCH(GRANITERAPIDS_D, ARRAY_GEN_0),
+ X86_MATCH(ATOM_CRESTMONT_X, ARRAY_GEN_1),
{}
};
MODULE_DEVICE_TABLE(x86cpu, ifs_cpu_ids);
@@ -100,6 +100,7 @@ static int __init ifs_init(void)
continue;
ifs_devices[i].rw_data.generation = FIELD_GET(MSR_INTEGRITY_CAPS_SAF_GEN_MASK,
msrval);
+ ifs_devices[i].rw_data.array_gen = (u32)m->driver_data;
ret = misc_register(&ifs_devices[i].misc);
if (ret)
goto err_exit;
diff --git a/drivers/platform/x86/intel/ifs/runtest.c b/drivers/platform/x86/intel/ifs/runtest.c
index 4fe544d79946..9ac75420c15e 100644
--- a/drivers/platform/x86/intel/ifs/runtest.c
+++ b/drivers/platform/x86/intel/ifs/runtest.c
@@ -329,6 +329,38 @@ static void ifs_array_test_core(int cpu, struct device *dev)
ifsd->status = SCAN_TEST_PASS;
}

+#define ARRAY_GEN1_TEST_ALL_ARRAYS 0x0ULL
+#define ARRAY_GEN1_STATUS_FAIL 0x1ULL
+
+static int do_array_test_gen1(void *status)
+{
+ int cpu = smp_processor_id();
+ int first;
+
+ first = cpumask_first(cpu_smt_mask(cpu));
+
+ if (cpu == first) {
+ wrmsrl(MSR_ARRAY_TRIGGER, ARRAY_GEN1_TEST_ALL_ARRAYS);
+ rdmsrl(MSR_ARRAY_STATUS, *((u64 *)status));
+ }
+
+ return 0;
+}
+
+static void ifs_array_test_gen1(int cpu, struct device *dev)
+{
+ struct ifs_data *ifsd = ifs_get_data(dev);
+ u64 status = 0;
+
+ stop_core_cpuslocked(cpu, do_array_test_gen1, &status);
+ ifsd->scan_details = status;
+
+ if (status & ARRAY_GEN1_STATUS_FAIL)
+ ifsd->status = SCAN_TEST_FAIL;
+ else
+ ifsd->status = SCAN_TEST_PASS;
+}
+
/*
* Initiate per core test. It wakes up work queue threads on the target cpu and
* its sibling cpu. Once all sibling threads wake up, the scan test gets executed and
@@ -356,7 +388,10 @@ int do_core_test(int cpu, struct device *dev)
ifs_test_core(cpu, dev);
break;
case IFS_TYPE_ARRAY_BIST:
- ifs_array_test_core(cpu, dev);
+ if (ifsd->array_gen == ARRAY_GEN_0)
+ ifs_array_test_core(cpu, dev);
+ else
+ ifs_array_test_gen1(cpu, dev);
break;
default:
return -EINVAL;
--
2.25.1