[PATCH v2 0/4] Add SBAF test to IFS

[PATCH v2 0/4] Add SBAF test to IFS

[Kuppuswamy Sathyanarayanan]

This patch series adds support for Structural Based Functional Test at
Field (SBAF) in the IFS driver. SBAF is a new type of testing that
provides comprehensive core test coverage, complementing existing IFS
tests like Scan at Field (SAF) and ArrayBist. Granite Rapids (GNR) is
the first platform that supports SBAF.

SBAF mimics the manufacturing screening environment and leverages the
same test suite. It makes use of Design For Test (DFT) observation
sites and features to maximize coverage in minimum time.

Similar to the SAF test, SBAF isolates the core under test from the
rest of the system during execution. Upon completion, the core
seamlessly resets to its pre-test state and resumes normal operation.
Any machine checks or hangs encountered during the test are confined to
the isolated core, preventing disruption to the overall system. Like
SAF test, the SBAF test is also divided into multiple batches, and each
batch test can take hundreds of milliseconds (100-200 ms) to complete.
If such a lengthy interruption is undesirable, it is recommended to
relocate the time-sensitive applications to other cores for the
duration of the test.

Patch Details:

Patch 1/4: Refactors MSR usage in IFS image loading code to share the
           code between SBAF and SAF tests.
Patch 2/4: Leverages SAF image loading logic and adds SBAF image loading support.
Patch 3/4: Adds support for user to trigger SBAF test.
Patch 4/4: Adds trace support for SBAF tests.

This series is originally authored by Jithu Joseph. I have made cleanups
related to code reuse between the SBAF and SAF tests and resubmitting it for
review.

Changes since v1:
 * Addressed trace struct hole issue (Steven)
 * Fixed initialization issue in ifs_sbaf_test_core() (Ilpo)

Jithu Joseph (3):
  platform/x86/intel/ifs: Add SBAF test image loading support
  platform/x86/intel/ifs: Add SBAF test support
  trace: platform/x86/intel/ifs: Add SBAF trace support

Kuppuswamy Sathyanarayanan (1):
  platform/x86/intel/ifs: Refactor MSR usage in IFS test code

 arch/x86/include/asm/msr-index.h         |   2 +
 drivers/platform/x86/intel/ifs/ifs.h     |  92 ++++++++-
 include/trace/events/intel_ifs.h         |  27 +++
 drivers/platform/x86/intel/ifs/core.c    |  33 ++++
 drivers/platform/x86/intel/ifs/load.c    |  39 ++--
 drivers/platform/x86/intel/ifs/runtest.c | 235 +++++++++++++++++++++++
 6 files changed, 413 insertions(+), 15 deletions(-)

-- 
2.25.1

[PATCH v2 1/4] platform/x86/intel/ifs: Refactor MSR usage in IFS test code

[Kuppuswamy Sathyanarayanan]

IFS tests such as Scan at Field (SAF) or Structural Based Functional
Test at Field (SBAF), require the user to load a test image. The image
loading process is similar across these tests, with the only difference
being MSR addresses used. To reuse the code between these tests, remove
the hard coding of MSR addresses and allow the driver to pass the MSR
addresses per IFS test (via driver device data).

Add a new structure named "struct ifs_test_msrs" to specify the
test-specific MSR addresses. Each IFS test will provide this structure,
enabling them to reuse the common code.

This is a preliminary patch in preparation for the addition of SBAF
support.

Reviewed-by: Ashok Raj <ashok.raj@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 drivers/platform/x86/intel/ifs/ifs.h  | 25 +++++++++++++++++++++++++
 drivers/platform/x86/intel/ifs/core.c |  9 +++++++++
 drivers/platform/x86/intel/ifs/load.c | 24 ++++++++++++++----------
 3 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
index 56b9f3e3cf76..738cbc7a5d00 100644
--- a/drivers/platform/x86/intel/ifs/ifs.h
+++ b/drivers/platform/x86/intel/ifs/ifs.h
@@ -266,6 +266,22 @@ struct ifs_test_caps {
 	int	test_num;
 };
 
+/**
+ * struct ifs_test_msrs - MSRs used in IFS tests
+ * @copy_hashes: Copy test hash data
+ * @copy_hashes_status: Status of copied test hash data
+ * @copy_chunks: Copy chunks of the test data
+ * @copy_chunks_status: Status of the copied test data chunks
+ * @test_ctrl: Control the test attributes
+ */
+struct ifs_test_msrs {
+	u32	copy_hashes;
+	u32	copy_hashes_status;
+	u32	copy_chunks;
+	u32	copy_chunks_status;
+	u32	test_ctrl;
+};
+
 /**
  * struct ifs_data - attributes related to intel IFS driver
  * @loaded_version: stores the currently loaded ifs image version.
@@ -299,6 +315,7 @@ struct ifs_work {
 
 struct ifs_device {
 	const struct ifs_test_caps *test_caps;
+	const struct ifs_test_msrs *test_msrs;
 	struct ifs_data rw_data;
 	struct miscdevice misc;
 };
@@ -319,6 +336,14 @@ static inline const struct ifs_test_caps *ifs_get_test_caps(struct device *dev)
 	return d->test_caps;
 }
 
+static inline const struct ifs_test_msrs *ifs_get_test_msrs(struct device *dev)
+{
+	struct miscdevice *m = dev_get_drvdata(dev);
+	struct ifs_device *d = container_of(m, struct ifs_device, misc);
+
+	return d->test_msrs;
+}
+
 extern bool *ifs_pkg_auth;
 int ifs_load_firmware(struct device *dev);
 int do_core_test(int cpu, struct device *dev);
diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
index 7b11198d85a1..1a7ca74abb61 100644
--- a/drivers/platform/x86/intel/ifs/core.c
+++ b/drivers/platform/x86/intel/ifs/core.c
@@ -40,9 +40,18 @@ static const struct ifs_test_caps array_test = {
 	.test_num = IFS_TYPE_ARRAY_BIST,
 };
 
+static const struct ifs_test_msrs scan_msrs = {
+	.copy_hashes = MSR_COPY_SCAN_HASHES,
+	.copy_hashes_status = MSR_SCAN_HASHES_STATUS,
+	.copy_chunks = MSR_AUTHENTICATE_AND_COPY_CHUNK,
+	.copy_chunks_status = MSR_CHUNKS_AUTHENTICATION_STATUS,
+	.test_ctrl = MSR_SAF_CTRL,
+};
+
 static struct ifs_device ifs_devices[] = {
 	[IFS_TYPE_SAF] = {
 		.test_caps = &scan_test,
+		.test_msrs = &scan_msrs,
 		.misc = {
 			.name = "intel_ifs_0",
 			.minor = MISC_DYNAMIC_MINOR,
diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
index 39f19cb51749..ad0c107f0922 100644
--- a/drivers/platform/x86/intel/ifs/load.c
+++ b/drivers/platform/x86/intel/ifs/load.c
@@ -118,15 +118,17 @@ static void copy_hashes_authenticate_chunks(struct work_struct *work)
 	union ifs_scan_hashes_status hashes_status;
 	union ifs_chunks_auth_status chunk_status;
 	struct device *dev = local_work->dev;
+	const struct ifs_test_msrs *msrs;
 	int i, num_chunks, chunk_size;
 	struct ifs_data *ifsd;
 	u64 linear_addr, base;
 	u32 err_code;
 
 	ifsd = ifs_get_data(dev);
+	msrs = ifs_get_test_msrs(dev);
 	/* run scan hash copy */
-	wrmsrl(MSR_COPY_SCAN_HASHES, ifs_hash_ptr);
-	rdmsrl(MSR_SCAN_HASHES_STATUS, hashes_status.data);
+	wrmsrl(msrs->copy_hashes, ifs_hash_ptr);
+	rdmsrl(msrs->copy_hashes_status, hashes_status.data);
 
 	/* enumerate the scan image information */
 	num_chunks = hashes_status.num_chunks;
@@ -147,8 +149,8 @@ static void copy_hashes_authenticate_chunks(struct work_struct *work)
 		linear_addr = base + i * chunk_size;
 		linear_addr |= i;
 
-		wrmsrl(MSR_AUTHENTICATE_AND_COPY_CHUNK, linear_addr);
-		rdmsrl(MSR_CHUNKS_AUTHENTICATION_STATUS, chunk_status.data);
+		wrmsrl(msrs->copy_chunks, linear_addr);
+		rdmsrl(msrs->copy_chunks_status, chunk_status.data);
 
 		ifsd->valid_chunks = chunk_status.valid_chunks;
 		err_code = chunk_status.error_code;
@@ -180,6 +182,7 @@ static int copy_hashes_authenticate_chunks_gen2(struct device *dev)
 	union ifs_scan_hashes_status_gen2 hashes_status;
 	union ifs_chunks_auth_status_gen2 chunk_status;
 	u32 err_code, valid_chunks, total_chunks;
+	const struct ifs_test_msrs *msrs;
 	int i, num_chunks, chunk_size;
 	union meta_data *ifs_meta;
 	int starting_chunk_nr;
@@ -189,10 +192,11 @@ static int copy_hashes_authenticate_chunks_gen2(struct device *dev)
 	int retry_count;
 
 	ifsd = ifs_get_data(dev);
+	msrs = ifs_get_test_msrs(dev);
 
 	if (need_copy_scan_hashes(ifsd)) {
-		wrmsrl(MSR_COPY_SCAN_HASHES, ifs_hash_ptr);
-		rdmsrl(MSR_SCAN_HASHES_STATUS, hashes_status.data);
+		wrmsrl(msrs->copy_hashes, ifs_hash_ptr);
+		rdmsrl(msrs->copy_hashes_status, hashes_status.data);
 
 		/* enumerate the scan image information */
 		chunk_size = hashes_status.chunk_size * SZ_1K;
@@ -212,8 +216,8 @@ static int copy_hashes_authenticate_chunks_gen2(struct device *dev)
 	}
 
 	if (ifsd->generation >= IFS_GEN_STRIDE_AWARE) {
-		wrmsrl(MSR_SAF_CTRL, INVALIDATE_STRIDE);
-		rdmsrl(MSR_CHUNKS_AUTHENTICATION_STATUS, chunk_status.data);
+		wrmsrl(msrs->test_ctrl, INVALIDATE_STRIDE);
+		rdmsrl(msrs->copy_chunks_status, chunk_status.data);
 		if (chunk_status.valid_chunks != 0) {
 			dev_err(dev, "Couldn't invalidate installed stride - %d\n",
 				chunk_status.valid_chunks);
@@ -234,9 +238,9 @@ static int copy_hashes_authenticate_chunks_gen2(struct device *dev)
 		chunk_table[1] = linear_addr;
 		do {
 			local_irq_disable();
-			wrmsrl(MSR_AUTHENTICATE_AND_COPY_CHUNK, (u64)chunk_table);
+			wrmsrl(msrs->copy_chunks, (u64)chunk_table);
 			local_irq_enable();
-			rdmsrl(MSR_CHUNKS_AUTHENTICATION_STATUS, chunk_status.data);
+			rdmsrl(msrs->copy_chunks_status, chunk_status.data);
 			err_code = chunk_status.error_code;
 		} while (err_code == AUTH_INTERRUPTED_ERROR && --retry_count);
 
-- 
2.25.1

Re: [PATCH v2 1/4] platform/x86/intel/ifs: Refactor MSR usage in IFS test code

[Ilpo Järvinen]

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

On Wed, 10 Jul 2024, Kuppuswamy Sathyanarayanan wrote:

> IFS tests such as Scan at Field (SAF) or Structural Based Functional
> Test at Field (SBAF), require the user to load a test image. The image
> loading process is similar across these tests, with the only difference
> being MSR addresses used. To reuse the code between these tests, remove
> the hard coding of MSR addresses and allow the driver to pass the MSR
> addresses per IFS test (via driver device data).
> 
> Add a new structure named "struct ifs_test_msrs" to specify the
> test-specific MSR addresses. Each IFS test will provide this structure,
> enabling them to reuse the common code.
> 
> This is a preliminary patch in preparation for the addition of SBAF
> support.
> 
> Reviewed-by: Ashok Raj <ashok.raj@intel.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

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

-- 
 i.

> ---
>  drivers/platform/x86/intel/ifs/ifs.h  | 25 +++++++++++++++++++++++++
>  drivers/platform/x86/intel/ifs/core.c |  9 +++++++++
>  drivers/platform/x86/intel/ifs/load.c | 24 ++++++++++++++----------
>  3 files changed, 48 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
> index 56b9f3e3cf76..738cbc7a5d00 100644
> --- a/drivers/platform/x86/intel/ifs/ifs.h
> +++ b/drivers/platform/x86/intel/ifs/ifs.h
> @@ -266,6 +266,22 @@ struct ifs_test_caps {
>  	int	test_num;
>  };
>  
> +/**
> + * struct ifs_test_msrs - MSRs used in IFS tests
> + * @copy_hashes: Copy test hash data
> + * @copy_hashes_status: Status of copied test hash data
> + * @copy_chunks: Copy chunks of the test data
> + * @copy_chunks_status: Status of the copied test data chunks
> + * @test_ctrl: Control the test attributes
> + */
> +struct ifs_test_msrs {
> +	u32	copy_hashes;
> +	u32	copy_hashes_status;
> +	u32	copy_chunks;
> +	u32	copy_chunks_status;
> +	u32	test_ctrl;
> +};
> +
>  /**
>   * struct ifs_data - attributes related to intel IFS driver
>   * @loaded_version: stores the currently loaded ifs image version.
> @@ -299,6 +315,7 @@ struct ifs_work {
>  
>  struct ifs_device {
>  	const struct ifs_test_caps *test_caps;
> +	const struct ifs_test_msrs *test_msrs;
>  	struct ifs_data rw_data;
>  	struct miscdevice misc;
>  };
> @@ -319,6 +336,14 @@ static inline const struct ifs_test_caps *ifs_get_test_caps(struct device *dev)
>  	return d->test_caps;
>  }
>  
> +static inline const struct ifs_test_msrs *ifs_get_test_msrs(struct device *dev)
> +{
> +	struct miscdevice *m = dev_get_drvdata(dev);
> +	struct ifs_device *d = container_of(m, struct ifs_device, misc);
> +
> +	return d->test_msrs;
> +}
> +
>  extern bool *ifs_pkg_auth;
>  int ifs_load_firmware(struct device *dev);
>  int do_core_test(int cpu, struct device *dev);
> diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
> index 7b11198d85a1..1a7ca74abb61 100644
> --- a/drivers/platform/x86/intel/ifs/core.c
> +++ b/drivers/platform/x86/intel/ifs/core.c
> @@ -40,9 +40,18 @@ static const struct ifs_test_caps array_test = {
>  	.test_num = IFS_TYPE_ARRAY_BIST,
>  };
>  
> +static const struct ifs_test_msrs scan_msrs = {
> +	.copy_hashes = MSR_COPY_SCAN_HASHES,
> +	.copy_hashes_status = MSR_SCAN_HASHES_STATUS,
> +	.copy_chunks = MSR_AUTHENTICATE_AND_COPY_CHUNK,
> +	.copy_chunks_status = MSR_CHUNKS_AUTHENTICATION_STATUS,
> +	.test_ctrl = MSR_SAF_CTRL,
> +};
> +
>  static struct ifs_device ifs_devices[] = {
>  	[IFS_TYPE_SAF] = {
>  		.test_caps = &scan_test,
> +		.test_msrs = &scan_msrs,
>  		.misc = {
>  			.name = "intel_ifs_0",
>  			.minor = MISC_DYNAMIC_MINOR,
> diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
> index 39f19cb51749..ad0c107f0922 100644
> --- a/drivers/platform/x86/intel/ifs/load.c
> +++ b/drivers/platform/x86/intel/ifs/load.c
> @@ -118,15 +118,17 @@ static void copy_hashes_authenticate_chunks(struct work_struct *work)
>  	union ifs_scan_hashes_status hashes_status;
>  	union ifs_chunks_auth_status chunk_status;
>  	struct device *dev = local_work->dev;
> +	const struct ifs_test_msrs *msrs;
>  	int i, num_chunks, chunk_size;
>  	struct ifs_data *ifsd;
>  	u64 linear_addr, base;
>  	u32 err_code;
>  
>  	ifsd = ifs_get_data(dev);
> +	msrs = ifs_get_test_msrs(dev);
>  	/* run scan hash copy */
> -	wrmsrl(MSR_COPY_SCAN_HASHES, ifs_hash_ptr);
> -	rdmsrl(MSR_SCAN_HASHES_STATUS, hashes_status.data);
> +	wrmsrl(msrs->copy_hashes, ifs_hash_ptr);
> +	rdmsrl(msrs->copy_hashes_status, hashes_status.data);
>  
>  	/* enumerate the scan image information */
>  	num_chunks = hashes_status.num_chunks;
> @@ -147,8 +149,8 @@ static void copy_hashes_authenticate_chunks(struct work_struct *work)
>  		linear_addr = base + i * chunk_size;
>  		linear_addr |= i;
>  
> -		wrmsrl(MSR_AUTHENTICATE_AND_COPY_CHUNK, linear_addr);
> -		rdmsrl(MSR_CHUNKS_AUTHENTICATION_STATUS, chunk_status.data);
> +		wrmsrl(msrs->copy_chunks, linear_addr);
> +		rdmsrl(msrs->copy_chunks_status, chunk_status.data);
>  
>  		ifsd->valid_chunks = chunk_status.valid_chunks;
>  		err_code = chunk_status.error_code;
> @@ -180,6 +182,7 @@ static int copy_hashes_authenticate_chunks_gen2(struct device *dev)
>  	union ifs_scan_hashes_status_gen2 hashes_status;
>  	union ifs_chunks_auth_status_gen2 chunk_status;
>  	u32 err_code, valid_chunks, total_chunks;
> +	const struct ifs_test_msrs *msrs;
>  	int i, num_chunks, chunk_size;
>  	union meta_data *ifs_meta;
>  	int starting_chunk_nr;
> @@ -189,10 +192,11 @@ static int copy_hashes_authenticate_chunks_gen2(struct device *dev)
>  	int retry_count;
>  
>  	ifsd = ifs_get_data(dev);
> +	msrs = ifs_get_test_msrs(dev);
>  
>  	if (need_copy_scan_hashes(ifsd)) {
> -		wrmsrl(MSR_COPY_SCAN_HASHES, ifs_hash_ptr);
> -		rdmsrl(MSR_SCAN_HASHES_STATUS, hashes_status.data);
> +		wrmsrl(msrs->copy_hashes, ifs_hash_ptr);
> +		rdmsrl(msrs->copy_hashes_status, hashes_status.data);
>  
>  		/* enumerate the scan image information */
>  		chunk_size = hashes_status.chunk_size * SZ_1K;
> @@ -212,8 +216,8 @@ static int copy_hashes_authenticate_chunks_gen2(struct device *dev)
>  	}
>  
>  	if (ifsd->generation >= IFS_GEN_STRIDE_AWARE) {
> -		wrmsrl(MSR_SAF_CTRL, INVALIDATE_STRIDE);
> -		rdmsrl(MSR_CHUNKS_AUTHENTICATION_STATUS, chunk_status.data);
> +		wrmsrl(msrs->test_ctrl, INVALIDATE_STRIDE);
> +		rdmsrl(msrs->copy_chunks_status, chunk_status.data);
>  		if (chunk_status.valid_chunks != 0) {
>  			dev_err(dev, "Couldn't invalidate installed stride - %d\n",
>  				chunk_status.valid_chunks);
> @@ -234,9 +238,9 @@ static int copy_hashes_authenticate_chunks_gen2(struct device *dev)
>  		chunk_table[1] = linear_addr;
>  		do {
>  			local_irq_disable();
> -			wrmsrl(MSR_AUTHENTICATE_AND_COPY_CHUNK, (u64)chunk_table);
> +			wrmsrl(msrs->copy_chunks, (u64)chunk_table);
>  			local_irq_enable();
> -			rdmsrl(MSR_CHUNKS_AUTHENTICATION_STATUS, chunk_status.data);
> +			rdmsrl(msrs->copy_chunks_status, chunk_status.data);
>  			err_code = chunk_status.error_code;
>  		} while (err_code == AUTH_INTERRUPTED_ERROR && --retry_count);
>  
> 

[PATCH v2 2/4] platform/x86/intel/ifs: Add SBAF test image loading support

[Kuppuswamy Sathyanarayanan]

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

Structural Based Functional Test at Field (SBAF) is a new type of
testing that provides comprehensive core test coverage complementing
existing IFS tests like Scan at Field (SAF) or ArrayBist.

SBAF device will appear as a new device instance (intel_ifs_2) under
/sys/devices/virtual/misc. The user interaction necessary to load the
test image and test a particular core is the same as the existing scan
test (intel_ifs_0).

During the loading stage, the driver will look for a file named
ff-mm-ss-<batch02x>.sbft in the /lib/firmware/intel/ifs_2 directory.
The hardware interaction needed for loading the image is similar to
SAF, with the only difference being the MSR addresses used. Reuse the
SAF image loading code, passing the SBAF-specific MSR addresses via
struct ifs_test_msrs in the driver device data.

Unlike SAF, the SBAF test image chunks are further divided into smaller
logical entities called bundles. Since the SBAF test is initiated per
bundle, cache the maximum number of bundles in the current image, which
is used for iterating through bundles during SBAF test execution.

Reviewed-by: Ashok Raj <ashok.raj@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
Co-developed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 arch/x86/include/asm/msr-index.h      |  2 ++
 drivers/platform/x86/intel/ifs/ifs.h  | 37 ++++++++++++++++++++++++++-
 drivers/platform/x86/intel/ifs/core.c | 24 +++++++++++++++++
 drivers/platform/x86/intel/ifs/load.c | 15 ++++++++---
 4 files changed, 73 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index e022e6eb766c..503d7acdda3f 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -247,6 +247,8 @@
 #define MSR_INTEGRITY_CAPS_ARRAY_BIST          BIT(MSR_INTEGRITY_CAPS_ARRAY_BIST_BIT)
 #define MSR_INTEGRITY_CAPS_PERIODIC_BIST_BIT	4
 #define MSR_INTEGRITY_CAPS_PERIODIC_BIST	BIT(MSR_INTEGRITY_CAPS_PERIODIC_BIST_BIT)
+#define MSR_INTEGRITY_CAPS_SBAF_BIT		8
+#define MSR_INTEGRITY_CAPS_SBAF			BIT(MSR_INTEGRITY_CAPS_SBAF_BIT)
 #define MSR_INTEGRITY_CAPS_SAF_GEN_MASK	GENMASK_ULL(10, 9)
 
 #define MSR_LBR_NHM_FROM		0x00000680
diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
index 738cbc7a5d00..600bb8a1b285 100644
--- a/drivers/platform/x86/intel/ifs/ifs.h
+++ b/drivers/platform/x86/intel/ifs/ifs.h
@@ -126,11 +126,38 @@
  * The driver does not make use of this, it only tests one core at a time.
  *
  * .. [#f1] https://github.com/intel/TBD
+ *
+ *
+ * Structural Based Functional Test at Field (SBAF):
+ * ------------------------------------------------
+ *
+ * SBAF is a new type of testing that provides comprehensive core test
+ * coverage complementing Scan at Field (SAF) testing. SBAF mimics the
+ * manufacturing screening environment and leverages the same test suite.
+ * It makes use of Design For Test (DFT) observation sites and features
+ * to maximize coverage in minimum time.
+ *
+ * Similar to the SAF test, SBAF isolates the core under test from the
+ * rest of the system during execution. Upon completion, the core
+ * seamlessly resets to its pre-test state and resumes normal operation.
+ * Any machine checks or hangs encountered during the test are confined to
+ * the isolated core, preventing disruption to the overall system.
+ *
+ * Like the SAF test, the SBAF test is also divided into multiple batches,
+ * and each batch test can take hundreds of milliseconds (100-200 ms) to
+ * complete. If such a lengthy interruption is undesirable, it is
+ * recommended to relocate the time-sensitive applications to other cores.
  */
 #include <linux/device.h>
 #include <linux/miscdevice.h>
 
 #define MSR_ARRAY_BIST				0x00000105
+
+#define MSR_COPY_SBAF_HASHES			0x000002b8
+#define MSR_SBAF_HASHES_STATUS			0x000002b9
+#define MSR_AUTHENTICATE_AND_COPY_SBAF_CHUNK	0x000002ba
+#define MSR_SBAF_CHUNKS_AUTHENTICATION_STATUS	0x000002bb
+
 #define MSR_COPY_SCAN_HASHES			0x000002c2
 #define MSR_SCAN_HASHES_STATUS			0x000002c3
 #define MSR_AUTHENTICATE_AND_COPY_CHUNK		0x000002c4
@@ -140,6 +167,7 @@
 #define MSR_ARRAY_TRIGGER			0x000002d6
 #define MSR_ARRAY_STATUS			0x000002d7
 #define MSR_SAF_CTRL				0x000004f0
+#define MSR_SBAF_CTRL				0x000004f8
 
 #define SCAN_NOT_TESTED				0
 #define SCAN_TEST_PASS				1
@@ -147,6 +175,7 @@
 
 #define IFS_TYPE_SAF			0
 #define IFS_TYPE_ARRAY_BIST		1
+#define IFS_TYPE_SBAF			2
 
 #define ARRAY_GEN0			0
 #define ARRAY_GEN1			1
@@ -196,7 +225,8 @@ union ifs_chunks_auth_status_gen2 {
 		u16	valid_chunks;
 		u16	total_chunks;
 		u32	error_code	:8;
-		u32	rsvd2		:24;
+		u32	rsvd2		:8;
+		u32	max_bundle	:16;
 	};
 };
 
@@ -261,9 +291,12 @@ union ifs_array {
 #define IFS_SW_TIMEOUT				0xFD
 #define IFS_SW_PARTIAL_COMPLETION		0xFE
 
+#define IFS_SUFFIX_SZ		5
+
 struct ifs_test_caps {
 	int	integrity_cap_bit;
 	int	test_num;
+	char	image_suffix[IFS_SUFFIX_SZ];
 };
 
 /**
@@ -294,6 +327,7 @@ struct ifs_test_msrs {
  * @generation: IFS test generation enumerated by hardware
  * @chunk_size: size of a test chunk
  * @array_gen: test generation of array test
+ * @max_bundle: maximum bundle index
  */
 struct ifs_data {
 	int	loaded_version;
@@ -306,6 +340,7 @@ struct ifs_data {
 	u32	generation;
 	u32	chunk_size;
 	u32	array_gen;
+	u32	max_bundle;
 };
 
 struct ifs_work {
diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
index 1a7ca74abb61..4f571b79f028 100644
--- a/drivers/platform/x86/intel/ifs/core.c
+++ b/drivers/platform/x86/intel/ifs/core.c
@@ -33,6 +33,7 @@ bool *ifs_pkg_auth;
 static const struct ifs_test_caps scan_test = {
 	.integrity_cap_bit = MSR_INTEGRITY_CAPS_PERIODIC_BIST_BIT,
 	.test_num = IFS_TYPE_SAF,
+	.image_suffix = "scan",
 };
 
 static const struct ifs_test_caps array_test = {
@@ -48,6 +49,20 @@ static const struct ifs_test_msrs scan_msrs = {
 	.test_ctrl = MSR_SAF_CTRL,
 };
 
+static const struct ifs_test_msrs sbaf_msrs = {
+	.copy_hashes = MSR_COPY_SBAF_HASHES,
+	.copy_hashes_status = MSR_SBAF_HASHES_STATUS,
+	.copy_chunks = MSR_AUTHENTICATE_AND_COPY_SBAF_CHUNK,
+	.copy_chunks_status = MSR_SBAF_CHUNKS_AUTHENTICATION_STATUS,
+	.test_ctrl = MSR_SBAF_CTRL,
+};
+
+static const struct ifs_test_caps sbaf_test = {
+	.integrity_cap_bit = MSR_INTEGRITY_CAPS_SBAF_BIT,
+	.test_num = IFS_TYPE_SBAF,
+	.image_suffix = "sbft",
+};
+
 static struct ifs_device ifs_devices[] = {
 	[IFS_TYPE_SAF] = {
 		.test_caps = &scan_test,
@@ -66,6 +81,15 @@ static struct ifs_device ifs_devices[] = {
 			.groups = plat_ifs_array_groups,
 		},
 	},
+	[IFS_TYPE_SBAF] = {
+		.test_caps = &sbaf_test,
+		.test_msrs = &sbaf_msrs,
+		.misc = {
+			.name = "intel_ifs_2",
+			.minor = MISC_DYNAMIC_MINOR,
+			.groups = plat_ifs_groups,
+		},
+	},
 };
 
 #define IFS_NUMTESTS ARRAY_SIZE(ifs_devices)
diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
index ad0c107f0922..8f2aded38f1c 100644
--- a/drivers/platform/x86/intel/ifs/load.c
+++ b/drivers/platform/x86/intel/ifs/load.c
@@ -261,20 +261,22 @@ static int copy_hashes_authenticate_chunks_gen2(struct device *dev)
 		return -EIO;
 	}
 	ifsd->valid_chunks = valid_chunks;
+	ifsd->max_bundle = chunk_status.max_bundle;
 
 	return 0;
 }
 
 static int validate_ifs_metadata(struct device *dev)
 {
+	const struct ifs_test_caps *test = ifs_get_test_caps(dev);
 	struct ifs_data *ifsd = ifs_get_data(dev);
 	union meta_data *ifs_meta;
 	char test_file[64];
 	int ret = -EINVAL;
 
-	snprintf(test_file, sizeof(test_file), "%02x-%02x-%02x-%02x.scan",
+	snprintf(test_file, sizeof(test_file), "%02x-%02x-%02x-%02x.%s",
 		 boot_cpu_data.x86, boot_cpu_data.x86_model,
-		 boot_cpu_data.x86_stepping, ifsd->cur_batch);
+		 boot_cpu_data.x86_stepping, ifsd->cur_batch, test->image_suffix);
 
 	ifs_meta = (union meta_data *)find_meta_data(ifs_header_ptr, META_TYPE_IFS);
 	if (!ifs_meta) {
@@ -304,6 +306,11 @@ static int validate_ifs_metadata(struct device *dev)
 		return ret;
 	}
 
+	if (ifs_meta->test_type != test->test_num) {
+		dev_warn(dev, "Metadata test_type %d mismatches with device type\n",
+			 ifs_meta->test_type);
+	}
+
 	return 0;
 }
 
@@ -391,9 +398,9 @@ int ifs_load_firmware(struct device *dev)
 	char scan_path[64];
 	int ret;
 
-	snprintf(scan_path, sizeof(scan_path), "intel/ifs_%d/%02x-%02x-%02x-%02x.scan",
+	snprintf(scan_path, sizeof(scan_path), "intel/ifs_%d/%02x-%02x-%02x-%02x.%s",
 		 test->test_num, boot_cpu_data.x86, boot_cpu_data.x86_model,
-		 boot_cpu_data.x86_stepping, ifsd->cur_batch);
+		 boot_cpu_data.x86_stepping, ifsd->cur_batch, test->image_suffix);
 
 	ret = request_firmware_direct(&fw, scan_path, dev);
 	if (ret) {
-- 
2.25.1

Re: [PATCH v2 2/4] platform/x86/intel/ifs: Add SBAF test image loading support

[Ilpo Järvinen]

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

On Wed, 10 Jul 2024, Kuppuswamy Sathyanarayanan wrote:

> From: Jithu Joseph <jithu.joseph@intel.com>
> 
> Structural Based Functional Test at Field (SBAF) is a new type of
> testing that provides comprehensive core test coverage complementing
> existing IFS tests like Scan at Field (SAF) or ArrayBist.
> 
> SBAF device will appear as a new device instance (intel_ifs_2) under
> /sys/devices/virtual/misc. The user interaction necessary to load the
> test image and test a particular core is the same as the existing scan
> test (intel_ifs_0).
> 
> During the loading stage, the driver will look for a file named
> ff-mm-ss-<batch02x>.sbft in the /lib/firmware/intel/ifs_2 directory.
> The hardware interaction needed for loading the image is similar to
> SAF, with the only difference being the MSR addresses used. Reuse the
> SAF image loading code, passing the SBAF-specific MSR addresses via
> struct ifs_test_msrs in the driver device data.
> 
> Unlike SAF, the SBAF test image chunks are further divided into smaller
> logical entities called bundles. Since the SBAF test is initiated per
> bundle, cache the maximum number of bundles in the current image, which
> is used for iterating through bundles during SBAF test execution.
> 
> Reviewed-by: Ashok Raj <ashok.raj@intel.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
> Co-developed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

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

Although the image_suffix change to the existing code could have been in 
own patch but this is simple enough change it doesn't hurt that much.

> ---
>  arch/x86/include/asm/msr-index.h      |  2 ++
>  drivers/platform/x86/intel/ifs/ifs.h  | 37 ++++++++++++++++++++++++++-
>  drivers/platform/x86/intel/ifs/core.c | 24 +++++++++++++++++
>  drivers/platform/x86/intel/ifs/load.c | 15 ++++++++---
>  4 files changed, 73 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index e022e6eb766c..503d7acdda3f 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -247,6 +247,8 @@
>  #define MSR_INTEGRITY_CAPS_ARRAY_BIST          BIT(MSR_INTEGRITY_CAPS_ARRAY_BIST_BIT)
>  #define MSR_INTEGRITY_CAPS_PERIODIC_BIST_BIT	4
>  #define MSR_INTEGRITY_CAPS_PERIODIC_BIST	BIT(MSR_INTEGRITY_CAPS_PERIODIC_BIST_BIT)
> +#define MSR_INTEGRITY_CAPS_SBAF_BIT		8
> +#define MSR_INTEGRITY_CAPS_SBAF			BIT(MSR_INTEGRITY_CAPS_SBAF_BIT)
>  #define MSR_INTEGRITY_CAPS_SAF_GEN_MASK	GENMASK_ULL(10, 9)
>  
>  #define MSR_LBR_NHM_FROM		0x00000680
> diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
> index 738cbc7a5d00..600bb8a1b285 100644
> --- a/drivers/platform/x86/intel/ifs/ifs.h
> +++ b/drivers/platform/x86/intel/ifs/ifs.h
> @@ -126,11 +126,38 @@
>   * The driver does not make use of this, it only tests one core at a time.
>   *
>   * .. [#f1] https://github.com/intel/TBD
> + *
> + *
> + * Structural Based Functional Test at Field (SBAF):
> + * ------------------------------------------------
> + *
> + * SBAF is a new type of testing that provides comprehensive core test
> + * coverage complementing Scan at Field (SAF) testing. SBAF mimics the
> + * manufacturing screening environment and leverages the same test suite.
> + * It makes use of Design For Test (DFT) observation sites and features
> + * to maximize coverage in minimum time.
> + *
> + * Similar to the SAF test, SBAF isolates the core under test from the
> + * rest of the system during execution. Upon completion, the core
> + * seamlessly resets to its pre-test state and resumes normal operation.
> + * Any machine checks or hangs encountered during the test are confined to
> + * the isolated core, preventing disruption to the overall system.
> + *
> + * Like the SAF test, the SBAF test is also divided into multiple batches,
> + * and each batch test can take hundreds of milliseconds (100-200 ms) to
> + * complete. If such a lengthy interruption is undesirable, it is
> + * recommended to relocate the time-sensitive applications to other cores.
>   */
>  #include <linux/device.h>
>  #include <linux/miscdevice.h>
>  
>  #define MSR_ARRAY_BIST				0x00000105
> +
> +#define MSR_COPY_SBAF_HASHES			0x000002b8
> +#define MSR_SBAF_HASHES_STATUS			0x000002b9
> +#define MSR_AUTHENTICATE_AND_COPY_SBAF_CHUNK	0x000002ba
> +#define MSR_SBAF_CHUNKS_AUTHENTICATION_STATUS	0x000002bb
> +
>  #define MSR_COPY_SCAN_HASHES			0x000002c2
>  #define MSR_SCAN_HASHES_STATUS			0x000002c3
>  #define MSR_AUTHENTICATE_AND_COPY_CHUNK		0x000002c4
> @@ -140,6 +167,7 @@
>  #define MSR_ARRAY_TRIGGER			0x000002d6
>  #define MSR_ARRAY_STATUS			0x000002d7
>  #define MSR_SAF_CTRL				0x000004f0
> +#define MSR_SBAF_CTRL				0x000004f8
>  
>  #define SCAN_NOT_TESTED				0
>  #define SCAN_TEST_PASS				1
> @@ -147,6 +175,7 @@
>  
>  #define IFS_TYPE_SAF			0
>  #define IFS_TYPE_ARRAY_BIST		1
> +#define IFS_TYPE_SBAF			2
>  
>  #define ARRAY_GEN0			0
>  #define ARRAY_GEN1			1
> @@ -196,7 +225,8 @@ union ifs_chunks_auth_status_gen2 {
>  		u16	valid_chunks;
>  		u16	total_chunks;
>  		u32	error_code	:8;
> -		u32	rsvd2		:24;
> +		u32	rsvd2		:8;
> +		u32	max_bundle	:16;
>  	};
>  };
>  
> @@ -261,9 +291,12 @@ union ifs_array {
>  #define IFS_SW_TIMEOUT				0xFD
>  #define IFS_SW_PARTIAL_COMPLETION		0xFE
>  
> +#define IFS_SUFFIX_SZ		5
> +
>  struct ifs_test_caps {
>  	int	integrity_cap_bit;
>  	int	test_num;
> +	char	image_suffix[IFS_SUFFIX_SZ];
>  };
>  
>  /**
> @@ -294,6 +327,7 @@ struct ifs_test_msrs {
>   * @generation: IFS test generation enumerated by hardware
>   * @chunk_size: size of a test chunk
>   * @array_gen: test generation of array test
> + * @max_bundle: maximum bundle index
>   */
>  struct ifs_data {
>  	int	loaded_version;
> @@ -306,6 +340,7 @@ struct ifs_data {
>  	u32	generation;
>  	u32	chunk_size;
>  	u32	array_gen;
> +	u32	max_bundle;
>  };
>  
>  struct ifs_work {
> diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
> index 1a7ca74abb61..4f571b79f028 100644
> --- a/drivers/platform/x86/intel/ifs/core.c
> +++ b/drivers/platform/x86/intel/ifs/core.c
> @@ -33,6 +33,7 @@ bool *ifs_pkg_auth;
>  static const struct ifs_test_caps scan_test = {
>  	.integrity_cap_bit = MSR_INTEGRITY_CAPS_PERIODIC_BIST_BIT,
>  	.test_num = IFS_TYPE_SAF,
> +	.image_suffix = "scan",
>  };
>  
>  static const struct ifs_test_caps array_test = {
> @@ -48,6 +49,20 @@ static const struct ifs_test_msrs scan_msrs = {
>  	.test_ctrl = MSR_SAF_CTRL,
>  };
>  
> +static const struct ifs_test_msrs sbaf_msrs = {
> +	.copy_hashes = MSR_COPY_SBAF_HASHES,
> +	.copy_hashes_status = MSR_SBAF_HASHES_STATUS,
> +	.copy_chunks = MSR_AUTHENTICATE_AND_COPY_SBAF_CHUNK,
> +	.copy_chunks_status = MSR_SBAF_CHUNKS_AUTHENTICATION_STATUS,
> +	.test_ctrl = MSR_SBAF_CTRL,
> +};
> +
> +static const struct ifs_test_caps sbaf_test = {
> +	.integrity_cap_bit = MSR_INTEGRITY_CAPS_SBAF_BIT,
> +	.test_num = IFS_TYPE_SBAF,
> +	.image_suffix = "sbft",
> +};
> +
>  static struct ifs_device ifs_devices[] = {
>  	[IFS_TYPE_SAF] = {
>  		.test_caps = &scan_test,
> @@ -66,6 +81,15 @@ static struct ifs_device ifs_devices[] = {
>  			.groups = plat_ifs_array_groups,
>  		},
>  	},
> +	[IFS_TYPE_SBAF] = {
> +		.test_caps = &sbaf_test,
> +		.test_msrs = &sbaf_msrs,
> +		.misc = {
> +			.name = "intel_ifs_2",
> +			.minor = MISC_DYNAMIC_MINOR,
> +			.groups = plat_ifs_groups,
> +		},
> +	},
>  };
>  
>  #define IFS_NUMTESTS ARRAY_SIZE(ifs_devices)
> diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
> index ad0c107f0922..8f2aded38f1c 100644
> --- a/drivers/platform/x86/intel/ifs/load.c
> +++ b/drivers/platform/x86/intel/ifs/load.c
> @@ -261,20 +261,22 @@ static int copy_hashes_authenticate_chunks_gen2(struct device *dev)
>  		return -EIO;
>  	}
>  	ifsd->valid_chunks = valid_chunks;
> +	ifsd->max_bundle = chunk_status.max_bundle;
>  
>  	return 0;
>  }
>  
>  static int validate_ifs_metadata(struct device *dev)
>  {
> +	const struct ifs_test_caps *test = ifs_get_test_caps(dev);
>  	struct ifs_data *ifsd = ifs_get_data(dev);
>  	union meta_data *ifs_meta;
>  	char test_file[64];
>  	int ret = -EINVAL;
>  
> -	snprintf(test_file, sizeof(test_file), "%02x-%02x-%02x-%02x.scan",
> +	snprintf(test_file, sizeof(test_file), "%02x-%02x-%02x-%02x.%s",
>  		 boot_cpu_data.x86, boot_cpu_data.x86_model,
> -		 boot_cpu_data.x86_stepping, ifsd->cur_batch);
> +		 boot_cpu_data.x86_stepping, ifsd->cur_batch, test->image_suffix);
>  
>  	ifs_meta = (union meta_data *)find_meta_data(ifs_header_ptr, META_TYPE_IFS);
>  	if (!ifs_meta) {
> @@ -304,6 +306,11 @@ static int validate_ifs_metadata(struct device *dev)
>  		return ret;
>  	}
>  
> +	if (ifs_meta->test_type != test->test_num) {
> +		dev_warn(dev, "Metadata test_type %d mismatches with device type\n",
> +			 ifs_meta->test_type);
> +	}
> +
>  	return 0;
>  }
>  
> @@ -391,9 +398,9 @@ int ifs_load_firmware(struct device *dev)
>  	char scan_path[64];
>  	int ret;
>  
> -	snprintf(scan_path, sizeof(scan_path), "intel/ifs_%d/%02x-%02x-%02x-%02x.scan",
> +	snprintf(scan_path, sizeof(scan_path), "intel/ifs_%d/%02x-%02x-%02x-%02x.%s",
>  		 test->test_num, boot_cpu_data.x86, boot_cpu_data.x86_model,
> -		 boot_cpu_data.x86_stepping, ifsd->cur_batch);
> +		 boot_cpu_data.x86_stepping, ifsd->cur_batch, test->image_suffix);
>  
>  	ret = request_firmware_direct(&fw, scan_path, dev);
>  	if (ret) {
> 

-- 
 i.

[PATCH v2 3/4] platform/x86/intel/ifs: Add SBAF test support

[Kuppuswamy Sathyanarayanan]

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

In a core, the SBAF test engine is shared between sibling CPUs.

An SBAF test image contains multiple bundles. Each bundle is further
composed of subunits called programs. When a SBAF test (for a particular
core) is triggered by the user, each SBAF bundle from the loaded test
image is executed sequentially on all the threads on the core using
the stop_core_cpuslocked mechanism. Each bundle execution is initiated by
writing to MSR_ACTIVATE_SBAF.

SBAF test bundle execution may be aborted when an interrupt occurs or
if the CPU does not have enough power budget for the test. In these
cases the kernel restarts the test from the aborted bundle. SBAF
execution is not retried if the test fails or if the test makes no
forward progress after 5 retries.

Reviewed-by: Ashok Raj <ashok.raj@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 drivers/platform/x86/intel/ifs/ifs.h     |  30 +++
 drivers/platform/x86/intel/ifs/runtest.c | 234 +++++++++++++++++++++++
 2 files changed, 264 insertions(+)

diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
index 600bb8a1b285..b261be46bce8 100644
--- a/drivers/platform/x86/intel/ifs/ifs.h
+++ b/drivers/platform/x86/intel/ifs/ifs.h
@@ -157,6 +157,8 @@
 #define MSR_SBAF_HASHES_STATUS			0x000002b9
 #define MSR_AUTHENTICATE_AND_COPY_SBAF_CHUNK	0x000002ba
 #define MSR_SBAF_CHUNKS_AUTHENTICATION_STATUS	0x000002bb
+#define MSR_ACTIVATE_SBAF			0x000002bc
+#define MSR_SBAF_STATUS				0x000002bd
 
 #define MSR_COPY_SCAN_HASHES			0x000002c2
 #define MSR_SCAN_HASHES_STATUS			0x000002c3
@@ -283,6 +285,34 @@ union ifs_array {
 	};
 };
 
+/* MSR_ACTIVATE_SBAF bit fields */
+union ifs_sbaf {
+	u64	data;
+	struct {
+		u32	bundle_idx	:9;
+		u32	rsvd1		:5;
+		u32	pgm_idx		:2;
+		u32	rsvd2		:16;
+		u32	delay		:31;
+		u32	sigmce		:1;
+	};
+};
+
+/* MSR_SBAF_STATUS bit fields */
+union ifs_sbaf_status {
+	u64	data;
+	struct {
+		u32	bundle_idx	:9;
+		u32	rsvd1		:5;
+		u32	pgm_idx		:2;
+		u32	rsvd2		:16;
+		u32	error_code	:8;
+		u32	rsvd3		:21;
+		u32	test_fail	:1;
+		u32	sbaf_status	:2;
+	};
+};
+
 /*
  * Driver populated error-codes
  * 0xFD: Test timed out before completing all the chunks.
diff --git a/drivers/platform/x86/intel/ifs/runtest.c b/drivers/platform/x86/intel/ifs/runtest.c
index 282e4bfe30da..a812ec7761b9 100644
--- a/drivers/platform/x86/intel/ifs/runtest.c
+++ b/drivers/platform/x86/intel/ifs/runtest.c
@@ -29,6 +29,13 @@ struct run_params {
 	union ifs_status status;
 };
 
+struct sbaf_run_params {
+	struct ifs_data *ifsd;
+	int *retry_cnt;
+	union ifs_sbaf *activate;
+	union ifs_sbaf_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).
@@ -146,6 +153,7 @@ static bool can_restart(union ifs_status status)
 #define SPINUNIT 100 /* 100 nsec */
 static atomic_t array_cpus_in;
 static atomic_t scan_cpus_in;
+static atomic_t sbaf_cpus_in;
 
 /*
  * Simplified cpu sibling rendezvous loop based on microcode loader __wait_for_cpus()
@@ -387,6 +395,226 @@ static void ifs_array_test_gen1(int cpu, struct device *dev)
 		ifsd->status = SCAN_TEST_PASS;
 }
 
+#define SBAF_STATUS_PASS			0
+#define SBAF_STATUS_SIGN_FAIL			1
+#define SBAF_STATUS_INTR			2
+#define SBAF_STATUS_TEST_FAIL			3
+
+enum sbaf_status_err_code {
+	IFS_SBAF_NO_ERROR				= 0,
+	IFS_SBAF_OTHER_THREAD_COULD_NOT_JOIN		= 1,
+	IFS_SBAF_INTERRUPTED_BEFORE_RENDEZVOUS		= 2,
+	IFS_SBAF_UNASSIGNED_ERROR_CODE3			= 3,
+	IFS_SBAF_INVALID_BUNDLE_INDEX			= 4,
+	IFS_SBAF_MISMATCH_ARGS_BETWEEN_THREADS		= 5,
+	IFS_SBAF_CORE_NOT_CAPABLE_CURRENTLY		= 6,
+	IFS_SBAF_UNASSIGNED_ERROR_CODE7			= 7,
+	IFS_SBAF_EXCEED_NUMBER_OF_THREADS_CONCURRENT	= 8,
+	IFS_SBAF_INTERRUPTED_DURING_EXECUTION		= 9,
+	IFS_SBAF_INVALID_PROGRAM_INDEX			= 0xA,
+	IFS_SBAF_CORRUPTED_CHUNK			= 0xB,
+	IFS_SBAF_DID_NOT_START				= 0xC,
+};
+
+static const char * const sbaf_test_status[] = {
+	[IFS_SBAF_NO_ERROR] = "SBAF no error",
+	[IFS_SBAF_OTHER_THREAD_COULD_NOT_JOIN] = "Other thread could not join.",
+	[IFS_SBAF_INTERRUPTED_BEFORE_RENDEZVOUS] = "Interrupt occurred prior to SBAF coordination.",
+	[IFS_SBAF_UNASSIGNED_ERROR_CODE3] = "Unassigned error code 0x3",
+	[IFS_SBAF_INVALID_BUNDLE_INDEX] = "Non valid sbaf bundles. Reload test image",
+	[IFS_SBAF_MISMATCH_ARGS_BETWEEN_THREADS] = "Mismatch in arguments between threads T0/T1.",
+	[IFS_SBAF_CORE_NOT_CAPABLE_CURRENTLY] = "Core not capable of performing SBAF currently",
+	[IFS_SBAF_UNASSIGNED_ERROR_CODE7] = "Unassigned error code 0x7",
+	[IFS_SBAF_EXCEED_NUMBER_OF_THREADS_CONCURRENT] = "Exceeded number of Logical Processors (LP) allowed to run Scan-At-Field concurrently",
+	[IFS_SBAF_INTERRUPTED_DURING_EXECUTION] = "Interrupt occurred prior to SBAF start",
+	[IFS_SBAF_INVALID_PROGRAM_INDEX] = "SBAF program index not valid",
+	[IFS_SBAF_CORRUPTED_CHUNK] = "SBAF operation aborted due to corrupted chunk",
+	[IFS_SBAF_DID_NOT_START] = "SBAF operation did not start",
+};
+
+static void sbaf_message_not_tested(struct device *dev, int cpu, u64 status_data)
+{
+	union ifs_sbaf_status status = (union ifs_sbaf_status)status_data;
+
+	if (status.error_code < ARRAY_SIZE(sbaf_test_status)) {
+		dev_info(dev, "CPU(s) %*pbl: SBAF operation did not start. %s\n",
+			 cpumask_pr_args(cpu_smt_mask(cpu)),
+			 sbaf_test_status[status.error_code]);
+	} else if (status.error_code == IFS_SW_TIMEOUT) {
+		dev_info(dev, "CPU(s) %*pbl: software timeout during scan\n",
+			 cpumask_pr_args(cpu_smt_mask(cpu)));
+	} else if (status.error_code == IFS_SW_PARTIAL_COMPLETION) {
+		dev_info(dev, "CPU(s) %*pbl: %s\n",
+			 cpumask_pr_args(cpu_smt_mask(cpu)),
+			 "Not all SBAF bundles executed. Maximum forward progress retries exceeded");
+	} else {
+		dev_info(dev, "CPU(s) %*pbl: SBAF unknown status %llx\n",
+			 cpumask_pr_args(cpu_smt_mask(cpu)), status.data);
+	}
+}
+
+static void sbaf_message_fail(struct device *dev, int cpu, union ifs_sbaf_status status)
+{
+	/* Failed signature check is set when SBAF signature did not match the expected value */
+	if (status.sbaf_status == SBAF_STATUS_SIGN_FAIL) {
+		dev_err(dev, "CPU(s) %*pbl: Failed signature check\n",
+			cpumask_pr_args(cpu_smt_mask(cpu)));
+	}
+
+	/* Failed to reach end of test */
+	if (status.sbaf_status == SBAF_STATUS_TEST_FAIL) {
+		dev_err(dev, "CPU(s) %*pbl: Failed to complete test\n",
+			cpumask_pr_args(cpu_smt_mask(cpu)));
+	}
+}
+
+static bool sbaf_bundle_completed(union ifs_sbaf_status status)
+{
+	if (status.sbaf_status || status.error_code)
+		return false;
+	return true;
+}
+
+static bool sbaf_can_restart(union ifs_sbaf_status status)
+{
+	enum sbaf_status_err_code err_code = status.error_code;
+
+	/* Signature for chunk is bad, or scan test failed */
+	if (status.sbaf_status == SBAF_STATUS_SIGN_FAIL ||
+	    status.sbaf_status == SBAF_STATUS_TEST_FAIL)
+		return false;
+
+	switch (err_code) {
+	case IFS_SBAF_NO_ERROR:
+	case IFS_SBAF_OTHER_THREAD_COULD_NOT_JOIN:
+	case IFS_SBAF_INTERRUPTED_BEFORE_RENDEZVOUS:
+	case IFS_SBAF_EXCEED_NUMBER_OF_THREADS_CONCURRENT:
+	case IFS_SBAF_INTERRUPTED_DURING_EXECUTION:
+		return true;
+	case IFS_SBAF_UNASSIGNED_ERROR_CODE3:
+	case IFS_SBAF_INVALID_BUNDLE_INDEX:
+	case IFS_SBAF_MISMATCH_ARGS_BETWEEN_THREADS:
+	case IFS_SBAF_CORE_NOT_CAPABLE_CURRENTLY:
+	case IFS_SBAF_UNASSIGNED_ERROR_CODE7:
+	case IFS_SBAF_INVALID_PROGRAM_INDEX:
+	case IFS_SBAF_CORRUPTED_CHUNK:
+	case IFS_SBAF_DID_NOT_START:
+		break;
+	}
+	return false;
+}
+
+/*
+ * Execute the SBAF test. Called "simultaneously" on all threads of a core
+ * at high priority using the stop_cpus mechanism.
+ */
+static int dosbaf(void *data)
+{
+	struct sbaf_run_params *run_params = data;
+	int cpu = smp_processor_id();
+	union ifs_sbaf_status status;
+	struct ifs_data *ifsd;
+	int first;
+
+	ifsd = run_params->ifsd;
+
+	/* Only the first logical CPU on a core reports result */
+	first = cpumask_first(cpu_smt_mask(cpu));
+	wait_for_sibling_cpu(&sbaf_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
+	 * starts scan of each requested bundle. The core test happens
+	 * during the "execution" of the WRMSR.
+	 */
+	wrmsrl(MSR_ACTIVATE_SBAF, run_params->activate->data);
+	rdmsrl(MSR_SBAF_STATUS, status.data);
+
+	/* Pass back the result of the test */
+	if (cpu == first)
+		run_params->status = status;
+
+	return 0;
+}
+
+static void ifs_sbaf_test_core(int cpu, struct device *dev)
+{
+	struct sbaf_run_params run_params;
+	union ifs_sbaf_status status = {};
+	union ifs_sbaf activate;
+	unsigned long timeout;
+	struct ifs_data *ifsd;
+	int stop_bundle;
+	int retries;
+
+	ifsd = ifs_get_data(dev);
+
+	activate.data = 0;
+	activate.delay = IFS_THREAD_WAIT;
+
+	timeout = jiffies + (2 * HZ);
+	retries = MAX_IFS_RETRIES;
+	activate.bundle_idx = 0;
+	stop_bundle = ifsd->max_bundle;
+
+	while (activate.bundle_idx <= stop_bundle) {
+		if (time_after(jiffies, timeout)) {
+			status.error_code = IFS_SW_TIMEOUT;
+			break;
+		}
+
+		atomic_set(&sbaf_cpus_in, 0);
+
+		run_params.ifsd = ifsd;
+		run_params.activate = &activate;
+		run_params.retry_cnt = &retries;
+		stop_core_cpuslocked(cpu, dosbaf, &run_params);
+
+		status = run_params.status;
+
+		if (sbaf_bundle_completed(status)) {
+			activate.bundle_idx = status.bundle_idx + 1;
+			activate.pgm_idx = 0;
+			retries = MAX_IFS_RETRIES;
+			continue;
+		}
+
+		/* Some cases can be retried, give up for others */
+		if (!sbaf_can_restart(status))
+			break;
+
+		if (status.pgm_idx == activate.pgm_idx) {
+			/* If no progress retry */
+			if (--retries == 0) {
+				if (status.error_code == IFS_NO_ERROR)
+					status.error_code = IFS_SW_PARTIAL_COMPLETION;
+				break;
+			}
+		} else {
+			/* if some progress, more pgms remaining in bundle, reset retries */
+			retries = MAX_IFS_RETRIES;
+			activate.bundle_idx = status.bundle_idx;
+			activate.pgm_idx = status.pgm_idx;
+		}
+	}
+
+	/* Update status for this core */
+	ifsd->scan_details = status.data;
+
+	if (status.sbaf_status == SBAF_STATUS_SIGN_FAIL ||
+	    status.sbaf_status == SBAF_STATUS_TEST_FAIL) {
+		ifsd->status = SCAN_TEST_FAIL;
+		sbaf_message_fail(dev, cpu, status);
+	} else if (status.error_code || status.sbaf_status == SBAF_STATUS_INTR ||
+		   (activate.bundle_idx < stop_bundle)) {
+		ifsd->status = SCAN_NOT_TESTED;
+		sbaf_message_not_tested(dev, cpu, status.data);
+	} 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
@@ -420,6 +648,12 @@ int do_core_test(int cpu, struct device *dev)
 		else
 			ifs_array_test_gen1(cpu, dev);
 		break;
+	case IFS_TYPE_SBAF:
+		if (!ifsd->loaded)
+			ret = -EPERM;
+		else
+			ifs_sbaf_test_core(cpu, dev);
+		break;
 	default:
 		ret = -EINVAL;
 	}
-- 
2.25.1

Re: [PATCH v2 3/4] platform/x86/intel/ifs: Add SBAF test support

[Ilpo Järvinen]

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

On Wed, 10 Jul 2024, Kuppuswamy Sathyanarayanan wrote:

> From: Jithu Joseph <jithu.joseph@intel.com>
> 
> In a core, the SBAF test engine is shared between sibling CPUs.
> 
> An SBAF test image contains multiple bundles. Each bundle is further
> composed of subunits called programs. When a SBAF test (for a particular
> core) is triggered by the user, each SBAF bundle from the loaded test
> image is executed sequentially on all the threads on the core using
> the stop_core_cpuslocked mechanism. Each bundle execution is initiated by
> writing to MSR_ACTIVATE_SBAF.
> 
> SBAF test bundle execution may be aborted when an interrupt occurs or
> if the CPU does not have enough power budget for the test. In these
> cases the kernel restarts the test from the aborted bundle. SBAF
> execution is not retried if the test fails or if the test makes no
> forward progress after 5 retries.
> 
> Reviewed-by: Ashok Raj <ashok.raj@intel.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> ---
>  drivers/platform/x86/intel/ifs/ifs.h     |  30 +++
>  drivers/platform/x86/intel/ifs/runtest.c | 234 +++++++++++++++++++++++
>  2 files changed, 264 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
> index 600bb8a1b285..b261be46bce8 100644
> --- a/drivers/platform/x86/intel/ifs/ifs.h
> +++ b/drivers/platform/x86/intel/ifs/ifs.h
> @@ -157,6 +157,8 @@
>  #define MSR_SBAF_HASHES_STATUS			0x000002b9
>  #define MSR_AUTHENTICATE_AND_COPY_SBAF_CHUNK	0x000002ba
>  #define MSR_SBAF_CHUNKS_AUTHENTICATION_STATUS	0x000002bb
> +#define MSR_ACTIVATE_SBAF			0x000002bc
> +#define MSR_SBAF_STATUS				0x000002bd
>  
>  #define MSR_COPY_SCAN_HASHES			0x000002c2
>  #define MSR_SCAN_HASHES_STATUS			0x000002c3
> @@ -283,6 +285,34 @@ union ifs_array {
>  	};
>  };
>  
> +/* MSR_ACTIVATE_SBAF bit fields */
> +union ifs_sbaf {
> +	u64	data;
> +	struct {
> +		u32	bundle_idx	:9;
> +		u32	rsvd1		:5;
> +		u32	pgm_idx		:2;
> +		u32	rsvd2		:16;
> +		u32	delay		:31;
> +		u32	sigmce		:1;
> +	};
> +};
> +
> +/* MSR_SBAF_STATUS bit fields */
> +union ifs_sbaf_status {
> +	u64	data;
> +	struct {
> +		u32	bundle_idx	:9;
> +		u32	rsvd1		:5;
> +		u32	pgm_idx		:2;
> +		u32	rsvd2		:16;
> +		u32	error_code	:8;
> +		u32	rsvd3		:21;
> +		u32	test_fail	:1;
> +		u32	sbaf_status	:2;
> +	};
> +};
> +
>  /*
>   * Driver populated error-codes
>   * 0xFD: Test timed out before completing all the chunks.
> diff --git a/drivers/platform/x86/intel/ifs/runtest.c b/drivers/platform/x86/intel/ifs/runtest.c
> index 282e4bfe30da..a812ec7761b9 100644
> --- a/drivers/platform/x86/intel/ifs/runtest.c
> +++ b/drivers/platform/x86/intel/ifs/runtest.c
> @@ -29,6 +29,13 @@ struct run_params {
>  	union ifs_status status;
>  };
>  
> +struct sbaf_run_params {
> +	struct ifs_data *ifsd;
> +	int *retry_cnt;
> +	union ifs_sbaf *activate;
> +	union ifs_sbaf_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).
> @@ -146,6 +153,7 @@ static bool can_restart(union ifs_status status)
>  #define SPINUNIT 100 /* 100 nsec */
>  static atomic_t array_cpus_in;
>  static atomic_t scan_cpus_in;
> +static atomic_t sbaf_cpus_in;
>  
>  /*
>   * Simplified cpu sibling rendezvous loop based on microcode loader __wait_for_cpus()
> @@ -387,6 +395,226 @@ static void ifs_array_test_gen1(int cpu, struct device *dev)
>  		ifsd->status = SCAN_TEST_PASS;
>  }
>  
> +#define SBAF_STATUS_PASS			0
> +#define SBAF_STATUS_SIGN_FAIL			1
> +#define SBAF_STATUS_INTR			2
> +#define SBAF_STATUS_TEST_FAIL			3
> +
> +enum sbaf_status_err_code {
> +	IFS_SBAF_NO_ERROR				= 0,
> +	IFS_SBAF_OTHER_THREAD_COULD_NOT_JOIN		= 1,
> +	IFS_SBAF_INTERRUPTED_BEFORE_RENDEZVOUS		= 2,
> +	IFS_SBAF_UNASSIGNED_ERROR_CODE3			= 3,
> +	IFS_SBAF_INVALID_BUNDLE_INDEX			= 4,
> +	IFS_SBAF_MISMATCH_ARGS_BETWEEN_THREADS		= 5,
> +	IFS_SBAF_CORE_NOT_CAPABLE_CURRENTLY		= 6,
> +	IFS_SBAF_UNASSIGNED_ERROR_CODE7			= 7,
> +	IFS_SBAF_EXCEED_NUMBER_OF_THREADS_CONCURRENT	= 8,
> +	IFS_SBAF_INTERRUPTED_DURING_EXECUTION		= 9,
> +	IFS_SBAF_INVALID_PROGRAM_INDEX			= 0xA,
> +	IFS_SBAF_CORRUPTED_CHUNK			= 0xB,
> +	IFS_SBAF_DID_NOT_START				= 0xC,
> +};
> +
> +static const char * const sbaf_test_status[] = {
> +	[IFS_SBAF_NO_ERROR] = "SBAF no error",
> +	[IFS_SBAF_OTHER_THREAD_COULD_NOT_JOIN] = "Other thread could not join.",
> +	[IFS_SBAF_INTERRUPTED_BEFORE_RENDEZVOUS] = "Interrupt occurred prior to SBAF coordination.",
> +	[IFS_SBAF_UNASSIGNED_ERROR_CODE3] = "Unassigned error code 0x3",
> +	[IFS_SBAF_INVALID_BUNDLE_INDEX] = "Non valid sbaf bundles. Reload test image",

Non-valid SBAF

...but given your define is named "INVALID", why not use just Invalid 
SBAF?

> +	[IFS_SBAF_MISMATCH_ARGS_BETWEEN_THREADS] = "Mismatch in arguments between threads T0/T1.",
> +	[IFS_SBAF_CORE_NOT_CAPABLE_CURRENTLY] = "Core not capable of performing SBAF currently",
> +	[IFS_SBAF_UNASSIGNED_ERROR_CODE7] = "Unassigned error code 0x7",
> +	[IFS_SBAF_EXCEED_NUMBER_OF_THREADS_CONCURRENT] = "Exceeded number of Logical Processors (LP) allowed to run Scan-At-Field concurrently",
> +	[IFS_SBAF_INTERRUPTED_DURING_EXECUTION] = "Interrupt occurred prior to SBAF start",
> +	[IFS_SBAF_INVALID_PROGRAM_INDEX] = "SBAF program index not valid",
> +	[IFS_SBAF_CORRUPTED_CHUNK] = "SBAF operation aborted due to corrupted chunk",
> +	[IFS_SBAF_DID_NOT_START] = "SBAF operation did not start",
> +};
> +
> +static void sbaf_message_not_tested(struct device *dev, int cpu, u64 status_data)
> +{
> +	union ifs_sbaf_status status = (union ifs_sbaf_status)status_data;
> +
> +	if (status.error_code < ARRAY_SIZE(sbaf_test_status)) {
> +		dev_info(dev, "CPU(s) %*pbl: SBAF operation did not start. %s\n",
> +			 cpumask_pr_args(cpu_smt_mask(cpu)),
> +			 sbaf_test_status[status.error_code]);
> +	} else if (status.error_code == IFS_SW_TIMEOUT) {
> +		dev_info(dev, "CPU(s) %*pbl: software timeout during scan\n",
> +			 cpumask_pr_args(cpu_smt_mask(cpu)));
> +	} else if (status.error_code == IFS_SW_PARTIAL_COMPLETION) {
> +		dev_info(dev, "CPU(s) %*pbl: %s\n",
> +			 cpumask_pr_args(cpu_smt_mask(cpu)),
> +			 "Not all SBAF bundles executed. Maximum forward progress retries exceeded");
> +	} else {
> +		dev_info(dev, "CPU(s) %*pbl: SBAF unknown status %llx\n",
> +			 cpumask_pr_args(cpu_smt_mask(cpu)), status.data);
> +	}
> +}
> +
> +static void sbaf_message_fail(struct device *dev, int cpu, union ifs_sbaf_status status)
> +{
> +	/* Failed signature check is set when SBAF signature did not match the expected value */
> +	if (status.sbaf_status == SBAF_STATUS_SIGN_FAIL) {
> +		dev_err(dev, "CPU(s) %*pbl: Failed signature check\n",
> +			cpumask_pr_args(cpu_smt_mask(cpu)));
> +	}
> +
> +	/* Failed to reach end of test */
> +	if (status.sbaf_status == SBAF_STATUS_TEST_FAIL) {
> +		dev_err(dev, "CPU(s) %*pbl: Failed to complete test\n",
> +			cpumask_pr_args(cpu_smt_mask(cpu)));
> +	}
> +}
> +
> +static bool sbaf_bundle_completed(union ifs_sbaf_status status)
> +{
> +	if (status.sbaf_status || status.error_code)
> +		return false;
> +	return true;

This is same as:

	return !status.sbaf_status && !status.error_code;

> +}
> +
> +static bool sbaf_can_restart(union ifs_sbaf_status status)
> +{
> +	enum sbaf_status_err_code err_code = status.error_code;
> +
> +	/* Signature for chunk is bad, or scan test failed */
> +	if (status.sbaf_status == SBAF_STATUS_SIGN_FAIL ||
> +	    status.sbaf_status == SBAF_STATUS_TEST_FAIL)
> +		return false;
> +
> +	switch (err_code) {
> +	case IFS_SBAF_NO_ERROR:
> +	case IFS_SBAF_OTHER_THREAD_COULD_NOT_JOIN:
> +	case IFS_SBAF_INTERRUPTED_BEFORE_RENDEZVOUS:
> +	case IFS_SBAF_EXCEED_NUMBER_OF_THREADS_CONCURRENT:
> +	case IFS_SBAF_INTERRUPTED_DURING_EXECUTION:
> +		return true;
> +	case IFS_SBAF_UNASSIGNED_ERROR_CODE3:
> +	case IFS_SBAF_INVALID_BUNDLE_INDEX:
> +	case IFS_SBAF_MISMATCH_ARGS_BETWEEN_THREADS:
> +	case IFS_SBAF_CORE_NOT_CAPABLE_CURRENTLY:
> +	case IFS_SBAF_UNASSIGNED_ERROR_CODE7:
> +	case IFS_SBAF_INVALID_PROGRAM_INDEX:
> +	case IFS_SBAF_CORRUPTED_CHUNK:
> +	case IFS_SBAF_DID_NOT_START:
> +		break;
> +	}
> +	return false;
> +}
> +
> +/*
> + * Execute the SBAF test. Called "simultaneously" on all threads of a core
> + * at high priority using the stop_cpus mechanism.
> + */
> +static int dosbaf(void *data)
> +{
> +	struct sbaf_run_params *run_params = data;
> +	int cpu = smp_processor_id();
> +	union ifs_sbaf_status status;
> +	struct ifs_data *ifsd;
> +	int first;
> +
> +	ifsd = run_params->ifsd;
> +
> +	/* Only the first logical CPU on a core reports result */
> +	first = cpumask_first(cpu_smt_mask(cpu));
> +	wait_for_sibling_cpu(&sbaf_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
> +	 * starts scan of each requested bundle. The core test happens
> +	 * during the "execution" of the WRMSR.
> +	 */
> +	wrmsrl(MSR_ACTIVATE_SBAF, run_params->activate->data);
> +	rdmsrl(MSR_SBAF_STATUS, status.data);
> +
> +	/* Pass back the result of the test */
> +	if (cpu == first)
> +		run_params->status = status;
> +
> +	return 0;
> +}
> +
> +static void ifs_sbaf_test_core(int cpu, struct device *dev)
> +{
> +	struct sbaf_run_params run_params;
> +	union ifs_sbaf_status status = {};
> +	union ifs_sbaf activate;
> +	unsigned long timeout;
> +	struct ifs_data *ifsd;
> +	int stop_bundle;
> +	int retries;
> +
> +	ifsd = ifs_get_data(dev);
> +
> +	activate.data = 0;
> +	activate.delay = IFS_THREAD_WAIT;
> +
> +	timeout = jiffies + (2 * HZ);

Unnecessary parenthesis.

Other than those things mentioned above,

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


-- 
 i.

Re: [PATCH v2 3/4] platform/x86/intel/ifs: Add SBAF test support

[Kuppuswamy Sathyanarayanan]

On 7/11/24 2:44 AM, Ilpo Järvinen wrote:
> On Wed, 10 Jul 2024, Kuppuswamy Sathyanarayanan wrote:
>
>> From: Jithu Joseph <jithu.joseph@intel.com>
>>
>> In a core, the SBAF test engine is shared between sibling CPUs.
>>
>> An SBAF test image contains multiple bundles. Each bundle is further
>> composed of subunits called programs. When a SBAF test (for a particular
>> core) is triggered by the user, each SBAF bundle from the loaded test
>> image is executed sequentially on all the threads on the core using
>> the stop_core_cpuslocked mechanism. Each bundle execution is initiated by
>> writing to MSR_ACTIVATE_SBAF.
>>
>> SBAF test bundle execution may be aborted when an interrupt occurs or
>> if the CPU does not have enough power budget for the test. In these
>> cases the kernel restarts the test from the aborted bundle. SBAF
>> execution is not retried if the test fails or if the test makes no
>> forward progress after 5 retries.
>>
>> Reviewed-by: Ashok Raj <ashok.raj@intel.com>
>> Reviewed-by: Tony Luck <tony.luck@intel.com>
>> Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
>> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>> ---
>>  drivers/platform/x86/intel/ifs/ifs.h     |  30 +++
>>  drivers/platform/x86/intel/ifs/runtest.c | 234 +++++++++++++++++++++++
>>  2 files changed, 264 insertions(+)
>>
>> diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
>> index 600bb8a1b285..b261be46bce8 100644
>> --- a/drivers/platform/x86/intel/ifs/ifs.h
>> +++ b/drivers/platform/x86/intel/ifs/ifs.h
>> @@ -157,6 +157,8 @@
>>  #define MSR_SBAF_HASHES_STATUS			0x000002b9
>>  #define MSR_AUTHENTICATE_AND_COPY_SBAF_CHUNK	0x000002ba
>>  #define MSR_SBAF_CHUNKS_AUTHENTICATION_STATUS	0x000002bb
>> +#define MSR_ACTIVATE_SBAF			0x000002bc
>> +#define MSR_SBAF_STATUS				0x000002bd
>>  
>>  #define MSR_COPY_SCAN_HASHES			0x000002c2
>>  #define MSR_SCAN_HASHES_STATUS			0x000002c3
>> @@ -283,6 +285,34 @@ union ifs_array {
>>  	};
>>  };
>>  
>> +/* MSR_ACTIVATE_SBAF bit fields */
>> +union ifs_sbaf {
>> +	u64	data;
>> +	struct {
>> +		u32	bundle_idx	:9;
>> +		u32	rsvd1		:5;
>> +		u32	pgm_idx		:2;
>> +		u32	rsvd2		:16;
>> +		u32	delay		:31;
>> +		u32	sigmce		:1;
>> +	};
>> +};
>> +
>> +/* MSR_SBAF_STATUS bit fields */
>> +union ifs_sbaf_status {
>> +	u64	data;
>> +	struct {
>> +		u32	bundle_idx	:9;
>> +		u32	rsvd1		:5;
>> +		u32	pgm_idx		:2;
>> +		u32	rsvd2		:16;
>> +		u32	error_code	:8;
>> +		u32	rsvd3		:21;
>> +		u32	test_fail	:1;
>> +		u32	sbaf_status	:2;
>> +	};
>> +};
>> +
>>  /*
>>   * Driver populated error-codes
>>   * 0xFD: Test timed out before completing all the chunks.
>> diff --git a/drivers/platform/x86/intel/ifs/runtest.c b/drivers/platform/x86/intel/ifs/runtest.c
>> index 282e4bfe30da..a812ec7761b9 100644
>> --- a/drivers/platform/x86/intel/ifs/runtest.c
>> +++ b/drivers/platform/x86/intel/ifs/runtest.c
>> @@ -29,6 +29,13 @@ struct run_params {
>>  	union ifs_status status;
>>  };
>>  
>> +struct sbaf_run_params {
>> +	struct ifs_data *ifsd;
>> +	int *retry_cnt;
>> +	union ifs_sbaf *activate;
>> +	union ifs_sbaf_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).
>> @@ -146,6 +153,7 @@ static bool can_restart(union ifs_status status)
>>  #define SPINUNIT 100 /* 100 nsec */
>>  static atomic_t array_cpus_in;
>>  static atomic_t scan_cpus_in;
>> +static atomic_t sbaf_cpus_in;
>>  
>>  /*
>>   * Simplified cpu sibling rendezvous loop based on microcode loader __wait_for_cpus()
>> @@ -387,6 +395,226 @@ static void ifs_array_test_gen1(int cpu, struct device *dev)
>>  		ifsd->status = SCAN_TEST_PASS;
>>  }
>>  
>> +#define SBAF_STATUS_PASS			0
>> +#define SBAF_STATUS_SIGN_FAIL			1
>> +#define SBAF_STATUS_INTR			2
>> +#define SBAF_STATUS_TEST_FAIL			3
>> +
>> +enum sbaf_status_err_code {
>> +	IFS_SBAF_NO_ERROR				= 0,
>> +	IFS_SBAF_OTHER_THREAD_COULD_NOT_JOIN		= 1,
>> +	IFS_SBAF_INTERRUPTED_BEFORE_RENDEZVOUS		= 2,
>> +	IFS_SBAF_UNASSIGNED_ERROR_CODE3			= 3,
>> +	IFS_SBAF_INVALID_BUNDLE_INDEX			= 4,
>> +	IFS_SBAF_MISMATCH_ARGS_BETWEEN_THREADS		= 5,
>> +	IFS_SBAF_CORE_NOT_CAPABLE_CURRENTLY		= 6,
>> +	IFS_SBAF_UNASSIGNED_ERROR_CODE7			= 7,
>> +	IFS_SBAF_EXCEED_NUMBER_OF_THREADS_CONCURRENT	= 8,
>> +	IFS_SBAF_INTERRUPTED_DURING_EXECUTION		= 9,
>> +	IFS_SBAF_INVALID_PROGRAM_INDEX			= 0xA,
>> +	IFS_SBAF_CORRUPTED_CHUNK			= 0xB,
>> +	IFS_SBAF_DID_NOT_START				= 0xC,
>> +};
>> +
>> +static const char * const sbaf_test_status[] = {
>> +	[IFS_SBAF_NO_ERROR] = "SBAF no error",
>> +	[IFS_SBAF_OTHER_THREAD_COULD_NOT_JOIN] = "Other thread could not join.",
>> +	[IFS_SBAF_INTERRUPTED_BEFORE_RENDEZVOUS] = "Interrupt occurred prior to SBAF coordination.",
>> +	[IFS_SBAF_UNASSIGNED_ERROR_CODE3] = "Unassigned error code 0x3",
>> +	[IFS_SBAF_INVALID_BUNDLE_INDEX] = "Non valid sbaf bundles. Reload test image",
> Non-valid SBAF
>
> ...but given your define is named "INVALID", why not use just Invalid 
> SBAF?

Above string is from the specification document.But I think it is ok to use
"Invalid" or "Non-valid".

Jithu, any concerns?

>> +	[IFS_SBAF_MISMATCH_ARGS_BETWEEN_THREADS] = "Mismatch in arguments between threads T0/T1.",
>> +	[IFS_SBAF_CORE_NOT_CAPABLE_CURRENTLY] = "Core not capable of performing SBAF currently",
>> +	[IFS_SBAF_UNASSIGNED_ERROR_CODE7] = "Unassigned error code 0x7",
>> +	[IFS_SBAF_EXCEED_NUMBER_OF_THREADS_CONCURRENT] = "Exceeded number of Logical Processors (LP) allowed to run Scan-At-Field concurrently",
>> +	[IFS_SBAF_INTERRUPTED_DURING_EXECUTION] = "Interrupt occurred prior to SBAF start",
>> +	[IFS_SBAF_INVALID_PROGRAM_INDEX] = "SBAF program index not valid",
>> +	[IFS_SBAF_CORRUPTED_CHUNK] = "SBAF operation aborted due to corrupted chunk",
>> +	[IFS_SBAF_DID_NOT_START] = "SBAF operation did not start",
>> +};
>> +
>> +static void sbaf_message_not_tested(struct device *dev, int cpu, u64 status_data)
>> +{
>> +	union ifs_sbaf_status status = (union ifs_sbaf_status)status_data;
>> +
>> +	if (status.error_code < ARRAY_SIZE(sbaf_test_status)) {
>> +		dev_info(dev, "CPU(s) %*pbl: SBAF operation did not start. %s\n",
>> +			 cpumask_pr_args(cpu_smt_mask(cpu)),
>> +			 sbaf_test_status[status.error_code]);
>> +	} else if (status.error_code == IFS_SW_TIMEOUT) {
>> +		dev_info(dev, "CPU(s) %*pbl: software timeout during scan\n",
>> +			 cpumask_pr_args(cpu_smt_mask(cpu)));
>> +	} else if (status.error_code == IFS_SW_PARTIAL_COMPLETION) {
>> +		dev_info(dev, "CPU(s) %*pbl: %s\n",
>> +			 cpumask_pr_args(cpu_smt_mask(cpu)),
>> +			 "Not all SBAF bundles executed. Maximum forward progress retries exceeded");
>> +	} else {
>> +		dev_info(dev, "CPU(s) %*pbl: SBAF unknown status %llx\n",
>> +			 cpumask_pr_args(cpu_smt_mask(cpu)), status.data);
>> +	}
>> +}
>> +
>> +static void sbaf_message_fail(struct device *dev, int cpu, union ifs_sbaf_status status)
>> +{
>> +	/* Failed signature check is set when SBAF signature did not match the expected value */
>> +	if (status.sbaf_status == SBAF_STATUS_SIGN_FAIL) {
>> +		dev_err(dev, "CPU(s) %*pbl: Failed signature check\n",
>> +			cpumask_pr_args(cpu_smt_mask(cpu)));
>> +	}
>> +
>> +	/* Failed to reach end of test */
>> +	if (status.sbaf_status == SBAF_STATUS_TEST_FAIL) {
>> +		dev_err(dev, "CPU(s) %*pbl: Failed to complete test\n",
>> +			cpumask_pr_args(cpu_smt_mask(cpu)));
>> +	}
>> +}
>> +
>> +static bool sbaf_bundle_completed(union ifs_sbaf_status status)
>> +{
>> +	if (status.sbaf_status || status.error_code)
>> +		return false;
>> +	return true;
> This is same as:
>
> 	return !status.sbaf_status && !status.error_code;

Yes. Your version looks good. Do you want me to send a version with
this change or you can include it when merging it?

>> +}
>> +
>> +static bool sbaf_can_restart(union ifs_sbaf_status status)
>> +{
>> +	enum sbaf_status_err_code err_code = status.error_code;
>> +
>> +	/* Signature for chunk is bad, or scan test failed */
>> +	if (status.sbaf_status == SBAF_STATUS_SIGN_FAIL ||
>> +	    status.sbaf_status == SBAF_STATUS_TEST_FAIL)
>> +		return false;
>> +
>> +	switch (err_code) {
>> +	case IFS_SBAF_NO_ERROR:
>> +	case IFS_SBAF_OTHER_THREAD_COULD_NOT_JOIN:
>> +	case IFS_SBAF_INTERRUPTED_BEFORE_RENDEZVOUS:
>> +	case IFS_SBAF_EXCEED_NUMBER_OF_THREADS_CONCURRENT:
>> +	case IFS_SBAF_INTERRUPTED_DURING_EXECUTION:
>> +		return true;
>> +	case IFS_SBAF_UNASSIGNED_ERROR_CODE3:
>> +	case IFS_SBAF_INVALID_BUNDLE_INDEX:
>> +	case IFS_SBAF_MISMATCH_ARGS_BETWEEN_THREADS:
>> +	case IFS_SBAF_CORE_NOT_CAPABLE_CURRENTLY:
>> +	case IFS_SBAF_UNASSIGNED_ERROR_CODE7:
>> +	case IFS_SBAF_INVALID_PROGRAM_INDEX:
>> +	case IFS_SBAF_CORRUPTED_CHUNK:
>> +	case IFS_SBAF_DID_NOT_START:
>> +		break;
>> +	}
>> +	return false;
>> +}
>> +
>> +/*
>> + * Execute the SBAF test. Called "simultaneously" on all threads of a core
>> + * at high priority using the stop_cpus mechanism.
>> + */
>> +static int dosbaf(void *data)
>> +{
>> +	struct sbaf_run_params *run_params = data;
>> +	int cpu = smp_processor_id();
>> +	union ifs_sbaf_status status;
>> +	struct ifs_data *ifsd;
>> +	int first;
>> +
>> +	ifsd = run_params->ifsd;
>> +
>> +	/* Only the first logical CPU on a core reports result */
>> +	first = cpumask_first(cpu_smt_mask(cpu));
>> +	wait_for_sibling_cpu(&sbaf_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
>> +	 * starts scan of each requested bundle. The core test happens
>> +	 * during the "execution" of the WRMSR.
>> +	 */
>> +	wrmsrl(MSR_ACTIVATE_SBAF, run_params->activate->data);
>> +	rdmsrl(MSR_SBAF_STATUS, status.data);
>> +
>> +	/* Pass back the result of the test */
>> +	if (cpu == first)
>> +		run_params->status = status;
>> +
>> +	return 0;
>> +}
>> +
>> +static void ifs_sbaf_test_core(int cpu, struct device *dev)
>> +{
>> +	struct sbaf_run_params run_params;
>> +	union ifs_sbaf_status status = {};
>> +	union ifs_sbaf activate;
>> +	unsigned long timeout;
>> +	struct ifs_data *ifsd;
>> +	int stop_bundle;
>> +	int retries;
>> +
>> +	ifsd = ifs_get_data(dev);
>> +
>> +	activate.data = 0;
>> +	activate.delay = IFS_THREAD_WAIT;
>> +
>> +	timeout = jiffies + (2 * HZ);
> Unnecessary parenthesis.
>
> Other than those things mentioned above,
>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>
>
-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

Re: [PATCH v2 3/4] platform/x86/intel/ifs: Add SBAF test support

[Joseph, Jithu]

On 7/11/2024 9:28 PM, Kuppuswamy Sathyanarayanan wrote:

>>> +
>>> +static const char * const sbaf_test_status[] = {
>>> +	[IFS_SBAF_NO_ERROR] = "SBAF no error",
>>> +	[IFS_SBAF_OTHER_THREAD_COULD_NOT_JOIN] = "Other thread could not join.",
>>> +	[IFS_SBAF_INTERRUPTED_BEFORE_RENDEZVOUS] = "Interrupt occurred prior to SBAF coordination.",
>>> +	[IFS_SBAF_UNASSIGNED_ERROR_CODE3] = "Unassigned error code 0x3",
>>> +	[IFS_SBAF_INVALID_BUNDLE_INDEX] = "Non valid sbaf bundles. Reload test image",
>> Non-valid SBAF
>>
>> ...but given your define is named "INVALID", why not use just Invalid 
>> SBAF?
> 
> Above string is from the specification document.But I think it is ok to use
> "Invalid" or "Non-valid".
> 
> Jithu, any concerns?
> 

Either seems okay, I think "Non-valid SBAF ..." is a bit more preferable as it is more in line with doc.

Jithu

Re: [PATCH v2 3/4] platform/x86/intel/ifs: Add SBAF test support

[Ilpo Järvinen]

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

On Thu, 11 Jul 2024, Kuppuswamy Sathyanarayanan wrote:
> On 7/11/24 2:44 AM, Ilpo Järvinen wrote:
> > On Wed, 10 Jul 2024, Kuppuswamy Sathyanarayanan wrote:
> >
> >> From: Jithu Joseph <jithu.joseph@intel.com>
> >>
> >> In a core, the SBAF test engine is shared between sibling CPUs.
> >>
> >> An SBAF test image contains multiple bundles. Each bundle is further
> >> composed of subunits called programs. When a SBAF test (for a particular
> >> core) is triggered by the user, each SBAF bundle from the loaded test
> >> image is executed sequentially on all the threads on the core using
> >> the stop_core_cpuslocked mechanism. Each bundle execution is initiated by
> >> writing to MSR_ACTIVATE_SBAF.
> >>
> >> SBAF test bundle execution may be aborted when an interrupt occurs or
> >> if the CPU does not have enough power budget for the test. In these
> >> cases the kernel restarts the test from the aborted bundle. SBAF
> >> execution is not retried if the test fails or if the test makes no
> >> forward progress after 5 retries.
> >>
> >> Reviewed-by: Ashok Raj <ashok.raj@intel.com>
> >> Reviewed-by: Tony Luck <tony.luck@intel.com>
> >> Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
> >> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> >> ---

> >> +static const char * const sbaf_test_status[] = {
> >> +	[IFS_SBAF_NO_ERROR] = "SBAF no error",
> >> +	[IFS_SBAF_OTHER_THREAD_COULD_NOT_JOIN] = "Other thread could not join.",
> >> +	[IFS_SBAF_INTERRUPTED_BEFORE_RENDEZVOUS] = "Interrupt occurred prior to SBAF coordination.",
> >> +	[IFS_SBAF_UNASSIGNED_ERROR_CODE3] = "Unassigned error code 0x3",
> >> +	[IFS_SBAF_INVALID_BUNDLE_INDEX] = "Non valid sbaf bundles. Reload test image",
> > Non-valid SBAF
> >
> > ...but given your define is named "INVALID", why not use just Invalid 
> > SBAF?
> 
> Above string is from the specification document.But I think it is ok to use
> "Invalid" or "Non-valid".
> 
> Jithu, any concerns?
> 
> >> +	[IFS_SBAF_MISMATCH_ARGS_BETWEEN_THREADS] = "Mismatch in arguments between threads T0/T1.",
> >> +	[IFS_SBAF_CORE_NOT_CAPABLE_CURRENTLY] = "Core not capable of performing SBAF currently",
> >> +	[IFS_SBAF_UNASSIGNED_ERROR_CODE7] = "Unassigned error code 0x7",
> >> +	[IFS_SBAF_EXCEED_NUMBER_OF_THREADS_CONCURRENT] = "Exceeded number of Logical Processors (LP) allowed to run Scan-At-Field concurrently",
> >> +	[IFS_SBAF_INTERRUPTED_DURING_EXECUTION] = "Interrupt occurred prior to SBAF start",
> >> +	[IFS_SBAF_INVALID_PROGRAM_INDEX] = "SBAF program index not valid",
> >> +	[IFS_SBAF_CORRUPTED_CHUNK] = "SBAF operation aborted due to corrupted chunk",
> >> +	[IFS_SBAF_DID_NOT_START] = "SBAF operation did not start",
> >> +};
> >> +
> >> +static void sbaf_message_not_tested(struct device *dev, int cpu, u64 status_data)
> >> +{
> >> +	union ifs_sbaf_status status = (union ifs_sbaf_status)status_data;
> >> +
> >> +	if (status.error_code < ARRAY_SIZE(sbaf_test_status)) {
> >> +		dev_info(dev, "CPU(s) %*pbl: SBAF operation did not start. %s\n",
> >> +			 cpumask_pr_args(cpu_smt_mask(cpu)),
> >> +			 sbaf_test_status[status.error_code]);
> >> +	} else if (status.error_code == IFS_SW_TIMEOUT) {
> >> +		dev_info(dev, "CPU(s) %*pbl: software timeout during scan\n",
> >> +			 cpumask_pr_args(cpu_smt_mask(cpu)));
> >> +	} else if (status.error_code == IFS_SW_PARTIAL_COMPLETION) {
> >> +		dev_info(dev, "CPU(s) %*pbl: %s\n",
> >> +			 cpumask_pr_args(cpu_smt_mask(cpu)),
> >> +			 "Not all SBAF bundles executed. Maximum forward progress retries exceeded");
> >> +	} else {
> >> +		dev_info(dev, "CPU(s) %*pbl: SBAF unknown status %llx\n",
> >> +			 cpumask_pr_args(cpu_smt_mask(cpu)), status.data);
> >> +	}
> >> +}
> >> +
> >> +static void sbaf_message_fail(struct device *dev, int cpu, union ifs_sbaf_status status)
> >> +{
> >> +	/* Failed signature check is set when SBAF signature did not match the expected value */
> >> +	if (status.sbaf_status == SBAF_STATUS_SIGN_FAIL) {
> >> +		dev_err(dev, "CPU(s) %*pbl: Failed signature check\n",
> >> +			cpumask_pr_args(cpu_smt_mask(cpu)));
> >> +	}
> >> +
> >> +	/* Failed to reach end of test */
> >> +	if (status.sbaf_status == SBAF_STATUS_TEST_FAIL) {
> >> +		dev_err(dev, "CPU(s) %*pbl: Failed to complete test\n",
> >> +			cpumask_pr_args(cpu_smt_mask(cpu)));
> >> +	}
> >> +}
> >> +
> >> +static bool sbaf_bundle_completed(union ifs_sbaf_status status)
> >> +{
> >> +	if (status.sbaf_status || status.error_code)
> >> +		return false;
> >> +	return true;
> > This is same as:
> >
> > 	return !status.sbaf_status && !status.error_code;
> 
> Yes. Your version looks good. Do you want me to send a version with
> this change or you can include it when merging it?

Please do send a new version, there were too many things to change for
me to do it while applying.

-- 
 i.

Re: [PATCH v2 3/4] platform/x86/intel/ifs: Add SBAF test support

[Ashok Raj]

On Thu, Jul 11, 2024 at 12:44:23PM +0300, Ilpo Järvinen wrote:
> > +static bool sbaf_bundle_completed(union ifs_sbaf_status status)
> > +{
> > +	if (status.sbaf_status || status.error_code)
> > +		return false;
> > +	return true;
> 
> This is same as:
> 
> 	return !status.sbaf_status && !status.error_code;
> 

Maybe another simplification

	return !(status.sbaf_status || status.error_code);


-- 
Cheers,
Ashok

Re: [PATCH v2 3/4] platform/x86/intel/ifs: Add SBAF test support

[Joseph, Jithu]

On 7/12/2024 2:51 AM, Ashok Raj wrote:
> On Thu, Jul 11, 2024 at 12:44:23PM +0300, Ilpo Järvinen wrote:
>>> +static bool sbaf_bundle_completed(union ifs_sbaf_status status)
>>> +{
>>> +	if (status.sbaf_status || status.error_code)
>>> +		return false;
>>> +	return true;
>>
>> This is same as:
>>
>> 	return !status.sbaf_status && !status.error_code;
>>
> 
> Maybe another simplification
> 
> 	return !(status.sbaf_status || status.error_code);
> 

Agree, this looks simpler


Jithu

[PATCH v2 4/4] trace: platform/x86/intel/ifs: Add SBAF trace support

[Kuppuswamy Sathyanarayanan]

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

Add tracing support for the SBAF IFS tests, which may be useful for
debugging systems that fail these tests. Log details like test content
batch number, SBAF bundle ID, program index and the exact errors or
warnings encountered by each HT thread during the test.

Reviewed-by: Ashok Raj <ashok.raj@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 include/trace/events/intel_ifs.h         | 27 ++++++++++++++++++++++++
 drivers/platform/x86/intel/ifs/runtest.c |  1 +
 2 files changed, 28 insertions(+)

diff --git a/include/trace/events/intel_ifs.h b/include/trace/events/intel_ifs.h
index 0d88ebf2c980..70323acde1de 100644
--- a/include/trace/events/intel_ifs.h
+++ b/include/trace/events/intel_ifs.h
@@ -35,6 +35,33 @@ TRACE_EVENT(ifs_status,
 		__entry->status)
 );
 
+TRACE_EVENT(ifs_sbaf,
+
+	TP_PROTO(int batch, union ifs_sbaf activate, union ifs_sbaf_status status),
+
+	TP_ARGS(batch, activate, status),
+
+	TP_STRUCT__entry(
+		__field(	u64,	status	)
+		__field(	int,	batch	)
+		__field(	u16,	bundle	)
+		__field(	u16,	pgm	)
+	),
+
+	TP_fast_assign(
+		__entry->status	= status.data;
+		__entry->batch	= batch;
+		__entry->bundle	= activate.bundle_idx;
+		__entry->pgm	= activate.pgm_idx;
+	),
+
+	TP_printk("batch: 0x%.2x, bundle_idx: 0x%.4x, pgm_idx: 0x%.4x, status: 0x%.16llx",
+		__entry->batch,
+		__entry->bundle,
+		__entry->pgm,
+		__entry->status)
+);
+
 #endif /* _TRACE_IFS_H */
 
 /* This part must be outside protection */
diff --git a/drivers/platform/x86/intel/ifs/runtest.c b/drivers/platform/x86/intel/ifs/runtest.c
index a812ec7761b9..2ec9a8c93d04 100644
--- a/drivers/platform/x86/intel/ifs/runtest.c
+++ b/drivers/platform/x86/intel/ifs/runtest.c
@@ -530,6 +530,7 @@ static int dosbaf(void *data)
 	 */
 	wrmsrl(MSR_ACTIVATE_SBAF, run_params->activate->data);
 	rdmsrl(MSR_SBAF_STATUS, status.data);
+	trace_ifs_sbaf(ifsd->cur_batch, *run_params->activate, status);
 
 	/* Pass back the result of the test */
 	if (cpu == first)
-- 
2.25.1

Re: [PATCH v2 4/4] trace: platform/x86/intel/ifs: Add SBAF trace support

[Ilpo Järvinen]

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

On Wed, 10 Jul 2024, Kuppuswamy Sathyanarayanan wrote:

> From: Jithu Joseph <jithu.joseph@intel.com>
> 
> Add tracing support for the SBAF IFS tests, which may be useful for
> debugging systems that fail these tests. Log details like test content
> batch number, SBAF bundle ID, program index and the exact errors or
> warnings encountered by each HT thread during the test.
> 
> Reviewed-by: Ashok Raj <ashok.raj@intel.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

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

-- 
 i.

Re: [PATCH v2 4/4] trace: platform/x86/intel/ifs: Add SBAF trace support

[Steven Rostedt]

On Wed, 10 Jul 2024 17:02:33 -0700
Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> wrote:

> From: Jithu Joseph <jithu.joseph@intel.com>
> 
> Add tracing support for the SBAF IFS tests, which may be useful for
> debugging systems that fail these tests. Log details like test content
> batch number, SBAF bundle ID, program index and the exact errors or
> warnings encountered by each HT thread during the test.
> 
> Reviewed-by: Ashok Raj <ashok.raj@intel.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>

-- Steve

> ---
>  include/trace/events/intel_ifs.h         | 27 ++++++++++++++++++++++++
>  drivers/platform/x86/intel/ifs/runtest.c |  1 +
>  2 files changed, 28 insertions(+)