lkml.org 
[lkml]   [2022]   [Nov]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v2 09/14] platform/x86/intel/ifs: Use generic microcode headers and functions
    From
    Hi all,

    On 11/7/22 23:53, Jithu Joseph wrote:
    > 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, use struct microcode_header_intel directly in
    > IFS driver instead of duplicating into a separate ifs_header structure.
    > Further use existing microcode_sanity_check() and find_matching_signature()
    > directly instead of implementing separate ones within the driver.
    >
    > More IFS specific checks will be added subsequently.
    >
    > Reviewed-by: Tony Luck <tony.luck@intel.com>
    > Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>

    Thanks, patch looks good to me:

    Reviewed-by: Hans de Goede <hdegoede@redhat.com>

    Regards,

    Hans




    > ---
    > arch/x86/include/asm/microcode_intel.h | 1 +
    > drivers/platform/x86/intel/ifs/load.c | 102 +++++--------------------
    > 2 files changed, 20 insertions(+), 83 deletions(-)
    >
    > diff --git a/arch/x86/include/asm/microcode_intel.h b/arch/x86/include/asm/microcode_intel.h
    > index 0ff4545f72d2..f905238748fc 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 MICROCODE_HEADER_VER_UCODE 1
    > +#define MICROCODE_HEADER_VER_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 60ba5a057f91..7c0d8602817b 100644
    > --- a/drivers/platform/x86/intel/ifs/load.c
    > +++ b/drivers/platform/x86/intel/ifs/load.c
    > @@ -8,22 +8,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);
    > @@ -150,33 +136,18 @@ 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;
    > struct ifs_data *ifsd = ifs_get_data(dev);
    > + int curr_pkg, cpu, ret = -ENOMEM;
    > 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;
    >
    > - 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();
    > @@ -203,67 +174,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 ifs_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 != MICROCODE_HEADER_VER_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, MICROCODE_HEADER_VER_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}.
    > @@ -284,12 +221,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 = ifs_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);

    \
     
     \ /
      Last update: 2022-11-10 22:13    [W:4.123 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site