Hi,
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@xxxxxxxxx>
> Suggested-by: Borislav Petkov <bp@xxxxxxxxx>
> Signed-off-by: Jithu Joseph <jithu.joseph@xxxxxxxxx>
> ---
> 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;
> }
