| Message ID | 20220428153849.295779-5-tony.luck@intel.com (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | Introduce In Field Scan driver | expand |
On Thu, Apr 28 2022 at 08:38, Tony Luck wrote: > +/* > + * Load ifs image. Before loading ifs module, the ifs image must be located > + * in /lib/firmware/intel/ifs and named as {family/model/stepping}.{testname}. > + */ > +void ifs_load_firmware(struct device *dev) > +{ > + const struct firmware *fw; > + char scan_path[32]; > + int ret; > + > + snprintf(scan_path, sizeof(scan_path), "intel/ifs/%02x-%02x-%02x.scan", > + boot_cpu_data.x86, boot_cpu_data.x86_model, boot_cpu_data.x86_stepping); > + > + ret = request_firmware_direct(&fw, scan_path, dev); > + if (ret) { > + dev_err(dev, "ifs file %s load failed\n", scan_path); > + return; Why is this not returning an error to the caller? Thanks, tglx
>> + ret = request_firmware_direct(&fw, scan_path, dev); >> + if (ret) { >> + dev_err(dev, "ifs file %s load failed\n", scan_path); >> + return; > > Why is this not returning an error to the caller? There are three call sequences that lead here: 1) CONFIG_INTEL_IFS=y At boot this is pretty much guaranteed to fail (unless some crazy person includes the scan file in the initramfs). In this case there isn't a useful caller to return the code to. In this case the driver init routine must ignore any error to make sure the sysfs reload file is present to load the scan file once the file system is available. 2) CONFIG_INTEL_IFS=m module load time Same code path as built-in ... so must ignore a failed load here too (unless there is a way to distinguish the built-in vs. module execution environment). 3) echo 1 > reload Hmmm. Some older revision did return an error ... but I seem to have factored it out during cleanups. Limited to standard -E???? error codes. The reload_store() function can just check whether the load succeeded by looking at ifsd->loaded instead of a return value from ifs_load_firmware() So, I will fix case 3. But probably by checking ifsd->loaded rather than by adding a return code to this function. -Tony
diff --git a/drivers/platform/x86/intel/ifs/Makefile b/drivers/platform/x86/intel/ifs/Makefile index af904880e959..98b6fde15689 100644 --- a/drivers/platform/x86/intel/ifs/Makefile +++ b/drivers/platform/x86/intel/ifs/Makefile @@ -1,3 +1,3 @@ obj-$(CONFIG_INTEL_IFS) += intel_ifs.o -intel_ifs-objs := core.o +intel_ifs-objs := core.o load.o diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c index e3623ac691b5..d4a54ff47447 100644 --- a/drivers/platform/x86/intel/ifs/core.c +++ b/drivers/platform/x86/intel/ifs/core.c @@ -6,6 +6,8 @@ #include <asm/cpu_device_id.h> +#include "ifs.h" + #define X86_MATCH(model) \ X86_MATCH_VENDOR_FAM_MODEL_FEATURE(INTEL, 6, \ INTEL_FAM6_##model, X86_FEATURE_CORE_CAPABILITIES, NULL) @@ -16,6 +18,17 @@ static const struct x86_cpu_id ifs_cpu_ids[] __initconst = { }; MODULE_DEVICE_TABLE(x86cpu, ifs_cpu_ids); +static struct ifs_device ifs_device = { + .data = { + .integrity_cap_bit = MSR_INTEGRITY_CAPS_PERIODIC_BIST_BIT, + }, + .misc = { + .name = "intel_ifs_0", + .nodename = "intel_ifs/0", + .minor = MISC_DYNAMIC_MINOR, + }, +}; + static int __init ifs_init(void) { const struct x86_cpu_id *m; @@ -34,11 +47,19 @@ static int __init ifs_init(void) if (rdmsrl_safe(MSR_INTEGRITY_CAPS, &msrval)) return -ENODEV; + if ((msrval & BIT(ifs_device.data.integrity_cap_bit)) && + !misc_register(&ifs_device.misc)) { + ifs_load_firmware(ifs_device.misc.this_device); + } else { + return -ENODEV; + } + return 0; } static void __exit ifs_exit(void) { + misc_deregister(&ifs_device.misc); } module_init(ifs_init); diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h new file mode 100644 index 000000000000..9a0f8e2077e2 --- /dev/null +++ b/drivers/platform/x86/intel/ifs/ifs.h @@ -0,0 +1,25 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* Copyright(c) 2022 Intel Corporation. */ + +#ifndef _IFS_H_ +#define _IFS_H_ + +#include <linux/device.h> +#include <linux/miscdevice.h> + +/** + * struct ifs_data - attributes related to intel IFS driver + * @integrity_cap_bit - MSR_INTEGRITY_CAPS bit enumerating this test + */ +struct ifs_data { + int integrity_cap_bit; +}; + +struct ifs_device { + struct ifs_data data; + struct miscdevice misc; +}; + +void ifs_load_firmware(struct device *dev); + +#endif diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c new file mode 100644 index 000000000000..9fb71d38c819 --- /dev/null +++ b/drivers/platform/x86/intel/ifs/load.c @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Copyright(c) 2022 Intel Corporation. */ + +#include <linux/firmware.h> + +#include "ifs.h" + +/* + * Load ifs image. Before loading ifs module, the ifs image must be located + * in /lib/firmware/intel/ifs and named as {family/model/stepping}.{testname}. + */ +void ifs_load_firmware(struct device *dev) +{ + const struct firmware *fw; + char scan_path[32]; + int ret; + + snprintf(scan_path, sizeof(scan_path), "intel/ifs/%02x-%02x-%02x.scan", + boot_cpu_data.x86, boot_cpu_data.x86_model, boot_cpu_data.x86_stepping); + + ret = request_firmware_direct(&fw, scan_path, dev); + if (ret) { + dev_err(dev, "ifs file %s load failed\n", scan_path); + return; + } + + release_firmware(fw); +}