[PATCH 0/5] Miscelleanous fixes and improvements to Infield Scan

[PATCH 0/5] Miscelleanous fixes and improvements to Infield Scan

[Ashok Raj]

Hi Hans,

This series has some bug fixes and improvements for Infield Scan.

Patch1 - fixes a bug in the driver that didn't release_firmware() in the
	 error path.

Patch2, 3 - Added some improved tracing.

Patch4 - Moves the rendezvous to function entry
Patch5 - Adds entry rendezvous for IFS.

Cheers,
Ashok


Ashok Raj (4):
  platform/x86/intel/ifs: Trace on all HT threads when executing a test
  platform/x86/intel/ifs: Add current batch number to trace output
  platform/x86/intel/ifs: Replace the exit rendezvous with an entry rendezvous for ARRAY_BIST
  platform/x86/intel/ifs: Add an entry rendezvous for SAF

Jithu Joseph (1):
  platform/x86/intel/ifs: Call release_firmware() when handling errors.

 include/trace/events/intel_ifs.h         |  12 +--
 drivers/platform/x86/intel/ifs/load.c    |   3 +-
 drivers/platform/x86/intel/ifs/runtest.c | 101 ++++++++++++++---------
 3 files changed, 70 insertions(+), 46 deletions(-)


base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
-- 
2.39.2

[PATCH 1/5] platform/x86/intel/ifs: Call release_firmware() when handling errors.

[Ashok Raj]

From: Jithu Joseph <jithu.joseph@intel.com>

Missing release_firmware() due to error handling blocked any future image
loading.

Fix the return code and release_fiwmare() to release the bad image.

Fixes: 25a76dbb36dd ("platform/x86/intel/ifs: Validate image size")
Reported-by: Pengfei Xu <pengfei.xu@intel.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
Signed-off-by: Ashok Raj <ashok.raj@intel.com>
Tested-by: Pengfei Xu <pengfei.xu@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
---
 drivers/platform/x86/intel/ifs/load.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
index a1ee1a74fc3c..2cf3b4a8813f 100644
--- a/drivers/platform/x86/intel/ifs/load.c
+++ b/drivers/platform/x86/intel/ifs/load.c
@@ -399,7 +399,8 @@ int ifs_load_firmware(struct device *dev)
 	if (fw->size != expected_size) {
 		dev_err(dev, "File size mismatch (expected %u, actual %zu). Corrupted IFS image.\n",
 			expected_size, fw->size);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto release;
 	}
 
 	ret = image_sanity_check(dev, (struct microcode_header_intel *)fw->data);
-- 
2.39.2

Re: [PATCH 1/5] platform/x86/intel/ifs: Call release_firmware() when handling errors.

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1385 bytes --]

On Thu, 25 Jan 2024, Ashok Raj wrote:

> From: Jithu Joseph <jithu.joseph@intel.com>
> 
> Missing release_firmware() due to error handling blocked any future image
> loading.
> 
> Fix the return code and release_fiwmare() to release the bad image.
> 
> Fixes: 25a76dbb36dd ("platform/x86/intel/ifs: Validate image size")
> Reported-by: Pengfei Xu <pengfei.xu@intel.com>
> Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
> Signed-off-by: Ashok Raj <ashok.raj@intel.com>
> Tested-by: Pengfei Xu <pengfei.xu@intel.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> ---
>  drivers/platform/x86/intel/ifs/load.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
> index a1ee1a74fc3c..2cf3b4a8813f 100644
> --- a/drivers/platform/x86/intel/ifs/load.c
> +++ b/drivers/platform/x86/intel/ifs/load.c
> @@ -399,7 +399,8 @@ int ifs_load_firmware(struct device *dev)
>  	if (fw->size != expected_size) {
>  		dev_err(dev, "File size mismatch (expected %u, actual %zu). Corrupted IFS image.\n",
>  			expected_size, fw->size);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto release;
>  	}
>  
>  	ret = image_sanity_check(dev, (struct microcode_header_intel *)fw->data);
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

Re: [PATCH 1/5] platform/x86/intel/ifs: Call release_firmware() when handling errors.

[Hans de Goede]

Hi,

On 1/25/24 09:22, Ashok Raj wrote:
> From: Jithu Joseph <jithu.joseph@intel.com>
> 
> Missing release_firmware() due to error handling blocked any future image
> loading.
> 
> Fix the return code and release_fiwmare() to release the bad image.
> 
> Fixes: 25a76dbb36dd ("platform/x86/intel/ifs: Validate image size")
> Reported-by: Pengfei Xu <pengfei.xu@intel.com>
> Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
> Signed-off-by: Ashok Raj <ashok.raj@intel.com>
> Tested-by: Pengfei Xu <pengfei.xu@intel.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>

Thank you for your patch/series, I've applied this patch
(series) to my review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in the pdx86 review-hans branch once I've
pushed my local branch there, which might take a while.

I will include this patch in my next fixes pull-req to Linus
for the current kernel development cycle.

Regards,

Hans




> ---
>  drivers/platform/x86/intel/ifs/load.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
> index a1ee1a74fc3c..2cf3b4a8813f 100644
> --- a/drivers/platform/x86/intel/ifs/load.c
> +++ b/drivers/platform/x86/intel/ifs/load.c
> @@ -399,7 +399,8 @@ int ifs_load_firmware(struct device *dev)
>  	if (fw->size != expected_size) {
>  		dev_err(dev, "File size mismatch (expected %u, actual %zu). Corrupted IFS image.\n",
>  			expected_size, fw->size);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto release;
>  	}
>  
>  	ret = image_sanity_check(dev, (struct microcode_header_intel *)fw->data);

Re: [PATCH 1/5] platform/x86/intel/ifs: Call release_firmware() when handling errors.

[Pengfei Xu]

On 2024-01-26 at 20:15:46 +0100, Hans de Goede wrote:
> Hi,
> 
> On 1/25/24 09:22, Ashok Raj wrote:
> > From: Jithu Joseph <jithu.joseph@intel.com>
> > 
> > Missing release_firmware() due to error handling blocked any future image
> > loading.
> > 
> > Fix the return code and release_fiwmare() to release the bad image.
> > 
> > Fixes: 25a76dbb36dd ("platform/x86/intel/ifs: Validate image size")
> > Reported-by: Pengfei Xu <pengfei.xu@intel.com>
> > Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
> > Signed-off-by: Ashok Raj <ashok.raj@intel.com>
> > Tested-by: Pengfei Xu <pengfei.xu@intel.com>
> > Reviewed-by: Tony Luck <tony.luck@intel.com>
> 
> Thank you for your patch/series, I've applied this patch
> (series) to my review-hans branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
> 
> Note it will show up in the pdx86 review-hans branch once I've
> pushed my local branch there, which might take a while.
> 
> I will include this patch in my next fixes pull-req to Linus
> for the current kernel development cycle.
> 

FYR.

[ CC stable@vger.kernel.org ]
Missed CC: Stable Tag.

This (follow-up) patch is now upstream into v6.7-rc1:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=25a76dbb36dd

Looks like linux-6.7.y needs the above fixed patch too.


Best Regards,
Thanks!


> Regards,
> 
> Hans
> 
> 
> 
> 
> > ---
> >  drivers/platform/x86/intel/ifs/load.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
> > index a1ee1a74fc3c..2cf3b4a8813f 100644
> > --- a/drivers/platform/x86/intel/ifs/load.c
> > +++ b/drivers/platform/x86/intel/ifs/load.c
> > @@ -399,7 +399,8 @@ int ifs_load_firmware(struct device *dev)
> >  	if (fw->size != expected_size) {
> >  		dev_err(dev, "File size mismatch (expected %u, actual %zu). Corrupted IFS image.\n",
> >  			expected_size, fw->size);
> > -		return -EINVAL;
> > +		ret = -EINVAL;
> > +		goto release;
> >  	}
> >  
> >  	ret = image_sanity_check(dev, (struct microcode_header_intel *)fw->data);
> 

Re: [PATCH 1/5] platform/x86/intel/ifs: Call release_firmware() when handling errors.

[Hans de Goede]

Hi,

On 1/29/24 04:55, Pengfei Xu wrote:
> On 2024-01-26 at 20:15:46 +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 1/25/24 09:22, Ashok Raj wrote:
>>> From: Jithu Joseph <jithu.joseph@intel.com>
>>>
>>> Missing release_firmware() due to error handling blocked any future image
>>> loading.
>>>
>>> Fix the return code and release_fiwmare() to release the bad image.
>>>
>>> Fixes: 25a76dbb36dd ("platform/x86/intel/ifs: Validate image size")
>>> Reported-by: Pengfei Xu <pengfei.xu@intel.com>
>>> Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
>>> Signed-off-by: Ashok Raj <ashok.raj@intel.com>
>>> Tested-by: Pengfei Xu <pengfei.xu@intel.com>
>>> Reviewed-by: Tony Luck <tony.luck@intel.com>
>>
>> Thank you for your patch/series, I've applied this patch
>> (series) to my review-hans branch:
>> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
>>
>> Note it will show up in the pdx86 review-hans branch once I've
>> pushed my local branch there, which might take a while.
>>
>> I will include this patch in my next fixes pull-req to Linus
>> for the current kernel development cycle.
>>
> 
> FYR.
> 
> [ CC stable@vger.kernel.org ]
> Missed CC: Stable Tag.
> 
> This (follow-up) patch is now upstream into v6.7-rc1:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=25a76dbb36dd
> 
> Looks like linux-6.7.y needs the above fixed patch too.

If you want to see this patch in the stable series please
submit it, with a reference to the upstream commit, to
stable@vger.kernel.org yourself.

Regards,

Hans



>>> ---
>>>  drivers/platform/x86/intel/ifs/load.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
>>> index a1ee1a74fc3c..2cf3b4a8813f 100644
>>> --- a/drivers/platform/x86/intel/ifs/load.c
>>> +++ b/drivers/platform/x86/intel/ifs/load.c
>>> @@ -399,7 +399,8 @@ int ifs_load_firmware(struct device *dev)
>>>  	if (fw->size != expected_size) {
>>>  		dev_err(dev, "File size mismatch (expected %u, actual %zu). Corrupted IFS image.\n",
>>>  			expected_size, fw->size);
>>> -		return -EINVAL;
>>> +		ret = -EINVAL;
>>> +		goto release;
>>>  	}
>>>  
>>>  	ret = image_sanity_check(dev, (struct microcode_header_intel *)fw->data);
>>
> 

[PATCH 2/5] platform/x86/intel/ifs: Trace on all HT threads when executing a test

[Ashok Raj]

Enable the trace function on all HT threads.  Currently, the trace is
called from some arbitrary CPU where the test was invoked.

This change gives visibility to the exact errors as seen by each
participating HT threads, and not just what was seen from the primary
thread.

Sample output below.

#           TASK-PID     CPU#  |||||  TIMESTAMP  FUNCTION
#              | |         |   |||||     |         |
     migration/0-18      [000] d..1. 527287.084668: start: 0000, stop: 007f, status: 0000000000007f80
   migration/128-785     [128] d..1. 527287.084669: start: 0000, stop: 007f, status: 0000000000007f80

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
---
 include/trace/events/intel_ifs.h         |  9 ++---
 drivers/platform/x86/intel/ifs/runtest.c | 46 +++++++++++++++++-------
 2 files changed, 36 insertions(+), 19 deletions(-)

diff --git a/include/trace/events/intel_ifs.h b/include/trace/events/intel_ifs.h
index af0af3f1d9b7..8462dfb7a020 100644
--- a/include/trace/events/intel_ifs.h
+++ b/include/trace/events/intel_ifs.h
@@ -10,26 +10,23 @@
 
 TRACE_EVENT(ifs_status,
 
-	TP_PROTO(int cpu, int start, int stop, u64 status),
+	TP_PROTO(int start, int stop, u64 status),
 
-	TP_ARGS(cpu, start, stop, status),
+	TP_ARGS(start, stop, status),
 
 	TP_STRUCT__entry(
 		__field(	u64,	status	)
-		__field(	int,	cpu	)
 		__field(	u16,	start	)
 		__field(	u16,	stop	)
 	),
 
 	TP_fast_assign(
-		__entry->cpu	= cpu;
 		__entry->start	= start;
 		__entry->stop	= stop;
 		__entry->status	= status;
 	),
 
-	TP_printk("cpu: %d, start: %.4x, stop: %.4x, status: %.16llx",
-		__entry->cpu,
+	TP_printk("start: %.4x, stop: %.4x, status: %.16llx",
 		__entry->start,
 		__entry->stop,
 		__entry->status)
diff --git a/drivers/platform/x86/intel/ifs/runtest.c b/drivers/platform/x86/intel/ifs/runtest.c
index 13ecd55c6668..c8352ffb9195 100644
--- a/drivers/platform/x86/intel/ifs/runtest.c
+++ b/drivers/platform/x86/intel/ifs/runtest.c
@@ -23,6 +23,12 @@
 /* Max retries on the same chunk */
 #define MAX_IFS_RETRIES  5
 
+struct run_params {
+	struct ifs_data *ifsd;
+	union ifs_scan *activate;
+	union ifs_status status;
+};
+
 /*
  * Number of TSC cycles that a logical CPU will wait for the other
  * logical CPU on the core in the WRMSR(ACTIVATE_SCAN).
@@ -140,10 +146,22 @@ static bool can_restart(union ifs_status status)
  */
 static int doscan(void *data)
 {
-	int cpu = smp_processor_id();
-	u64 *msrs = data;
+	int cpu = smp_processor_id(), start, stop;
+	struct run_params *params = data;
+	union ifs_status status;
+	struct ifs_data *ifsd;
 	int first;
 
+	ifsd = params->ifsd;
+
+	if (ifsd->generation) {
+		start = params->activate->gen2.start;
+		stop = params->activate->gen2.stop;
+	} else {
+		start = params->activate->gen0.start;
+		stop = params->activate->gen0.stop;
+	}
+
 	/* Only the first logical CPU on a core reports result */
 	first = cpumask_first(cpu_smt_mask(cpu));
 
@@ -155,12 +173,14 @@ static int doscan(void *data)
 	 * take up to 200 milliseconds (in the case where all chunks
 	 * are processed in a single pass) before it retires.
 	 */
-	wrmsrl(MSR_ACTIVATE_SCAN, msrs[0]);
+	wrmsrl(MSR_ACTIVATE_SCAN, params->activate->data);
+	rdmsrl(MSR_SCAN_STATUS, status.data);
 
-	if (cpu == first) {
-		/* Pass back the result of the scan */
-		rdmsrl(MSR_SCAN_STATUS, msrs[1]);
-	}
+	trace_ifs_status(start, stop, status.data);
+
+	/* Pass back the result of the scan */
+	if (cpu == first)
+		params->status = status;
 
 	return 0;
 }
@@ -179,7 +199,7 @@ static void ifs_test_core(int cpu, struct device *dev)
 	struct ifs_data *ifsd;
 	int to_start, to_stop;
 	int status_chunk;
-	u64 msrvals[2];
+	struct run_params params;
 	int retries;
 
 	ifsd = ifs_get_data(dev);
@@ -190,6 +210,8 @@ static void ifs_test_core(int cpu, struct device *dev)
 	to_start = 0;
 	to_stop = ifsd->valid_chunks - 1;
 
+	params.ifsd = ifs_get_data(dev);
+
 	if (ifsd->generation) {
 		activate.gen2.start = to_start;
 		activate.gen2.stop = to_stop;
@@ -207,12 +229,10 @@ static void ifs_test_core(int cpu, struct device *dev)
 			break;
 		}
 
-		msrvals[0] = activate.data;
-		stop_core_cpuslocked(cpu, doscan, msrvals);
-
-		status.data = msrvals[1];
+		params.activate = &activate;
+		stop_core_cpuslocked(cpu, doscan, &params);
 
-		trace_ifs_status(cpu, to_start, to_stop, status.data);
+		status = params.status;
 
 		/* Some cases can be retried, give up for others */
 		if (!can_restart(status))
-- 
2.39.2

[PATCH 3/5] platform/x86/intel/ifs: Add current batch number to trace output

[Ashok Raj]

Add the current batch number in the trace output. When there are failures,
it's important to know which test content resulted in failure.

#           TASK-PID     CPU#  |||||  TIMESTAMP  FUNCTION
#              | |         |   |||||     |         |
     migration/0-18      [000] d..1. 527287.084668: ifs_status: batch: 02, start: 0000, stop: 007f, status: 0000000000007f80
   migration/128-785     [128] d..1. 527287.084669: ifs_status: batch: 02, start: 0000, stop: 007f, status: 0000000000007f80

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
---
 include/trace/events/intel_ifs.h         | 9 ++++++---
 drivers/platform/x86/intel/ifs/runtest.c | 2 +-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/include/trace/events/intel_ifs.h b/include/trace/events/intel_ifs.h
index 8462dfb7a020..8ce2de120f2d 100644
--- a/include/trace/events/intel_ifs.h
+++ b/include/trace/events/intel_ifs.h
@@ -10,23 +10,26 @@
 
 TRACE_EVENT(ifs_status,
 
-	TP_PROTO(int start, int stop, u64 status),
+	TP_PROTO(int batch, int start, int stop, u64 status),
 
-	TP_ARGS(start, stop, status),
+	TP_ARGS(batch, start, stop, status),
 
 	TP_STRUCT__entry(
+		__field(	int,	batch	)
 		__field(	u64,	status	)
 		__field(	u16,	start	)
 		__field(	u16,	stop	)
 	),
 
 	TP_fast_assign(
+		__entry->batch	= batch;
 		__entry->start	= start;
 		__entry->stop	= stop;
 		__entry->status	= status;
 	),
 
-	TP_printk("start: %.4x, stop: %.4x, status: %.16llx",
+	TP_printk("batch: %.2d, start: %.4x, stop: %.4x, status: %.16llx",
+		__entry->batch,
 		__entry->start,
 		__entry->stop,
 		__entry->status)
diff --git a/drivers/platform/x86/intel/ifs/runtest.c b/drivers/platform/x86/intel/ifs/runtest.c
index c8352ffb9195..21dc0046fd9b 100644
--- a/drivers/platform/x86/intel/ifs/runtest.c
+++ b/drivers/platform/x86/intel/ifs/runtest.c
@@ -176,7 +176,7 @@ static int doscan(void *data)
 	wrmsrl(MSR_ACTIVATE_SCAN, params->activate->data);
 	rdmsrl(MSR_SCAN_STATUS, status.data);
 
-	trace_ifs_status(start, stop, status.data);
+	trace_ifs_status(ifsd->cur_batch, start, stop, status.data);
 
 	/* Pass back the result of the scan */
 	if (cpu == first)
-- 
2.39.2

[PATCH 4/5] platform/x86/intel/ifs: Replace the exit rendezvous with an entry rendezvous for ARRAY_BIST

[Ashok Raj]

ARRAY_BIST requires the test to be invoked only from one of the HT
siblings of a core.  If the other sibling was in mwait(), that didn't permit
the test to complete and resulted in several retries before the test could
finish.

The exit rendezvous was introduced to keep the HT sibling busy until the primary
CPU completed the test to avoid those retries. What is actually needed is
to ensure that both the threads rendezvous *before* the wrmsr to trigger the
test to give good chance to complete the test.

The `stop_machine()` function returns only after all the CPUs complete
running the function, and provides an exit rendezvous implicitly.

In kernel/stop_machine.c::multi_cpu_stop(), every CPU in the mask
needs to complete reaching MULTI_STOP_RUN. When all CPUs complete, the
state machine moves to next state, i.e MULTI_STOP_EXIT. Thus the underlying API
stop_core_cpuslocked() already provides an exit rendezvous.

Add the rendezvous earlier in order to  ensure the wrmsr is triggered after all
CPUs reach the do_array_test(). Remove the exit rendezvous since
stop_core_cpuslocked() already gaurantees that.

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
---
 drivers/platform/x86/intel/ifs/runtest.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/intel/ifs/runtest.c b/drivers/platform/x86/intel/ifs/runtest.c
index 21dc0046fd9b..e3307dd8e3c4 100644
--- a/drivers/platform/x86/intel/ifs/runtest.c
+++ b/drivers/platform/x86/intel/ifs/runtest.c
@@ -271,7 +271,7 @@ static void ifs_test_core(int cpu, struct device *dev)
 }
 
 #define SPINUNIT 100 /* 100 nsec */
-static atomic_t array_cpus_out;
+static atomic_t array_cpus_in;
 
 /*
  * Simplified cpu sibling rendezvous loop based on microcode loader __wait_for_cpus()
@@ -298,6 +298,8 @@ static int do_array_test(void *data)
 	int cpu = smp_processor_id();
 	int first;
 
+	wait_for_sibling_cpu(&array_cpus_in, NSEC_PER_SEC);
+
 	/*
 	 * Only one logical CPU on a core needs to trigger the Array test via MSR write.
 	 */
@@ -309,9 +311,6 @@ static int do_array_test(void *data)
 		rdmsrl(MSR_ARRAY_BIST, command->data);
 	}
 
-	/* Tests complete faster if the sibling is spinning here */
-	wait_for_sibling_cpu(&array_cpus_out, NSEC_PER_SEC);
-
 	return 0;
 }
 
@@ -332,7 +331,7 @@ static void ifs_array_test_core(int cpu, struct device *dev)
 			timed_out = true;
 			break;
 		}
-		atomic_set(&array_cpus_out, 0);
+		atomic_set(&array_cpus_in, 0);
 		stop_core_cpuslocked(cpu, do_array_test, &command);
 
 		if (command.ctrl_result)
-- 
2.39.2

[PATCH 5/5] platform/x86/intel/ifs: Add an entry rendezvous for SAF

[Ashok Raj]

The activation for SAF includes a parameter to make microcode wait for both
threads to join. It's preferable to perform an entry rendezvous before
the activation to ensure that they start the `wrmsr` close enough to each
other. In some cases it has been observed that one of the threads might be
just a bit late to arrive. An entry rendezvous reduces the likelihood of
these cases occurring.

Add an entry rendezvous to ensure the activation on both threads happen
close enough to each other.

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
---
 drivers/platform/x86/intel/ifs/runtest.c | 48 +++++++++++++-----------
 1 file changed, 26 insertions(+), 22 deletions(-)

diff --git a/drivers/platform/x86/intel/ifs/runtest.c b/drivers/platform/x86/intel/ifs/runtest.c
index e3307dd8e3c4..95b4b71fab53 100644
--- a/drivers/platform/x86/intel/ifs/runtest.c
+++ b/drivers/platform/x86/intel/ifs/runtest.c
@@ -140,6 +140,29 @@ static bool can_restart(union ifs_status status)
 	return false;
 }
 
+#define SPINUNIT 100 /* 100 nsec */
+static atomic_t array_cpus_in;
+static atomic_t scan_cpus_in;
+
+/*
+ * Simplified cpu sibling rendezvous loop based on microcode loader __wait_for_cpus()
+ */
+static void wait_for_sibling_cpu(atomic_t *t, long long timeout)
+{
+	int cpu = smp_processor_id();
+	const struct cpumask *smt_mask = cpu_smt_mask(cpu);
+	int all_cpus = cpumask_weight(smt_mask);
+
+	atomic_inc(t);
+	while (atomic_read(t) < all_cpus) {
+		if (timeout < SPINUNIT)
+			return;
+		ndelay(SPINUNIT);
+		timeout -= SPINUNIT;
+		touch_nmi_watchdog();
+	}
+}
+
 /*
  * Execute the scan. Called "simultaneously" on all threads of a core
  * at high priority using the stop_cpus mechanism.
@@ -165,6 +188,8 @@ static int doscan(void *data)
 	/* Only the first logical CPU on a core reports result */
 	first = cpumask_first(cpu_smt_mask(cpu));
 
+	wait_for_sibling_cpu(&scan_cpus_in, NSEC_PER_SEC);
+
 	/*
 	 * This WRMSR will wait for other HT threads to also write
 	 * to this MSR (at most for activate.delay cycles). Then it
@@ -230,6 +255,7 @@ static void ifs_test_core(int cpu, struct device *dev)
 		}
 
 		params.activate = &activate;
+		atomic_set(&scan_cpus_in, 0);
 		stop_core_cpuslocked(cpu, doscan, &params);
 
 		status = params.status;
@@ -270,28 +296,6 @@ static void ifs_test_core(int cpu, struct device *dev)
 	}
 }
 
-#define SPINUNIT 100 /* 100 nsec */
-static atomic_t array_cpus_in;
-
-/*
- * Simplified cpu sibling rendezvous loop based on microcode loader __wait_for_cpus()
- */
-static void wait_for_sibling_cpu(atomic_t *t, long long timeout)
-{
-	int cpu = smp_processor_id();
-	const struct cpumask *smt_mask = cpu_smt_mask(cpu);
-	int all_cpus = cpumask_weight(smt_mask);
-
-	atomic_inc(t);
-	while (atomic_read(t) < all_cpus) {
-		if (timeout < SPINUNIT)
-			return;
-		ndelay(SPINUNIT);
-		timeout -= SPINUNIT;
-		touch_nmi_watchdog();
-	}
-}
-
 static int do_array_test(void *data)
 {
 	union ifs_array *command = data;
-- 
2.39.2

Re: [PATCH 0/5] Miscelleanous fixes and improvements to Infield Scan

[Ilpo Järvinen]

On Thu, 25 Jan 2024, Ashok Raj wrote:

> Hi Hans,
> 
> This series has some bug fixes and improvements for Infield Scan.
> 
> Patch1 - fixes a bug in the driver that didn't release_firmware() in the
> 	 error path.
> 
> Patch2, 3 - Added some improved tracing.
> 
> Patch4 - Moves the rendezvous to function entry
> Patch5 - Adds entry rendezvous for IFS.
> 
> Cheers,
> Ashok
> 
> 
> Ashok Raj (4):
>   platform/x86/intel/ifs: Trace on all HT threads when executing a test
>   platform/x86/intel/ifs: Add current batch number to trace output
>   platform/x86/intel/ifs: Replace the exit rendezvous with an entry rendezvous for ARRAY_BIST
>   platform/x86/intel/ifs: Add an entry rendezvous for SAF

Patches 2-5 applied to review-ilpo.

-- 
 i.