arch/x86/include/asm/cpu.h | 2 + arch/x86/include/asm/microcode_intel.h | 5 +- drivers/platform/x86/intel/ifs/ifs.h | 27 ++- arch/x86/kernel/cpu/intel.c | 129 +++++++++++ arch/x86/kernel/cpu/microcode/intel.c | 146 +------------ drivers/platform/x86/intel/ifs/core.c | 7 +- drivers/platform/x86/intel/ifs/load.c | 205 ++++++++++-------- drivers/platform/x86/intel/ifs/runtest.c | 10 +- drivers/platform/x86/intel/ifs/sysfs.c | 41 ++-- .../ABI/testing/sysfs-platform-intel-ifs | 30 +-- drivers/platform/x86/intel/ifs/Kconfig | 4 - 11 files changed, 330 insertions(+), 276 deletions(-)
Changes in v2
- Rebased ontop of v6.1-rc4
Boris
- Moved exported functions (microcode_sanity_check(),
find_matching_signature ) from microcode/intel.c to cpu/intel.c
(patch4,6)
- Removed microcode metadata specific code changes to
microcode_sanity_check() (patch6)
- Moved find_meta_data() from common to IFS driver (Patch 8)
Sohil
- Dropped portions of Patch2 from v1 and folded remaining to Patch12
- Rewording multiple commits
- Avoid meta prefix in fields of metadata_header (patch8)
- Defining MICROCODE_HEADER_VER* into common location (patch6, 9)
- Elaborating error messages w.r.t processor flags (patch9)
- Changed sysfs_emit() parameter (patch 11)
v1 patches available from
Link: https://lore.kernel.org/lkml/20221021203413.1220137-1-jithu.joseph@intel.com/
Initial implementation of IFS driver assumed a single IFS test image
file with a fixed name.
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).
This series makes the driver aware of multiple scan test image files,
modifies IFS test image file headers to make it fully compatible
with microcode headers and adds a few other bug fixes and changes.
Patch organization:
Patches 1, and 2: bug fixes
Patch 3: Removes image loading during init path
Patch 4, 5 and 6: exports a couple of microcode/intel.c functions
for use by driver
Rest of them are IFS driver changes
Patches 9: IFS header format changes to make it fully compatible
to intel microcode header format
Patch 7,8 and 10: microcode/IFS metadata section related
Patches 11, 12 and 13: native support for multiple scan test image files
Patch 14: reverts the broken flag
Ashok Raj (1):
platform/x86/intel/ifs: Add metadata support
Jithu Joseph (13):
platform/x86/intel/ifs: Remove unused selection
platform/x86/intel/ifs: return a more appropriate Error code
platform/x86/intel/ifs: Remove image loading during init
x86/microcode/intel: Expose find_matching_signature() for IFS
x86/microcode/intel: Use appropriate type in microcode_sanity_check()
x86/microcode/intel: Expose microcode_sanity_check()
x86/microcode/intel: Use a reserved field for metasize
platform/x86/intel/ifs: Use generic microcode headers and functions
platform/x86/intel/ifs: Add metadata validation
platform/x86/intel/ifs: Remove reload sysfs entry
platform/x86/intel/ifs: Add current_batch sysfs entry
Documentation/ABI: Update IFS ABI doc
Revert "platform/x86/intel/ifs: Mark as BROKEN"
arch/x86/include/asm/cpu.h | 2 +
arch/x86/include/asm/microcode_intel.h | 5 +-
drivers/platform/x86/intel/ifs/ifs.h | 27 ++-
arch/x86/kernel/cpu/intel.c | 129 +++++++++++
arch/x86/kernel/cpu/microcode/intel.c | 146 +------------
drivers/platform/x86/intel/ifs/core.c | 7 +-
drivers/platform/x86/intel/ifs/load.c | 205 ++++++++++--------
drivers/platform/x86/intel/ifs/runtest.c | 10 +-
drivers/platform/x86/intel/ifs/sysfs.c | 41 ++--
.../ABI/testing/sysfs-platform-intel-ifs | 30 +--
drivers/platform/x86/intel/ifs/Kconfig | 4 -
11 files changed, 330 insertions(+), 276 deletions(-)
base-commit: f0c4d9fc9cc9462659728d168387191387e903cc
--
2.25.1
Changes in v3
- Rebased ontop of v6.1-rc5
- Added reviewed-by tags from Sohil and Hans
Boris
- Moved dynamic memory allocation from scan_chunks_sanity_check()
to driver init (patch 4)
- Split v2's Expose microcode_sanity_check(), into 2 separate patches
(patch 7 and patch8).
- Add kerneldoc style comment to intel_microcode_sanity_check() and
change parameter name to hdr_type instead of hdr_ver (patch 8)
- Remove ifs_ prefix from static functions (patch 10, patch11)
- Rename macro names more appropriately (patch10, patch8, patch12)
- Remove obvious "what" from certain commit messages
- Fix typos and wordings
Dave
- Use union to describe ifs meta_data structure and use u32 types
Sohil
- Use unsigned int to store current_batch (patch 11, patch 14)
- Fix an inadvertent mistake in microcode_sanity_check (patch 7)
- Fix wordings
v2 patches available from
Link: https://lore.kernel.org/lkml/20221107225323.2733518-1-jithu.joseph@intel.com/
Changes in v2
- Rebased ontop of v6.1-rc4
Boris
- Moved exported functions (microcode_sanity_check(),
find_matching_signature ) from microcode/intel.c to cpu/intel.c
(patch4,6)
- Removed microcode metadata specific code changes to
microcode_sanity_check() (patch6)
- Moved find_meta_data() from common to IFS driver (Patch 8)
Sohil
- Dropped portions of Patch2 from v1 and folded remaining to Patch12
- Rewording multiple commits
- Avoid meta prefix in fields of metadata_header (patch8)
- Defining MICROCODE_HEADER_VER* into common location (patch6, 9)
- Elaborating error messages w.r.t processor flags (patch9)
- Changed sysfs_emit() parameter (patch 11)
v1 patches available from
Link: https://lore.kernel.org/lkml/20221021203413.1220137-1-jithu.joseph@intel.com/
Initial implementation of IFS driver assumed a single IFS test image
file with a fixed name.
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).
This series makes the driver aware of multiple scan test image files,
modifies IFS test image file headers to make it fully compatible
with microcode headers and adds a few other bug fixes and changes.
Patch organization:
Patches 1, and 2: bug fixes
Patch 3: Removes image loading during init path
Patch 4: Move memory allocation from load to init path
Patch 5, 6, 7, 8: exports a couple of microcode/intel.c functions
for use by driver
Rest of them are IFS driver changes
Patch 9,10 and 12: microcode/IFS metadata section related
Patches 11: IFS header format changes to make it fully compatible
to intel microcode header format
Patches 13, 14 and 15: native support for multiple scan test image files
Patch 16: reverts the broken flag
Ashok Raj (1):
platform/x86/intel/ifs: Add metadata support
Jithu Joseph (15):
platform/x86/intel/ifs: Remove unused selection
platform/x86/intel/ifs: Return a more appropriate Error code
platform/x86/intel/ifs: Remove image loading during init
platform/x86/intel/ifs: Remove memory allocation from load path
x86/microcode/intel: Reuse find_matching_signature()
x86/microcode/intel: Use appropriate type in microcode_sanity_check()
x86/microcode/intel: Reuse microcode_sanity_check()
x86/microcode/intel: Add hdr_type to intel_microcode_sanity_check()
x86/microcode/intel: Use a reserved field for metasize
platform/x86/intel/ifs: Use generic microcode headers and functions
platform/x86/intel/ifs: Add metadata validation
platform/x86/intel/ifs: Remove reload sysfs entry
platform/x86/intel/ifs: Add current_batch sysfs entry
Documentation/ABI: Update IFS ABI doc
Revert "platform/x86/intel/ifs: Mark as BROKEN"
arch/x86/include/asm/cpu.h | 2 +
arch/x86/include/asm/microcode_intel.h | 5 +-
drivers/platform/x86/intel/ifs/ifs.h | 27 ++-
arch/x86/kernel/cpu/intel.c | 141 +++++++++++
arch/x86/kernel/cpu/microcode/intel.c | 146 +-----------
drivers/platform/x86/intel/ifs/core.c | 14 +-
drivers/platform/x86/intel/ifs/load.c | 218 ++++++++++--------
drivers/platform/x86/intel/ifs/runtest.c | 10 +-
drivers/platform/x86/intel/ifs/sysfs.c | 41 ++--
.../ABI/testing/sysfs-platform-intel-ifs | 30 +--
drivers/platform/x86/intel/ifs/Kconfig | 4 -
11 files changed, 356 insertions(+), 282 deletions(-)
base-commit: 094226ad94f471a9f19e8f8e7140a09c2625abaa
--
2.25.1
CONFIG_INTEL_IFS_DEVICE is not used anywhere. The selection in
Kconfig is therefore pointless. Delete it.
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
---
drivers/platform/x86/intel/ifs/Kconfig | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/platform/x86/intel/ifs/Kconfig b/drivers/platform/x86/intel/ifs/Kconfig
index c341a27cc1a3..89152d46deee 100644
--- a/drivers/platform/x86/intel/ifs/Kconfig
+++ b/drivers/platform/x86/intel/ifs/Kconfig
@@ -4,7 +4,6 @@ config INTEL_IFS
# Discussion on the list has shown that the sysfs API needs a bit
# more work, mark this as broken for now
depends on BROKEN
- select INTEL_IFS_DEVICE
help
Enable support for the In Field Scan capability in select
CPUs. The capability allows for running low level tests via
--
2.25.1The following commit has been merged into the x86/microcode branch of tip:
Commit-ID: 10d4853e4c5cd64b9ef1e5579bb2e89bceab4175
Gitweb: https://git.kernel.org/tip/10d4853e4c5cd64b9ef1e5579bb2e89bceab4175
Author: Jithu Joseph <jithu.joseph@intel.com>
AuthorDate: Wed, 16 Nov 2022 19:59:20 -08:00
Committer: Borislav Petkov <bp@suse.de>
CommitterDate: Fri, 18 Nov 2022 14:59:20 +01:00
platform/x86/intel/ifs: Remove unused selection
CONFIG_INTEL_IFS_DEVICE is not used anywhere. The selection in
Kconfig is therefore pointless. Delete it.
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20221117035935.4136738-2-jithu.joseph@intel.com
---
drivers/platform/x86/intel/ifs/Kconfig | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/platform/x86/intel/ifs/Kconfig b/drivers/platform/x86/intel/ifs/Kconfig
index c341a27..89152d4 100644
--- a/drivers/platform/x86/intel/ifs/Kconfig
+++ b/drivers/platform/x86/intel/ifs/Kconfig
@@ -4,7 +4,6 @@ config INTEL_IFS
# Discussion on the list has shown that the sysfs API needs a bit
# more work, mark this as broken for now
depends on BROKEN
- select INTEL_IFS_DEVICE
help
Enable support for the In Field Scan capability in select
CPUs. The capability allows for running low level tests viascan_chunks_sanity_check() returns -ENOMEM if it encounters
an error while copying IFS test image from memory to Secure Memory.
Return -EIO in this scenario, as it is more appropriate.
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
---
drivers/platform/x86/intel/ifs/load.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
index d056617ddc85..89ce265887ea 100644
--- a/drivers/platform/x86/intel/ifs/load.c
+++ b/drivers/platform/x86/intel/ifs/load.c
@@ -157,8 +157,10 @@ static int scan_chunks_sanity_check(struct device *dev)
INIT_WORK(&local_work.w, copy_hashes_authenticate_chunks);
schedule_work_on(cpu, &local_work.w);
wait_for_completion(&ifs_done);
- if (ifsd->loading_error)
+ if (ifsd->loading_error) {
+ ret = -EIO;
goto out;
+ }
package_authenticated[curr_pkg] = 1;
}
ret = 0;
--
2.25.1The following commit has been merged into the x86/microcode branch of tip:
Commit-ID: f4e209e956b5d66f0e6e34e89f19811c2c1e596e
Gitweb: https://git.kernel.org/tip/f4e209e956b5d66f0e6e34e89f19811c2c1e596e
Author: Jithu Joseph <jithu.joseph@intel.com>
AuthorDate: Wed, 16 Nov 2022 19:59:21 -08:00
Committer: Borislav Petkov <bp@suse.de>
CommitterDate: Fri, 18 Nov 2022 21:30:36 +01:00
platform/x86/intel/ifs: Return a more appropriate error code
scan_chunks_sanity_check() returns -ENOMEM if it encounters an error
while copying IFS test image from memory to Secure Memory.
Return -EIO in this scenario, as it is more appropriate.
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20221117035935.4136738-3-jithu.joseph@intel.com
---
drivers/platform/x86/intel/ifs/load.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
index d056617..89ce265 100644
--- a/drivers/platform/x86/intel/ifs/load.c
+++ b/drivers/platform/x86/intel/ifs/load.c
@@ -157,8 +157,10 @@ static int scan_chunks_sanity_check(struct device *dev)
INIT_WORK(&local_work.w, copy_hashes_authenticate_chunks);
schedule_work_on(cpu, &local_work.w);
wait_for_completion(&ifs_done);
- if (ifsd->loading_error)
+ if (ifsd->loading_error) {
+ ret = -EIO;
goto out;
+ }
package_authenticated[curr_pkg] = 1;
}
ret = 0;IFS test image is unnecessarily loaded during driver initialization.
Drop image loading during ifs_init() and improve module load time.
With this change, user has to load one when starting the tests.
As a consequence, make ifs_sem static as it is only used within sysfs.c
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Suggested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
---
drivers/platform/x86/intel/ifs/ifs.h | 2 --
drivers/platform/x86/intel/ifs/core.c | 6 +-----
drivers/platform/x86/intel/ifs/sysfs.c | 2 +-
3 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
index 73c8e91cf144..3ff1d9aaeaa9 100644
--- a/drivers/platform/x86/intel/ifs/ifs.h
+++ b/drivers/platform/x86/intel/ifs/ifs.h
@@ -229,6 +229,4 @@ void ifs_load_firmware(struct device *dev);
int do_core_test(int cpu, struct device *dev);
const struct attribute_group **ifs_get_groups(void);
-extern struct semaphore ifs_sem;
-
#endif
diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
index 27204e3d674d..5fb7f655c291 100644
--- a/drivers/platform/x86/intel/ifs/core.c
+++ b/drivers/platform/x86/intel/ifs/core.c
@@ -51,12 +51,8 @@ static int __init ifs_init(void)
ifs_device.misc.groups = ifs_get_groups();
if ((msrval & BIT(ifs_device.data.integrity_cap_bit)) &&
- !misc_register(&ifs_device.misc)) {
- down(&ifs_sem);
- ifs_load_firmware(ifs_device.misc.this_device);
- up(&ifs_sem);
+ !misc_register(&ifs_device.misc))
return 0;
- }
return -ENODEV;
}
diff --git a/drivers/platform/x86/intel/ifs/sysfs.c b/drivers/platform/x86/intel/ifs/sysfs.c
index 37d8380d6fa8..65dd6fea5342 100644
--- a/drivers/platform/x86/intel/ifs/sysfs.c
+++ b/drivers/platform/x86/intel/ifs/sysfs.c
@@ -13,7 +13,7 @@
* Protects against simultaneous tests on multiple cores, or
* reloading can file while a test is in progress
*/
-DEFINE_SEMAPHORE(ifs_sem);
+static DEFINE_SEMAPHORE(ifs_sem);
/*
* The sysfs interface to check additional details of last test
--
2.25.1The following commit has been merged into the x86/microcode branch of tip:
Commit-ID: a4c30fa4ead5e6628e5ff5a45664ba7181acf6f1
Gitweb: https://git.kernel.org/tip/a4c30fa4ead5e6628e5ff5a45664ba7181acf6f1
Author: Jithu Joseph <jithu.joseph@intel.com>
AuthorDate: Wed, 16 Nov 2022 19:59:22 -08:00
Committer: Borislav Petkov <bp@suse.de>
CommitterDate: Fri, 18 Nov 2022 21:37:04 +01:00
platform/x86/intel/ifs: Remove image loading during init
IFS test image is unnecessarily loaded during driver initialization.
Drop image loading during ifs_init() and improve module load time. With
this change, user has to load one when starting the tests.
As a consequence, make ifs_sem static as it is only used within sysfs.c
Suggested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20221117035935.4136738-4-jithu.joseph@intel.com
---
drivers/platform/x86/intel/ifs/core.c | 6 +-----
drivers/platform/x86/intel/ifs/ifs.h | 2 --
drivers/platform/x86/intel/ifs/sysfs.c | 2 +-
3 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
index 27204e3..5fb7f65 100644
--- a/drivers/platform/x86/intel/ifs/core.c
+++ b/drivers/platform/x86/intel/ifs/core.c
@@ -51,12 +51,8 @@ static int __init ifs_init(void)
ifs_device.misc.groups = ifs_get_groups();
if ((msrval & BIT(ifs_device.data.integrity_cap_bit)) &&
- !misc_register(&ifs_device.misc)) {
- down(&ifs_sem);
- ifs_load_firmware(ifs_device.misc.this_device);
- up(&ifs_sem);
+ !misc_register(&ifs_device.misc))
return 0;
- }
return -ENODEV;
}
diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
index 73c8e91..3ff1d9a 100644
--- a/drivers/platform/x86/intel/ifs/ifs.h
+++ b/drivers/platform/x86/intel/ifs/ifs.h
@@ -229,6 +229,4 @@ void ifs_load_firmware(struct device *dev);
int do_core_test(int cpu, struct device *dev);
const struct attribute_group **ifs_get_groups(void);
-extern struct semaphore ifs_sem;
-
#endif
diff --git a/drivers/platform/x86/intel/ifs/sysfs.c b/drivers/platform/x86/intel/ifs/sysfs.c
index 37d8380..65dd6fe 100644
--- a/drivers/platform/x86/intel/ifs/sysfs.c
+++ b/drivers/platform/x86/intel/ifs/sysfs.c
@@ -13,7 +13,7 @@
* Protects against simultaneous tests on multiple cores, or
* reloading can file while a test is in progress
*/
-DEFINE_SEMAPHORE(ifs_sem);
+static DEFINE_SEMAPHORE(ifs_sem);
/*
* The sysfs interface to check additional details of last testIFS requires tests to be authenticated once for each CPU socket
on a system.
scan_chunks_sanity_check() was dynamically allocating memory
to store the state of whether tests have been authenticated on
each socket for every load operation.
Move the memory allocation to init path.
Reviewed-by: Tony Luck <tony.luck@intel.com>
Suggested-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
---
drivers/platform/x86/intel/ifs/ifs.h | 2 ++
drivers/platform/x86/intel/ifs/core.c | 13 +++++++++++--
drivers/platform/x86/intel/ifs/load.c | 14 ++++----------
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
index 3ff1d9aaeaa9..3a051890d9e7 100644
--- a/drivers/platform/x86/intel/ifs/ifs.h
+++ b/drivers/platform/x86/intel/ifs/ifs.h
@@ -229,4 +229,6 @@ void ifs_load_firmware(struct device *dev);
int do_core_test(int cpu, struct device *dev);
const struct attribute_group **ifs_get_groups(void);
+extern bool *ifs_pkg_auth;
+
#endif
diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
index 5fb7f655c291..4b39f2359180 100644
--- a/drivers/platform/x86/intel/ifs/core.c
+++ b/drivers/platform/x86/intel/ifs/core.c
@@ -4,6 +4,7 @@
#include <linux/module.h>
#include <linux/kdev_t.h>
#include <linux/semaphore.h>
+#include <linux/slab.h>
#include <asm/cpu_device_id.h>
@@ -30,6 +31,8 @@ static struct ifs_device ifs_device = {
},
};
+bool *ifs_pkg_auth;
+
static int __init ifs_init(void)
{
const struct x86_cpu_id *m;
@@ -51,8 +54,13 @@ static int __init ifs_init(void)
ifs_device.misc.groups = ifs_get_groups();
if ((msrval & BIT(ifs_device.data.integrity_cap_bit)) &&
- !misc_register(&ifs_device.misc))
- return 0;
+ !misc_register(&ifs_device.misc)) {
+ ifs_pkg_auth = kmalloc_array(topology_max_packages(), sizeof(bool), GFP_KERNEL);
+ if (!ifs_pkg_auth)
+ return -ENOMEM;
+ else
+ return 0;
+ }
return -ENODEV;
}
@@ -60,6 +68,7 @@ static int __init ifs_init(void)
static void __exit ifs_exit(void)
{
misc_deregister(&ifs_device.misc);
+ 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 89ce265887ea..c914e4d359db 100644
--- a/drivers/platform/x86/intel/ifs/load.c
+++ b/drivers/platform/x86/intel/ifs/load.c
@@ -3,7 +3,6 @@
#include <linux/firmware.h>
#include <asm/cpu.h>
-#include <linux/slab.h>
#include <asm/microcode_intel.h>
#include "ifs.h"
@@ -118,16 +117,12 @@ static void copy_hashes_authenticate_chunks(struct work_struct *work)
*/
static int scan_chunks_sanity_check(struct device *dev)
{
- int metadata_size, curr_pkg, cpu, ret = -ENOMEM;
+ int metadata_size, curr_pkg, cpu, ret;
struct ifs_data *ifsd = ifs_get_data(dev);
- bool *package_authenticated;
struct ifs_work local_work;
char *test_ptr;
- package_authenticated = kcalloc(topology_max_packages(), sizeof(bool), GFP_KERNEL);
- if (!package_authenticated)
- return ret;
-
+ memset(ifs_pkg_auth, 0, (topology_max_packages() * sizeof(bool)));
metadata_size = ifs_header_ptr->metadata_size;
/* Spec says that if the Meta Data Size = 0 then it should be treated as 2000 */
@@ -150,7 +145,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 (package_authenticated[curr_pkg])
+ if (ifs_pkg_auth[curr_pkg])
continue;
reinit_completion(&ifs_done);
local_work.dev = dev;
@@ -161,12 +156,11 @@ static int scan_chunks_sanity_check(struct device *dev)
ret = -EIO;
goto out;
}
- package_authenticated[curr_pkg] = 1;
+ ifs_pkg_auth[curr_pkg] = 1;
}
ret = 0;
out:
cpus_read_unlock();
- kfree(package_authenticated);
return ret;
}
--
2.25.1Hi,
On 11/17/22 04:59, Jithu Joseph wrote:
> IFS requires tests to be authenticated once for each CPU socket
> on a system.
>
> scan_chunks_sanity_check() was dynamically allocating memory
> to store the state of whether tests have been authenticated on
> each socket for every load operation.
>
> Move the memory allocation to init path.
>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> Suggested-by: Borislav Petkov <bp@alien8.de>
> Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
> ---
> drivers/platform/x86/intel/ifs/ifs.h | 2 ++
> drivers/platform/x86/intel/ifs/core.c | 13 +++++++++++--
> drivers/platform/x86/intel/ifs/load.c | 14 ++++----------
> 3 files changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
> index 3ff1d9aaeaa9..3a051890d9e7 100644
> --- a/drivers/platform/x86/intel/ifs/ifs.h
> +++ b/drivers/platform/x86/intel/ifs/ifs.h
> @@ -229,4 +229,6 @@ void ifs_load_firmware(struct device *dev);
> int do_core_test(int cpu, struct device *dev);
> const struct attribute_group **ifs_get_groups(void);
>
> +extern bool *ifs_pkg_auth;
> +
This is not necessary and ugly, nack for this patch as-is (sorry).
You can simply add this pointer to "struct ifs_data" and then
alloc it in ifs_init() before the misc_register call.
scan_chunks_sanity_check() already has a "struct ifs_data *ifsd",
so it can easily access ifs_pkg_auth through that when you make
ifs_pkg_auth part of "struct ifs_data".
Regards,
Hans
\
> #endif
> diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
> index 5fb7f655c291..4b39f2359180 100644
> --- a/drivers/platform/x86/intel/ifs/core.c
> +++ b/drivers/platform/x86/intel/ifs/core.c
> @@ -4,6 +4,7 @@
> #include <linux/module.h>
> #include <linux/kdev_t.h>
> #include <linux/semaphore.h>
> +#include <linux/slab.h>
>
> #include <asm/cpu_device_id.h>
>
> @@ -30,6 +31,8 @@ static struct ifs_device ifs_device = {
> },
> };
>
> +bool *ifs_pkg_auth;
> +
> static int __init ifs_init(void)
> {
> const struct x86_cpu_id *m;
> @@ -51,8 +54,13 @@ static int __init ifs_init(void)
> ifs_device.misc.groups = ifs_get_groups();
>
> if ((msrval & BIT(ifs_device.data.integrity_cap_bit)) &&
> - !misc_register(&ifs_device.misc))
> - return 0;
> + !misc_register(&ifs_device.misc)) {
> + ifs_pkg_auth = kmalloc_array(topology_max_packages(), sizeof(bool), GFP_KERNEL);
> + if (!ifs_pkg_auth)
> + return -ENOMEM;
> + else
> + return 0;
> + }
>
> return -ENODEV;
> }
> @@ -60,6 +68,7 @@ static int __init ifs_init(void)
> static void __exit ifs_exit(void)
> {
> misc_deregister(&ifs_device.misc);
> + 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 89ce265887ea..c914e4d359db 100644
> --- a/drivers/platform/x86/intel/ifs/load.c
> +++ b/drivers/platform/x86/intel/ifs/load.c
> @@ -3,7 +3,6 @@
>
> #include <linux/firmware.h>
> #include <asm/cpu.h>
> -#include <linux/slab.h>
> #include <asm/microcode_intel.h>
>
> #include "ifs.h"
> @@ -118,16 +117,12 @@ static void copy_hashes_authenticate_chunks(struct work_struct *work)
> */
> static int scan_chunks_sanity_check(struct device *dev)
> {
> - int metadata_size, curr_pkg, cpu, ret = -ENOMEM;
> + int metadata_size, curr_pkg, cpu, ret;
> struct ifs_data *ifsd = ifs_get_data(dev);
> - bool *package_authenticated;
> struct ifs_work local_work;
> char *test_ptr;
>
> - package_authenticated = kcalloc(topology_max_packages(), sizeof(bool), GFP_KERNEL);
> - if (!package_authenticated)
> - return ret;
> -
> + memset(ifs_pkg_auth, 0, (topology_max_packages() * sizeof(bool)));
> metadata_size = ifs_header_ptr->metadata_size;
>
> /* Spec says that if the Meta Data Size = 0 then it should be treated as 2000 */
> @@ -150,7 +145,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 (package_authenticated[curr_pkg])
> + if (ifs_pkg_auth[curr_pkg])
> continue;
> reinit_completion(&ifs_done);
> local_work.dev = dev;
> @@ -161,12 +156,11 @@ static int scan_chunks_sanity_check(struct device *dev)
> ret = -EIO;
> goto out;
> }
> - package_authenticated[curr_pkg] = 1;
> + ifs_pkg_auth[curr_pkg] = 1;
> }
> ret = 0;
> out:
> cpus_read_unlock();
> - kfree(package_authenticated);
>
> return ret;
> }
IFS requires tests to be authenticated once for each CPU socket
on a system.
scan_chunks_sanity_check() was dynamically allocating memory
to store the state of whether tests have been authenticated on
each socket for every load operation.
Move the memory allocation to init path and store the pointer
in ifs_data struct.
Reviewed-by: Tony Luck <tony.luck@intel.com>
Suggested-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
---
- Replaced global pkg_auth pointer to struct ifs_data (Hans)
- With this change there are conflicts in patches 11 and 12 (I will
post the updated 11 and 12 if this is satisfactory)
drivers/platform/x86/intel/ifs/ifs.h | 2 ++
drivers/platform/x86/intel/ifs/core.c | 12 ++++++++++--
drivers/platform/x86/intel/ifs/load.c | 14 ++++----------
3 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
index 3ff1d9aaeaa9..8de1952a1b7b 100644
--- a/drivers/platform/x86/intel/ifs/ifs.h
+++ b/drivers/platform/x86/intel/ifs/ifs.h
@@ -191,6 +191,7 @@ union ifs_status {
* 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.
@@ -199,6 +200,7 @@ union ifs_status {
*/
struct ifs_data {
int integrity_cap_bit;
+ bool *pkg_auth;
int loaded_version;
bool loaded;
bool loading_error;
diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
index 5fb7f655c291..6980a31e9786 100644
--- a/drivers/platform/x86/intel/ifs/core.c
+++ b/drivers/platform/x86/intel/ifs/core.c
@@ -4,6 +4,7 @@
#include <linux/module.h>
#include <linux/kdev_t.h>
#include <linux/semaphore.h>
+#include <linux/slab.h>
#include <asm/cpu_device_id.h>
@@ -51,8 +52,14 @@ static int __init ifs_init(void)
ifs_device.misc.groups = ifs_get_groups();
if ((msrval & BIT(ifs_device.data.integrity_cap_bit)) &&
- !misc_register(&ifs_device.misc))
- return 0;
+ !misc_register(&ifs_device.misc)) {
+ ifs_device.data.pkg_auth = kmalloc_array(topology_max_packages(),
+ sizeof(bool), GFP_KERNEL);
+ if (!ifs_device.data.pkg_auth)
+ return -ENOMEM;
+ else
+ return 0;
+ }
return -ENODEV;
}
@@ -60,6 +67,7 @@ static int __init ifs_init(void)
static void __exit ifs_exit(void)
{
misc_deregister(&ifs_device.misc);
+ kfree(ifs_device.data.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 89ce265887ea..8423c486d11b 100644
--- a/drivers/platform/x86/intel/ifs/load.c
+++ b/drivers/platform/x86/intel/ifs/load.c
@@ -3,7 +3,6 @@
#include <linux/firmware.h>
#include <asm/cpu.h>
-#include <linux/slab.h>
#include <asm/microcode_intel.h>
#include "ifs.h"
@@ -118,16 +117,12 @@ static void copy_hashes_authenticate_chunks(struct work_struct *work)
*/
static int scan_chunks_sanity_check(struct device *dev)
{
- int metadata_size, curr_pkg, cpu, ret = -ENOMEM;
+ int metadata_size, curr_pkg, cpu, ret;
struct ifs_data *ifsd = ifs_get_data(dev);
- bool *package_authenticated;
struct ifs_work local_work;
char *test_ptr;
- package_authenticated = kcalloc(topology_max_packages(), sizeof(bool), GFP_KERNEL);
- if (!package_authenticated)
- return ret;
-
+ memset(ifsd->pkg_auth, 0, (topology_max_packages() * sizeof(bool)));
metadata_size = ifs_header_ptr->metadata_size;
/* Spec says that if the Meta Data Size = 0 then it should be treated as 2000 */
@@ -150,7 +145,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 (package_authenticated[curr_pkg])
+ if (ifsd->pkg_auth[curr_pkg])
continue;
reinit_completion(&ifs_done);
local_work.dev = dev;
@@ -161,12 +156,11 @@ static int scan_chunks_sanity_check(struct device *dev)
ret = -EIO;
goto out;
}
- package_authenticated[curr_pkg] = 1;
+ ifsd->pkg_auth[curr_pkg] = 1;
}
ret = 0;
out:
cpus_read_unlock();
- kfree(package_authenticated);
return ret;
}
--
2.25.1Hi Jithu,
On 11/17/22 18:29, Jithu Joseph wrote:
> IFS requires tests to be authenticated once for each CPU socket
> on a system.
>
> scan_chunks_sanity_check() was dynamically allocating memory
> to store the state of whether tests have been authenticated on
> each socket for every load operation.
>
> Move the memory allocation to init path and store the pointer
> in ifs_data struct.
>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> Suggested-by: Borislav Petkov <bp@alien8.de>
> Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
> ---
> - Replaced global pkg_auth pointer to struct ifs_data (Hans)
> - With this change there are conflicts in patches 11 and 12 (I will
> post the updated 11 and 12 if this is satisfactory)
>
> drivers/platform/x86/intel/ifs/ifs.h | 2 ++
> drivers/platform/x86/intel/ifs/core.c | 12 ++++++++++--
> drivers/platform/x86/intel/ifs/load.c | 14 ++++----------
> 3 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
> index 3ff1d9aaeaa9..8de1952a1b7b 100644
> --- a/drivers/platform/x86/intel/ifs/ifs.h
> +++ b/drivers/platform/x86/intel/ifs/ifs.h
> @@ -191,6 +191,7 @@ union ifs_status {
> * 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.
> @@ -199,6 +200,7 @@ union ifs_status {
> */
> struct ifs_data {
> int integrity_cap_bit;
> + bool *pkg_auth;
> int loaded_version;
> bool loaded;
> bool loading_error;
> diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
> index 5fb7f655c291..6980a31e9786 100644
> --- a/drivers/platform/x86/intel/ifs/core.c
> +++ b/drivers/platform/x86/intel/ifs/core.c
> @@ -4,6 +4,7 @@
> #include <linux/module.h>
> #include <linux/kdev_t.h>
> #include <linux/semaphore.h>
> +#include <linux/slab.h>
>
> #include <asm/cpu_device_id.h>
>
> @@ -51,8 +52,14 @@ static int __init ifs_init(void)
> ifs_device.misc.groups = ifs_get_groups();
>
> if ((msrval & BIT(ifs_device.data.integrity_cap_bit)) &&
> - !misc_register(&ifs_device.misc))
> - return 0;
> + !misc_register(&ifs_device.misc)) {
> + ifs_device.data.pkg_auth = kmalloc_array(topology_max_packages(),
> + sizeof(bool), GFP_KERNEL);
Thank you for the new version, but as I mentioned in my review, this kmalloc
must be done *before* the misc_register(&ifs_device.misc), because as soon
as that is done the other code may get triggered creating a race condition.
More in general && the misc_register to gether with the integrity_cap_bit
is not really nice. If someone does not pay close attention they may
mis that the check of the if has the pretty big side-effect of
registering the actual misc device.
Generally speaking test-conditions for if-s should not have side
effects if possible.
> + if (!ifs_device.data.pkg_auth)
> + return -ENOMEM;
> + else
> + return 0;
> + }
>
> return -ENODEV;
> }
This also makes me realize that you have your -ENODEV error exit and
your normal success exit paths switched around from what is normal.
Why not just write the above as (can be done as part of this
patch since you need to touch it all anyways):
if (!(msrval & BIT(ifs_device.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)
return -ENOMEM
ret = misc_register(&ifs_device.misc);
if (ret) {
kfree(ifs_device.data.pkg_auth);
return ret;
}
return 0;
}
That makes this all look much more like a normal probe() function
with the success 0 return at the end.
Where as your version has the success 0 return nested 2 levels
deep in the else of a kmalloc() error check...
Regards,
Hans
> @@ -60,6 +67,7 @@ static int __init ifs_init(void)
> static void __exit ifs_exit(void)
> {
> misc_deregister(&ifs_device.misc);
> + kfree(ifs_device.data.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 89ce265887ea..8423c486d11b 100644
> --- a/drivers/platform/x86/intel/ifs/load.c
> +++ b/drivers/platform/x86/intel/ifs/load.c
> @@ -3,7 +3,6 @@
>
> #include <linux/firmware.h>
> #include <asm/cpu.h>
> -#include <linux/slab.h>
> #include <asm/microcode_intel.h>
>
> #include "ifs.h"
> @@ -118,16 +117,12 @@ static void copy_hashes_authenticate_chunks(struct work_struct *work)
> */
> static int scan_chunks_sanity_check(struct device *dev)
> {
> - int metadata_size, curr_pkg, cpu, ret = -ENOMEM;
> + int metadata_size, curr_pkg, cpu, ret;
> struct ifs_data *ifsd = ifs_get_data(dev);
> - bool *package_authenticated;
> struct ifs_work local_work;
> char *test_ptr;
>
> - package_authenticated = kcalloc(topology_max_packages(), sizeof(bool), GFP_KERNEL);
> - if (!package_authenticated)
> - return ret;
> -
> + memset(ifsd->pkg_auth, 0, (topology_max_packages() * sizeof(bool)));
> metadata_size = ifs_header_ptr->metadata_size;
>
> /* Spec says that if the Meta Data Size = 0 then it should be treated as 2000 */
> @@ -150,7 +145,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 (package_authenticated[curr_pkg])
> + if (ifsd->pkg_auth[curr_pkg])
> continue;
> reinit_completion(&ifs_done);
> local_work.dev = dev;
> @@ -161,12 +156,11 @@ static int scan_chunks_sanity_check(struct device *dev)
> ret = -EIO;
> goto out;
> }
> - package_authenticated[curr_pkg] = 1;
> + ifsd->pkg_auth[curr_pkg] = 1;
> }
> ret = 0;
> out:
> cpus_read_unlock();
> - kfree(package_authenticated);
>
> return ret;
> }
IFS requires tests to be authenticated once for each CPU socket
on a system.
scan_chunks_sanity_check() was dynamically allocating memory
to store the state of whether tests have been authenticated on
each socket for every load operation.
Move the memory allocation to init path and store the pointer
in ifs_data struct.
Also rearrange the adjacent error checking in init for a
more simplified and natural flow.
Reviewed-by: Tony Luck <tony.luck@intel.com>
Suggested-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
---
- Replaced global pkg_auth pointer to struct ifs_data (Hans)
- Rearrange the adjacent error checking flow in ifs_init (Hans)
- With this change there are conflicts in patches 11 and 12 (I will
post the updated 11 and 12 if this is satisfactory)
drivers/platform/x86/intel/ifs/ifs.h | 2 ++
drivers/platform/x86/intel/ifs/core.c | 20 ++++++++++++++++----
drivers/platform/x86/intel/ifs/load.c | 14 ++++----------
3 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
index 3ff1d9aaeaa9..8de1952a1b7b 100644
--- a/drivers/platform/x86/intel/ifs/ifs.h
+++ b/drivers/platform/x86/intel/ifs/ifs.h
@@ -191,6 +191,7 @@ union ifs_status {
* 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.
@@ -199,6 +200,7 @@ union ifs_status {
*/
struct ifs_data {
int integrity_cap_bit;
+ bool *pkg_auth;
int loaded_version;
bool loaded;
bool loading_error;
diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
index 5fb7f655c291..943eb2a17c64 100644
--- a/drivers/platform/x86/intel/ifs/core.c
+++ b/drivers/platform/x86/intel/ifs/core.c
@@ -4,6 +4,7 @@
#include <linux/module.h>
#include <linux/kdev_t.h>
#include <linux/semaphore.h>
+#include <linux/slab.h>
#include <asm/cpu_device_id.h>
@@ -34,6 +35,7 @@ static int __init ifs_init(void)
{
const struct x86_cpu_id *m;
u64 msrval;
+ int ret;
m = x86_match_cpu(ifs_cpu_ids);
if (!m)
@@ -50,16 +52,26 @@ static int __init ifs_init(void)
ifs_device.misc.groups = ifs_get_groups();
- if ((msrval & BIT(ifs_device.data.integrity_cap_bit)) &&
- !misc_register(&ifs_device.misc))
- return 0;
+ if (!(msrval & BIT(ifs_device.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)
+ return -ENOMEM;
+
+ ret = misc_register(&ifs_device.misc);
+ if (ret) {
+ kfree(ifs_device.data.pkg_auth);
+ return ret;
+ }
- return -ENODEV;
+ return 0;
}
static void __exit ifs_exit(void)
{
misc_deregister(&ifs_device.misc);
+ kfree(ifs_device.data.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 89ce265887ea..8423c486d11b 100644
--- a/drivers/platform/x86/intel/ifs/load.c
+++ b/drivers/platform/x86/intel/ifs/load.c
@@ -3,7 +3,6 @@
#include <linux/firmware.h>
#include <asm/cpu.h>
-#include <linux/slab.h>
#include <asm/microcode_intel.h>
#include "ifs.h"
@@ -118,16 +117,12 @@ static void copy_hashes_authenticate_chunks(struct work_struct *work)
*/
static int scan_chunks_sanity_check(struct device *dev)
{
- int metadata_size, curr_pkg, cpu, ret = -ENOMEM;
+ int metadata_size, curr_pkg, cpu, ret;
struct ifs_data *ifsd = ifs_get_data(dev);
- bool *package_authenticated;
struct ifs_work local_work;
char *test_ptr;
- package_authenticated = kcalloc(topology_max_packages(), sizeof(bool), GFP_KERNEL);
- if (!package_authenticated)
- return ret;
-
+ memset(ifsd->pkg_auth, 0, (topology_max_packages() * sizeof(bool)));
metadata_size = ifs_header_ptr->metadata_size;
/* Spec says that if the Meta Data Size = 0 then it should be treated as 2000 */
@@ -150,7 +145,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 (package_authenticated[curr_pkg])
+ if (ifsd->pkg_auth[curr_pkg])
continue;
reinit_completion(&ifs_done);
local_work.dev = dev;
@@ -161,12 +156,11 @@ static int scan_chunks_sanity_check(struct device *dev)
ret = -EIO;
goto out;
}
- package_authenticated[curr_pkg] = 1;
+ ifsd->pkg_auth[curr_pkg] = 1;
}
ret = 0;
out:
cpus_read_unlock();
- kfree(package_authenticated);
return ret;
}
--
2.25.1Hi,
On 11/17/22 20:59, Jithu Joseph wrote:
> IFS requires tests to be authenticated once for each CPU socket
> on a system.
>
> scan_chunks_sanity_check() was dynamically allocating memory
> to store the state of whether tests have been authenticated on
> each socket for every load operation.
>
> Move the memory allocation to init path and store the pointer
> in ifs_data struct.
>
> Also rearrange the adjacent error checking in init for a
> more simplified and natural flow.
>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> Suggested-by: Borislav Petkov <bp@alien8.de>
> Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
> ---
> - Replaced global pkg_auth pointer to struct ifs_data (Hans)
> - Rearrange the adjacent error checking flow in ifs_init (Hans)
> - With this change there are conflicts in patches 11 and 12 (I will
> post the updated 11 and 12 if this is satisfactory)
Thanks, this patch looks good to me now:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
>
> drivers/platform/x86/intel/ifs/ifs.h | 2 ++
> drivers/platform/x86/intel/ifs/core.c | 20 ++++++++++++++++----
> drivers/platform/x86/intel/ifs/load.c | 14 ++++----------
> 3 files changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
> index 3ff1d9aaeaa9..8de1952a1b7b 100644
> --- a/drivers/platform/x86/intel/ifs/ifs.h
> +++ b/drivers/platform/x86/intel/ifs/ifs.h
> @@ -191,6 +191,7 @@ union ifs_status {
> * 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.
> @@ -199,6 +200,7 @@ union ifs_status {
> */
> struct ifs_data {
> int integrity_cap_bit;
> + bool *pkg_auth;
> int loaded_version;
> bool loaded;
> bool loading_error;
> diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
> index 5fb7f655c291..943eb2a17c64 100644
> --- a/drivers/platform/x86/intel/ifs/core.c
> +++ b/drivers/platform/x86/intel/ifs/core.c
> @@ -4,6 +4,7 @@
> #include <linux/module.h>
> #include <linux/kdev_t.h>
> #include <linux/semaphore.h>
> +#include <linux/slab.h>
>
> #include <asm/cpu_device_id.h>
>
> @@ -34,6 +35,7 @@ static int __init ifs_init(void)
> {
> const struct x86_cpu_id *m;
> u64 msrval;
> + int ret;
>
> m = x86_match_cpu(ifs_cpu_ids);
> if (!m)
> @@ -50,16 +52,26 @@ static int __init ifs_init(void)
>
> ifs_device.misc.groups = ifs_get_groups();
>
> - if ((msrval & BIT(ifs_device.data.integrity_cap_bit)) &&
> - !misc_register(&ifs_device.misc))
> - return 0;
> + if (!(msrval & BIT(ifs_device.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)
> + return -ENOMEM;
> +
> + ret = misc_register(&ifs_device.misc);
> + if (ret) {
> + kfree(ifs_device.data.pkg_auth);
> + return ret;
> + }
>
> - return -ENODEV;
> + return 0;
> }
>
> static void __exit ifs_exit(void)
> {
> misc_deregister(&ifs_device.misc);
> + kfree(ifs_device.data.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 89ce265887ea..8423c486d11b 100644
> --- a/drivers/platform/x86/intel/ifs/load.c
> +++ b/drivers/platform/x86/intel/ifs/load.c
> @@ -3,7 +3,6 @@
>
> #include <linux/firmware.h>
> #include <asm/cpu.h>
> -#include <linux/slab.h>
> #include <asm/microcode_intel.h>
>
> #include "ifs.h"
> @@ -118,16 +117,12 @@ static void copy_hashes_authenticate_chunks(struct work_struct *work)
> */
> static int scan_chunks_sanity_check(struct device *dev)
> {
> - int metadata_size, curr_pkg, cpu, ret = -ENOMEM;
> + int metadata_size, curr_pkg, cpu, ret;
> struct ifs_data *ifsd = ifs_get_data(dev);
> - bool *package_authenticated;
> struct ifs_work local_work;
> char *test_ptr;
>
> - package_authenticated = kcalloc(topology_max_packages(), sizeof(bool), GFP_KERNEL);
> - if (!package_authenticated)
> - return ret;
> -
> + memset(ifsd->pkg_auth, 0, (topology_max_packages() * sizeof(bool)));
> metadata_size = ifs_header_ptr->metadata_size;
>
> /* Spec says that if the Meta Data Size = 0 then it should be treated as 2000 */
> @@ -150,7 +145,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 (package_authenticated[curr_pkg])
> + if (ifsd->pkg_auth[curr_pkg])
> continue;
> reinit_completion(&ifs_done);
> local_work.dev = dev;
> @@ -161,12 +156,11 @@ static int scan_chunks_sanity_check(struct device *dev)
> ret = -EIO;
> goto out;
> }
> - package_authenticated[curr_pkg] = 1;
> + ifsd->pkg_auth[curr_pkg] = 1;
> }
> ret = 0;
> out:
> cpus_read_unlock();
> - kfree(package_authenticated);
>
> return ret;
> }
On 11/17/2022 1:13 PM, Hans de Goede wrote: > Hi, > > On 11/17/22 20:59, Jithu Joseph wrote: >> IFS requires tests to be authenticated once for each CPU socket >> on a system. >> >> scan_chunks_sanity_check() was dynamically allocating memory >> to store the state of whether tests have been authenticated on >> each socket for every load operation. >> >> Move the memory allocation to init path and store the pointer >> in ifs_data struct. >> >> Also rearrange the adjacent error checking in init for a >> more simplified and natural flow. >> >> Reviewed-by: Tony Luck <tony.luck@intel.com> >> Suggested-by: Borislav Petkov <bp@alien8.de> >> Signed-off-by: Jithu Joseph <jithu.joseph@intel.com> >> --- >> - Replaced global pkg_auth pointer to struct ifs_data (Hans) >> - Rearrange the adjacent error checking flow in ifs_init (Hans) >> - With this change there are conflicts in patches 11 and 12 (I will >> post the updated 11 and 12 if this is satisfactory) > > Thanks, this patch looks good to me now: > > Reviewed-by: Hans de Goede <hdegoede@redhat.com> > Thanks for the detailed review and suggestions. I will now resend patches 11 and 12 which will apply ontop of this revised patch4 . Jithu
The following commit has been merged into the x86/microcode branch of tip:
Commit-ID: cb5eceee816bf05667089869d822b9cbc919465a
Gitweb: https://git.kernel.org/tip/cb5eceee816bf05667089869d822b9cbc919465a
Author: Jithu Joseph <jithu.joseph@intel.com>
AuthorDate: Thu, 17 Nov 2022 11:59:57 -08:00
Committer: Borislav Petkov <bp@suse.de>
CommitterDate: Fri, 18 Nov 2022 21:43:18 +01:00
platform/x86/intel/ifs: Remove memory allocation from load path
IFS requires tests to be authenticated once for each CPU socket on a
system.
scan_chunks_sanity_check() was dynamically allocating memory to store
the state of whether tests have been authenticated on each socket for
every load operation.
Move the memory allocation to init path and store the pointer in
ifs_data struct.
Also rearrange the adjacent error checking in init for a more simplified
and natural flow.
Suggested-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20221117195957.28225-1-jithu.joseph@intel.com
---
drivers/platform/x86/intel/ifs/core.c | 20 ++++++++++++++++----
drivers/platform/x86/intel/ifs/ifs.h | 2 ++
drivers/platform/x86/intel/ifs/load.c | 14 ++++----------
3 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
index 5fb7f65..943eb2a 100644
--- a/drivers/platform/x86/intel/ifs/core.c
+++ b/drivers/platform/x86/intel/ifs/core.c
@@ -4,6 +4,7 @@
#include <linux/module.h>
#include <linux/kdev_t.h>
#include <linux/semaphore.h>
+#include <linux/slab.h>
#include <asm/cpu_device_id.h>
@@ -34,6 +35,7 @@ static int __init ifs_init(void)
{
const struct x86_cpu_id *m;
u64 msrval;
+ int ret;
m = x86_match_cpu(ifs_cpu_ids);
if (!m)
@@ -50,16 +52,26 @@ static int __init ifs_init(void)
ifs_device.misc.groups = ifs_get_groups();
- if ((msrval & BIT(ifs_device.data.integrity_cap_bit)) &&
- !misc_register(&ifs_device.misc))
- return 0;
+ if (!(msrval & BIT(ifs_device.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)
+ return -ENOMEM;
+
+ ret = misc_register(&ifs_device.misc);
+ if (ret) {
+ kfree(ifs_device.data.pkg_auth);
+ return ret;
+ }
- return -ENODEV;
+ return 0;
}
static void __exit ifs_exit(void)
{
misc_deregister(&ifs_device.misc);
+ kfree(ifs_device.data.pkg_auth);
}
module_init(ifs_init);
diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
index 3ff1d9a..8de1952 100644
--- a/drivers/platform/x86/intel/ifs/ifs.h
+++ b/drivers/platform/x86/intel/ifs/ifs.h
@@ -191,6 +191,7 @@ union ifs_status {
* 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.
@@ -199,6 +200,7 @@ union ifs_status {
*/
struct ifs_data {
int integrity_cap_bit;
+ bool *pkg_auth;
int loaded_version;
bool loaded;
bool loading_error;
diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
index 89ce265..8423c48 100644
--- a/drivers/platform/x86/intel/ifs/load.c
+++ b/drivers/platform/x86/intel/ifs/load.c
@@ -3,7 +3,6 @@
#include <linux/firmware.h>
#include <asm/cpu.h>
-#include <linux/slab.h>
#include <asm/microcode_intel.h>
#include "ifs.h"
@@ -118,16 +117,12 @@ done:
*/
static int scan_chunks_sanity_check(struct device *dev)
{
- int metadata_size, curr_pkg, cpu, ret = -ENOMEM;
+ int metadata_size, curr_pkg, cpu, ret;
struct ifs_data *ifsd = ifs_get_data(dev);
- bool *package_authenticated;
struct ifs_work local_work;
char *test_ptr;
- package_authenticated = kcalloc(topology_max_packages(), sizeof(bool), GFP_KERNEL);
- if (!package_authenticated)
- return ret;
-
+ memset(ifsd->pkg_auth, 0, (topology_max_packages() * sizeof(bool)));
metadata_size = ifs_header_ptr->metadata_size;
/* Spec says that if the Meta Data Size = 0 then it should be treated as 2000 */
@@ -150,7 +145,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 (package_authenticated[curr_pkg])
+ if (ifsd->pkg_auth[curr_pkg])
continue;
reinit_completion(&ifs_done);
local_work.dev = dev;
@@ -161,12 +156,11 @@ static int scan_chunks_sanity_check(struct device *dev)
ret = -EIO;
goto out;
}
- package_authenticated[curr_pkg] = 1;
+ ifsd->pkg_auth[curr_pkg] = 1;
}
ret = 0;
out:
cpus_read_unlock();
- kfree(package_authenticated);
return ret;
}IFS uses test images provided by Intel that can be regarded as
firmware. IFS test image carries microcode header with extended signature
table.
Reuse find_matching_signature() for verifying if the test image
header or the extended signature table indicate whether an IFS test image
is fit to run on a system.
No functional change
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Ashok Raj <ashok.raj@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
---
arch/x86/include/asm/cpu.h | 1 +
arch/x86/kernel/cpu/intel.c | 29 ++++++++++++++++++
arch/x86/kernel/cpu/microcode/intel.c | 44 +++++----------------------
3 files changed, 38 insertions(+), 36 deletions(-)
diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index b472ef76826a..e853440b5c65 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -95,5 +95,6 @@ static inline bool intel_cpu_signatures_match(unsigned int s1, unsigned int p1,
}
extern u64 x86_read_arch_cap_msr(void);
+int intel_find_matching_signature(void *mc, unsigned int csig, int cpf);
#endif /* _ASM_X86_CPU_H */
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 2d7ea5480ec3..b6f9210fb31a 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -216,6 +216,35 @@ int intel_cpu_collect_info(struct ucode_cpu_info *uci)
}
EXPORT_SYMBOL_GPL(intel_cpu_collect_info);
+/*
+ * Returns 1 if update has been found, 0 otherwise.
+ */
+int intel_find_matching_signature(void *mc, unsigned int csig, int cpf)
+{
+ struct microcode_header_intel *mc_hdr = mc;
+ struct extended_sigtable *ext_hdr;
+ struct extended_signature *ext_sig;
+ int i;
+
+ if (intel_cpu_signatures_match(csig, cpf, mc_hdr->sig, mc_hdr->pf))
+ return 1;
+
+ /* Look for ext. headers: */
+ if (get_totalsize(mc_hdr) <= get_datasize(mc_hdr) + MC_HEADER_SIZE)
+ return 0;
+
+ ext_hdr = mc + get_datasize(mc_hdr) + MC_HEADER_SIZE;
+ ext_sig = (void *)ext_hdr + EXT_HEADER_SIZE;
+
+ for (i = 0; i < ext_hdr->count; i++) {
+ if (intel_cpu_signatures_match(csig, cpf, ext_sig->sig, ext_sig->pf))
+ return 1;
+ ext_sig++;
+ }
+ return 0;
+}
+EXPORT_SYMBOL_GPL(intel_find_matching_signature);
+
static void early_init_intel(struct cpuinfo_x86 *c)
{
u64 misc_enable;
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index 1fcbd671f1df..4e611a708718 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -45,34 +45,6 @@ static struct microcode_intel *intel_ucode_patch;
/* last level cache size per core */
static int llc_size_per_core;
-/*
- * Returns 1 if update has been found, 0 otherwise.
- */
-static int find_matching_signature(void *mc, unsigned int csig, int cpf)
-{
- struct microcode_header_intel *mc_hdr = mc;
- struct extended_sigtable *ext_hdr;
- struct extended_signature *ext_sig;
- int i;
-
- if (intel_cpu_signatures_match(csig, cpf, mc_hdr->sig, mc_hdr->pf))
- return 1;
-
- /* Look for ext. headers: */
- if (get_totalsize(mc_hdr) <= get_datasize(mc_hdr) + MC_HEADER_SIZE)
- return 0;
-
- ext_hdr = mc + get_datasize(mc_hdr) + MC_HEADER_SIZE;
- ext_sig = (void *)ext_hdr + EXT_HEADER_SIZE;
-
- for (i = 0; i < ext_hdr->count; i++) {
- if (intel_cpu_signatures_match(csig, cpf, ext_sig->sig, ext_sig->pf))
- return 1;
- ext_sig++;
- }
- return 0;
-}
-
/*
* Returns 1 if update has been found, 0 otherwise.
*/
@@ -83,7 +55,7 @@ static int has_newer_microcode(void *mc, unsigned int csig, int cpf, int new_rev
if (mc_hdr->rev <= new_rev)
return 0;
- return find_matching_signature(mc, csig, cpf);
+ return intel_find_matching_signature(mc, csig, cpf);
}
static struct ucode_patch *memdup_patch(void *data, unsigned int size)
@@ -117,7 +89,7 @@ static void save_microcode_patch(struct ucode_cpu_info *uci, void *data, unsigne
sig = mc_saved_hdr->sig;
pf = mc_saved_hdr->pf;
- if (find_matching_signature(data, sig, pf)) {
+ if (intel_find_matching_signature(data, sig, pf)) {
prev_found = true;
if (mc_hdr->rev <= mc_saved_hdr->rev)
@@ -149,7 +121,7 @@ static void save_microcode_patch(struct ucode_cpu_info *uci, void *data, unsigne
if (!p)
return;
- if (!find_matching_signature(p->data, uci->cpu_sig.sig, uci->cpu_sig.pf))
+ if (!intel_find_matching_signature(p->data, uci->cpu_sig.sig, uci->cpu_sig.pf))
return;
/*
@@ -286,8 +258,8 @@ scan_microcode(void *data, size_t size, struct ucode_cpu_info *uci, bool save)
size -= mc_size;
- if (!find_matching_signature(data, uci->cpu_sig.sig,
- uci->cpu_sig.pf)) {
+ if (!intel_find_matching_signature(data, uci->cpu_sig.sig,
+ uci->cpu_sig.pf)) {
data += mc_size;
continue;
}
@@ -652,9 +624,9 @@ static struct microcode_intel *find_patch(struct ucode_cpu_info *uci)
if (phdr->rev <= uci->cpu_sig.rev)
continue;
- if (!find_matching_signature(phdr,
- uci->cpu_sig.sig,
- uci->cpu_sig.pf))
+ if (!intel_find_matching_signature(phdr,
+ uci->cpu_sig.sig,
+ uci->cpu_sig.pf))
continue;
return iter->data;
--
2.25.1The following commit has been merged into the x86/microcode branch of tip:
Commit-ID: 716f380275866350ee44447b3c7c999f39c3178d
Gitweb: https://git.kernel.org/tip/716f380275866350ee44447b3c7c999f39c3178d
Author: Jithu Joseph <jithu.joseph@intel.com>
AuthorDate: Wed, 16 Nov 2022 19:59:24 -08:00
Committer: Borislav Petkov <bp@suse.de>
CommitterDate: Fri, 18 Nov 2022 21:50:01 +01:00
x86/microcode/intel: Reuse find_matching_signature()
IFS uses test images provided by Intel that can be regarded as firmware.
An IFS test image carries microcode header with an extended signature
table.
Reuse find_matching_signature() for verifying if the test image header
or the extended signature table indicate whether that image is fit to
run on a system.
No functional changes.
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Ashok Raj <ashok.raj@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Link: https://lore.kernel.org/r/20221117035935.4136738-6-jithu.joseph@intel.com
---
arch/x86/include/asm/cpu.h | 1 +-
arch/x86/kernel/cpu/intel.c | 29 +++++++++++++++++-
arch/x86/kernel/cpu/microcode/intel.c | 44 ++++----------------------
3 files changed, 38 insertions(+), 36 deletions(-)
diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index b472ef7..e853440 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -95,5 +95,6 @@ static inline bool intel_cpu_signatures_match(unsigned int s1, unsigned int p1,
}
extern u64 x86_read_arch_cap_msr(void);
+int intel_find_matching_signature(void *mc, unsigned int csig, int cpf);
#endif /* _ASM_X86_CPU_H */
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index beb8ca5..c7331ec 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -215,6 +215,35 @@ int intel_cpu_collect_info(struct ucode_cpu_info *uci)
}
EXPORT_SYMBOL_GPL(intel_cpu_collect_info);
+/*
+ * Returns 1 if update has been found, 0 otherwise.
+ */
+int intel_find_matching_signature(void *mc, unsigned int csig, int cpf)
+{
+ struct microcode_header_intel *mc_hdr = mc;
+ struct extended_sigtable *ext_hdr;
+ struct extended_signature *ext_sig;
+ int i;
+
+ if (intel_cpu_signatures_match(csig, cpf, mc_hdr->sig, mc_hdr->pf))
+ return 1;
+
+ /* Look for ext. headers: */
+ if (get_totalsize(mc_hdr) <= get_datasize(mc_hdr) + MC_HEADER_SIZE)
+ return 0;
+
+ ext_hdr = mc + get_datasize(mc_hdr) + MC_HEADER_SIZE;
+ ext_sig = (void *)ext_hdr + EXT_HEADER_SIZE;
+
+ for (i = 0; i < ext_hdr->count; i++) {
+ if (intel_cpu_signatures_match(csig, cpf, ext_sig->sig, ext_sig->pf))
+ return 1;
+ ext_sig++;
+ }
+ return 0;
+}
+EXPORT_SYMBOL_GPL(intel_find_matching_signature);
+
static void early_init_intel(struct cpuinfo_x86 *c)
{
u64 misc_enable;
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index 8c35c70..c77ec1b 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -48,34 +48,6 @@ static int llc_size_per_core;
/*
* Returns 1 if update has been found, 0 otherwise.
*/
-static int find_matching_signature(void *mc, unsigned int csig, int cpf)
-{
- struct microcode_header_intel *mc_hdr = mc;
- struct extended_sigtable *ext_hdr;
- struct extended_signature *ext_sig;
- int i;
-
- if (intel_cpu_signatures_match(csig, cpf, mc_hdr->sig, mc_hdr->pf))
- return 1;
-
- /* Look for ext. headers: */
- if (get_totalsize(mc_hdr) <= get_datasize(mc_hdr) + MC_HEADER_SIZE)
- return 0;
-
- ext_hdr = mc + get_datasize(mc_hdr) + MC_HEADER_SIZE;
- ext_sig = (void *)ext_hdr + EXT_HEADER_SIZE;
-
- for (i = 0; i < ext_hdr->count; i++) {
- if (intel_cpu_signatures_match(csig, cpf, ext_sig->sig, ext_sig->pf))
- return 1;
- ext_sig++;
- }
- return 0;
-}
-
-/*
- * Returns 1 if update has been found, 0 otherwise.
- */
static int has_newer_microcode(void *mc, unsigned int csig, int cpf, int new_rev)
{
struct microcode_header_intel *mc_hdr = mc;
@@ -83,7 +55,7 @@ static int has_newer_microcode(void *mc, unsigned int csig, int cpf, int new_rev
if (mc_hdr->rev <= new_rev)
return 0;
- return find_matching_signature(mc, csig, cpf);
+ return intel_find_matching_signature(mc, csig, cpf);
}
static struct ucode_patch *memdup_patch(void *data, unsigned int size)
@@ -117,7 +89,7 @@ static void save_microcode_patch(struct ucode_cpu_info *uci, void *data, unsigne
sig = mc_saved_hdr->sig;
pf = mc_saved_hdr->pf;
- if (find_matching_signature(data, sig, pf)) {
+ if (intel_find_matching_signature(data, sig, pf)) {
prev_found = true;
if (mc_hdr->rev <= mc_saved_hdr->rev)
@@ -149,7 +121,7 @@ static void save_microcode_patch(struct ucode_cpu_info *uci, void *data, unsigne
if (!p)
return;
- if (!find_matching_signature(p->data, uci->cpu_sig.sig, uci->cpu_sig.pf))
+ if (!intel_find_matching_signature(p->data, uci->cpu_sig.sig, uci->cpu_sig.pf))
return;
/*
@@ -286,8 +258,8 @@ scan_microcode(void *data, size_t size, struct ucode_cpu_info *uci, bool save)
size -= mc_size;
- if (!find_matching_signature(data, uci->cpu_sig.sig,
- uci->cpu_sig.pf)) {
+ if (!intel_find_matching_signature(data, uci->cpu_sig.sig,
+ uci->cpu_sig.pf)) {
data += mc_size;
continue;
}
@@ -652,9 +624,9 @@ static struct microcode_intel *find_patch(struct ucode_cpu_info *uci)
if (phdr->rev <= uci->cpu_sig.rev)
continue;
- if (!find_matching_signature(phdr,
- uci->cpu_sig.sig,
- uci->cpu_sig.pf))
+ if (!intel_find_matching_signature(phdr,
+ uci->cpu_sig.sig,
+ uci->cpu_sig.pf))
continue;
return iter->data;The data type of print_err parameter used by microcode_sanity_check()
is int. In preparation for exporting this function, to be used by
the IFS driver, convert it to a more appropriate bool type for readability.
No functional change intended.
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Ashok Raj <ashok.raj@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Suggested-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
---
arch/x86/kernel/cpu/microcode/intel.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index 4e611a708718..234b163806ea 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -135,7 +135,7 @@ static void save_microcode_patch(struct ucode_cpu_info *uci, void *data, unsigne
intel_ucode_patch = p->data;
}
-static int microcode_sanity_check(void *mc, int print_err)
+static int microcode_sanity_check(void *mc, bool print_err)
{
unsigned long total_size, data_size, ext_table_size;
struct microcode_header_intel *mc_header = mc;
@@ -253,7 +253,7 @@ scan_microcode(void *data, size_t size, struct ucode_cpu_info *uci, bool save)
mc_size = get_totalsize(mc_header);
if (!mc_size ||
mc_size > size ||
- microcode_sanity_check(data, 0) < 0)
+ microcode_sanity_check(data, false) < 0)
break;
size -= mc_size;
@@ -792,7 +792,7 @@ static enum ucode_state generic_load_microcode(int cpu, struct iov_iter *iter)
memcpy(mc, &mc_header, sizeof(mc_header));
data = mc + sizeof(mc_header);
if (!copy_from_iter_full(data, data_size, iter) ||
- microcode_sanity_check(mc, 1) < 0) {
+ microcode_sanity_check(mc, true) < 0) {
break;
}
--
2.25.1The following commit has been merged into the x86/microcode branch of tip:
Commit-ID: 2e13ab0158dd4a033d61a5baa8f9b5b7c9ea8431
Gitweb: https://git.kernel.org/tip/2e13ab0158dd4a033d61a5baa8f9b5b7c9ea8431
Author: Jithu Joseph <jithu.joseph@intel.com>
AuthorDate: Wed, 16 Nov 2022 19:59:25 -08:00
Committer: Borislav Petkov <bp@suse.de>
CommitterDate: Fri, 18 Nov 2022 21:51:56 +01:00
x86/microcode/intel: Use appropriate type in microcode_sanity_check()
The data type of the @print_err parameter used by microcode_sanity_check()
is int. In preparation for exporting this function to be used by
the IFS driver convert it to a more appropriate bool type for readability.
No functional change intended.
Suggested-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Ashok Raj <ashok.raj@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Link: https://lore.kernel.org/r/20221117035935.4136738-7-jithu.joseph@intel.com
---
arch/x86/kernel/cpu/microcode/intel.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index c77ec1b..e48f05e 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -135,7 +135,7 @@ static void save_microcode_patch(struct ucode_cpu_info *uci, void *data, unsigne
intel_ucode_patch = p->data;
}
-static int microcode_sanity_check(void *mc, int print_err)
+static int microcode_sanity_check(void *mc, bool print_err)
{
unsigned long total_size, data_size, ext_table_size;
struct microcode_header_intel *mc_header = mc;
@@ -253,7 +253,7 @@ scan_microcode(void *data, size_t size, struct ucode_cpu_info *uci, bool save)
mc_size = get_totalsize(mc_header);
if (!mc_size ||
mc_size > size ||
- microcode_sanity_check(data, 0) < 0)
+ microcode_sanity_check(data, false) < 0)
break;
size -= mc_size;
@@ -792,7 +792,7 @@ static enum ucode_state generic_load_microcode(int cpu, struct iov_iter *iter)
memcpy(mc, &mc_header, sizeof(mc_header));
data = mc + sizeof(mc_header);
if (!copy_from_iter_full(data, data_size, iter) ||
- microcode_sanity_check(mc, 1) < 0) {
+ microcode_sanity_check(mc, true) < 0) {
break;
}IFS test image carries the same microcode header as regular Intel
microcode blobs.
microcode_sanity_check() can be used by IFS driver to perform
sanity check of the IFS test images too.
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Ashok Raj <ashok.raj@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
---
arch/x86/include/asm/cpu.h | 1 +
arch/x86/kernel/cpu/intel.c | 99 +++++++++++++++++++++++++
arch/x86/kernel/cpu/microcode/intel.c | 102 +-------------------------
3 files changed, 102 insertions(+), 100 deletions(-)
diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index e853440b5c65..9e3ac95acf2d 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -96,5 +96,6 @@ static inline bool intel_cpu_signatures_match(unsigned int s1, unsigned int p1,
extern u64 x86_read_arch_cap_msr(void);
int intel_find_matching_signature(void *mc, unsigned int csig, int cpf);
+int intel_microcode_sanity_check(void *mc, bool print_err);
#endif /* _ASM_X86_CPU_H */
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index b6f9210fb31a..01e73ec1d585 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -245,6 +245,105 @@ int intel_find_matching_signature(void *mc, unsigned int csig, int cpf)
}
EXPORT_SYMBOL_GPL(intel_find_matching_signature);
+int intel_microcode_sanity_check(void *mc, bool print_err)
+{
+ unsigned long total_size, data_size, ext_table_size;
+ struct microcode_header_intel *mc_header = mc;
+ struct extended_sigtable *ext_header = NULL;
+ u32 sum, orig_sum, ext_sigcount = 0, i;
+ struct extended_signature *ext_sig;
+
+ total_size = get_totalsize(mc_header);
+ data_size = get_datasize(mc_header);
+
+ if (data_size + MC_HEADER_SIZE > total_size) {
+ if (print_err)
+ pr_err("Error: bad microcode data file size.\n");
+ return -EINVAL;
+ }
+
+ if (mc_header->ldrver != 1 || mc_header->hdrver != 1) {
+ if (print_err)
+ pr_err("Error: invalid/unknown microcode update format.\n");
+ return -EINVAL;
+ }
+
+ ext_table_size = total_size - (MC_HEADER_SIZE + data_size);
+ if (ext_table_size) {
+ u32 ext_table_sum = 0;
+ u32 *ext_tablep;
+
+ if (ext_table_size < EXT_HEADER_SIZE ||
+ ((ext_table_size - EXT_HEADER_SIZE) % EXT_SIGNATURE_SIZE)) {
+ if (print_err)
+ pr_err("Error: truncated extended signature table.\n");
+ return -EINVAL;
+ }
+
+ ext_header = mc + MC_HEADER_SIZE + data_size;
+ if (ext_table_size != exttable_size(ext_header)) {
+ if (print_err)
+ pr_err("Error: extended signature table size mismatch.\n");
+ return -EFAULT;
+ }
+
+ ext_sigcount = ext_header->count;
+
+ /*
+ * Check extended table checksum: the sum of all dwords that
+ * comprise a valid table must be 0.
+ */
+ ext_tablep = (u32 *)ext_header;
+
+ i = ext_table_size / sizeof(u32);
+ while (i--)
+ ext_table_sum += ext_tablep[i];
+
+ if (ext_table_sum) {
+ if (print_err)
+ pr_warn("Bad extended signature table checksum, aborting.\n");
+ return -EINVAL;
+ }
+ }
+
+ /*
+ * Calculate the checksum of update data and header. The checksum of
+ * valid update data and header including the extended signature table
+ * must be 0.
+ */
+ orig_sum = 0;
+ i = (MC_HEADER_SIZE + data_size) / sizeof(u32);
+ while (i--)
+ orig_sum += ((u32 *)mc)[i];
+
+ if (orig_sum) {
+ if (print_err)
+ pr_err("Bad microcode data checksum, aborting.\n");
+ return -EINVAL;
+ }
+
+ if (!ext_table_size)
+ return 0;
+
+ /*
+ * Check extended signature checksum: 0 => valid.
+ */
+ for (i = 0; i < ext_sigcount; i++) {
+ ext_sig = (void *)ext_header + EXT_HEADER_SIZE +
+ EXT_SIGNATURE_SIZE * i;
+
+ sum = (mc_header->sig + mc_header->pf + mc_header->cksum) -
+ (ext_sig->sig + ext_sig->pf + ext_sig->cksum);
+ if (sum) {
+ if (print_err)
+ pr_err("Bad extended signature checksum, aborting.\n");
+ return -EINVAL;
+ }
+ }
+ return 0;
+}
+EXPORT_SYMBOL_GPL(intel_microcode_sanity_check);
+
static void early_init_intel(struct cpuinfo_x86 *c)
{
u64 misc_enable;
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index 234b163806ea..af7134073e65 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -135,104 +135,6 @@ static void save_microcode_patch(struct ucode_cpu_info *uci, void *data, unsigne
intel_ucode_patch = p->data;
}
-static int microcode_sanity_check(void *mc, bool print_err)
-{
- unsigned long total_size, data_size, ext_table_size;
- struct microcode_header_intel *mc_header = mc;
- struct extended_sigtable *ext_header = NULL;
- u32 sum, orig_sum, ext_sigcount = 0, i;
- struct extended_signature *ext_sig;
-
- total_size = get_totalsize(mc_header);
- data_size = get_datasize(mc_header);
-
- if (data_size + MC_HEADER_SIZE > total_size) {
- if (print_err)
- pr_err("Error: bad microcode data file size.\n");
- return -EINVAL;
- }
-
- if (mc_header->ldrver != 1 || mc_header->hdrver != 1) {
- if (print_err)
- pr_err("Error: invalid/unknown microcode update format.\n");
- return -EINVAL;
- }
-
- ext_table_size = total_size - (MC_HEADER_SIZE + data_size);
- if (ext_table_size) {
- u32 ext_table_sum = 0;
- u32 *ext_tablep;
-
- if ((ext_table_size < EXT_HEADER_SIZE)
- || ((ext_table_size - EXT_HEADER_SIZE) % EXT_SIGNATURE_SIZE)) {
- if (print_err)
- pr_err("Error: truncated extended signature table.\n");
- return -EINVAL;
- }
-
- ext_header = mc + MC_HEADER_SIZE + data_size;
- if (ext_table_size != exttable_size(ext_header)) {
- if (print_err)
- pr_err("Error: extended signature table size mismatch.\n");
- return -EFAULT;
- }
-
- ext_sigcount = ext_header->count;
-
- /*
- * Check extended table checksum: the sum of all dwords that
- * comprise a valid table must be 0.
- */
- ext_tablep = (u32 *)ext_header;
-
- i = ext_table_size / sizeof(u32);
- while (i--)
- ext_table_sum += ext_tablep[i];
-
- if (ext_table_sum) {
- if (print_err)
- pr_warn("Bad extended signature table checksum, aborting.\n");
- return -EINVAL;
- }
- }
-
- /*
- * Calculate the checksum of update data and header. The checksum of
- * valid update data and header including the extended signature table
- * must be 0.
- */
- orig_sum = 0;
- i = (MC_HEADER_SIZE + data_size) / sizeof(u32);
- while (i--)
- orig_sum += ((u32 *)mc)[i];
-
- if (orig_sum) {
- if (print_err)
- pr_err("Bad microcode data checksum, aborting.\n");
- return -EINVAL;
- }
-
- if (!ext_table_size)
- return 0;
-
- /*
- * Check extended signature checksum: 0 => valid.
- */
- for (i = 0; i < ext_sigcount; i++) {
- ext_sig = (void *)ext_header + EXT_HEADER_SIZE +
- EXT_SIGNATURE_SIZE * i;
-
- sum = (mc_header->sig + mc_header->pf + mc_header->cksum) -
- (ext_sig->sig + ext_sig->pf + ext_sig->cksum);
- if (sum) {
- if (print_err)
- pr_err("Bad extended signature checksum, aborting.\n");
- return -EINVAL;
- }
- }
- return 0;
-}
-
/*
* Get microcode matching with BSP's model. Only CPUs with the same model as
* BSP can stay in the platform.
@@ -253,7 +155,7 @@ scan_microcode(void *data, size_t size, struct ucode_cpu_info *uci, bool save)
mc_size = get_totalsize(mc_header);
if (!mc_size ||
mc_size > size ||
- microcode_sanity_check(data, false) < 0)
+ intel_microcode_sanity_check(data, false) < 0)
break;
size -= mc_size;
@@ -792,7 +694,7 @@ static enum ucode_state generic_load_microcode(int cpu, struct iov_iter *iter)
memcpy(mc, &mc_header, sizeof(mc_header));
data = mc + sizeof(mc_header);
if (!copy_from_iter_full(data, data_size, iter) ||
- microcode_sanity_check(mc, true) < 0) {
+ intel_microcode_sanity_check(mc, true) < 0) {
break;
}
--
2.25.1The following commit has been merged into the x86/microcode branch of tip:
Commit-ID: 514ee839c6d0750c1c4456502e6fa08599e57931
Gitweb: https://git.kernel.org/tip/514ee839c6d0750c1c4456502e6fa08599e57931
Author: Jithu Joseph <jithu.joseph@intel.com>
AuthorDate: Wed, 16 Nov 2022 19:59:26 -08:00
Committer: Borislav Petkov <bp@suse.de>
CommitterDate: Fri, 18 Nov 2022 22:00:17 +01:00
x86/microcode/intel: Reuse microcode_sanity_check()
IFS test image carries the same microcode header as regular Intel
microcode blobs.
Reuse microcode_sanity_check() in the IFS driver to perform sanity check
of the IFS test images too.
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Ashok Raj <ashok.raj@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Link: https://lore.kernel.org/r/20221117035935.4136738-8-jithu.joseph@intel.com
---
arch/x86/include/asm/cpu.h | 1 +-
arch/x86/kernel/cpu/intel.c | 99 ++++++++++++++++++++++++-
arch/x86/kernel/cpu/microcode/intel.c | 102 +-------------------------
3 files changed, 102 insertions(+), 100 deletions(-)
diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index e853440..9e3ac95 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -96,5 +96,6 @@ static inline bool intel_cpu_signatures_match(unsigned int s1, unsigned int p1,
extern u64 x86_read_arch_cap_msr(void);
int intel_find_matching_signature(void *mc, unsigned int csig, int cpf);
+int intel_microcode_sanity_check(void *mc, bool print_err);
#endif /* _ASM_X86_CPU_H */
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index c7331ec..bef06a1 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -244,6 +244,105 @@ int intel_find_matching_signature(void *mc, unsigned int csig, int cpf)
}
EXPORT_SYMBOL_GPL(intel_find_matching_signature);
+int intel_microcode_sanity_check(void *mc, bool print_err)
+{
+ unsigned long total_size, data_size, ext_table_size;
+ struct microcode_header_intel *mc_header = mc;
+ struct extended_sigtable *ext_header = NULL;
+ u32 sum, orig_sum, ext_sigcount = 0, i;
+ struct extended_signature *ext_sig;
+
+ total_size = get_totalsize(mc_header);
+ data_size = get_datasize(mc_header);
+
+ if (data_size + MC_HEADER_SIZE > total_size) {
+ if (print_err)
+ pr_err("Error: bad microcode data file size.\n");
+ return -EINVAL;
+ }
+
+ if (mc_header->ldrver != 1 || mc_header->hdrver != 1) {
+ if (print_err)
+ pr_err("Error: invalid/unknown microcode update format.\n");
+ return -EINVAL;
+ }
+
+ ext_table_size = total_size - (MC_HEADER_SIZE + data_size);
+ if (ext_table_size) {
+ u32 ext_table_sum = 0;
+ u32 *ext_tablep;
+
+ if (ext_table_size < EXT_HEADER_SIZE ||
+ ((ext_table_size - EXT_HEADER_SIZE) % EXT_SIGNATURE_SIZE)) {
+ if (print_err)
+ pr_err("Error: truncated extended signature table.\n");
+ return -EINVAL;
+ }
+
+ ext_header = mc + MC_HEADER_SIZE + data_size;
+ if (ext_table_size != exttable_size(ext_header)) {
+ if (print_err)
+ pr_err("Error: extended signature table size mismatch.\n");
+ return -EFAULT;
+ }
+
+ ext_sigcount = ext_header->count;
+
+ /*
+ * Check extended table checksum: the sum of all dwords that
+ * comprise a valid table must be 0.
+ */
+ ext_tablep = (u32 *)ext_header;
+
+ i = ext_table_size / sizeof(u32);
+ while (i--)
+ ext_table_sum += ext_tablep[i];
+
+ if (ext_table_sum) {
+ if (print_err)
+ pr_warn("Bad extended signature table checksum, aborting.\n");
+ return -EINVAL;
+ }
+ }
+
+ /*
+ * Calculate the checksum of update data and header. The checksum of
+ * valid update data and header including the extended signature table
+ * must be 0.
+ */
+ orig_sum = 0;
+ i = (MC_HEADER_SIZE + data_size) / sizeof(u32);
+ while (i--)
+ orig_sum += ((u32 *)mc)[i];
+
+ if (orig_sum) {
+ if (print_err)
+ pr_err("Bad microcode data checksum, aborting.\n");
+ return -EINVAL;
+ }
+
+ if (!ext_table_size)
+ return 0;
+
+ /*
+ * Check extended signature checksum: 0 => valid.
+ */
+ for (i = 0; i < ext_sigcount; i++) {
+ ext_sig = (void *)ext_header + EXT_HEADER_SIZE +
+ EXT_SIGNATURE_SIZE * i;
+
+ sum = (mc_header->sig + mc_header->pf + mc_header->cksum) -
+ (ext_sig->sig + ext_sig->pf + ext_sig->cksum);
+ if (sum) {
+ if (print_err)
+ pr_err("Bad extended signature checksum, aborting.\n");
+ return -EINVAL;
+ }
+ }
+ return 0;
+}
+EXPORT_SYMBOL_GPL(intel_microcode_sanity_check);
+
static void early_init_intel(struct cpuinfo_x86 *c)
{
u64 misc_enable;
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index e48f05e..fb6ff71 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -135,104 +135,6 @@ static void save_microcode_patch(struct ucode_cpu_info *uci, void *data, unsigne
intel_ucode_patch = p->data;
}
-static int microcode_sanity_check(void *mc, bool print_err)
-{
- unsigned long total_size, data_size, ext_table_size;
- struct microcode_header_intel *mc_header = mc;
- struct extended_sigtable *ext_header = NULL;
- u32 sum, orig_sum, ext_sigcount = 0, i;
- struct extended_signature *ext_sig;
-
- total_size = get_totalsize(mc_header);
- data_size = get_datasize(mc_header);
-
- if (data_size + MC_HEADER_SIZE > total_size) {
- if (print_err)
- pr_err("Error: bad microcode data file size.\n");
- return -EINVAL;
- }
-
- if (mc_header->ldrver != 1 || mc_header->hdrver != 1) {
- if (print_err)
- pr_err("Error: invalid/unknown microcode update format.\n");
- return -EINVAL;
- }
-
- ext_table_size = total_size - (MC_HEADER_SIZE + data_size);
- if (ext_table_size) {
- u32 ext_table_sum = 0;
- u32 *ext_tablep;
-
- if ((ext_table_size < EXT_HEADER_SIZE)
- || ((ext_table_size - EXT_HEADER_SIZE) % EXT_SIGNATURE_SIZE)) {
- if (print_err)
- pr_err("Error: truncated extended signature table.\n");
- return -EINVAL;
- }
-
- ext_header = mc + MC_HEADER_SIZE + data_size;
- if (ext_table_size != exttable_size(ext_header)) {
- if (print_err)
- pr_err("Error: extended signature table size mismatch.\n");
- return -EFAULT;
- }
-
- ext_sigcount = ext_header->count;
-
- /*
- * Check extended table checksum: the sum of all dwords that
- * comprise a valid table must be 0.
- */
- ext_tablep = (u32 *)ext_header;
-
- i = ext_table_size / sizeof(u32);
- while (i--)
- ext_table_sum += ext_tablep[i];
-
- if (ext_table_sum) {
- if (print_err)
- pr_warn("Bad extended signature table checksum, aborting.\n");
- return -EINVAL;
- }
- }
-
- /*
- * Calculate the checksum of update data and header. The checksum of
- * valid update data and header including the extended signature table
- * must be 0.
- */
- orig_sum = 0;
- i = (MC_HEADER_SIZE + data_size) / sizeof(u32);
- while (i--)
- orig_sum += ((u32 *)mc)[i];
-
- if (orig_sum) {
- if (print_err)
- pr_err("Bad microcode data checksum, aborting.\n");
- return -EINVAL;
- }
-
- if (!ext_table_size)
- return 0;
-
- /*
- * Check extended signature checksum: 0 => valid.
- */
- for (i = 0; i < ext_sigcount; i++) {
- ext_sig = (void *)ext_header + EXT_HEADER_SIZE +
- EXT_SIGNATURE_SIZE * i;
-
- sum = (mc_header->sig + mc_header->pf + mc_header->cksum) -
- (ext_sig->sig + ext_sig->pf + ext_sig->cksum);
- if (sum) {
- if (print_err)
- pr_err("Bad extended signature checksum, aborting.\n");
- return -EINVAL;
- }
- }
- return 0;
-}
-
/*
* Get microcode matching with BSP's model. Only CPUs with the same model as
* BSP can stay in the platform.
@@ -253,7 +155,7 @@ scan_microcode(void *data, size_t size, struct ucode_cpu_info *uci, bool save)
mc_size = get_totalsize(mc_header);
if (!mc_size ||
mc_size > size ||
- microcode_sanity_check(data, false) < 0)
+ intel_microcode_sanity_check(data, false) < 0)
break;
size -= mc_size;
@@ -792,7 +694,7 @@ static enum ucode_state generic_load_microcode(int cpu, struct iov_iter *iter)
memcpy(mc, &mc_header, sizeof(mc_header));
data = mc + sizeof(mc_header);
if (!copy_from_iter_full(data, data_size, iter) ||
- microcode_sanity_check(mc, true) < 0) {
+ intel_microcode_sanity_check(mc, true) < 0) {
break;
}IFS test images and microcode blobs use the same header format.
Microcode blobs use header type of 1, whereas IFS test images
will use header type of 2.
In preparation for IFS reusing intel_microcode_sanity_check(),
add header type as a parameter for sanity check.
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
---
arch/x86/include/asm/cpu.h | 2 +-
arch/x86/include/asm/microcode_intel.h | 1 +
arch/x86/kernel/cpu/intel.c | 19 ++++++++++++++++---
arch/x86/kernel/cpu/microcode/intel.c | 4 ++--
4 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index 9e3ac95acf2d..78796b98a544 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -96,6 +96,6 @@ static inline bool intel_cpu_signatures_match(unsigned int s1, unsigned int p1,
extern u64 x86_read_arch_cap_msr(void);
int intel_find_matching_signature(void *mc, unsigned int csig, int cpf);
-int intel_microcode_sanity_check(void *mc, bool print_err);
+int intel_microcode_sanity_check(void *mc, bool print_err, int hdr_type);
#endif /* _ASM_X86_CPU_H */
diff --git a/arch/x86/include/asm/microcode_intel.h b/arch/x86/include/asm/microcode_intel.h
index 4c92cea7e4b5..2a999bf91ef0 100644
--- a/arch/x86/include/asm/microcode_intel.h
+++ b/arch/x86/include/asm/microcode_intel.h
@@ -41,6 +41,7 @@ struct extended_sigtable {
#define DEFAULT_UCODE_TOTALSIZE (DEFAULT_UCODE_DATASIZE + MC_HEADER_SIZE)
#define EXT_HEADER_SIZE (sizeof(struct extended_sigtable))
#define EXT_SIGNATURE_SIZE (sizeof(struct extended_signature))
+#define MC_HEADER_TYPE_MICROCODE 1
#define get_totalsize(mc) \
(((struct microcode_intel *)mc)->hdr.datasize ? \
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 01e73ec1d585..ff305e3784d1 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -245,7 +245,19 @@ int intel_find_matching_signature(void *mc, unsigned int csig, int cpf)
}
EXPORT_SYMBOL_GPL(intel_find_matching_signature);
-int intel_microcode_sanity_check(void *mc, bool print_err)
+/**
+ * intel_microcode_sanity_check() - Sanity check microcode file.
+ * @mc: Pointer to the microcode file contents.
+ * @print_err: Display failure reason if true, silent if false.
+ * @hdr_type: Type of file, i.e. normal microcode file or In Field Scan file.
+ * Validate if the microcode header type matches with the type specified here.
+ *
+ * Validate certain header fields and verify if computed checksum matches
+ * with the one specified in the header.
+ *
+ * Return: 0 if the file passes all the checks, -ERR for invalid files
+ */
+int intel_microcode_sanity_check(void *mc, bool print_err, int hdr_type)
{
unsigned long total_size, data_size, ext_table_size;
struct microcode_header_intel *mc_header = mc;
@@ -262,9 +274,10 @@ int intel_microcode_sanity_check(void *mc, bool print_err)
return -EINVAL;
}
- if (mc_header->ldrver != 1 || mc_header->hdrver != 1) {
+ if (mc_header->ldrver != 1 || mc_header->hdrver != hdr_type) {
if (print_err)
- pr_err("Error: invalid/unknown microcode update format.\n");
+ pr_err("Error: invalid/unknown microcode update format. Header type %d\n",
+ mc_header->hdrver);
return -EINVAL;
}
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index af7134073e65..bf8026304f6f 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -155,7 +155,7 @@ scan_microcode(void *data, size_t size, struct ucode_cpu_info *uci, bool save)
mc_size = get_totalsize(mc_header);
if (!mc_size ||
mc_size > size ||
- intel_microcode_sanity_check(data, false) < 0)
+ intel_microcode_sanity_check(data, false, MC_HEADER_TYPE_MICROCODE) < 0)
break;
size -= mc_size;
@@ -694,7 +694,7 @@ static enum ucode_state generic_load_microcode(int cpu, struct iov_iter *iter)
memcpy(mc, &mc_header, sizeof(mc_header));
data = mc + sizeof(mc_header);
if (!copy_from_iter_full(data, data_size, iter) ||
- intel_microcode_sanity_check(mc, true) < 0) {
+ intel_microcode_sanity_check(mc, true, MC_HEADER_TYPE_MICROCODE) < 0) {
break;
}
--
2.25.1The following commit has been merged into the x86/microcode branch of tip:
Commit-ID: e0788c3281a72386e75b53a010de4bfbac7e80db
Gitweb: https://git.kernel.org/tip/e0788c3281a72386e75b53a010de4bfbac7e80db
Author: Jithu Joseph <jithu.joseph@intel.com>
AuthorDate: Wed, 16 Nov 2022 19:59:27 -08:00
Committer: Borislav Petkov <bp@suse.de>
CommitterDate: Fri, 18 Nov 2022 22:08:19 +01:00
x86/microcode/intel: Add hdr_type to intel_microcode_sanity_check()
IFS test images and microcode blobs use the same header format.
Microcode blobs use header type of 1, whereas IFS test images
will use header type of 2.
In preparation for IFS reusing intel_microcode_sanity_check(),
add header type as a parameter for sanity check.
[ bp: Touchups. ]
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Ashok Raj <ashok.raj@intel.com>
Link: https://lore.kernel.org/r/20221117035935.4136738-9-jithu.joseph@intel.com
---
arch/x86/include/asm/cpu.h | 2 +-
arch/x86/include/asm/microcode_intel.h | 1 +
arch/x86/kernel/cpu/intel.c | 21 ++++++++++++++++++---
arch/x86/kernel/cpu/microcode/intel.c | 4 ++--
4 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index 9e3ac95..78796b9 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -96,6 +96,6 @@ static inline bool intel_cpu_signatures_match(unsigned int s1, unsigned int p1,
extern u64 x86_read_arch_cap_msr(void);
int intel_find_matching_signature(void *mc, unsigned int csig, int cpf);
-int intel_microcode_sanity_check(void *mc, bool print_err);
+int intel_microcode_sanity_check(void *mc, bool print_err, int hdr_type);
#endif /* _ASM_X86_CPU_H */
diff --git a/arch/x86/include/asm/microcode_intel.h b/arch/x86/include/asm/microcode_intel.h
index 4c92cea..2a999bf 100644
--- a/arch/x86/include/asm/microcode_intel.h
+++ b/arch/x86/include/asm/microcode_intel.h
@@ -41,6 +41,7 @@ struct extended_sigtable {
#define DEFAULT_UCODE_TOTALSIZE (DEFAULT_UCODE_DATASIZE + MC_HEADER_SIZE)
#define EXT_HEADER_SIZE (sizeof(struct extended_sigtable))
#define EXT_SIGNATURE_SIZE (sizeof(struct extended_signature))
+#define MC_HEADER_TYPE_MICROCODE 1
#define get_totalsize(mc) \
(((struct microcode_intel *)mc)->hdr.datasize ? \
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index bef06a1..b6997eb 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -244,7 +244,21 @@ int intel_find_matching_signature(void *mc, unsigned int csig, int cpf)
}
EXPORT_SYMBOL_GPL(intel_find_matching_signature);
-int intel_microcode_sanity_check(void *mc, bool print_err)
+/**
+ * intel_microcode_sanity_check() - Sanity check microcode file.
+ * @mc: Pointer to the microcode file contents.
+ * @print_err: Display failure reason if true, silent if false.
+ * @hdr_type: Type of file, i.e. normal microcode file or In Field Scan file.
+ * Validate if the microcode header type matches with the type
+ * specified here.
+ *
+ * Validate certain header fields and verify if computed checksum matches
+ * with the one specified in the header.
+ *
+ * Return: 0 if the file passes all the checks, -EINVAL if any of the checks
+ * fail.
+ */
+int intel_microcode_sanity_check(void *mc, bool print_err, int hdr_type)
{
unsigned long total_size, data_size, ext_table_size;
struct microcode_header_intel *mc_header = mc;
@@ -261,9 +275,10 @@ int intel_microcode_sanity_check(void *mc, bool print_err)
return -EINVAL;
}
- if (mc_header->ldrver != 1 || mc_header->hdrver != 1) {
+ if (mc_header->ldrver != 1 || mc_header->hdrver != hdr_type) {
if (print_err)
- pr_err("Error: invalid/unknown microcode update format.\n");
+ pr_err("Error: invalid/unknown microcode update format. Header type %d\n",
+ mc_header->hdrver);
return -EINVAL;
}
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index fb6ff71..c4a00fb 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -155,7 +155,7 @@ scan_microcode(void *data, size_t size, struct ucode_cpu_info *uci, bool save)
mc_size = get_totalsize(mc_header);
if (!mc_size ||
mc_size > size ||
- intel_microcode_sanity_check(data, false) < 0)
+ intel_microcode_sanity_check(data, false, MC_HEADER_TYPE_MICROCODE) < 0)
break;
size -= mc_size;
@@ -694,7 +694,7 @@ static enum ucode_state generic_load_microcode(int cpu, struct iov_iter *iter)
memcpy(mc, &mc_header, sizeof(mc_header));
data = mc + sizeof(mc_header);
if (!copy_from_iter_full(data, data_size, iter) ||
- intel_microcode_sanity_check(mc, true) < 0) {
+ intel_microcode_sanity_check(mc, true, MC_HEADER_TYPE_MICROCODE) < 0) {
break;
}Intel is using microcode file format for IFS test images too.
IFS test images use one of the existing reserved fields in microcode
header to indicate the size of the region in the file allocated for
metadata structures.
In preparation for this, rename first of the existing reserved fields
in microcode header to metasize. In subsequent patches IFS specific
code will make use of this field while parsing IFS images.
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Ashok Raj <ashok.raj@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
---
arch/x86/include/asm/microcode_intel.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/microcode_intel.h b/arch/x86/include/asm/microcode_intel.h
index 2a999bf91ef0..6af1e703cb2e 100644
--- a/arch/x86/include/asm/microcode_intel.h
+++ b/arch/x86/include/asm/microcode_intel.h
@@ -14,7 +14,8 @@ struct microcode_header_intel {
unsigned int pf;
unsigned int datasize;
unsigned int totalsize;
- unsigned int reserved[3];
+ unsigned int metasize;
+ unsigned int reserved[2];
};
struct microcode_intel {
--
2.25.1The following commit has been merged into the x86/microcode branch of tip:
Commit-ID: 28377e56ed22ecce60260eb8afdf0c9837b3505f
Gitweb: https://git.kernel.org/tip/28377e56ed22ecce60260eb8afdf0c9837b3505f
Author: Jithu Joseph <jithu.joseph@intel.com>
AuthorDate: Wed, 16 Nov 2022 19:59:28 -08:00
Committer: Borislav Petkov <bp@suse.de>
CommitterDate: Fri, 18 Nov 2022 22:10:12 +01:00
x86/microcode/intel: Use a reserved field for metasize
Intel is using microcode file format for IFS test images too.
IFS test images use one of the existing reserved fields in microcode
header to indicate the size of the region in the file allocated for
metadata structures.
In preparation for this, rename first of the existing reserved fields
in microcode header to metasize. In subsequent patches IFS specific
code will make use of this field while parsing IFS images.
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Ashok Raj <ashok.raj@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Link: https://lore.kernel.org/r/20221117035935.4136738-10-jithu.joseph@intel.com
---
arch/x86/include/asm/microcode_intel.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/microcode_intel.h b/arch/x86/include/asm/microcode_intel.h
index 2a999bf..6af1e70 100644
--- a/arch/x86/include/asm/microcode_intel.h
+++ b/arch/x86/include/asm/microcode_intel.h
@@ -14,7 +14,8 @@ struct microcode_header_intel {
unsigned int pf;
unsigned int datasize;
unsigned int totalsize;
- unsigned int reserved[3];
+ unsigned int metasize;
+ unsigned int reserved[2];
};
struct microcode_intel {From: Ashok Raj <ashok.raj@intel.com>
One of the existing reserved fields in microcode header has been
allocated to indicate the size for metadata structures.
The location of metadata section within microcode header is as shown
below:
Microcode Format
+----------------------+ Base
|Header Version |
+----------------------+
|Update revision |
+----------------------+
|Date DDMMYYYY |
+----------------------+
|Sig |
+----------------------+
|Checksum |
+----------------------+
|Loader Version |
+----------------------+
|Processor Flags |
+----------------------+
|Data Size |
+----------------------+
|Total Size |
+----------------------+
|Meta Size |
+----------------------+
|Reserved |
+----------------------+
|Reserved |
+----------------------+ Base+48
| |
| |
| |
| |
| Microcode |
| |
| Data |
| |
| |
+----------------------+ Base+48+data_size-
| | meta_size
| Meta Data |
| structure(s) |
| |
+----------------------+ Base+48+data_size
| Extended Signature |
| Table |
| |
| |
| |
| |
| |
+----------------------+ Base+total_size
Add an accessor function which will return a pointer to the
start of a specific meta_type being queried.
Upcoming changes will introduce the layout of IFS metadata
structure and will make use of metadata section.
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
---
drivers/platform/x86/intel/ifs/load.c | 32 +++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
index c914e4d359db..713f18ce00a1 100644
--- a/drivers/platform/x86/intel/ifs/load.c
+++ b/drivers/platform/x86/intel/ifs/load.c
@@ -43,6 +43,38 @@ static const char * const scan_authentication_status[] = {
[2] = "Chunk authentication error. The hash of chunk did not match expected value"
};
+#define MC_HEADER_META_TYPE_END (0)
+
+struct metadata_header {
+ unsigned int type;
+ unsigned int blk_size;
+};
+
+static struct metadata_header *find_meta_data(void *ucode, unsigned int meta_type)
+{
+ struct metadata_header *meta_header;
+ unsigned long data_size, total_meta;
+ unsigned long meta_size = 0;
+
+ data_size = get_datasize(ucode);
+ total_meta = ((struct microcode_intel *)ucode)->hdr.metasize;
+ if (!total_meta)
+ return NULL;
+
+ meta_header = (ucode + MC_HEADER_SIZE + data_size) - total_meta;
+
+ while (meta_header->type != MC_HEADER_META_TYPE_END &&
+ meta_header->blk_size &&
+ meta_size < total_meta) {
+ meta_size += meta_header->blk_size;
+ if (meta_header->type == meta_type)
+ return meta_header;
+
+ meta_header = (void *)meta_header + meta_header->blk_size;
+ }
+ return NULL;
+}
+
/*
* To copy scan hashes and authenticate test chunks, the initiating cpu must point
* to the EDX:EAX to the test image in linear address.
--
2.25.1The following commit has been merged into the x86/microcode branch of tip:
Commit-ID: 8382fee3bb86526bde1bfb1a06834f056140e0dd
Gitweb: https://git.kernel.org/tip/8382fee3bb86526bde1bfb1a06834f056140e0dd
Author: Ashok Raj <ashok.raj@intel.com>
AuthorDate: Wed, 16 Nov 2022 19:59:29 -08:00
Committer: Borislav Petkov <bp@suse.de>
CommitterDate: Sat, 19 Nov 2022 10:39:08 +01:00
platform/x86/intel/ifs: Add metadata support
One of the existing reserved fields in the microcode header has been
allocated to indicate the size of metadata structures.
The location of metadata section within microcode header is as shown
below:
Microcode Blob Format
+----------------------+ Base
|Header Version |
+----------------------+
|Update revision |
+----------------------+
|Date DDMMYYYY |
+----------------------+
|Sig |
+----------------------+
|Checksum |
+----------------------+
|Loader Version |
+----------------------+
|Processor Flags |
+----------------------+
|Data Size |
+----------------------+
|Total Size |
+----------------------+
|Meta Size |
+----------------------+
|Reserved |
+----------------------+
|Reserved |
+----------------------+ Base+48
| |
| Microcode |
| Data |
| |
+----------------------+ Base+48+data_size-
| | meta_size
| Meta Data |
| structure(s) |
| |
+----------------------+ Base+48+data_size
| |
| Extended Signature |
| Table |
| |
+----------------------+ Base+total_size
Add an accessor function which will return a pointer to the start of a
specific meta_type being queried.
[ bp: Massage commit message. ]
Signed-off-by: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20221117035935.4136738-11-jithu.joseph@intel.com
---
drivers/platform/x86/intel/ifs/load.c | 32 ++++++++++++++++++++++++++-
1 file changed, 32 insertions(+)
diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
index 8423c48..9228da5 100644
--- a/drivers/platform/x86/intel/ifs/load.c
+++ b/drivers/platform/x86/intel/ifs/load.c
@@ -43,6 +43,38 @@ static const char * const scan_authentication_status[] = {
[2] = "Chunk authentication error. The hash of chunk did not match expected value"
};
+#define MC_HEADER_META_TYPE_END (0)
+
+struct metadata_header {
+ unsigned int type;
+ unsigned int blk_size;
+};
+
+static struct metadata_header *find_meta_data(void *ucode, unsigned int meta_type)
+{
+ struct metadata_header *meta_header;
+ unsigned long data_size, total_meta;
+ unsigned long meta_size = 0;
+
+ data_size = get_datasize(ucode);
+ total_meta = ((struct microcode_intel *)ucode)->hdr.metasize;
+ if (!total_meta)
+ return NULL;
+
+ meta_header = (ucode + MC_HEADER_SIZE + data_size) - total_meta;
+
+ while (meta_header->type != MC_HEADER_META_TYPE_END &&
+ meta_header->blk_size &&
+ meta_size < total_meta) {
+ meta_size += meta_header->blk_size;
+ if (meta_header->type == meta_type)
+ return meta_header;
+
+ meta_header = (void *)meta_header + meta_header->blk_size;
+ }
+ return NULL;
+}
+
/*
* To copy scan hashes and authenticate test chunks, the initiating cpu must point
* to the EDX:EAX to the test image in linear address.Existing implementation (broken) of IFS used a header format (for IFS
test images) which was very similar to microcode format, but didn’t
accommodate extended signatures. This meant same IFS test image had to
be duplicated for different steppings and the validation code in the
driver was only looking at the primary header parameters. Going forward
IFS test image headers has been tweaked to become fully compatible with
microcode format.
Newer IFS test image headers will use microcode_header_intel->hdrver = 2,
so as to distinguish it from microcode images and older IFS test images.
In light of the above, reuse struct microcode_header_intel directly in
IFS driver and reuse microcode functions for validation and sanity
checking.
More IFS specific checks will be added subsequently.
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
---
arch/x86/include/asm/microcode_intel.h | 1 +
drivers/platform/x86/intel/ifs/load.c | 103 +++++--------------------
2 files changed, 20 insertions(+), 84 deletions(-)
diff --git a/arch/x86/include/asm/microcode_intel.h b/arch/x86/include/asm/microcode_intel.h
index 6af1e703cb2e..f1fa979e05bf 100644
--- a/arch/x86/include/asm/microcode_intel.h
+++ b/arch/x86/include/asm/microcode_intel.h
@@ -43,6 +43,7 @@ struct extended_sigtable {
#define EXT_HEADER_SIZE (sizeof(struct extended_sigtable))
#define EXT_SIGNATURE_SIZE (sizeof(struct extended_signature))
#define MC_HEADER_TYPE_MICROCODE 1
+#define MC_HEADER_TYPE_IFS 2
#define get_totalsize(mc) \
(((struct microcode_intel *)mc)->hdr.datasize ? \
diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
index 713f18ce00a1..6caa98cc6cac 100644
--- a/drivers/platform/x86/intel/ifs/load.c
+++ b/drivers/platform/x86/intel/ifs/load.c
@@ -7,22 +7,8 @@
#include "ifs.h"
-struct ifs_header {
- u32 header_ver;
- u32 blob_revision;
- u32 date;
- u32 processor_sig;
- u32 check_sum;
- u32 loader_rev;
- u32 processor_flags;
- u32 metadata_size;
- u32 total_size;
- u32 fusa_info;
- u64 reserved;
-};
-
-#define IFS_HEADER_SIZE (sizeof(struct ifs_header))
-static struct ifs_header *ifs_header_ptr; /* pointer to the ifs image header */
+#define IFS_HEADER_SIZE (sizeof(struct microcode_header_intel))
+static struct microcode_header_intel *ifs_header_ptr; /* pointer to the ifs image header */
static u64 ifs_hash_ptr; /* Address of ifs metadata (hash) */
static u64 ifs_test_image_ptr; /* 256B aligned address of test pattern */
static DECLARE_COMPLETION(ifs_done);
@@ -149,29 +135,13 @@ static void copy_hashes_authenticate_chunks(struct work_struct *work)
*/
static int scan_chunks_sanity_check(struct device *dev)
{
- int metadata_size, curr_pkg, cpu, ret;
struct ifs_data *ifsd = ifs_get_data(dev);
struct ifs_work local_work;
- char *test_ptr;
+ int curr_pkg, cpu, ret;
memset(ifs_pkg_auth, 0, (topology_max_packages() * sizeof(bool)));
- metadata_size = ifs_header_ptr->metadata_size;
-
- /* Spec says that if the Meta Data Size = 0 then it should be treated as 2000 */
- if (metadata_size == 0)
- metadata_size = 2000;
-
- /* Scan chunk start must be 256 byte aligned */
- if ((metadata_size + IFS_HEADER_SIZE) % 256) {
- dev_err(dev, "Scan pattern offset within the binary is not 256 byte aligned\n");
- return -EINVAL;
- }
-
- test_ptr = (char *)ifs_header_ptr + IFS_HEADER_SIZE + metadata_size;
ifsd->loading_error = false;
-
- ifs_test_image_ptr = (u64)test_ptr;
- ifsd->loaded_version = ifs_header_ptr->blob_revision;
+ ifsd->loaded_version = ifs_header_ptr->rev;
/* copy the scan hash and authenticate per package */
cpus_read_lock();
@@ -197,67 +167,33 @@ static int scan_chunks_sanity_check(struct device *dev)
return ret;
}
-static int ifs_sanity_check(struct device *dev,
- const struct microcode_header_intel *mc_header)
+static int image_sanity_check(struct device *dev, const struct microcode_header_intel *data)
{
- unsigned long total_size, data_size;
- u32 sum, *mc;
-
- total_size = get_totalsize(mc_header);
- data_size = get_datasize(mc_header);
+ struct ucode_cpu_info uci;
- if ((data_size + MC_HEADER_SIZE > total_size) || (total_size % sizeof(u32))) {
- dev_err(dev, "bad ifs data file size.\n");
+ /* Provide a specific error message when loading an older/unsupported image */
+ if (data->hdrver != MC_HEADER_TYPE_IFS) {
+ dev_err(dev, "Header version %d not supported\n", data->hdrver);
return -EINVAL;
}
- if (mc_header->ldrver != 1 || mc_header->hdrver != 1) {
- dev_err(dev, "invalid/unknown ifs update format.\n");
+ if (intel_microcode_sanity_check((void *)data, true, MC_HEADER_TYPE_IFS)) {
+ dev_err(dev, "sanity check failed\n");
return -EINVAL;
}
- mc = (u32 *)mc_header;
- sum = 0;
- for (int i = 0; i < total_size / sizeof(u32); i++)
- sum += mc[i];
+ intel_cpu_collect_info(&uci);
- if (sum) {
- dev_err(dev, "bad ifs data checksum, aborting.\n");
+ if (!intel_find_matching_signature((void *)data,
+ uci.cpu_sig.sig,
+ uci.cpu_sig.pf)) {
+ dev_err(dev, "cpu signature, processor flags not matching\n");
return -EINVAL;
}
return 0;
}
-static bool find_ifs_matching_signature(struct device *dev, struct ucode_cpu_info *uci,
- const struct microcode_header_intel *shdr)
-{
- unsigned int mc_size;
-
- mc_size = get_totalsize(shdr);
-
- if (!mc_size || ifs_sanity_check(dev, shdr) < 0) {
- dev_err(dev, "ifs sanity check failure\n");
- return false;
- }
-
- if (!intel_cpu_signatures_match(uci->cpu_sig.sig, uci->cpu_sig.pf, shdr->sig, shdr->pf)) {
- dev_err(dev, "ifs signature, pf not matching\n");
- return false;
- }
-
- return true;
-}
-
-static bool ifs_image_sanity_check(struct device *dev, const struct microcode_header_intel *data)
-{
- struct ucode_cpu_info uci;
-
- intel_cpu_collect_info(&uci);
-
- return find_ifs_matching_signature(dev, &uci, data);
-}
-
/*
* 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}.
@@ -278,12 +214,11 @@ void ifs_load_firmware(struct device *dev)
goto done;
}
- if (!ifs_image_sanity_check(dev, (struct microcode_header_intel *)fw->data)) {
- dev_err(dev, "ifs header sanity check failed\n");
+ ret = image_sanity_check(dev, (struct microcode_header_intel *)fw->data);
+ if (ret)
goto release;
- }
- ifs_header_ptr = (struct ifs_header *)fw->data;
+ ifs_header_ptr = (struct microcode_header_intel *)fw->data;
ifs_hash_ptr = (u64)(ifs_header_ptr + 1);
ret = scan_chunks_sanity_check(dev);
--
2.25.1
Existing implementation (broken) of IFS used a header format (for IFS
test images) which was very similar to microcode format, but didn’t
accommodate extended signatures. This meant same IFS test image had to
be duplicated for different steppings and the validation code in the
driver was only looking at the primary header parameters. Going forward
IFS test image headers has been tweaked to become fully compatible with
microcode format.
Newer IFS test image headers will use microcode_header_intel->hdrver = 2,
so as to distinguish it from microcode images and older IFS test images.
In light of the above, reuse struct microcode_header_intel directly in
IFS driver and reuse microcode functions for validation and sanity
checking.
More IFS specific checks will be added subsequently.
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
---
- Rebased to apply alongside the updated 4/16 patch
arch/x86/include/asm/microcode_intel.h | 1 +
drivers/platform/x86/intel/ifs/load.c | 104 +++++--------------------
2 files changed, 21 insertions(+), 84 deletions(-)
diff --git a/arch/x86/include/asm/microcode_intel.h b/arch/x86/include/asm/microcode_intel.h
index 6af1e703cb2e..f1fa979e05bf 100644
--- a/arch/x86/include/asm/microcode_intel.h
+++ b/arch/x86/include/asm/microcode_intel.h
@@ -43,6 +43,7 @@ struct extended_sigtable {
#define EXT_HEADER_SIZE (sizeof(struct extended_sigtable))
#define EXT_SIGNATURE_SIZE (sizeof(struct extended_signature))
#define MC_HEADER_TYPE_MICROCODE 1
+#define MC_HEADER_TYPE_IFS 2
#define get_totalsize(mc) \
(((struct microcode_intel *)mc)->hdr.datasize ? \
diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
index 9228da560736..83434160bc4c 100644
--- a/drivers/platform/x86/intel/ifs/load.c
+++ b/drivers/platform/x86/intel/ifs/load.c
@@ -7,22 +7,8 @@
#include "ifs.h"
-struct ifs_header {
- u32 header_ver;
- u32 blob_revision;
- u32 date;
- u32 processor_sig;
- u32 check_sum;
- u32 loader_rev;
- u32 processor_flags;
- u32 metadata_size;
- u32 total_size;
- u32 fusa_info;
- u64 reserved;
-};
-
-#define IFS_HEADER_SIZE (sizeof(struct ifs_header))
-static struct ifs_header *ifs_header_ptr; /* pointer to the ifs image header */
+#define IFS_HEADER_SIZE (sizeof(struct microcode_header_intel))
+static struct microcode_header_intel *ifs_header_ptr; /* pointer to the ifs image header */
static u64 ifs_hash_ptr; /* Address of ifs metadata (hash) */
static u64 ifs_test_image_ptr; /* 256B aligned address of test pattern */
static DECLARE_COMPLETION(ifs_done);
@@ -149,29 +135,14 @@ static void copy_hashes_authenticate_chunks(struct work_struct *work)
*/
static int scan_chunks_sanity_check(struct device *dev)
{
- int metadata_size, curr_pkg, cpu, ret;
struct ifs_data *ifsd = ifs_get_data(dev);
struct ifs_work local_work;
- char *test_ptr;
-
- memset(ifsd->pkg_auth, 0, (topology_max_packages() * sizeof(bool)));
- metadata_size = ifs_header_ptr->metadata_size;
+ int curr_pkg, cpu, ret;
- /* Spec says that if the Meta Data Size = 0 then it should be treated as 2000 */
- if (metadata_size == 0)
- metadata_size = 2000;
- /* Scan chunk start must be 256 byte aligned */
- if ((metadata_size + IFS_HEADER_SIZE) % 256) {
- dev_err(dev, "Scan pattern offset within the binary is not 256 byte aligned\n");
- return -EINVAL;
- }
-
- test_ptr = (char *)ifs_header_ptr + IFS_HEADER_SIZE + metadata_size;
+ memset(ifsd->pkg_auth, 0, (topology_max_packages() * sizeof(bool)));
ifsd->loading_error = false;
-
- ifs_test_image_ptr = (u64)test_ptr;
- ifsd->loaded_version = ifs_header_ptr->blob_revision;
+ ifsd->loaded_version = ifs_header_ptr->rev;
/* copy the scan hash and authenticate per package */
cpus_read_lock();
@@ -197,67 +168,33 @@ static int scan_chunks_sanity_check(struct device *dev)
return ret;
}
-static int ifs_sanity_check(struct device *dev,
- const struct microcode_header_intel *mc_header)
+static int image_sanity_check(struct device *dev, const struct microcode_header_intel *data)
{
- unsigned long total_size, data_size;
- u32 sum, *mc;
-
- total_size = get_totalsize(mc_header);
- data_size = get_datasize(mc_header);
+ struct ucode_cpu_info uci;
- if ((data_size + MC_HEADER_SIZE > total_size) || (total_size % sizeof(u32))) {
- dev_err(dev, "bad ifs data file size.\n");
+ /* Provide a specific error message when loading an older/unsupported image */
+ if (data->hdrver != MC_HEADER_TYPE_IFS) {
+ dev_err(dev, "Header version %d not supported\n", data->hdrver);
return -EINVAL;
}
- if (mc_header->ldrver != 1 || mc_header->hdrver != 1) {
- dev_err(dev, "invalid/unknown ifs update format.\n");
+ if (intel_microcode_sanity_check((void *)data, true, MC_HEADER_TYPE_IFS)) {
+ dev_err(dev, "sanity check failed\n");
return -EINVAL;
}
- mc = (u32 *)mc_header;
- sum = 0;
- for (int i = 0; i < total_size / sizeof(u32); i++)
- sum += mc[i];
+ intel_cpu_collect_info(&uci);
- if (sum) {
- dev_err(dev, "bad ifs data checksum, aborting.\n");
+ if (!intel_find_matching_signature((void *)data,
+ uci.cpu_sig.sig,
+ uci.cpu_sig.pf)) {
+ dev_err(dev, "cpu signature, processor flags not matching\n");
return -EINVAL;
}
return 0;
}
-static bool find_ifs_matching_signature(struct device *dev, struct ucode_cpu_info *uci,
- const struct microcode_header_intel *shdr)
-{
- unsigned int mc_size;
-
- mc_size = get_totalsize(shdr);
-
- if (!mc_size || ifs_sanity_check(dev, shdr) < 0) {
- dev_err(dev, "ifs sanity check failure\n");
- return false;
- }
-
- if (!intel_cpu_signatures_match(uci->cpu_sig.sig, uci->cpu_sig.pf, shdr->sig, shdr->pf)) {
- dev_err(dev, "ifs signature, pf not matching\n");
- return false;
- }
-
- return true;
-}
-
-static bool ifs_image_sanity_check(struct device *dev, const struct microcode_header_intel *data)
-{
- struct ucode_cpu_info uci;
-
- intel_cpu_collect_info(&uci);
-
- return find_ifs_matching_signature(dev, &uci, data);
-}
-
/*
* 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}.
@@ -278,12 +215,11 @@ void ifs_load_firmware(struct device *dev)
goto done;
}
- if (!ifs_image_sanity_check(dev, (struct microcode_header_intel *)fw->data)) {
- dev_err(dev, "ifs header sanity check failed\n");
+ ret = image_sanity_check(dev, (struct microcode_header_intel *)fw->data);
+ if (ret)
goto release;
- }
- ifs_header_ptr = (struct ifs_header *)fw->data;
+ ifs_header_ptr = (struct microcode_header_intel *)fw->data;
ifs_hash_ptr = (u64)(ifs_header_ptr + 1);
ret = scan_chunks_sanity_check(dev);
--
2.25.1
The following commit has been merged into the x86/microcode branch of tip:
Commit-ID: aa63e0fda85edf9a8431fc31a2b2d4f3f40592f9
Gitweb: https://git.kernel.org/tip/aa63e0fda85edf9a8431fc31a2b2d4f3f40592f9
Author: Jithu Joseph <jithu.joseph@intel.com>
AuthorDate: Thu, 17 Nov 2022 14:50:39 -08:00
Committer: Borislav Petkov <bp@suse.de>
CommitterDate: Sat, 19 Nov 2022 11:12:06 +01:00
platform/x86/intel/ifs: Use generic microcode headers and functions
Existing implementation (broken) of IFS used a header format (for IFS
test images) which was very similar to microcode format, but didn’t
accommodate extended signatures. This meant same IFS test image had to
be duplicated for different steppings and the validation code in the
driver was only looking at the primary header parameters. Going forward,
IFS test image headers have been tweaked to become fully compatible with
the microcode format.
Newer IFS test image headers will use header version 2 in order to
distinguish it from microcode images and older IFS test images.
In light of the above, reuse struct microcode_header_intel directly in
the IFS driver and reuse microcode functions for validation and sanity
checking.
[ bp: Massage commit message. ]
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20221117225039.30166-1-jithu.joseph@intel.com
---
arch/x86/include/asm/microcode_intel.h | 1 +-
drivers/platform/x86/intel/ifs/load.c | 104 ++++--------------------
2 files changed, 21 insertions(+), 84 deletions(-)
diff --git a/arch/x86/include/asm/microcode_intel.h b/arch/x86/include/asm/microcode_intel.h
index 6af1e70..f1fa979 100644
--- a/arch/x86/include/asm/microcode_intel.h
+++ b/arch/x86/include/asm/microcode_intel.h
@@ -43,6 +43,7 @@ struct extended_sigtable {
#define EXT_HEADER_SIZE (sizeof(struct extended_sigtable))
#define EXT_SIGNATURE_SIZE (sizeof(struct extended_signature))
#define MC_HEADER_TYPE_MICROCODE 1
+#define MC_HEADER_TYPE_IFS 2
#define get_totalsize(mc) \
(((struct microcode_intel *)mc)->hdr.datasize ? \
diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
index 9228da5..8343416 100644
--- a/drivers/platform/x86/intel/ifs/load.c
+++ b/drivers/platform/x86/intel/ifs/load.c
@@ -7,22 +7,8 @@
#include "ifs.h"
-struct ifs_header {
- u32 header_ver;
- u32 blob_revision;
- u32 date;
- u32 processor_sig;
- u32 check_sum;
- u32 loader_rev;
- u32 processor_flags;
- u32 metadata_size;
- u32 total_size;
- u32 fusa_info;
- u64 reserved;
-};
-
-#define IFS_HEADER_SIZE (sizeof(struct ifs_header))
-static struct ifs_header *ifs_header_ptr; /* pointer to the ifs image header */
+#define IFS_HEADER_SIZE (sizeof(struct microcode_header_intel))
+static struct microcode_header_intel *ifs_header_ptr; /* pointer to the ifs image header */
static u64 ifs_hash_ptr; /* Address of ifs metadata (hash) */
static u64 ifs_test_image_ptr; /* 256B aligned address of test pattern */
static DECLARE_COMPLETION(ifs_done);
@@ -149,29 +135,14 @@ done:
*/
static int scan_chunks_sanity_check(struct device *dev)
{
- int metadata_size, curr_pkg, cpu, ret;
struct ifs_data *ifsd = ifs_get_data(dev);
struct ifs_work local_work;
- char *test_ptr;
-
- memset(ifsd->pkg_auth, 0, (topology_max_packages() * sizeof(bool)));
- metadata_size = ifs_header_ptr->metadata_size;
+ int curr_pkg, cpu, ret;
- /* Spec says that if the Meta Data Size = 0 then it should be treated as 2000 */
- if (metadata_size == 0)
- metadata_size = 2000;
- /* Scan chunk start must be 256 byte aligned */
- if ((metadata_size + IFS_HEADER_SIZE) % 256) {
- dev_err(dev, "Scan pattern offset within the binary is not 256 byte aligned\n");
- return -EINVAL;
- }
-
- test_ptr = (char *)ifs_header_ptr + IFS_HEADER_SIZE + metadata_size;
+ memset(ifsd->pkg_auth, 0, (topology_max_packages() * sizeof(bool)));
ifsd->loading_error = false;
-
- ifs_test_image_ptr = (u64)test_ptr;
- ifsd->loaded_version = ifs_header_ptr->blob_revision;
+ ifsd->loaded_version = ifs_header_ptr->rev;
/* copy the scan hash and authenticate per package */
cpus_read_lock();
@@ -197,67 +168,33 @@ out:
return ret;
}
-static int ifs_sanity_check(struct device *dev,
- const struct microcode_header_intel *mc_header)
+static int image_sanity_check(struct device *dev, const struct microcode_header_intel *data)
{
- unsigned long total_size, data_size;
- u32 sum, *mc;
-
- total_size = get_totalsize(mc_header);
- data_size = get_datasize(mc_header);
+ struct ucode_cpu_info uci;
- if ((data_size + MC_HEADER_SIZE > total_size) || (total_size % sizeof(u32))) {
- dev_err(dev, "bad ifs data file size.\n");
+ /* Provide a specific error message when loading an older/unsupported image */
+ if (data->hdrver != MC_HEADER_TYPE_IFS) {
+ dev_err(dev, "Header version %d not supported\n", data->hdrver);
return -EINVAL;
}
- if (mc_header->ldrver != 1 || mc_header->hdrver != 1) {
- dev_err(dev, "invalid/unknown ifs update format.\n");
+ if (intel_microcode_sanity_check((void *)data, true, MC_HEADER_TYPE_IFS)) {
+ dev_err(dev, "sanity check failed\n");
return -EINVAL;
}
- mc = (u32 *)mc_header;
- sum = 0;
- for (int i = 0; i < total_size / sizeof(u32); i++)
- sum += mc[i];
+ intel_cpu_collect_info(&uci);
- if (sum) {
- dev_err(dev, "bad ifs data checksum, aborting.\n");
+ if (!intel_find_matching_signature((void *)data,
+ uci.cpu_sig.sig,
+ uci.cpu_sig.pf)) {
+ dev_err(dev, "cpu signature, processor flags not matching\n");
return -EINVAL;
}
return 0;
}
-static bool find_ifs_matching_signature(struct device *dev, struct ucode_cpu_info *uci,
- const struct microcode_header_intel *shdr)
-{
- unsigned int mc_size;
-
- mc_size = get_totalsize(shdr);
-
- if (!mc_size || ifs_sanity_check(dev, shdr) < 0) {
- dev_err(dev, "ifs sanity check failure\n");
- return false;
- }
-
- if (!intel_cpu_signatures_match(uci->cpu_sig.sig, uci->cpu_sig.pf, shdr->sig, shdr->pf)) {
- dev_err(dev, "ifs signature, pf not matching\n");
- return false;
- }
-
- return true;
-}
-
-static bool ifs_image_sanity_check(struct device *dev, const struct microcode_header_intel *data)
-{
- struct ucode_cpu_info uci;
-
- intel_cpu_collect_info(&uci);
-
- return find_ifs_matching_signature(dev, &uci, data);
-}
-
/*
* 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}.
@@ -278,12 +215,11 @@ void ifs_load_firmware(struct device *dev)
goto done;
}
- if (!ifs_image_sanity_check(dev, (struct microcode_header_intel *)fw->data)) {
- dev_err(dev, "ifs header sanity check failed\n");
+ ret = image_sanity_check(dev, (struct microcode_header_intel *)fw->data);
+ if (ret)
goto release;
- }
- ifs_header_ptr = (struct ifs_header *)fw->data;
+ ifs_header_ptr = (struct microcode_header_intel *)fw->data;
ifs_hash_ptr = (u64)(ifs_header_ptr + 1);
ret = scan_chunks_sanity_check(dev);The data portion of IFS test image file contains a metadata
region containing possibly multiple metadata structures in
addition to test data and hashes.
IFS Metadata layout
+----------------------+ 0
|META_TYPE_IFS (=1) |
+----------------------+
|meta_size |
+----------------------+
|test type |
+----------------------+
|fusa info |
+----------------------+
|total images |
+----------------------+
|current image# |
+----------------------+
|total chunks |
+----------------------+
|starting chunk |
+----------------------+
|size per chunk |
+----------------------+
|chunks per stride |
+----------------------+
|Reserved[54] |
+----------------------+ 256
| |
| |
| |
| |
|Test Data/Chunks |
| |
| |
| |
| |
+----------------------+ meta_size
| META_TYPE_END (=0) |
+----------------------+ meta_size + 4
| size of end (=8) |
| |
+----------------------+ meta_size + 8
Introduce the layout of this meta_data structure and validate
the sanity of certain fields of the new image before loading.
Tweak references to IFS test image chunks to reflect the updated
layout of the test image.
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
---
drivers/platform/x86/intel/ifs/ifs.h | 2 +
drivers/platform/x86/intel/ifs/load.c | 57 +++++++++++++++++++++++++++
2 files changed, 59 insertions(+)
diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
index 3a051890d9e7..e3e8210ebd57 100644
--- a/drivers/platform/x86/intel/ifs/ifs.h
+++ b/drivers/platform/x86/intel/ifs/ifs.h
@@ -196,6 +196,7 @@ union ifs_status {
* @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
*/
struct ifs_data {
int integrity_cap_bit;
@@ -205,6 +206,7 @@ struct ifs_data {
int valid_chunks;
int status;
u64 scan_details;
+ u32 cur_batch;
};
struct ifs_work {
diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
index 6caa98cc6cac..3f1de9d4cf4b 100644
--- a/drivers/platform/x86/intel/ifs/load.c
+++ b/drivers/platform/x86/intel/ifs/load.c
@@ -7,7 +7,25 @@
#include "ifs.h"
+#define IFS_CHUNK_ALIGNMENT 256
+union meta_data {
+ struct {
+ u32 meta_type; // metadata type
+ u32 meta_size; // size of this entire struct including hdrs.
+ u32 test_type; // IFS test type
+ u32 fusa_info; // Fusa info
+ u32 total_images; // Total number of images
+ u32 current_image; // Current Image #
+ u32 total_chunks; // Total number of chunks in this image
+ u32 starting_chunk; // Starting chunk number in this image
+ u32 size_per_chunk; // size of each chunk
+ u32 chunks_per_stride; // number of chunks in a stride
+ };
+ u8 padding[IFS_CHUNK_ALIGNMENT];
+};
+
#define IFS_HEADER_SIZE (sizeof(struct microcode_header_intel))
+#define META_TYPE_IFS 1
static struct microcode_header_intel *ifs_header_ptr; /* pointer to the ifs image header */
static u64 ifs_hash_ptr; /* Address of ifs metadata (hash) */
static u64 ifs_test_image_ptr; /* 256B aligned address of test pattern */
@@ -128,6 +146,41 @@ static void copy_hashes_authenticate_chunks(struct work_struct *work)
complete(&ifs_done);
}
+static int validate_ifs_metadata(struct device *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",
+ boot_cpu_data.x86, boot_cpu_data.x86_model,
+ boot_cpu_data.x86_stepping, ifsd->cur_batch);
+
+ ifs_meta = (union meta_data *)find_meta_data(ifs_header_ptr, META_TYPE_IFS);
+ if (!ifs_meta) {
+ dev_err(dev, "IFS Metadata missing in file %s\n", test_file);
+ return ret;
+ }
+
+ ifs_test_image_ptr = (u64)ifs_meta + sizeof(union meta_data);
+
+ /* Scan chunk start must be 256 byte aligned */
+ if (!IS_ALIGNED(ifs_test_image_ptr, IFS_CHUNK_ALIGNMENT)) {
+ dev_err(dev, "Scan pattern is not aligned on %d bytes aligned in %s\n",
+ IFS_CHUNK_ALIGNMENT, test_file);
+ return ret;
+ }
+
+ if (ifs_meta->current_image != ifsd->cur_batch) {
+ dev_warn(dev, "Mismatch between filename %s and batch metadata 0x%02x\n",
+ test_file, ifs_meta->current_image);
+ return ret;
+ }
+
+ return 0;
+}
+
/*
* IFS requires scan chunks authenticated per each socket in the platform.
* Once the test chunk is authenticated, it is automatically copied to secured memory
@@ -140,6 +193,10 @@ static int scan_chunks_sanity_check(struct device *dev)
int curr_pkg, cpu, ret;
memset(ifs_pkg_auth, 0, (topology_max_packages() * sizeof(bool)));
+ ret = validate_ifs_metadata(dev);
+ if (ret)
+ return ret;
+
ifsd->loading_error = false;
ifsd->loaded_version = ifs_header_ptr->rev;
--
2.25.1The data portion of IFS test image file contains a metadata
region containing possibly multiple metadata structures in
addition to test data and hashes.
IFS Metadata layout
+----------------------+ 0
|META_TYPE_IFS (=1) |
+----------------------+
|meta_size |
+----------------------+
|test type |
+----------------------+
|fusa info |
+----------------------+
|total images |
+----------------------+
|current image# |
+----------------------+
|total chunks |
+----------------------+
|starting chunk |
+----------------------+
|size per chunk |
+----------------------+
|chunks per stride |
+----------------------+
|Reserved[54] |
+----------------------+ 256
| |
| |
| |
| |
|Test Data/Chunks |
| |
| |
| |
| |
+----------------------+ meta_size
| META_TYPE_END (=0) |
+----------------------+ meta_size + 4
| size of end (=8) |
| |
+----------------------+ meta_size + 8
Introduce the layout of this meta_data structure and validate
the sanity of certain fields of the new image before loading.
Tweak references to IFS test image chunks to reflect the updated
layout of the test image.
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
---
- Rebased to apply alongside the updated 4/16 patch
drivers/platform/x86/intel/ifs/ifs.h | 2 +
drivers/platform/x86/intel/ifs/load.c | 58 ++++++++++++++++++++++++++-
2 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
index 8de1952a1b7b..74c051c544f4 100644
--- a/drivers/platform/x86/intel/ifs/ifs.h
+++ b/drivers/platform/x86/intel/ifs/ifs.h
@@ -197,6 +197,7 @@ union ifs_status {
* @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
*/
struct ifs_data {
int integrity_cap_bit;
@@ -207,6 +208,7 @@ struct ifs_data {
int valid_chunks;
int status;
u64 scan_details;
+ u32 cur_batch;
};
struct ifs_work {
diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
index 83434160bc4c..edc7baa976bf 100644
--- a/drivers/platform/x86/intel/ifs/load.c
+++ b/drivers/platform/x86/intel/ifs/load.c
@@ -7,7 +7,25 @@
#include "ifs.h"
+#define IFS_CHUNK_ALIGNMENT 256
+union meta_data {
+ struct {
+ u32 meta_type; // metadata type
+ u32 meta_size; // size of this entire struct including hdrs.
+ u32 test_type; // IFS test type
+ u32 fusa_info; // Fusa info
+ u32 total_images; // Total number of images
+ u32 current_image; // Current Image #
+ u32 total_chunks; // Total number of chunks in this image
+ u32 starting_chunk; // Starting chunk number in this image
+ u32 size_per_chunk; // size of each chunk
+ u32 chunks_per_stride; // number of chunks in a stride
+ };
+ u8 padding[IFS_CHUNK_ALIGNMENT];
+};
+
#define IFS_HEADER_SIZE (sizeof(struct microcode_header_intel))
+#define META_TYPE_IFS 1
static struct microcode_header_intel *ifs_header_ptr; /* pointer to the ifs image header */
static u64 ifs_hash_ptr; /* Address of ifs metadata (hash) */
static u64 ifs_test_image_ptr; /* 256B aligned address of test pattern */
@@ -128,6 +146,41 @@ static void copy_hashes_authenticate_chunks(struct work_struct *work)
complete(&ifs_done);
}
+static int validate_ifs_metadata(struct device *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",
+ boot_cpu_data.x86, boot_cpu_data.x86_model,
+ boot_cpu_data.x86_stepping, ifsd->cur_batch);
+
+ ifs_meta = (union meta_data *)find_meta_data(ifs_header_ptr, META_TYPE_IFS);
+ if (!ifs_meta) {
+ dev_err(dev, "IFS Metadata missing in file %s\n", test_file);
+ return ret;
+ }
+
+ ifs_test_image_ptr = (u64)ifs_meta + sizeof(union meta_data);
+
+ /* Scan chunk start must be 256 byte aligned */
+ if (!IS_ALIGNED(ifs_test_image_ptr, IFS_CHUNK_ALIGNMENT)) {
+ dev_err(dev, "Scan pattern is not aligned on %d bytes aligned in %s\n",
+ IFS_CHUNK_ALIGNMENT, test_file);
+ return ret;
+ }
+
+ if (ifs_meta->current_image != ifsd->cur_batch) {
+ dev_warn(dev, "Mismatch between filename %s and batch metadata 0x%02x\n",
+ test_file, ifs_meta->current_image);
+ return ret;
+ }
+
+ return 0;
+}
+
/*
* IFS requires scan chunks authenticated per each socket in the platform.
* Once the test chunk is authenticated, it is automatically copied to secured memory
@@ -139,8 +192,11 @@ 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)));
+ ret = validate_ifs_metadata(dev);
+ if (ret)
+ return ret;
+
ifsd->loading_error = false;
ifsd->loaded_version = ifs_header_ptr->rev;
--
2.25.1The following commit has been merged into the x86/microcode branch of tip:
Commit-ID: 48c6e7dc19051c5ef725490cf8673d768cda7748
Gitweb: https://git.kernel.org/tip/48c6e7dc19051c5ef725490cf8673d768cda7748
Author: Jithu Joseph <jithu.joseph@intel.com>
AuthorDate: Thu, 17 Nov 2022 15:04:08 -08:00
Committer: Borislav Petkov <bp@suse.de>
CommitterDate: Sat, 19 Nov 2022 11:22:29 +01:00
platform/x86/intel/ifs: Add metadata validation
The data portion of a IFS test image file contains a metadata region
containing possibly multiple metadata structures in addition to test
data and hashes.
IFS Metadata layout
+----------------------+ 0
|META_TYPE_IFS (=1) |
+----------------------+
|meta_size |
+----------------------+
|test type |
+----------------------+
|fusa info |
+----------------------+
|total images |
+----------------------+
|current image# |
+----------------------+
|total chunks |
+----------------------+
|starting chunk |
+----------------------+
|size per chunk |
+----------------------+
|chunks per stride |
+----------------------+
|Reserved[54] |
+----------------------+ 256
| |
| Test Data/Chunks |
| |
+----------------------+ meta_size
| META_TYPE_END (=0) |
+----------------------+ meta_size + 4
| size of end (=8) |
+----------------------+ meta_size + 8
Introduce the layout of this meta_data structure and validate
the sanity of certain fields of the new image before loading.
Tweak references to IFS test image chunks to reflect the updated
layout of the test image.
[ bp: Massage commit message. ]
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20221117230408.30331-1-jithu.joseph@intel.com
---
drivers/platform/x86/intel/ifs/ifs.h | 2 +-
drivers/platform/x86/intel/ifs/load.c | 58 +++++++++++++++++++++++++-
2 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
index 8de1952..74c051c 100644
--- a/drivers/platform/x86/intel/ifs/ifs.h
+++ b/drivers/platform/x86/intel/ifs/ifs.h
@@ -197,6 +197,7 @@ union ifs_status {
* @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
*/
struct ifs_data {
int integrity_cap_bit;
@@ -207,6 +208,7 @@ struct ifs_data {
int valid_chunks;
int status;
u64 scan_details;
+ u32 cur_batch;
};
struct ifs_work {
diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
index 8343416..edc7baa 100644
--- a/drivers/platform/x86/intel/ifs/load.c
+++ b/drivers/platform/x86/intel/ifs/load.c
@@ -7,7 +7,25 @@
#include "ifs.h"
+#define IFS_CHUNK_ALIGNMENT 256
+union meta_data {
+ struct {
+ u32 meta_type; // metadata type
+ u32 meta_size; // size of this entire struct including hdrs.
+ u32 test_type; // IFS test type
+ u32 fusa_info; // Fusa info
+ u32 total_images; // Total number of images
+ u32 current_image; // Current Image #
+ u32 total_chunks; // Total number of chunks in this image
+ u32 starting_chunk; // Starting chunk number in this image
+ u32 size_per_chunk; // size of each chunk
+ u32 chunks_per_stride; // number of chunks in a stride
+ };
+ u8 padding[IFS_CHUNK_ALIGNMENT];
+};
+
#define IFS_HEADER_SIZE (sizeof(struct microcode_header_intel))
+#define META_TYPE_IFS 1
static struct microcode_header_intel *ifs_header_ptr; /* pointer to the ifs image header */
static u64 ifs_hash_ptr; /* Address of ifs metadata (hash) */
static u64 ifs_test_image_ptr; /* 256B aligned address of test pattern */
@@ -128,6 +146,41 @@ done:
complete(&ifs_done);
}
+static int validate_ifs_metadata(struct device *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",
+ boot_cpu_data.x86, boot_cpu_data.x86_model,
+ boot_cpu_data.x86_stepping, ifsd->cur_batch);
+
+ ifs_meta = (union meta_data *)find_meta_data(ifs_header_ptr, META_TYPE_IFS);
+ if (!ifs_meta) {
+ dev_err(dev, "IFS Metadata missing in file %s\n", test_file);
+ return ret;
+ }
+
+ ifs_test_image_ptr = (u64)ifs_meta + sizeof(union meta_data);
+
+ /* Scan chunk start must be 256 byte aligned */
+ if (!IS_ALIGNED(ifs_test_image_ptr, IFS_CHUNK_ALIGNMENT)) {
+ dev_err(dev, "Scan pattern is not aligned on %d bytes aligned in %s\n",
+ IFS_CHUNK_ALIGNMENT, test_file);
+ return ret;
+ }
+
+ if (ifs_meta->current_image != ifsd->cur_batch) {
+ dev_warn(dev, "Mismatch between filename %s and batch metadata 0x%02x\n",
+ test_file, ifs_meta->current_image);
+ return ret;
+ }
+
+ return 0;
+}
+
/*
* IFS requires scan chunks authenticated per each socket in the platform.
* Once the test chunk is authenticated, it is automatically copied to secured memory
@@ -139,8 +192,11 @@ 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)));
+ ret = validate_ifs_metadata(dev);
+ if (ret)
+ return ret;
+
ifsd->loading_error = false;
ifsd->loaded_version = ifs_header_ptr->rev;Reload sysfs entry will be replaced by current_batch, drop it.
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
---
drivers/platform/x86/intel/ifs/sysfs.c | 29 --------------------------
1 file changed, 29 deletions(-)
diff --git a/drivers/platform/x86/intel/ifs/sysfs.c b/drivers/platform/x86/intel/ifs/sysfs.c
index 65dd6fea5342..e077910c5d28 100644
--- a/drivers/platform/x86/intel/ifs/sysfs.c
+++ b/drivers/platform/x86/intel/ifs/sysfs.c
@@ -87,34 +87,6 @@ static ssize_t run_test_store(struct device *dev,
static DEVICE_ATTR_WO(run_test);
-/*
- * Reload the IFS image. When user wants to install new IFS image
- */
-static ssize_t reload_store(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t count)
-{
- struct ifs_data *ifsd = ifs_get_data(dev);
- bool res;
-
-
- if (kstrtobool(buf, &res))
- return -EINVAL;
- if (!res)
- return count;
-
- if (down_interruptible(&ifs_sem))
- return -EINTR;
-
- ifs_load_firmware(dev);
-
- up(&ifs_sem);
-
- return ifsd->loaded ? count : -ENODEV;
-}
-
-static DEVICE_ATTR_WO(reload);
-
/*
* Display currently loaded IFS image version.
*/
@@ -136,7 +108,6 @@ static struct attribute *plat_ifs_attrs[] = {
&dev_attr_details.attr,
&dev_attr_status.attr,
&dev_attr_run_test.attr,
- &dev_attr_reload.attr,
&dev_attr_image_version.attr,
NULL
};
--
2.25.1The following commit has been merged into the x86/microcode branch of tip:
Commit-ID: bf835ee852be38e9fab1fdb330eccdd9728aec34
Gitweb: https://git.kernel.org/tip/bf835ee852be38e9fab1fdb330eccdd9728aec34
Author: Jithu Joseph <jithu.joseph@intel.com>
AuthorDate: Wed, 16 Nov 2022 19:59:32 -08:00
Committer: Borislav Petkov <bp@suse.de>
CommitterDate: Sat, 19 Nov 2022 11:23:53 +01:00
platform/x86/intel/ifs: Remove reload sysfs entry
Reload sysfs entry will be replaced by current_batch, drop it.
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20221117035935.4136738-14-jithu.joseph@intel.com
---
drivers/platform/x86/intel/ifs/sysfs.c | 29 +-------------------------
1 file changed, 29 deletions(-)
diff --git a/drivers/platform/x86/intel/ifs/sysfs.c b/drivers/platform/x86/intel/ifs/sysfs.c
index 65dd6fe..e077910 100644
--- a/drivers/platform/x86/intel/ifs/sysfs.c
+++ b/drivers/platform/x86/intel/ifs/sysfs.c
@@ -88,34 +88,6 @@ static ssize_t run_test_store(struct device *dev,
static DEVICE_ATTR_WO(run_test);
/*
- * Reload the IFS image. When user wants to install new IFS image
- */
-static ssize_t reload_store(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t count)
-{
- struct ifs_data *ifsd = ifs_get_data(dev);
- bool res;
-
-
- if (kstrtobool(buf, &res))
- return -EINVAL;
- if (!res)
- return count;
-
- if (down_interruptible(&ifs_sem))
- return -EINTR;
-
- ifs_load_firmware(dev);
-
- up(&ifs_sem);
-
- return ifsd->loaded ? count : -ENODEV;
-}
-
-static DEVICE_ATTR_WO(reload);
-
-/*
* Display currently loaded IFS image version.
*/
static ssize_t image_version_show(struct device *dev,
@@ -136,7 +108,6 @@ static struct attribute *plat_ifs_attrs[] = {
&dev_attr_details.attr,
&dev_attr_status.attr,
&dev_attr_run_test.attr,
- &dev_attr_reload.attr,
&dev_attr_image_version.attr,
NULL
};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. Range check of the input is done to verify
it not greater than 0xff.
For e.g if the scan test image comprises of 6 files, they would be named
as show below:
06-8f-06-01.scan
06-8f-06-02.scan
06-8f-06-03.scan
06-8f-06-04.scan
06-8f-06-05.scan
06-8f-06-06.scan
And writing 3 to current_batch would result in loading 06-8f-06-03.scan
in the above e.g. The file can also be read to know the currently loaded
file.
And testing a system looks like:
for each scan file
do
load the IFS test image file (write to the batch file)
for each core
do
test the core with this set of tests
done
done
Qualify few error messages with the test image file suffix to
provide better context.
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
---
drivers/platform/x86/intel/ifs/ifs.h | 23 ++++++++++----
drivers/platform/x86/intel/ifs/core.c | 1 +
drivers/platform/x86/intel/ifs/load.c | 18 +++++++----
drivers/platform/x86/intel/ifs/runtest.c | 10 ++++---
drivers/platform/x86/intel/ifs/sysfs.c | 38 ++++++++++++++++++++++++
5 files changed, 74 insertions(+), 16 deletions(-)
diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
index e3e8210ebd57..6ebedff4d6b8 100644
--- a/drivers/platform/x86/intel/ifs/ifs.h
+++ b/drivers/platform/x86/intel/ifs/ifs.h
@@ -33,13 +33,23 @@
* The driver loads the tests into memory reserved BIOS local to each CPU
* socket in a two step process using writes to MSRs to first load the
* SHA hashes for the test. Then the tests themselves. Status MSRs provide
- * feedback on the success/failure of these steps. When a new test file
- * is installed it can be loaded by writing to the driver reload file::
+ * feedback on the success/failure of these steps.
*
- * # echo 1 > /sys/devices/virtual/misc/intel_ifs_0/reload
+ * The test files are kept in a fixed location: /lib/firmware/intel/ifs_0/
+ * For e.g if there are 3 test files, they would be named in the following
+ * fashion:
+ * ff-mm-ss-01.scan
+ * ff-mm-ss-02.scan
+ * ff-mm-ss-03.scan
+ * (where ff refers to family, mm indicates model and ss indicates stepping)
*
- * Similar to microcode, the current version of the scan tests is stored
- * in a fixed location: /lib/firmware/intel/ifs.0/family-model-stepping.scan
+ * A different testfile can be loaded by writing the numerical portion
+ * (e.g 1, 2 or 3 in the above scenario) into the curent_batch file.
+ * To load ff-mm-ss-02.scan, the following command can be used::
+ *
+ * # echo 2 > /sys/devices/virtual/misc/intel_ifs_0/current_batch
+ *
+ * The above file can also be read to know the currently loaded image.
*
* Running tests
* -------------
@@ -207,6 +217,7 @@ struct ifs_data {
int status;
u64 scan_details;
u32 cur_batch;
+ int test_num;
};
struct ifs_work {
@@ -227,7 +238,7 @@ static inline struct ifs_data *ifs_get_data(struct device *dev)
return &d->data;
}
-void ifs_load_firmware(struct device *dev);
+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 4b39f2359180..c74cd8138ee6 100644
--- a/drivers/platform/x86/intel/ifs/core.c
+++ b/drivers/platform/x86/intel/ifs/core.c
@@ -23,6 +23,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,
},
.misc = {
.name = "intel_ifs_0",
diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
index 3f1de9d4cf4b..74a50e99cacd 100644
--- a/drivers/platform/x86/intel/ifs/load.c
+++ b/drivers/platform/x86/intel/ifs/load.c
@@ -253,17 +253,18 @@ static int image_sanity_check(struct device *dev, const struct microcode_header_
/*
* 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}.
+ * in /lib/firmware/intel/ifs_x/ and named as family-model-stepping-02x.{testname}.
*/
-void ifs_load_firmware(struct device *dev)
+int ifs_load_firmware(struct device *dev)
{
struct ifs_data *ifsd = ifs_get_data(dev);
const struct firmware *fw;
- char scan_path[32];
- int ret;
+ char scan_path[64];
+ int ret = -EINVAL;
- 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);
+ 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,
+ boot_cpu_data.x86_stepping, ifsd->cur_batch);
ret = request_firmware_direct(&fw, scan_path, dev);
if (ret) {
@@ -279,8 +280,13 @@ void ifs_load_firmware(struct device *dev)
ifs_hash_ptr = (u64)(ifs_header_ptr + 1);
ret = scan_chunks_sanity_check(dev);
+ if (ret)
+ dev_err(dev, "Load failure for batch: %02x\n", ifsd->cur_batch);
+
release:
release_firmware(fw);
done:
ifsd->loaded = (ret == 0);
+
+ return ret;
}
diff --git a/drivers/platform/x86/intel/ifs/runtest.c b/drivers/platform/x86/intel/ifs/runtest.c
index b2ca2bb4501f..0bfd8fcdd7e8 100644
--- a/drivers/platform/x86/intel/ifs/runtest.c
+++ b/drivers/platform/x86/intel/ifs/runtest.c
@@ -78,14 +78,16 @@ static void message_not_tested(struct device *dev, int cpu, union ifs_status sta
static void message_fail(struct device *dev, int cpu, union ifs_status status)
{
+ struct ifs_data *ifsd = ifs_get_data(dev);
+
/*
* control_error is set when the microcode runs into a problem
* loading the image from the reserved BIOS memory, or it has
* been corrupted. Reloading the image may fix this issue.
*/
if (status.control_error) {
- dev_err(dev, "CPU(s) %*pbl: could not execute from loaded scan image\n",
- cpumask_pr_args(cpu_smt_mask(cpu)));
+ dev_err(dev, "CPU(s) %*pbl: could not execute from loaded scan image. Batch: %02x version: 0x%x\n",
+ cpumask_pr_args(cpu_smt_mask(cpu)), ifsd->cur_batch, ifsd->loaded_version);
}
/*
@@ -96,8 +98,8 @@ static void message_fail(struct device *dev, int cpu, union ifs_status status)
* the core being tested.
*/
if (status.signature_error) {
- dev_err(dev, "CPU(s) %*pbl: test signature incorrect.\n",
- cpumask_pr_args(cpu_smt_mask(cpu)));
+ dev_err(dev, "CPU(s) %*pbl: test signature incorrect. Batch: %02x version: 0x%x\n",
+ cpumask_pr_args(cpu_smt_mask(cpu)), ifsd->cur_batch, ifsd->loaded_version);
}
}
diff --git a/drivers/platform/x86/intel/ifs/sysfs.c b/drivers/platform/x86/intel/ifs/sysfs.c
index e077910c5d28..ee636a76b083 100644
--- a/drivers/platform/x86/intel/ifs/sysfs.c
+++ b/drivers/platform/x86/intel/ifs/sysfs.c
@@ -87,6 +87,43 @@ static ssize_t run_test_store(struct device *dev,
static DEVICE_ATTR_WO(run_test);
+static ssize_t current_batch_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct ifs_data *ifsd = ifs_get_data(dev);
+ unsigned int cur_batch;
+ int rc;
+
+ rc = kstrtouint(buf, 0, &cur_batch);
+ if (rc < 0 || cur_batch > 0xff)
+ return -EINVAL;
+
+ if (down_interruptible(&ifs_sem))
+ return -EINTR;
+
+ ifsd->cur_batch = cur_batch;
+
+ rc = ifs_load_firmware(dev);
+
+ up(&ifs_sem);
+
+ return (rc == 0) ? count : rc;
+}
+
+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, "none\n");
+ else
+ return sysfs_emit(buf, "0x%02x\n", ifsd->cur_batch);
+}
+
+static DEVICE_ATTR_RW(current_batch);
+
/*
* Display currently loaded IFS image version.
*/
@@ -108,6 +145,7 @@ static struct attribute *plat_ifs_attrs[] = {
&dev_attr_details.attr,
&dev_attr_status.attr,
&dev_attr_run_test.attr,
+ &dev_attr_current_batch.attr,
&dev_attr_image_version.attr,
NULL
};
--
2.25.1Remove reload documentation and add current_batch documentation.
Update the kernel version and date for all the entries.
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
---
.../ABI/testing/sysfs-platform-intel-ifs | 30 ++++++++++---------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-platform-intel-ifs b/Documentation/ABI/testing/sysfs-platform-intel-ifs
index 486d6d2ff8a0..55991983d0d0 100644
--- a/Documentation/ABI/testing/sysfs-platform-intel-ifs
+++ b/Documentation/ABI/testing/sysfs-platform-intel-ifs
@@ -1,39 +1,41 @@
What: /sys/devices/virtual/misc/intel_ifs_<N>/run_test
-Date: April 21 2022
-KernelVersion: 5.19
+Date: Nov 16 2022
+KernelVersion: 6.2
Contact: "Jithu Joseph" <jithu.joseph@intel.com>
Description: Write <cpu#> to trigger IFS test for one online core.
Note that the test is per core. The cpu# can be
for any thread on the core. Running on one thread
completes the test for the core containing that thread.
Example: to test the core containing cpu5: echo 5 >
- /sys/devices/platform/intel_ifs.<N>/run_test
+ /sys/devices/virtual/misc/intel_ifs_<N>/run_test
What: /sys/devices/virtual/misc/intel_ifs_<N>/status
-Date: April 21 2022
-KernelVersion: 5.19
+Date: Nov 16 2022
+KernelVersion: 6.2
Contact: "Jithu Joseph" <jithu.joseph@intel.com>
Description: The status of the last test. It can be one of "pass", "fail"
or "untested".
What: /sys/devices/virtual/misc/intel_ifs_<N>/details
-Date: April 21 2022
-KernelVersion: 5.19
+Date: Nov 16 2022
+KernelVersion: 6.2
Contact: "Jithu Joseph" <jithu.joseph@intel.com>
Description: Additional information regarding the last test. The details file reports
the hex value of the SCAN_STATUS MSR. Note that the error_code field
may contain driver defined software code not defined in the Intel SDM.
What: /sys/devices/virtual/misc/intel_ifs_<N>/image_version
-Date: April 21 2022
-KernelVersion: 5.19
+Date: Nov 16 2022
+KernelVersion: 6.2
Contact: "Jithu Joseph" <jithu.joseph@intel.com>
Description: Version (hexadecimal) of loaded IFS binary image. If no scan image
is loaded reports "none".
-What: /sys/devices/virtual/misc/intel_ifs_<N>/reload
-Date: April 21 2022
-KernelVersion: 5.19
+What: /sys/devices/virtual/misc/intel_ifs_<N>/current_batch
+Date: Nov 16 2022
+KernelVersion: 6.2
Contact: "Jithu Joseph" <jithu.joseph@intel.com>
-Description: Write "1" (or "y" or "Y") to reload the IFS image from
- /lib/firmware/intel/ifs/ff-mm-ss.scan.
+Description: Write a number less than or equal to 0xff to load an IFS test image.
+ The number written treated as the 2 digit suffix in the following file name:
+ /lib/firmware/intel/ifs_<N>/ff-mm-ss-02x.scan
+ Reading the file will provide the suffix of the currently loaded IFS test image.
--
2.25.1The following commit has been merged into the x86/microcode branch of tip:
Commit-ID: 72a0f445fc091bd18873b10b9ab56573e490f00d
Gitweb: https://git.kernel.org/tip/72a0f445fc091bd18873b10b9ab56573e490f00d
Author: Jithu Joseph <jithu.joseph@intel.com>
AuthorDate: Wed, 16 Nov 2022 19:59:34 -08:00
Committer: Borislav Petkov <bp@suse.de>
CommitterDate: Sat, 19 Nov 2022 11:30:00 +01:00
Documentation/ABI: Update IFS ABI doc
Remove reload documentation and add current_batch documentation.
Update the kernel version and date for all the entries.
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20221117035935.4136738-16-jithu.joseph@intel.com
---
Documentation/ABI/testing/sysfs-platform-intel-ifs | 30 ++++++-------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-platform-intel-ifs b/Documentation/ABI/testing/sysfs-platform-intel-ifs
index 486d6d2..5599198 100644
--- a/Documentation/ABI/testing/sysfs-platform-intel-ifs
+++ b/Documentation/ABI/testing/sysfs-platform-intel-ifs
@@ -1,39 +1,41 @@
What: /sys/devices/virtual/misc/intel_ifs_<N>/run_test
-Date: April 21 2022
-KernelVersion: 5.19
+Date: Nov 16 2022
+KernelVersion: 6.2
Contact: "Jithu Joseph" <jithu.joseph@intel.com>
Description: Write <cpu#> to trigger IFS test for one online core.
Note that the test is per core. The cpu# can be
for any thread on the core. Running on one thread
completes the test for the core containing that thread.
Example: to test the core containing cpu5: echo 5 >
- /sys/devices/platform/intel_ifs.<N>/run_test
+ /sys/devices/virtual/misc/intel_ifs_<N>/run_test
What: /sys/devices/virtual/misc/intel_ifs_<N>/status
-Date: April 21 2022
-KernelVersion: 5.19
+Date: Nov 16 2022
+KernelVersion: 6.2
Contact: "Jithu Joseph" <jithu.joseph@intel.com>
Description: The status of the last test. It can be one of "pass", "fail"
or "untested".
What: /sys/devices/virtual/misc/intel_ifs_<N>/details
-Date: April 21 2022
-KernelVersion: 5.19
+Date: Nov 16 2022
+KernelVersion: 6.2
Contact: "Jithu Joseph" <jithu.joseph@intel.com>
Description: Additional information regarding the last test. The details file reports
the hex value of the SCAN_STATUS MSR. Note that the error_code field
may contain driver defined software code not defined in the Intel SDM.
What: /sys/devices/virtual/misc/intel_ifs_<N>/image_version
-Date: April 21 2022
-KernelVersion: 5.19
+Date: Nov 16 2022
+KernelVersion: 6.2
Contact: "Jithu Joseph" <jithu.joseph@intel.com>
Description: Version (hexadecimal) of loaded IFS binary image. If no scan image
is loaded reports "none".
-What: /sys/devices/virtual/misc/intel_ifs_<N>/reload
-Date: April 21 2022
-KernelVersion: 5.19
+What: /sys/devices/virtual/misc/intel_ifs_<N>/current_batch
+Date: Nov 16 2022
+KernelVersion: 6.2
Contact: "Jithu Joseph" <jithu.joseph@intel.com>
-Description: Write "1" (or "y" or "Y") to reload the IFS image from
- /lib/firmware/intel/ifs/ff-mm-ss.scan.
+Description: Write a number less than or equal to 0xff to load an IFS test image.
+ The number written treated as the 2 digit suffix in the following file name:
+ /lib/firmware/intel/ifs_<N>/ff-mm-ss-02x.scan
+ Reading the file will provide the suffix of the currently loaded IFS test image.Issues with user interface [1] to load scan test images
has been addressed, so the following can be reverted.
commit c483e7ea10fa ("platform/x86/intel/ifs: Mark as BROKEN")
Link: https://lore.kernel.org/lkml/26102aca-a730-ddf8-d024-2e7367696757@redhat.com/ [1]
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
---
drivers/platform/x86/intel/ifs/Kconfig | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/platform/x86/intel/ifs/Kconfig b/drivers/platform/x86/intel/ifs/Kconfig
index 89152d46deee..3eded966757e 100644
--- a/drivers/platform/x86/intel/ifs/Kconfig
+++ b/drivers/platform/x86/intel/ifs/Kconfig
@@ -1,9 +1,6 @@
config INTEL_IFS
tristate "Intel In Field Scan"
depends on X86 && CPU_SUP_INTEL && 64BIT && SMP
- # Discussion on the list has shown that the sysfs API needs a bit
- # more work, mark this as broken for now
- depends on BROKEN
help
Enable support for the In Field Scan capability in select
CPUs. The capability allows for running low level tests via
--
2.25.1The following commit has been merged into the x86/microcode branch of tip:
Commit-ID: 1a63b58082869273bfbab1b945007193f7bd3a78
Gitweb: https://git.kernel.org/tip/1a63b58082869273bfbab1b945007193f7bd3a78
Author: Jithu Joseph <jithu.joseph@intel.com>
AuthorDate: Wed, 16 Nov 2022 19:59:35 -08:00
Committer: Borislav Petkov <bp@suse.de>
CommitterDate: Sat, 19 Nov 2022 11:31:20 +01:00
Revert "platform/x86/intel/ifs: Mark as BROKEN"
Issues with user interface [1] to load scan test images have been
addressed so remove the dependency on BROKEN.
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/lkml/26102aca-a730-ddf8-d024-2e7367696757@redhat.com/ [1]
Link: https://lore.kernel.org/r/20221117035935.4136738-17-jithu.joseph@intel.com
---
drivers/platform/x86/intel/ifs/Kconfig | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/platform/x86/intel/ifs/Kconfig b/drivers/platform/x86/intel/ifs/Kconfig
index 89152d4..3eded96 100644
--- a/drivers/platform/x86/intel/ifs/Kconfig
+++ b/drivers/platform/x86/intel/ifs/Kconfig
@@ -1,9 +1,6 @@
config INTEL_IFS
tristate "Intel In Field Scan"
depends on X86 && CPU_SUP_INTEL && 64BIT && SMP
- # Discussion on the list has shown that the sysfs API needs a bit
- # more work, mark this as broken for now
- depends on BROKEN
help
Enable support for the In Field Scan capability in select
CPUs. The capability allows for running low level tests viaOn Mon, Nov 07, 2022 at 02:53:09PM -0800, Jithu Joseph wrote:
> Changes in v2
> - Rebased ontop of v6.1-rc4
> Boris
> - Moved exported functions (microcode_sanity_check(),
> find_matching_signature ) from microcode/intel.c to cpu/intel.c
> (patch4,6)
> - Removed microcode metadata specific code changes to
> microcode_sanity_check() (patch6)
> - Moved find_meta_data() from common to IFS driver (Patch 8)
What's the upstreaming plan here - I'm assuming I should take the
microcode patches through the tip tree?
Or should I take the whole thing through tip so that there's no
confusion and having to sync and share branches between trees?
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Hi Boris, On 11/10/22 10:59, Borislav Petkov wrote: > On Mon, Nov 07, 2022 at 02:53:09PM -0800, Jithu Joseph wrote: >> Changes in v2 >> - Rebased ontop of v6.1-rc4 >> Boris >> - Moved exported functions (microcode_sanity_check(), >> find_matching_signature ) from microcode/intel.c to cpu/intel.c >> (patch4,6) >> - Removed microcode metadata specific code changes to >> microcode_sanity_check() (patch6) >> - Moved find_meta_data() from common to IFS driver (Patch 8) > > What's the upstreaming plan here - I'm assuming I should take the > microcode patches through the tip tree? > > Or should I take the whole thing through tip so that there's no > confusion and having to sync and share branches between trees? I have just reviewed all the platform/x86/intel/ifs changes and they all look good to me. I think it is the best and easiest if you just take the whole branch. I don't have any changes pending under drivers/platform/x86/intel/ifs so there should not be any conflicts. Regards, Hans
On 11/10/2022 1:37 PM, Hans de Goede wrote: > Hi Boris, > > On 11/10/22 10:59, Borislav Petkov wrote: >> On Mon, Nov 07, 2022 at 02:53:09PM -0800, Jithu Joseph wrote: >>> Changes in v2 >>> - Rebased ontop of v6.1-rc4 >>> Boris >>> - Moved exported functions (microcode_sanity_check(), >>> find_matching_signature ) from microcode/intel.c to cpu/intel.c >>> (patch4,6) >>> - Removed microcode metadata specific code changes to >>> microcode_sanity_check() (patch6) >>> - Moved find_meta_data() from common to IFS driver (Patch 8) >> >> What's the upstreaming plan here - I'm assuming I should take the >> microcode patches through the tip tree? >> >> Or should I take the whole thing through tip so that there's no >> confusion and having to sync and share branches between trees? > > I have just reviewed all the platform/x86/intel/ifs changes > and they all look good to me. > Hans, Thanks for reviewing. Boris, I have fixes from Sohil's comments (mainly for patch 06/14). Let me know if you have any other issues that need touching up in x86 parts (04 - 07). I will post a cleaned-up series next week. Jithu
© 2016 - 2026 Red Hat, Inc.