On Wed, Jan 31, 2018 at 05:12:03PM +0100, Patrick Wildt wrote:
> Hi,
> 
> this diff allows us to load the Intel microcode much earlier.  So far
> we load it after the CPUs have identified and then have to update the
> CPU flags afterwards.  This is not good since we have to assume that
> those updates can remove and add instructions and other features.  We
> need to load it earlier.  The only other option is to have the boot-
> blocks load the ucode for us.
> 
> One issue though is actually loading and passing the binaries that can
> range from 2k to 90k of binary size.  As far as I know we don't use the
> lower 16M of memory on amd64, which the bootblocks use as heap.  Thus
> allocating a buffer for using alloc() should be "fine" if we make sure
> that we read the ucode after we early enough.  kettenis@ tells me that
> cpu_startup() is the earliest MD code which can use malloc(9), so that
> is where we can copy the ucode from the bootloader to kernel land.
> 
> I have tested this with efiboot(8), more tests would be appreciated.
> 
> Thanks,
> Patrick

Updated diff.  RAMDISK compiles and runs through make release without
any further issues.

This diff also enables the debug printfs, so people testing this diff
should see something happening in dmesg.  I will not commit that diff
with the prints enabled.

Please report back.  Some more stuff should be seen when doing

        $ dmesg | grep ucode

Please not that you have to build and install the bootloader in addtion
to installing a new kernel.

Thanks,
Patrick

diff --git a/sys/arch/amd64/amd64/cpu.c b/sys/arch/amd64/amd64/cpu.c
index 8a779aa65eb..5ec7e7f6409 100644
--- a/sys/arch/amd64/amd64/cpu.c
+++ b/sys/arch/amd64/amd64/cpu.c
@@ -406,21 +406,24 @@ cpu_attach(struct device *parent, struct device *self, 
void *aux)
                printf("(uniprocessor)\n");
                ci->ci_flags |= CPUF_PRESENT | CPUF_SP | CPUF_PRIMARY;
                cpu_intr_init(ci);
+#ifndef SMALL_KERNEL
+               cpu_ucode_apply(ci);
+#endif
                identifycpu(ci);
 #ifdef MTRR
                mem_range_attach();
 #endif /* MTRR */
                cpu_init(ci);
                cpu_init_mwait(sc);
-#ifndef SMALL_KERNEL
-               config_mountroot(NULL, cpu_ucode_attachhook);
-#endif
                break;
 
        case CPU_ROLE_BP:
                printf("apid %d (boot processor)\n", caa->cpu_apicid);
                ci->ci_flags |= CPUF_PRESENT | CPUF_BSP | CPUF_PRIMARY;
                cpu_intr_init(ci);
+#ifndef SMALL_KERNEL
+               cpu_ucode_apply(ci);
+#endif
                identifycpu(ci);
 #ifdef MTRR
                mem_range_attach();
@@ -438,9 +441,6 @@ cpu_attach(struct device *parent, struct device *self, void 
*aux)
                ioapic_bsp_id = caa->cpu_apicid;
 #endif
                cpu_init_mwait(sc);
-#ifndef SMALL_KERNEL
-               config_mountroot(NULL, cpu_ucode_attachhook);
-#endif
                break;
 
        case CPU_ROLE_AP:
@@ -698,6 +698,7 @@ cpu_hatch(void *v)
 
        lapic_enable();
        lapic_startclock();
+       cpu_ucode_apply(ci);
 
        if ((ci->ci_flags & CPUF_IDENTIFIED) == 0) {
                /*
@@ -738,8 +739,6 @@ cpu_hatch(void *v)
        lldt(0);
 
        cpu_init(ci);
-       cpu_ucode_apply(ci);
-       cpu_flags_update(ci);
 #if NPVBUS > 0
        pvbus_init_cpu();
 #endif
@@ -939,20 +938,3 @@ cpu_activate(struct device *self, int act)
 
        return (0);
 }
-
-void
-cpu_flags_update(struct cpu_info *ci)
-{
-       uint32_t dummy;
-
-       /* XXX this update is much later than we want it to be */
-       if (cpuid_level >= 0x07) {
-               CPUID_LEAF(0x7, 0, dummy, dummy, dummy,
-                   ci->ci_feature_sefflags_edx);
-       }
-       if (!strcmp(cpu_vendor, "AuthenticAMD") &&
-           ci->ci_pnfeatset >= 0x80000008) {
-               CPUID(0x80000008, dummy, ci->ci_feature_amdspec_ebx,
-                   dummy, dummy);
-       }
-}
diff --git a/sys/arch/amd64/amd64/machdep.c b/sys/arch/amd64/amd64/machdep.c
index 33d1d38395b..940716c174c 100644
--- a/sys/arch/amd64/amd64/machdep.c
+++ b/sys/arch/amd64/amd64/machdep.c
@@ -241,6 +241,7 @@ bios_diskinfo_t     *bios_diskinfo;
 bios_memmap_t  *bios_memmap;
 u_int32_t      bios_cksumlen;
 bios_efiinfo_t *bios_efiinfo;
+bios_ucode_t   *bios_ucode;
 
 /*
  * Size of memory segments, before any memory is stolen.
@@ -308,6 +309,10 @@ cpu_startup(void)
 
        /* Safe for i/o port / memory space allocation to use malloc now. */
        x86_bus_space_mallocok();
+
+#ifndef SMALL_KERNEL
+       cpu_ucode_setup();
+#endif
 }
 
 /*
@@ -1886,6 +1891,10 @@ getbootinfo(char *bootinfo, int bootinfo_size)
                                docninit++;
                        break;
 
+               case BOOTARG_UCODE:
+                       bios_ucode = (bios_ucode_t *)q->ba_arg;
+                       break;
+
                default:
 #ifdef BOOTINFO_DEBUG
                        printf(" unsupported arg (%d) %p", q->ba_type,
diff --git a/sys/arch/amd64/amd64/ucode.c b/sys/arch/amd64/amd64/ucode.c
index ceec53dafea..7b807125782 100644
--- a/sys/arch/amd64/amd64/ucode.c
+++ b/sys/arch/amd64/amd64/ucode.c
@@ -19,12 +19,16 @@
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/mutex.h>
+#include <sys/malloc.h>
+
+#include <uvm/uvm_extern.h>
 
 #include <machine/cpu.h>
 #include <machine/cpufunc.h>
 #include <machine/specialreg.h>
+#include <machine/biosvar.h>
 
-/* #define UCODE_DEBUG */
+#define UCODE_DEBUG
 #ifdef UCODE_DEBUG
 #define DPRINTF(x)     do { if (cpu_ucode_debug > 0) printf x; } while (0)
 #define DPRINTFN(n, x) do { if (cpu_ucode_debug >= (n)) printf x; } while (0)
@@ -65,7 +69,7 @@ struct intel_ucode_ext_sig {
 char *  cpu_ucode_data;
 size_t  cpu_ucode_size;
 
-void    cpu_ucode_setup(struct cpu_info *);
+void    cpu_ucode_setup(void);
 void    cpu_ucode_apply(struct cpu_info *);
 
 /* Intel */
@@ -81,32 +85,18 @@ struct intel_ucode_header   *cpu_ucode_intel_applied;
 struct mutex                    cpu_ucode_intel_mtx = 
MUTEX_INITIALIZER(IPL_HIGH);
 
 void
-cpu_ucode_attachhook(struct device *dv)
+cpu_ucode_setup(void)
 {
-       struct cpu_info *ci = curcpu();
-
-       cpu_ucode_setup(ci);
-       cpu_ucode_apply(ci);
-       cpu_flags_update(ci);
-}
-
-void
-cpu_ucode_setup(struct cpu_info *ci)
-{
-       char name[128];
-       u_char *ucode;
-       size_t size;
-
-       snprintf(name, sizeof(name), "intel/%02x-%02x-%02x", ci->ci_family,
-           ci->ci_model, (ci->ci_signature >> 0) & 0x0f);
+       if (bios_ucode == NULL)
+               return;
 
-       if (loadfirmware(name, &ucode, &size) != 0) {
-               DPRINTF(("%s: no microcode found: %s\n", __func__, name));
+       if (!bios_ucode->uc_addr || !bios_ucode->uc_size)
                return;
-       }
 
-       cpu_ucode_data = ucode;
-       cpu_ucode_size = size;
+       cpu_ucode_size = bios_ucode->uc_size;
+       cpu_ucode_data = malloc(cpu_ucode_size, M_DEVBUF, M_WAITOK);
+       memcpy(cpu_ucode_data, (void *)PMAP_DIRECT_MAP(bios_ucode->uc_addr),
+           cpu_ucode_size);
 }
 
 /*
@@ -126,8 +116,10 @@ cpu_ucode_intel_apply(struct cpu_info *ci)
        uint32_t old_rev, new_rev;
        paddr_t data;
 
-       if (cpu_ucode_data == NULL || cpu_ucode_size == 0)
+       if (cpu_ucode_data == NULL || cpu_ucode_size == 0) {
+               DPRINTF(("%s: no microcode provided\n", __func__));
                return;
+       }
 
        /*
         * Grab a mutex, because we are not allowed to run updates
@@ -140,10 +132,14 @@ cpu_ucode_intel_apply(struct cpu_info *ci)
        if (update == NULL)
                update = cpu_ucode_intel_find(cpu_ucode_data,
                    cpu_ucode_size, old_rev);
-       if (update == NULL)
+       if (update == NULL) {
+               DPRINTF(("%s: no microcode update found\n", __func__));
                goto out;
-       if (update->update_revision == old_rev)
+       }
+       if (update->update_revision == old_rev) {
+               DPRINTF(("%s: microcode already up-to-date\n", __func__));
                goto out;
+       }
 
        /* Apply microcode. */
        data = (paddr_t)update;
diff --git a/sys/arch/amd64/include/biosvar.h b/sys/arch/amd64/include/biosvar.h
index dcb59d766ca..9c7f3b5e7b4 100644
--- a/sys/arch/amd64/include/biosvar.h
+++ b/sys/arch/amd64/include/biosvar.h
@@ -206,6 +206,12 @@ typedef struct _bios_efiinfo {
        uint32_t        fb_reserved_mask;
 } __packed bios_efiinfo_t;
 
+#define        BOOTARG_UCODE 12
+typedef struct _bios_ucode {
+       uint64_t        uc_addr;
+       uint64_t        uc_size;
+} __packed bios_ucode_t;
+
 #if defined(_KERNEL) || defined (_STANDALONE)
 
 #ifdef _LOCORE
@@ -254,6 +260,7 @@ bios_diskinfo_t *bios_getdiskinfo(dev_t);
 extern u_int bootapiver;
 extern bios_memmap_t *bios_memmap;
 extern bios_efiinfo_t *bios_efiinfo;
+extern bios_ucode_t *bios_ucode;
 
 #endif /* _KERNEL */
 #endif /* _LOCORE */
diff --git a/sys/arch/amd64/include/cpufunc.h b/sys/arch/amd64/include/cpufunc.h
index a86ac711a80..67398c5f077 100644
--- a/sys/arch/amd64/include/cpufunc.h
+++ b/sys/arch/amd64/include/cpufunc.h
@@ -314,9 +314,8 @@ breakpoint(void)
 }
 
 void amd64_errata(struct cpu_info *);
-void cpu_flags_update(struct cpu_info *);
+void cpu_ucode_setup(void);
 void cpu_ucode_apply(struct cpu_info *);
-void cpu_ucode_attachhook(struct device *);
 
 #endif /* _KERNEL */
 
diff --git a/sys/arch/amd64/stand/boot/conf.c b/sys/arch/amd64/stand/boot/conf.c
index 6bb7271de00..b00fec88696 100644
--- a/sys/arch/amd64/stand/boot/conf.c
+++ b/sys/arch/amd64/stand/boot/conf.c
@@ -40,7 +40,7 @@
 #include <biosdev.h>
 #include <dev/cons.h>
 
-const char version[] = "3.33";
+const char version[] = "3.34";
 int    debug = 1;
 
 
diff --git a/sys/arch/amd64/stand/cdboot/conf.c 
b/sys/arch/amd64/stand/cdboot/conf.c
index 29088565aa8..acca472568a 100644
--- a/sys/arch/amd64/stand/cdboot/conf.c
+++ b/sys/arch/amd64/stand/cdboot/conf.c
@@ -41,7 +41,7 @@
 #include <biosdev.h>
 #include <dev/cons.h>
 
-const char version[] = "3.28";
+const char version[] = "3.29";
 int    debug = 1;
 
 
diff --git a/sys/arch/amd64/stand/efiboot/conf.c 
b/sys/arch/amd64/stand/efiboot/conf.c
index b91d7485dea..7c0e8361d6b 100644
--- a/sys/arch/amd64/stand/efiboot/conf.c
+++ b/sys/arch/amd64/stand/efiboot/conf.c
@@ -39,7 +39,7 @@
 #include "efidev.h"
 #include "efipxe.h"
 
-const char version[] = "3.37";
+const char version[] = "3.38";
 
 #ifdef EFI_DEBUG
 int    debug = 0;
diff --git a/sys/arch/amd64/stand/libsa/exec_i386.c 
b/sys/arch/amd64/stand/libsa/exec_i386.c
index eecefb2c2d8..335109aeab4 100644
--- a/sys/arch/amd64/stand/libsa/exec_i386.c
+++ b/sys/arch/amd64/stand/libsa/exec_i386.c
@@ -33,8 +33,10 @@
 #include <dev/cons.h>
 #include <lib/libsa/loadfile.h>
 #include <machine/biosvar.h>
+#include <machine/specialreg.h>
 #include <stand/boot/bootarg.h>
 
+#include "cmd.h"
 #include "disk.h"
 #include "libsa.h"
 
@@ -51,6 +53,9 @@
 typedef void (*startfuncp)(int, int, int, int, int, int, int, int)
     __attribute__ ((noreturn));
 
+void ucode_load(void);
+extern struct cmd_state cmd;
+
 char *bootmac = NULL;
 
 void
@@ -102,6 +107,8 @@ run_loadfile(u_long *marks, int howto)
        bcopy(bootdev_dip->disklabel.d_uid, &bootduid.duid, sizeof(bootduid));
        addbootarg(BOOTARG_BOOTDUID, sizeof(bootduid), &bootduid);
 
+       ucode_load();
+
 #ifdef SOFTRAID
        if (bootdev_dip->sr_vol != NULL) {
                bv = bootdev_dip->sr_vol;
@@ -160,3 +167,54 @@ run_loadfile(u_long *marks, int howto)
 #endif
        /* not reached */
 }
+
+void
+ucode_load(void)
+{
+       uint32_t model, family, stepping;
+       uint32_t dummy, signature;
+       uint32_t vendor[4];
+       bios_ucode_t uc;
+       struct stat sb;
+       char path[128];
+       size_t buflen;
+       char *buf;
+       int fd;
+
+       CPUID(0, dummy, vendor[0], vendor[2], vendor[1]);
+       vendor[3] = 0; /* NULL-terminate */
+       if (strcmp((char *)vendor, "GenuineIntel") != 0)
+               return;
+
+       CPUID(1, signature, dummy, dummy, dummy);
+       family = (signature >> 8) & 0x0f;
+       model = (signature >> 4) & 0x0f;
+       if (family == 0x6 || family == 0xf) {
+               family += (signature >> 20) & 0xff;
+               model += ((signature >> 16) & 0x0f) << 4;
+       }
+       stepping = (signature >> 0) & 0x0f;
+
+       snprintf(path, sizeof(path), "%s:/etc/firmware/intel/%02x-%02x-%02x",
+           cmd.bootdev, family, model, stepping);
+
+       fd = open(path, 0);
+       if (fd == -1)
+               return;
+
+       if (fstat(fd, &sb) == -1)
+               return;
+
+       buflen = sb.st_size;
+       if ((buf = alloc(buflen)) == NULL)
+               return;
+
+       if (read(fd, buf, buflen) != buflen) {
+               free(buf, buflen);
+               return;
+       }
+
+       uc.uc_addr = (uint64_t)buf;
+       uc.uc_size = (uint64_t)buflen;
+       addbootarg(BOOTARG_UCODE, sizeof(uc), &uc);
+}
diff --git a/sys/arch/amd64/stand/pxeboot/conf.c 
b/sys/arch/amd64/stand/pxeboot/conf.c
index 4e2484374c3..0e23f9f52c9 100644
--- a/sys/arch/amd64/stand/pxeboot/conf.c
+++ b/sys/arch/amd64/stand/pxeboot/conf.c
@@ -43,7 +43,7 @@
 #include "pxeboot.h"
 #include "pxe_net.h"
 
-const char version[] = "3.28";
+const char version[] = "3.29";
 int    debug = 0;
 
 void (*sa_cleanup)(void) = pxe_shutdown;

Reply via email to