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 <jithu.joseph@xxxxxxxxx> > Reviewed-by: Tony Luck <tony.luck@xxxxxxxxx> > Tested-by: Pengfei Xu <pengfei.xu@xxxxxxxxx> > --- > 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; >