Re: [PATCH 1/2] microcode: Scan the initramfs payload for microcode blob.

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 25/09/13 15:11, Konrad Rzeszutek Wilk wrote:
> . snip..
>>> -        ucode_mod_idx = simple_strtol(s, NULL, 0);
>>> +    if ( ucode_mod_forced ) /* Forced by EFI */
>>> +       return;
>>> +
>>> +    ucode_mod_idx = simple_strtol(s, NULL, 0);
>>> +
>>> +    if ( ucode_mod_idx == 0 && !strncmp(s, "scan", 4) )
>>> +        ucode_scan = 1;
>> This code matches neither the description in the comment above, nor the
>> description in the command line markdown document.
>>
>> Realistically, "scan" is mutually exclusive with specifying an index.
>>
>> Might it be better to have:
>>
>> if ( !strncmp(s, "scan", 4) )
>>   ucode_scan = 1;
>> else
>>   ucode_mod_index = simple_strtol(s, NULL, 0);
>>
>> ~Andrew
> .. snip.. How does this patch look like (in microcode.v3.1 branch):
>
> From 586e18b79f9db8de460ea1bfc66d5a2f8b5cec04 Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Date: Wed, 10 Jul 2013 16:49:30 -0400
> Subject: [PATCH 1/2] microcode: Scan the initramfs payload for microcode blob.
>
> The Linux kernel is able to update the microcode during early
> bootup via inspection of the initramfs blob to see if there is an
> cpio image with certain microcode files. Linux is able to function
> with two (or more) cpio archives in the initrd b/c it unpacks
> all of the cpio archives.
>
> The format of the early initramfs is nicely documented
> in Linux's Documentation/x86/early-microcode.txt:
>
> Early load microcode
> ====================
> By Fenghua Yu <fenghua.yu@intel.com>
>
> Kernel can update microcode in early phase of boot time. Loading microcode early
> can fix CPU issues before they are observed during kernel boot time.
>
> Microcode is stored in an initrd file. The microcode is read from the initrd
> file and loaded to CPUs during boot time.
>
> The format of the combined initrd image is microcode in cpio format followed by
> the initrd image (maybe compressed). Kernel parses the combined initrd image
> during boot time. The microcode file in cpio name space is:
> kernel/x86/microcode/GenuineIntel.bin
>
> During BSP boot (before SMP starts), if the kernel finds the microcode file in
> the initrd file, it parses the microcode and saves matching microcode in memory.
> If matching microcode is found, it will be uploaded in BSP and later on in all
> APs.
>
> The cached microcode patch is applied when CPUs resume from a sleep state.
>
> There are two legacy user space interfaces to load microcode, either through
> /dev/cpu/microcode or through /sys/devices/system/cpu/microcode/reload file
> in sysfs.
>
> In addition to these two legacy methods, the early loading method described
> here is the third method with which microcode can be uploaded to a system's
> CPUs.
>
> The following example script shows how to generate a new combined initrd file in
> /boot/initrd-3.5.0.ucode.img with original microcode microcode.bin and
> original initrd image /boot/initrd-3.5.0.img.
>
> mkdir initrd
> cd initrd
> mkdir kernel
> mkdir kernel/x86
> mkdir kernel/x86/microcode
> cp ../microcode.bin kernel/x86/microcode/GenuineIntel.bin
> find .|cpio -oc >../ucode.cpio
> cd ..
> cat ucode.cpio /boot/initrd-3.5.0.img >/boot/initrd-3.5.0.ucode.img
>
> As such this code inspects the initrd to see if the microcode
> signatures are present and if so updates the hypervisor.
>
> The option to turn this scan on/off is gated by the 'ucode'
> parameter. The options are now:
>  'scan'      Scan for the microcode in any multiboot payload.
>  <index>     Attempt to load microcode blob (not the cpio archive
>              format) from the multiboot payload number.
>
> This option alters slightly the 'ucode' parameter by only allowing
> either parameter:
>   ucode=[<index>|scan]
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> [v1: Revised code based on Boris Ostrovsky's comments]
> [v2: Fix memory leak (forgot to call xfree)]
> [v3: Added comments from David, stuck the option under ucode per Jan's
>      review]
> [v4: Review comments from Jan]
> [v5: Review comments from Andrew]
> ---
>  docs/misc/xen-command-line.markdown |  14 +++-
>  xen/arch/x86/microcode.c            | 158 ++++++++++++++++++++++++++++++++----
>  xen/common/Makefile                 |   2 +-
>  xen/common/earlycpio.c              | 151 ++++++++++++++++++++++++++++++++++
>  xen/include/xen/earlycpio.h         |  14 ++++
>  5 files changed, 320 insertions(+), 19 deletions(-)
>  create mode 100644 xen/common/earlycpio.c
>  create mode 100644 xen/include/xen/earlycpio.h
>
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
> index 56c9729..60892d6 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -897,10 +897,12 @@ pages) must also be specified via the tbuf\_size parameter.
>  > `= unstable | skewed`
>  
>  ### ucode
> -> `= <integer>`
> +> `= <integer> , scan`

+> `= [<integer> | scan]`

~Andrew

> +
> +Specify how and where to find CPU microcode update blob.
>  
> -Specify the CPU microcode update blob module index. When positive, this
> -specifies the n-th module (in the GrUB entry, zero based) to be used
> +'integer' specifies the CPU microcode update blob module index. When positive,
> +this specifies the n-th module (in the GrUB entry, zero based) to be used
>  for updating CPU micrcode. When negative, counting starts at the end of
>  the modules in the GrUB entry (so with the blob commonly being last,
>  one could specify `ucode=-1`). Note that the value of zero is not valid
> @@ -910,6 +912,12 @@ when used with xen.efi (there the concept of modules doesn't exist, and
>  the blob gets specified via the `ucode=<filename>` config file/section
>  entry; see [EFI configuration file description](efi.html)).
>  
> +'scan' instructs the hypervisor to scan the multiboot images for an cpio
> +image that contains microcode. Depending on the platform the format of
> +the microcode blobs must be:
> +  - on Intel: kernel/x86/microcode/GenuineIntel.bin
> +  - on AMD  : kernel/x86/microcode/AuthenticAMD.bin
> +
>  ### unrestricted\_guest
>  > `= <boolean>`
>  
> diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
> index fbbf95b..e6344cf 100644
> --- a/xen/arch/x86/microcode.c
> +++ b/xen/arch/x86/microcode.c
> @@ -33,6 +33,7 @@
>  #include <xen/spinlock.h>
>  #include <xen/tasklet.h>
>  #include <xen/guest_access.h>
> +#include <xen/earlycpio.h>
>  
>  #include <asm/msr.h>
>  #include <asm/processor.h>
> @@ -45,19 +46,123 @@ static signed int __initdata ucode_mod_idx;
>  static bool_t __initdata ucode_mod_forced;
>  static cpumask_t __initdata init_mask;
>  
> +/*
> + * If we scan the initramfs.cpio for the early microcode code
> + * and find it, then 'ucode_blob' will contain the pointer
> + * and the size of said blob. It is allocated from Xen's heap
> + * memory.
> + */
> +struct ucode_mod_blob {
> +    void *data;
> +    size_t size;
> +};
> +
> +static struct ucode_mod_blob __initdata ucode_blob;
> +/*
> + * By default we will NOT parse the multiboot modules to see if there is
> + * cpio image with the microcode images.
> + */
> +static bool_t __initdata ucode_scan;
> +
>  void __init microcode_set_module(unsigned int idx)
>  {
>      ucode_mod_idx = idx;
>      ucode_mod_forced = 1;
>  }
>  
> +/*
> + * The format is '[<integer>|scan]'. Both options are optional.
> + * If the EFI has forced which of the multiboot payloads is to be used,
> + * no parsing will be attempted.
> + */
>  static void __init parse_ucode(char *s)
>  {
> -    if ( !ucode_mod_forced )
> +    if ( ucode_mod_forced ) /* Forced by EFI */
> +       return;
> +
> +    if ( !strncmp(s, "scan", 4) )
> +        ucode_scan = 1;
> +    else
>          ucode_mod_idx = simple_strtol(s, NULL, 0);
>  }
>  custom_param("ucode", parse_ucode);
>  
> +/*
> + * 8MB ought to be enough.
> + */
> +#define MAX_EARLY_CPIO_MICROCODE (8 << 20)
> +
> +void __init microcode_scan_module(
> +    unsigned long *module_map,
> +    const multiboot_info_t *mbi,
> +    void *(*bootmap)(const module_t *))
> +{
> +    module_t *mod = (module_t *)__va(mbi->mods_addr);
> +    uint64_t *_blob_start;
> +    unsigned long _blob_size;
> +    struct cpio_data cd;
> +    long offset;
> +    const char *p = NULL;
> +    int i;
> +
> +    ucode_blob.size = 0;
> +    if ( !ucode_scan )
> +        return;
> +
> +    if ( boot_cpu_data.x86_vendor == X86_VENDOR_AMD )
> +        p = "kernel/x86/microcode/AuthenticAMD.bin";
> +    else if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
> +        p = "kernel/x86/microcode/GenuineIntel.bin";
> +    else
> +        return;
> +
> +    /*
> +     * Try all modules and see whichever could be the microcode blob.
> +     */
> +    for ( i = 1 /* Ignore dom0 kernel */; i < mbi->mods_count; i++ )
> +    {
> +        if ( !test_bit(i, module_map) )
> +            continue;
> +
> +        _blob_start = bootmap(&mod[i]);
> +        _blob_size = mod[i].mod_end;
> +        if ( !_blob_start )
> +        {
> +            printk("Could not map multiboot module #%d (size: %ld)\n",
> +                   i, _blob_size);
> +            continue;
> +        }
> +        cd.data = NULL;
> +        cd.size = 0;
> +        cd = find_cpio_data(p, _blob_start, _blob_size, &offset /* ignore */);
> +        if ( cd.data )
> +        {
> +                /*
> +                 * This is an arbitrary check - it would be sad if the blob
> +                 * consumed most of the memory and did not allow guests
> +                 * to launch.
> +                 */
> +                if ( cd.size > MAX_EARLY_CPIO_MICROCODE )
> +                {
> +                    printk("Multiboot %d microcode payload too big! (%ld, we can do %d)\n",
> +                           i, cd.size, MAX_EARLY_CPIO_MICROCODE);
> +                    goto err;
> +                }
> +                ucode_blob.size = cd.size;
> +                ucode_blob.data = xmalloc_bytes(cd.size);
> +                if ( !ucode_blob.data )
> +                    cd.data = NULL;
> +                else
> +                    memcpy(ucode_blob.data, cd.data, cd.size);
> +        }
> +        bootmap(NULL);
> +        if ( cd.data )
> +            break;
> +    }
> +    return;
> +err:
> +    bootmap(NULL);
> +}
>  void __init microcode_grab_module(
>      unsigned long *module_map,
>      const multiboot_info_t *mbi,
> @@ -69,9 +174,12 @@ void __init microcode_grab_module(
>          ucode_mod_idx += mbi->mods_count;
>      if ( ucode_mod_idx <= 0 || ucode_mod_idx >= mbi->mods_count ||
>           !__test_and_clear_bit(ucode_mod_idx, module_map) )
> -        return;
> +        goto scan;
>      ucode_mod = mod[ucode_mod_idx];
>      ucode_mod_map = map;
> +scan:
> +    if ( ucode_scan )
> +        microcode_scan_module(module_map, mbi, map);
>  }
>  
>  const struct microcode_ops *microcode_ops;
> @@ -236,7 +344,10 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) buf, unsigned long len)
>  
>  static void __init _do_microcode_update(unsigned long data)
>  {
> -    microcode_update_cpu((void *)data, ucode_mod.mod_end);
> +    void *_data = (void *)data;
> +    size_t len = ucode_blob.size ? ucode_blob.size : ucode_mod.mod_end;
> +
> +    microcode_update_cpu(_data, len);
>      cpumask_set_cpu(smp_processor_id(), &init_mask);
>  }
>  
> @@ -246,18 +357,19 @@ static int __init microcode_init(void)
>      static struct tasklet __initdata tasklet;
>      unsigned int cpu;
>  
> -    if ( !microcode_ops || !ucode_mod.mod_end )
> +    if ( !microcode_ops )
> +        return 0;
> +
> +    if ( !ucode_mod.mod_end && !ucode_blob.size )
>          return 0;
>  
> -    data = ucode_mod_map(&ucode_mod);
> +    data = ucode_blob.size ? ucode_blob.data : ucode_mod_map(&ucode_mod);
> +
>      if ( !data )
>          return -ENOMEM;
>  
>      if ( microcode_ops->start_update && microcode_ops->start_update() != 0 )
> -    {
> -        ucode_mod_map(NULL);
> -        return 0;
> -    }
> +        goto out;
>  
>      softirq_tasklet_init(&tasklet, _do_microcode_update, (unsigned long)data);
>  
> @@ -269,7 +381,11 @@ static int __init microcode_init(void)
>          } while ( !cpumask_test_cpu(cpu, &init_mask) );
>      }
>  
> -    ucode_mod_map(NULL);
> +out:
> +    if ( ucode_blob.size )
> +        xfree(data);
> +    else
> +        ucode_mod_map(NULL);
>  
>      return 0;
>  }
> @@ -298,14 +414,26 @@ static int __init microcode_presmp_init(void)
>  {
>      if ( microcode_ops )
>      {
> -        if ( ucode_mod.mod_end )
> +        if ( ucode_mod.mod_end || ucode_blob.size )
>          {
> -            void *data = ucode_mod_map(&ucode_mod);
> -
> +            void *data;
> +            size_t len;
> +
> +            if ( ucode_blob.size )
> +            {
> +                len = ucode_blob.size;
> +                data = ucode_blob.data;
> +            }
> +            else
> +            {
> +                len = ucode_mod.mod_end;
> +                data = ucode_mod_map(&ucode_mod);
> +            }
>              if ( data )
> -                microcode_update_cpu(data, ucode_mod.mod_end);
> +                microcode_update_cpu(data, len);
>  
> -            ucode_mod_map(NULL);
> +            if ( !ucode_blob.size )
> +                ucode_mod_map(NULL);
>          }
>  
>          register_cpu_notifier(&microcode_percpu_nfb);
> diff --git a/xen/common/Makefile b/xen/common/Makefile
> index 5486140..6da4651 100644
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -49,7 +49,7 @@ obj-y += radix-tree.o
>  obj-y += rbtree.o
>  obj-y += lzo.o
>  
> -obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma unlzo,$(n).init.o)
> +obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma unlzo earlycpio,$(n).init.o)
>  
>  obj-$(perfc)       += perfc.o
>  obj-$(crash_debug) += gdbstub.o
> diff --git a/xen/common/earlycpio.c b/xen/common/earlycpio.c
> new file mode 100644
> index 0000000..5e54142
> --- /dev/null
> +++ b/xen/common/earlycpio.c
> @@ -0,0 +1,151 @@
> +/* ----------------------------------------------------------------------- *
> + *
> + *   Copyright 2012 Intel Corporation; author H. Peter Anvin
> + *
> + *   This file is part of the Linux kernel, and is made available
> + *   under the terms of the GNU General Public License version 2, as
> + *   published by the Free Software Foundation.
> + *
> + *   This program is distributed in the hope it will be useful, but
> + *   WITHOUT ANY WARRANTY; without even the implied warranty of
> + *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *   General Public License for more details.
> + *
> + * ----------------------------------------------------------------------- */
> +
> +/*
> + * earlycpio.c
> + *
> + * Find a specific cpio member; must precede any compressed content.
> + * This is used to locate data items in the initramfs used by the
> + * kernel itself during early boot (before the main initramfs is
> + * decompressed.)  It is the responsibility of the initramfs creator
> + * to ensure that these items are uncompressed at the head of the
> + * blob.  Depending on the boot loader or package tool that may be a
> + * separate file or part of the same file.
> + */
> +
> +#include <xen/config.h>
> +#include <xen/init.h>
> +#include <xen/lib.h>
> +#include <xen/string.h>
> +#include <xen/earlycpio.h>
> +
> +#define ALIGN(x, a) ((x + (a) - 1) & ~((a) - 1))
> +#define PTR_ALIGN(p, a)         ((typeof(p))ALIGN((unsigned long)(p), (a)))
> +
> +enum cpio_fields {
> +	C_MAGIC,
> +	C_INO,
> +	C_MODE,
> +	C_UID,
> +	C_GID,
> +	C_NLINK,
> +	C_MTIME,
> +	C_FILESIZE,
> +	C_MAJ,
> +	C_MIN,
> +	C_RMAJ,
> +	C_RMIN,
> +	C_NAMESIZE,
> +	C_CHKSUM,
> +	C_NFIELDS
> +};
> +
> +/**
> + * cpio_data find_cpio_data - Search for files in an uncompressed cpio
> + * @path:   The directory to search for, including a slash at the end
> + * @data:   Pointer to the the cpio archive or a header inside
> + * @len:    Remaining length of the cpio based on data pointer
> + * @offset: When a matching file is found, this is the offset to the
> + *          beginning of the cpio. It can be used to iterate through
> + *          the cpio to find all files inside of a directory path
> + *
> + * @return: struct cpio_data containing the address, length and
> + *          filename (with the directory path cut off) of the found file.
> + *          If you search for a filename and not for files in a directory,
> + *          pass the absolute path of the filename in the cpio and make sure
> + *          the match returned an empty filename string.
> + */
> +
> +struct cpio_data __init find_cpio_data(const char *path, void *data,
> +					  size_t len,  long *offset)
> +{
> +	const size_t cpio_header_len = 8*C_NFIELDS - 2;
> +	struct cpio_data cd = { NULL, 0 };
> +	const char *p, *dptr, *nptr;
> +	unsigned int ch[C_NFIELDS], *chp, v;
> +	unsigned char c, x;
> +	size_t mypathsize = strlen(path);
> +	int i, j;
> +
> +	p = data;
> +
> +	while (len > cpio_header_len) {
> +		if (!*p) {
> +			/* All cpio headers need to be 4-byte aligned */
> +			p += 4;
> +			len -= 4;
> +			continue;
> +		}
> +
> +		j = 6;		/* The magic field is only 6 characters */
> +		chp = ch;
> +		for (i = C_NFIELDS; i; i--) {
> +			v = 0;
> +			while (j--) {
> +				v <<= 4;
> +				c = *p++;
> +
> +				x = c - '0';
> +				if (x < 10) {
> +					v += x;
> +					continue;
> +				}
> +
> +				x = (c | 0x20) - 'a';
> +				if (x < 6) {
> +					v += x + 10;
> +					continue;
> +				}
> +
> +				goto quit; /* Invalid hexadecimal */
> +			}
> +			*chp++ = v;
> +			j = 8;	/* All other fields are 8 characters */
> +		}
> +
> +		if ((ch[C_MAGIC] - 0x070701) > 1)
> +			goto quit; /* Invalid magic */
> +
> +		len -= cpio_header_len;
> +
> +		dptr = PTR_ALIGN(p + ch[C_NAMESIZE], 4);
> +		nptr = PTR_ALIGN(dptr + ch[C_FILESIZE], 4);
> +
> +		if (nptr > p + len || dptr < p || nptr < dptr)
> +			goto quit; /* Buffer overrun */
> +
> +		if ((ch[C_MODE] & 0170000) == 0100000 &&
> +		    ch[C_NAMESIZE] >= mypathsize &&
> +		    !memcmp(p, path, mypathsize)) {
> +			*offset = (long)nptr - (long)data;
> +			if (ch[C_NAMESIZE] - mypathsize >= MAX_CPIO_FILE_NAME) {
> +				printk(
> +				"File %s exceeding MAX_CPIO_FILE_NAME [%d]\n",
> +				p, MAX_CPIO_FILE_NAME);
> +			}
> +			if (ch[C_NAMESIZE] - 1 /* includes \0 */ == mypathsize) {
> +				cd.data = (void *)dptr;
> +				cd.size = ch[C_FILESIZE];
> +				return cd; /* Found it! */
> +			}
> +		}
> +		len -= (nptr - p);
> +		p = nptr;
> +	}
> +
> +quit:
> +	return cd;
> +}
> +
> diff --git a/xen/include/xen/earlycpio.h b/xen/include/xen/earlycpio.h
> new file mode 100644
> index 0000000..85d144a
> --- /dev/null
> +++ b/xen/include/xen/earlycpio.h
> @@ -0,0 +1,14 @@
> +#ifndef _EARLYCPIO_H
> +#define _EARLYCPIO_H
> +
> +#define MAX_CPIO_FILE_NAME 18
> +
> +struct cpio_data {
> +	void *data;
> +	size_t size;
> +};
> +
> +struct cpio_data find_cpio_data(const char *path, void *data, size_t len,
> +				long *offset);
> +
> +#endif /* _EARLYCPIO_H */