[U-Boot] [PATCH 5/8] x86: ifdtool: Add support for early microcode access
Bin Meng
bmeng.cn at gmail.com
Mon Dec 15 10:33:02 CET 2014
Hi Simon,
On Mon, Dec 15, 2014 at 8:15 AM, Simon Glass <sjg at chromium.org> wrote:
> Some Intel CPUs use an 'FSP' binary blob which provides an inflexible
> means of starting up the CPU. One result is that microcode updates can only
> be done before RAM is available and therefore parsing of the device tree
> is impracticle.
>
> Worse, the addess of the microcode update must be stored in ROM since a
> pointer to its start address and size is passed to the 'FSP' blob. It is
> not possible to perform any calculations to obtain the address and size.
>
> To work around this, ifdtool is enhanced to work out the address and size of
> the first microcode update it finds in the supplied device tree. It then
> writes these into the correct place in the ROM. U-Boot can then start up
> the FSP correctly.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> tools/Makefile | 1 +
> tools/ifdtool.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 104 insertions(+), 8 deletions(-)
>
> diff --git a/tools/Makefile b/tools/Makefile
> index a4216a1..e549f8e 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -126,6 +126,7 @@ hostprogs-$(CONFIG_EXYNOS5250) += mkexynosspl
> hostprogs-$(CONFIG_EXYNOS5420) += mkexynosspl
> HOSTCFLAGS_mkexynosspl.o := -pedantic
>
> +ifdtool-objs := $(LIBFDT_OBJS) ifdtool.o
> hostprogs-$(CONFIG_X86) += ifdtool
>
> hostprogs-$(CONFIG_MX23) += mxsboot
> diff --git a/tools/ifdtool.c b/tools/ifdtool.c
> index 4077ba8..50b28d4 100644
> --- a/tools/ifdtool.c
> +++ b/tools/ifdtool.c
> @@ -18,6 +18,7 @@
> #include <unistd.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> +#include <libfdt.h>
> #include "ifdtool.h"
>
> #undef DEBUG
> @@ -34,6 +35,8 @@
>
> enum input_file_type_t {
> IF_normal,
> + IF_fdt,
> + IF_uboot,
> };
>
> struct input_file {
> @@ -703,7 +706,7 @@ int inject_region(char *image, int size, int region_type, char *region_fname)
> * 0xffffffff so use an address relative to that. For an
> * 8MB ROM the start address is 0xfff80000.
> * @write_fname: Filename to add to the image
> - * @return 0 if OK, -ve on error
> + * @return number of bytes written if OK, -ve on error
> */
> static int write_data(char *image, int size, unsigned int addr,
> const char *write_fname)
> @@ -715,7 +718,7 @@ static int write_data(char *image, int size, unsigned int addr,
> if (write_fd < 0)
> return write_fd;
>
> - offset = addr + size;
> + offset = (uint32_t)(addr + size);
> debug("Writing %s to offset %#x\n", write_fname, offset);
>
> if (offset < 0 || offset + write_size > size) {
> @@ -731,6 +734,69 @@ static int write_data(char *image, int size, unsigned int addr,
>
> close(write_fd);
>
> + return write_size;
> +}
> +
> +/**
> + * write_uboot() - Write U-Boot, device tree and microcode pointer
> + *
> + * This writes U-Boot into a place in the flash, followed by its device tree.
> + * The microcode pointer is written so that U-Boot can find the microcode in
> + * the device tree very early in boot.
> + *
> + * @image: Pointer to image
> + * @size: Size of image in bytes
> + *
Can we remove the blank lines here?
> + *
> + *
> + * @return number of bytes written if OK, -ve on error
> + */
However the codes below says the return value is always 0 if OK.
> +static int write_uboot(char *image, int size, struct input_file *uboot,
> + struct input_file *fdt, unsigned int ucode_ptr)
> +{
> + const void *blob;
> + const char *data;
> + int uboot_size;
> + uint32_t *ptr;
> + int data_size;
> + int offset;
> + int node;
> + int ret;
> +
> + uboot_size = write_data(image, size, uboot->addr, uboot->fname);
> + if (uboot_size < 0)
> + return uboot_size;
> + fdt->addr = uboot->addr + uboot_size;
> + debug("U-Boot size %#x, FDT at %#x\n", uboot_size, fdt->addr);
> + ret = write_data(image, size, fdt->addr, fdt->fname);
> + if (ret < 0)
> + return ret;
> +
> + if (ucode_ptr) {
> + blob = (void *)image + (uint32_t)(fdt->addr + size);
> + debug("DTB at %lx\n", (char *)blob - image);
> + node = fdt_node_offset_by_compatible(blob, 0,
> + "intel,microcode");
> + if (node < 0) {
> + debug("No microcode found in FDT: %s\n",
> + fdt_strerror(node));
> + return -ENOENT;
> + }
> + data = fdt_getprop(blob, node, "data", &data_size);
> + if (!data) {
> + debug("No microcode data found in FDT: %s\n",
> + fdt_strerror(data_size));
> + return -ENOENT;
> + }
> + offset = ucode_ptr - uboot->addr;
> + ptr = (void *)image + offset;
> + ptr[0] = fdt->addr + (data - image);
This is the bug that causes Crown Bay fails to boot. It should be:
ptr[0] = uboot->addr + (data - image);
> + ptr[1] = data_size;
> + debug("Wrote microcode pointer at %x: addr=%x, size=%x\n",
> + ucode_ptr, ptr[0], ptr[1]);
> + }
> +
> return 0;
> }
>
> @@ -800,7 +866,7 @@ int main(int argc, char *argv[])
> char *desc_fname = NULL, *addr_str = NULL;
> int region_type = -1, inputfreq = 0;
> enum spi_frequency spifreq = SPI_FREQUENCY_20MHZ;
> - struct input_file input_file[WRITE_MAX], *ifile;
> + struct input_file input_file[WRITE_MAX], *ifile, *fdt = NULL;
> unsigned char wr_idx, wr_num = 0;
> int rom_size = -1;
> bool write_it;
> @@ -808,6 +874,8 @@ int main(int argc, char *argv[])
> char *outfile = NULL;
> struct stat buf;
> int size = 0;
> + unsigned int ucode_ptr = 0;
> + bool have_uboot = false;
> int bios_fd;
> char *image;
> int ret;
> @@ -817,18 +885,21 @@ int main(int argc, char *argv[])
> {"descriptor", 1, NULL, 'D'},
> {"em100", 0, NULL, 'e'},
> {"extract", 0, NULL, 'x'},
> + {"fdt", 1, NULL, 'f'},
> {"inject", 1, NULL, 'i'},
> {"lock", 0, NULL, 'l'},
> + {"microcode", 1, NULL, 'm'},
> {"romsize", 1, NULL, 'r'},
> {"spifreq", 1, NULL, 's'},
> {"unlock", 0, NULL, 'u'},
> + {"uboot", 1, NULL, 'U'},
> {"write", 1, NULL, 'w'},
> {"version", 0, NULL, 'v'},
> {"help", 0, NULL, 'h'},
> {0, 0, 0, 0}
> };
>
> - while ((opt = getopt_long(argc, argv, "cdD:ehi:lr:s:uvw:x?",
> + while ((opt = getopt_long(argc, argv, "cdD:ef:hi:lm:r:s:uU:vw:x?",
> long_options, &option_index)) != EOF) {
> switch (opt) {
> case 'c':
> @@ -871,6 +942,9 @@ int main(int argc, char *argv[])
> case 'l':
> mode_locked = 1;
> break;
> + case 'm':
> + ucode_ptr = strtoul(optarg, NULL, 0);
> + break;
> case 'r':
> rom_size = strtol(optarg, NULL, 0);
> debug("ROM size %d\n", rom_size);
> @@ -904,6 +978,8 @@ int main(int argc, char *argv[])
> exit(EXIT_SUCCESS);
> break;
> case 'w':
> + case 'U':
> + case 'f':
> ifile = &input_file[wr_num];
> mode_write = 1;
> if (wr_num < WRITE_MAX) {
> @@ -913,7 +989,12 @@ int main(int argc, char *argv[])
> exit(EXIT_FAILURE);
> }
> ifile->addr = strtol(optarg, NULL, 0);
> - ifile->type = IF_normal;
> + ifile->type = opt == 'f' ? IF_fdt :
> + opt == 'U' ? IF_uboot : IF_normal;
> + if (ifile->type == IF_fdt)
> + fdt = ifile;
> + else if (ifile->type == IF_uboot)
> + have_uboot = true;
> wr_num++;
> } else {
> fprintf(stderr,
> @@ -970,6 +1051,13 @@ int main(int argc, char *argv[])
> exit(EXIT_FAILURE);
> }
>
> + if (have_uboot && !fdt) {
> + fprintf(stderr,
> + "You must supply a device tree file for U-Boot\n\n");
> + print_usage(argv[0]);
> + exit(EXIT_FAILURE);
> + }
> +
> filename = argv[optind];
> if (optind + 2 != argc)
> outfile = argv[optind + 1];
> @@ -1034,9 +1122,16 @@ int main(int argc, char *argv[])
> if (mode_write) {
> for (wr_idx = 0; wr_idx < wr_num; wr_idx++) {
> ifile = &input_file[wr_idx];
> - ret = write_data(image, size, ifile->addr,
> + if (ifile->type == IF_fdt) {
> + continue;
> + } else if (ifile->type == IF_uboot) {
> + ret = write_uboot(image, size, ifile, fdt,
> + ucode_ptr);
> + } else {
> + ret = write_data(image, size, ifile->addr,
> ifile->fname);
> - if (ret)
> + }
> + if (ret < 0)
> break;
> }
> }
> @@ -1071,5 +1166,5 @@ int main(int argc, char *argv[])
> free(image);
> close(bios_fd);
>
> - return ret ? 1 : 0;
> + return ret < 0 ? 1 : 0;
> }
> --
Regards,
Bin
More information about the U-Boot
mailing list