On 10/21/2022 1:34 PM, Jithu Joseph wrote:
Initial implementation assumed a single IFS test image file with a
fixed name ff-mm-ss.scan. (where ff, mm, ss refers to family, model
and stepping of the core)
Subsequently, it became evident that supporting more than one
test image file is needed to provide more comprehensive
test coverage. (Test coverage in this scenario refers to testing
more transistors in the core to identify faults)
The other alternative of increasing the size of a single scan test image
file would not work as the upper bound is limited by the size of memory
area reserved by BIOS for loading IFS test image.
Introduce "current_batch" file which accepts a number. Writing a
number to the current_batch file would load the test image file by name
ff-mm-ss-<xy>.scan, where <xy> is the number written to the
"current_batch" file in hex.
Any specific reasoning why the name "current_batch" was chosen? To me,
batch seems to suggest multiple or a group of files. But in reality only
one test file is loaded at a time.
Naming can sometimes be quite subjective so it might be useful to get
multiple opinions here.
As per my understanding, there is sysfs file called run_test which runs
a loaded test. Instead of current_batch how about the name load_test (or
maybe current_test)?
load_test - Write a number less than or equal to 0xff to load an IFS
test image. (Description as-is from the documentation patch)
* Running tests
* -------------
@@ -207,6 +217,7 @@ struct ifs_data {
int status;
u64 scan_details;
int cur_batch;
+ int test_num;
};
struct ifs_work {
diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
index 5fb7f655c291..1f040837e8eb 100644
--- a/drivers/platform/x86/intel/ifs/core.c
+++ b/drivers/platform/x86/intel/ifs/core.c
@@ -22,6 +22,7 @@ MODULE_DEVICE_TABLE(x86cpu, ifs_cpu_ids);
static struct ifs_device ifs_device = {
.data = {
.integrity_cap_bit = MSR_INTEGRITY_CAPS_PERIODIC_BIST_BIT,
+ .test_num = 0,
Is this initialization really needed? Wouldn't it default to 0?
Maybe if you explain what does test_num refer to it might answer the above?
+static ssize_t current_batch_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct ifs_data *ifsd = ifs_get_data(dev);
+
+ if (!ifsd->loaded)
+ return sysfs_emit(buf, "%s\n", "none");
Why not:
sysfs_emit(buf, "none\n");
+ else
+ return sysfs_emit(buf, "0x%02x\n", ifsd->cur_batch);
+}
Sohil
