| Message ID | 1419733246-5612-22-git-send-email-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Simon, On Sun, Dec 28, 2014 at 10:20 AM, Simon Glass <sjg@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@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? > - 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@0 { > -#include "microcode/m12206a7_00000029.dtsi" > - }; > - update@1 { > #include "microcode/m12306a9_0000001b.dtsi" > }; > }; > -- Regards, Bin
On Wed, Dec 31, 2014 at 2:45 PM, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Sun, Dec 28, 2014 at 10:20 AM, Simon Glass <sjg@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@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. > I realized UCODE_HEADER_LEN might be better. At least shorter than microcode :-), and is consistent with the MSR_IA32_UCODE_WRITE. [snip] Regards, Bin
Hi Bin, On 30 December 2014 at 23:45, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Sun, Dec 28, 2014 at 10:20 AM, Simon Glass <sjg@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@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@0 { >> -#include "microcode/m12206a7_00000029.dtsi" >> - }; >> - update@1 { >> #include "microcode/m12306a9_0000001b.dtsi" >> }; >> }; >> -- > Regards, Simon
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 + xorl %edx, %edx + movl $_dt_ucode_base_size, %eax + movl (%eax), %eax + addl $0x30, %eax + 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(); - 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@0 { -#include "microcode/m12206a7_00000029.dtsi" - }; - update@1 { #include "microcode/m12306a9_0000001b.dtsi" }; };
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@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(-)