[PATCH v3 1/8] platform/x86/intel/ifs: Reorganize driver data

From: Jithu Joseph
Date: Tue Feb 28 2023 - 21:01:27 EST


The struct holding device driver data contained both read only(ro)
and read write(rw) fields.

Separating ro fields from rw fields was recommended as
a preferable design pattern during review[1].

Group the rw fields into a separate struct whose memory is allocated
during driver_init(). Associate it to the miscdevice being registered
by keeping it in the same container struct as the miscdevice.

Also in prepration to supporting additional tests, move ifs_pkg_auth
to a global as it is only applicable for the first test type.

Link: https://lore.kernel.org/lkml/Y+9H9otxLYPqMkUh@xxxxxxxxx/ [1]

Signed-off-by: Jithu Joseph <jithu.joseph@xxxxxxxxx>
Reviewed-by: Tony Luck <tony.luck@xxxxxxxxx>
---
drivers/platform/x86/intel/ifs/ifs.h | 19 +++++++++-------
drivers/platform/x86/intel/ifs/core.c | 31 ++++++++++++++++++---------
drivers/platform/x86/intel/ifs/load.c | 8 +++----
3 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
index 046e39304fd5..e07463c794d4 100644
--- a/drivers/platform/x86/intel/ifs/ifs.h
+++ b/drivers/platform/x86/intel/ifs/ifs.h
@@ -197,22 +197,23 @@ union ifs_status {
#define IFS_SW_TIMEOUT 0xFD
#define IFS_SW_PARTIAL_COMPLETION 0xFE

+struct ifs_const_data {
+ int integrity_cap_bit;
+ int test_num;
+};
+
/**
* struct ifs_data - attributes related to intel IFS driver
- * @integrity_cap_bit: MSR_INTEGRITY_CAPS bit enumerating this test
* @loaded_version: stores the currently loaded ifs image version.
- * @pkg_auth: array of bool storing per package auth status
* @loaded: If a valid test binary has been loaded into the memory
* @loading_error: Error occurred on another CPU while loading image
* @valid_chunks: number of chunks which could be validated.
* @status: it holds simple status pass/fail/untested
* @scan_details: opaque scan status code from h/w
* @cur_batch: number indicating the currently loaded test file
- * @test_num: number indicating the test type
+ * @ro_info: ptr to struct holding fixed details
*/
struct ifs_data {
- int integrity_cap_bit;
- bool *pkg_auth;
int loaded_version;
bool loaded;
bool loading_error;
@@ -220,7 +221,7 @@ struct ifs_data {
int status;
u64 scan_details;
u32 cur_batch;
- int test_num;
+ struct ifs_const_data *ro_info;
};

struct ifs_work {
@@ -229,7 +230,8 @@ struct ifs_work {
};

struct ifs_device {
- struct ifs_data data;
+ struct ifs_const_data ro_data;
+ struct ifs_data *rw_data;
struct miscdevice misc;
};

@@ -238,9 +240,10 @@ static inline struct ifs_data *ifs_get_data(struct device *dev)
struct miscdevice *m = dev_get_drvdata(dev);
struct ifs_device *d = container_of(m, struct ifs_device, misc);

- return &d->data;
+ return d->rw_data;
}

+extern bool *ifs_pkg_auth;
int ifs_load_firmware(struct device *dev);
int do_core_test(int cpu, struct device *dev);
const struct attribute_group **ifs_get_groups(void);
diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
index 206a617c2e02..b518b661daf0 100644
--- a/drivers/platform/x86/intel/ifs/core.c
+++ b/drivers/platform/x86/intel/ifs/core.c
@@ -20,8 +20,10 @@ static const struct x86_cpu_id ifs_cpu_ids[] __initconst = {
};
MODULE_DEVICE_TABLE(x86cpu, ifs_cpu_ids);

+bool *ifs_pkg_auth;
+
static struct ifs_device ifs_device = {
- .data = {
+ .ro_data = {
.integrity_cap_bit = MSR_INTEGRITY_CAPS_PERIODIC_BIST_BIT,
.test_num = 0,
},
@@ -35,8 +37,8 @@ static struct ifs_device ifs_device = {
static int __init ifs_init(void)
{
const struct x86_cpu_id *m;
+ struct ifs_data *ifsd;
u64 msrval;
- int ret;

m = x86_match_cpu(ifs_cpu_ids);
if (!m)
@@ -53,26 +55,35 @@ static int __init ifs_init(void)

ifs_device.misc.groups = ifs_get_groups();

- if (!(msrval & BIT(ifs_device.data.integrity_cap_bit)))
+ if (!(msrval & BIT(ifs_device.ro_data.integrity_cap_bit)))
return -ENODEV;

- ifs_device.data.pkg_auth = kmalloc_array(topology_max_packages(), sizeof(bool), GFP_KERNEL);
- if (!ifs_device.data.pkg_auth)
+ ifs_pkg_auth = kmalloc_array(topology_max_packages(), sizeof(bool), GFP_KERNEL);
+ if (!ifs_pkg_auth)
+ return -ENOMEM;
+
+ ifsd = kzalloc(sizeof(*ifsd), GFP_KERNEL);
+ if (!ifsd)
return -ENOMEM;

- ret = misc_register(&ifs_device.misc);
- if (ret) {
- kfree(ifs_device.data.pkg_auth);
- return ret;
+ ifsd->ro_info = &ifs_device.ro_data;
+ ifs_device.rw_data = ifsd;
+
+ if (misc_register(&ifs_device.misc)) {
+ kfree(ifsd);
+ kfree(ifs_pkg_auth);
+ return -ENODEV;
}

return 0;
+
}

static void __exit ifs_exit(void)
{
misc_deregister(&ifs_device.misc);
- kfree(ifs_device.data.pkg_auth);
+ kfree(ifs_device.rw_data);
+ kfree(ifs_pkg_auth);
}

module_init(ifs_init);
diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
index c5c24e6fdc43..cdec3316c08d 100644
--- a/drivers/platform/x86/intel/ifs/load.c
+++ b/drivers/platform/x86/intel/ifs/load.c
@@ -192,7 +192,7 @@ static int scan_chunks_sanity_check(struct device *dev)
struct ifs_work local_work;
int curr_pkg, cpu, ret;

- memset(ifsd->pkg_auth, 0, (topology_max_packages() * sizeof(bool)));
+ memset(ifs_pkg_auth, 0, (topology_max_packages() * sizeof(bool)));
ret = validate_ifs_metadata(dev);
if (ret)
return ret;
@@ -204,7 +204,7 @@ static int scan_chunks_sanity_check(struct device *dev)
cpus_read_lock();
for_each_online_cpu(cpu) {
curr_pkg = topology_physical_package_id(cpu);
- if (ifsd->pkg_auth[curr_pkg])
+ if (ifs_pkg_auth[curr_pkg])
continue;
reinit_completion(&ifs_done);
local_work.dev = dev;
@@ -215,7 +215,7 @@ static int scan_chunks_sanity_check(struct device *dev)
ret = -EIO;
goto out;
}
- ifsd->pkg_auth[curr_pkg] = 1;
+ ifs_pkg_auth[curr_pkg] = 1;
}
ret = 0;
out:
@@ -263,7 +263,7 @@ int ifs_load_firmware(struct device *dev)
int ret = -EINVAL;

snprintf(scan_path, sizeof(scan_path), "intel/ifs_%d/%02x-%02x-%02x-%02x.scan",
- ifsd->test_num, boot_cpu_data.x86, boot_cpu_data.x86_model,
+ ifsd->ro_info->test_num, boot_cpu_data.x86, boot_cpu_data.x86_model,
boot_cpu_data.x86_stepping, ifsd->cur_batch);

ret = request_firmware_direct(&fw, scan_path, dev);
--
2.25.1