lkml.org 
[lkml]   [2022]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v3 04/16] platform/x86/intel/ifs: Remove memory allocation from load path
    From
    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@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;
    > }

    \
     
     \ /
      Last update: 2022-11-17 09:53    [W:2.607 / U:0.036 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site