[U-Boot] [PATCH 21/22] x86: ivybridge: Update microcode early in boot

[Simon Glass]

Hi  Bin,

On 30 December 2014 at 23:45, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Simon,
>
> On Sun, Dec 28, 2014 at 10:20 AM, Simon Glass <sjg at chromium.org> wrote:
>> At present the normal update (which happens much later) does not work. This
>> seems to have something to do with the 'no eviction' mode in the CAR, or at
>> least moving the microcode update after that causes it not to work.
>>
>> For now, do an update early on so that it definitely works. Also refuse to
>> continue unless the microcode update check (later in boot) is successful.
>>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>> ---
>>
>>  arch/x86/cpu/ivybridge/car.S             | 14 ++++++++++++++
>>  arch/x86/cpu/ivybridge/cpu.c             |  2 +-
>>  arch/x86/cpu/ivybridge/microcode_intel.c |  9 +++++++--
>>  arch/x86/dts/link.dts                    |  3 ---
>>  4 files changed, 22 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/x86/cpu/ivybridge/car.S b/arch/x86/cpu/ivybridge/car.S
>> index 6e7e1e4..95da087 100644
>> --- a/arch/x86/cpu/ivybridge/car.S
>> +++ b/arch/x86/cpu/ivybridge/car.S
>> @@ -45,6 +45,14 @@ car_init:
>>         movl    $0xFEE00300, %esi
>>         movl    %eax, (%esi)
>>
>> +       /* TODO: Load microcode later - the 'no eviction' mode breaks this */
>> +       movl    $0x79, %ecx
>
> Replace 0x79 to MSR_IA32_UCODE_WRITE from msr-index.h
>
>> +       xorl    %edx, %edx
>> +       movl    $_dt_ucode_base_size, %eax
>> +       movl    (%eax), %eax
>> +       addl    $0x30, %eax
>
> And here 0x30 to something like MICROCODE_HEADER_LEN.
>
>> +       wrmsr
>> +
>>         post_code(POST_CAR_SIPI)
>>         /* Zero out all fixed range and variable range MTRRs */
>>         movl    $mtrr_table, %esi
>> @@ -228,3 +236,9 @@ mtrr_table:
>>         .word 0x20C, 0x20D, 0x20E, 0x20F
>>         .word 0x210, 0x211, 0x212, 0x213
>>  mtrr_table_end:
>> +
>> +       .align 4
>> +_dt_ucode_base_size:
>> +       /* These next two fields are filled in by ifdtool */
>> +       .long   0                       /* microcode base */
>> +       .long   0                       /* microcode size */
>> diff --git a/arch/x86/cpu/ivybridge/cpu.c b/arch/x86/cpu/ivybridge/cpu.c
>> index 0543e06..e925310 100644
>> --- a/arch/x86/cpu/ivybridge/cpu.c
>> +++ b/arch/x86/cpu/ivybridge/cpu.c
>> @@ -263,7 +263,7 @@ int print_cpuinfo(void)
>>         enable_lapic();
>>
>>         ret = microcode_update_intel();
>
> Since we already did the microcode update in car_init, why should we
> do this again here?

The car_init() version is a work-around that I would like to remove.

This update actually checks that it happened and fails with an error
if not. For car we don't have a way yet of displaying an error.

>
>> -       if (ret && ret != -ENOENT && ret != -EEXIST)
>> +       if (ret)
>>                 return ret;
>>
>>         /* Enable upper 128bytes of CMOS */
>> diff --git a/arch/x86/cpu/ivybridge/microcode_intel.c b/arch/x86/cpu/ivybridge/microcode_intel.c
>> index 0817751..c012820 100644
>> --- a/arch/x86/cpu/ivybridge/microcode_intel.c
>> +++ b/arch/x86/cpu/ivybridge/microcode_intel.c
>> @@ -120,6 +120,7 @@ int microcode_update_intel(void)
>>         int count;
>>         int node;
>>         int ret;
>> +       int rev;
>>
>>         microcode_read_cpu(&cpu);
>>         node = 0;
>> @@ -147,12 +148,16 @@ int microcode_update_intel(void)
>>                         skipped++;
>>                         continue;
>>                 }
>> -               ret = microcode_read_rev();
>>                 wrmsr(0x79, (ulong)update.data, 0);
>> +               rev = microcode_read_rev();
>>                 debug("microcode: updated to revision 0x%x date=%04x-%02x-%02x\n",
>> -                     microcode_read_rev(), update.date_code & 0xffff,
>> +                     rev, update.date_code & 0xffff,
>>                       (update.date_code >> 24) & 0xff,
>>                       (update.date_code >> 16) & 0xff);
>> +               if (update.update_revision != rev) {
>> +                       printf("Microcode update failed\n");
>> +                       return -EFAULT;
>> +               }
>>                 count++;
>>         } while (1);
>>  }
>> diff --git a/arch/x86/dts/link.dts b/arch/x86/dts/link.dts
>> index a739080..1ebc334 100644
>> --- a/arch/x86/dts/link.dts
>> +++ b/arch/x86/dts/link.dts
>> @@ -214,9 +214,6 @@
>>
>>         microcode {
>>                 update at 0 {
>> -#include "microcode/m12206a7_00000029.dtsi"
>> -               };
>> -               update at 1 {
>>  #include "microcode/m12306a9_0000001b.dtsi"
>>                 };
>>         };
>> --
>

Regards,
Simon