| Message ID | BLU436-SMTP989D08F6764A45BA3A5703BFB30@phx.gbl |
|---|---|
| State | Changes Requested |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Simon, On Thu, Jun 4, 2015 at 8:12 PM, Bin Meng <bmeng.cn@gmail.com> wrote: > This is a temparory hacking for testing U-Boot on a newer version > MinnowMax board. > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > --- > > arch/x86/dts/minnowmax.dts | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/dts/minnowmax.dts b/arch/x86/dts/minnowmax.dts > index 7103bc5..9e1fcfc 100644 > --- a/arch/x86/dts/minnowmax.dts > +++ b/arch/x86/dts/minnowmax.dts > @@ -101,7 +101,7 @@ > > microcode { > update@0 { > -#include "microcode/m0130673322.dtsi" > +#include "microcode/m0130679901.dtsi" > }; > }; > > -- Saket confirmed these two patches resolved the boot problem he saw. So we will need think about how to better support such scenario that different revision board with different stepping CPUs. Could be multiple microcodes in one U-Boot image, or still one microcode with some #ifdef #endif. Note FSP has the capability to accept multiple microcodes as parameters in the FspTempRamInit(), but right now U-Boot only specifies one. How do you think? Regards, Bin
Hi Bin, On 06/04 22:21, Bin Meng wrote: > Hi Simon, > > On Thu, Jun 4, 2015 at 8:12 PM, Bin Meng <bmeng.cn@gmail.com> wrote: > > This is a temparory hacking for testing U-Boot on a newer version > > MinnowMax board. > > > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > > --- > > > > arch/x86/dts/minnowmax.dts | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/x86/dts/minnowmax.dts b/arch/x86/dts/minnowmax.dts > > index 7103bc5..9e1fcfc 100644 > > --- a/arch/x86/dts/minnowmax.dts > > +++ b/arch/x86/dts/minnowmax.dts > > @@ -101,7 +101,7 @@ > > > > microcode { > > update@0 { > > -#include "microcode/m0130673322.dtsi" > > +#include "microcode/m0130679901.dtsi" > > }; > > }; > > > > -- > > Saket confirmed these two patches resolved the boot problem he saw. So > we will need think about how to better support such scenario that > different revision board with different stepping CPUs. Could be > multiple microcodes in one U-Boot image, or still one microcode with > some #ifdef #endif. Note FSP has the capability to accept multiple > microcodes as parameters in the FspTempRamInit(), but right now U-Boot > only specifies one. How do you think? Why not just have a minnowmax common dtsi and then top level dts files for each revision of the board containing the ways they are different (such as microcode)? Thanks, Andrew
Hi, On 4 June 2015 at 10:27, Andrew Bradford <andrew@bradfordembedded.com> wrote: > > Hi Bin, > > On 06/04 22:21, Bin Meng wrote: > > Hi Simon, > > > > On Thu, Jun 4, 2015 at 8:12 PM, Bin Meng <bmeng.cn@gmail.com> wrote: > > > This is a temparory hacking for testing U-Boot on a newer version > > > MinnowMax board. > > > > > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > > > --- > > > > > > arch/x86/dts/minnowmax.dts | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/arch/x86/dts/minnowmax.dts b/arch/x86/dts/minnowmax.dts > > > index 7103bc5..9e1fcfc 100644 > > > --- a/arch/x86/dts/minnowmax.dts > > > +++ b/arch/x86/dts/minnowmax.dts > > > @@ -101,7 +101,7 @@ > > > > > > microcode { > > > update@0 { > > > -#include "microcode/m0130673322.dtsi" > > > +#include "microcode/m0130679901.dtsi" > > > }; > > > }; > > > > > > -- > > > > Saket confirmed these two patches resolved the boot problem he saw. So > > we will need think about how to better support such scenario that > > different revision board with different stepping CPUs. Could be > > multiple microcodes in one U-Boot image, or still one microcode with > > some #ifdef #endif. Note FSP has the capability to accept multiple > > microcodes as parameters in the FspTempRamInit(), but right now U-Boot > > only specifies one. How do you think? > > Why not just have a minnowmax common dtsi and then top level dts files > for each revision of the board containing the ways they are different > (such as microcode)? My preference would be to include all the microcode needed by the board and then pass the correct one to the FSP. Now that we can access the device tree that should be possible. There is logic in ./arch/x86/cpu/ivybridge/microcode_intel.c to do this but it may need a bit of refactorng. The existing microcode approach (with ifdtool adding a pointer to the microcode) is a work-around for the FSP problem. Now that Bin has solved this I'd be keen to remove it an just use device tree. Regards, Simon
Hi Simon, On Fri, Jun 5, 2015 at 2:31 AM, Simon Glass <sjg@chromium.org> wrote: > Hi, > > On 4 June 2015 at 10:27, Andrew Bradford <andrew@bradfordembedded.com> wrote: >> >> Hi Bin, >> >> On 06/04 22:21, Bin Meng wrote: >> > Hi Simon, >> > >> > On Thu, Jun 4, 2015 at 8:12 PM, Bin Meng <bmeng.cn@gmail.com> wrote: >> > > This is a temparory hacking for testing U-Boot on a newer version >> > > MinnowMax board. >> > > >> > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >> > > --- >> > > >> > > arch/x86/dts/minnowmax.dts | 2 +- >> > > 1 file changed, 1 insertion(+), 1 deletion(-) >> > > >> > > diff --git a/arch/x86/dts/minnowmax.dts b/arch/x86/dts/minnowmax.dts >> > > index 7103bc5..9e1fcfc 100644 >> > > --- a/arch/x86/dts/minnowmax.dts >> > > +++ b/arch/x86/dts/minnowmax.dts >> > > @@ -101,7 +101,7 @@ >> > > >> > > microcode { >> > > update@0 { >> > > -#include "microcode/m0130673322.dtsi" >> > > +#include "microcode/m0130679901.dtsi" >> > > }; >> > > }; >> > > >> > > -- >> > >> > Saket confirmed these two patches resolved the boot problem he saw. So >> > we will need think about how to better support such scenario that >> > different revision board with different stepping CPUs. Could be >> > multiple microcodes in one U-Boot image, or still one microcode with >> > some #ifdef #endif. Note FSP has the capability to accept multiple >> > microcodes as parameters in the FspTempRamInit(), but right now U-Boot >> > only specifies one. How do you think? >> >> Why not just have a minnowmax common dtsi and then top level dts files >> for each revision of the board containing the ways they are different >> (such as microcode)? > > My preference would be to include all the microcode needed by the > board and then pass the correct one to the FSP. Now that we can access > the device tree that should be possible. There is logic in > ./arch/x86/cpu/ivybridge/microcode_intel.c to do this but it may need > a bit of refactorng. > The device tree still cannot be accessed in that early phase before CAR is initialized. The issue is that FSP is designed to have FspTempRamInit() to do both microcode loading and CAR initialization. > The existing microcode approach (with ifdtool adding a pointer to the > microcode) is a work-around for the FSP problem. Now that Bin has > solved this I'd be keen to remove it an just use device tree. > Regards, Bin
Hi Bin, On 4 June 2015 at 20:03, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Fri, Jun 5, 2015 at 2:31 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi, >> >> On 4 June 2015 at 10:27, Andrew Bradford <andrew@bradfordembedded.com> wrote: >>> >>> Hi Bin, >>> >>> On 06/04 22:21, Bin Meng wrote: >>> > Hi Simon, >>> > >>> > On Thu, Jun 4, 2015 at 8:12 PM, Bin Meng <bmeng.cn@gmail.com> wrote: >>> > > This is a temparory hacking for testing U-Boot on a newer version >>> > > MinnowMax board. >>> > > >>> > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >>> > > --- >>> > > >>> > > arch/x86/dts/minnowmax.dts | 2 +- >>> > > 1 file changed, 1 insertion(+), 1 deletion(-) >>> > > >>> > > diff --git a/arch/x86/dts/minnowmax.dts b/arch/x86/dts/minnowmax.dts >>> > > index 7103bc5..9e1fcfc 100644 >>> > > --- a/arch/x86/dts/minnowmax.dts >>> > > +++ b/arch/x86/dts/minnowmax.dts >>> > > @@ -101,7 +101,7 @@ >>> > > >>> > > microcode { >>> > > update@0 { >>> > > -#include "microcode/m0130673322.dtsi" >>> > > +#include "microcode/m0130679901.dtsi" >>> > > }; >>> > > }; >>> > > >>> > > -- >>> > >>> > Saket confirmed these two patches resolved the boot problem he saw. So >>> > we will need think about how to better support such scenario that >>> > different revision board with different stepping CPUs. Could be >>> > multiple microcodes in one U-Boot image, or still one microcode with >>> > some #ifdef #endif. Note FSP has the capability to accept multiple >>> > microcodes as parameters in the FspTempRamInit(), but right now U-Boot >>> > only specifies one. How do you think? >>> >>> Why not just have a minnowmax common dtsi and then top level dts files >>> for each revision of the board containing the ways they are different >>> (such as microcode)? >> >> My preference would be to include all the microcode needed by the >> board and then pass the correct one to the FSP. Now that we can access >> the device tree that should be possible. There is logic in >> ./arch/x86/cpu/ivybridge/microcode_intel.c to do this but it may need >> a bit of refactorng. >> > > The device tree still cannot be accessed in that early phase before > CAR is initialized. The issue is that FSP is designed to have > FspTempRamInit() to do both microcode loading and CAR initialization. > Ah yes of course. Most unfortunate. Maybe we could have ifdtool or the Makefile put all the microcode in a single big blob somewhere else in u-boot.rom so that the whole thing can be passed to the FSP. In that case I think the FSP will select the correct one I also wonder if it is possible to load the microcode in our own code and pass nothing to the FSP? I think I tried that though and it hung. The FSP design leaves much to b desired. >> The existing microcode approach (with ifdtool adding a pointer to the >> microcode) is a work-around for the FSP problem. Now that Bin has >> solved this I'd be keen to remove it an just use device tree. >> > > Regards, > Bin Regards, Simon
Hi Simon, On Sat, Jun 6, 2015 at 12:17 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Bin, > > On 4 June 2015 at 20:03, Bin Meng <bmeng.cn@gmail.com> wrote: >> Hi Simon, >> >> On Fri, Jun 5, 2015 at 2:31 AM, Simon Glass <sjg@chromium.org> wrote: >>> Hi, >>> >>> On 4 June 2015 at 10:27, Andrew Bradford <andrew@bradfordembedded.com> wrote: >>>> >>>> Hi Bin, >>>> >>>> On 06/04 22:21, Bin Meng wrote: >>>> > Hi Simon, >>>> > >>>> > On Thu, Jun 4, 2015 at 8:12 PM, Bin Meng <bmeng.cn@gmail.com> wrote: >>>> > > This is a temparory hacking for testing U-Boot on a newer version >>>> > > MinnowMax board. >>>> > > >>>> > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >>>> > > --- >>>> > > >>>> > > arch/x86/dts/minnowmax.dts | 2 +- >>>> > > 1 file changed, 1 insertion(+), 1 deletion(-) >>>> > > >>>> > > diff --git a/arch/x86/dts/minnowmax.dts b/arch/x86/dts/minnowmax.dts >>>> > > index 7103bc5..9e1fcfc 100644 >>>> > > --- a/arch/x86/dts/minnowmax.dts >>>> > > +++ b/arch/x86/dts/minnowmax.dts >>>> > > @@ -101,7 +101,7 @@ >>>> > > >>>> > > microcode { >>>> > > update@0 { >>>> > > -#include "microcode/m0130673322.dtsi" >>>> > > +#include "microcode/m0130679901.dtsi" >>>> > > }; >>>> > > }; >>>> > > >>>> > > -- >>>> > >>>> > Saket confirmed these two patches resolved the boot problem he saw. So >>>> > we will need think about how to better support such scenario that >>>> > different revision board with different stepping CPUs. Could be >>>> > multiple microcodes in one U-Boot image, or still one microcode with >>>> > some #ifdef #endif. Note FSP has the capability to accept multiple >>>> > microcodes as parameters in the FspTempRamInit(), but right now U-Boot >>>> > only specifies one. How do you think? >>>> >>>> Why not just have a minnowmax common dtsi and then top level dts files >>>> for each revision of the board containing the ways they are different >>>> (such as microcode)? >>> >>> My preference would be to include all the microcode needed by the >>> board and then pass the correct one to the FSP. Now that we can access >>> the device tree that should be possible. There is logic in >>> ./arch/x86/cpu/ivybridge/microcode_intel.c to do this but it may need >>> a bit of refactorng. >>> >> >> The device tree still cannot be accessed in that early phase before >> CAR is initialized. The issue is that FSP is designed to have >> FspTempRamInit() to do both microcode loading and CAR initialization. >> > > Ah yes of course. Most unfortunate. > > Maybe we could have ifdtool or the Makefile put all the microcode in a > single big blob somewhere else in u-boot.rom so that the whole thing > can be passed to the FSP. In that case I think the FSP will select the > correct one Maybe an easy way to handle this is to create a special microcode section in u-boot.lds and just include all these header files in a single array in the section. This does not need ifdtool or device tree support. > I also wonder if it is possible to load the microcode in our own code > and pass nothing to the FSP? I think I tried that though and it hung. Yes, according to FSP external architecture spec, the microcode is a must. The reason is that on some processors without microcode update the CAR initialization will fail. > The FSP design leaves much to b desired. > >>> The existing microcode approach (with ifdtool adding a pointer to the >>> microcode) is a work-around for the FSP problem. Now that Bin has >>> solved this I'd be keen to remove it an just use device tree. >>> Regards, Bin
Hi Bin, On 5 June 2015 at 19:30, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Sat, Jun 6, 2015 at 12:17 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi Bin, >> >> On 4 June 2015 at 20:03, Bin Meng <bmeng.cn@gmail.com> wrote: >>> Hi Simon, >>> >>> On Fri, Jun 5, 2015 at 2:31 AM, Simon Glass <sjg@chromium.org> wrote: >>>> Hi, >>>> >>>> On 4 June 2015 at 10:27, Andrew Bradford <andrew@bradfordembedded.com> wrote: >>>>> >>>>> Hi Bin, >>>>> >>>>> On 06/04 22:21, Bin Meng wrote: >>>>> > Hi Simon, >>>>> > >>>>> > On Thu, Jun 4, 2015 at 8:12 PM, Bin Meng <bmeng.cn@gmail.com> wrote: >>>>> > > This is a temparory hacking for testing U-Boot on a newer version >>>>> > > MinnowMax board. >>>>> > > >>>>> > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >>>>> > > --- >>>>> > > >>>>> > > arch/x86/dts/minnowmax.dts | 2 +- >>>>> > > 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> > > >>>>> > > diff --git a/arch/x86/dts/minnowmax.dts b/arch/x86/dts/minnowmax.dts >>>>> > > index 7103bc5..9e1fcfc 100644 >>>>> > > --- a/arch/x86/dts/minnowmax.dts >>>>> > > +++ b/arch/x86/dts/minnowmax.dts >>>>> > > @@ -101,7 +101,7 @@ >>>>> > > >>>>> > > microcode { >>>>> > > update@0 { >>>>> > > -#include "microcode/m0130673322.dtsi" >>>>> > > +#include "microcode/m0130679901.dtsi" >>>>> > > }; >>>>> > > }; >>>>> > > >>>>> > > -- >>>>> > >>>>> > Saket confirmed these two patches resolved the boot problem he saw. So >>>>> > we will need think about how to better support such scenario that >>>>> > different revision board with different stepping CPUs. Could be >>>>> > multiple microcodes in one U-Boot image, or still one microcode with >>>>> > some #ifdef #endif. Note FSP has the capability to accept multiple >>>>> > microcodes as parameters in the FspTempRamInit(), but right now U-Boot >>>>> > only specifies one. How do you think? >>>>> >>>>> Why not just have a minnowmax common dtsi and then top level dts files >>>>> for each revision of the board containing the ways they are different >>>>> (such as microcode)? >>>> >>>> My preference would be to include all the microcode needed by the >>>> board and then pass the correct one to the FSP. Now that we can access >>>> the device tree that should be possible. There is logic in >>>> ./arch/x86/cpu/ivybridge/microcode_intel.c to do this but it may need >>>> a bit of refactorng. >>>> >>> >>> The device tree still cannot be accessed in that early phase before >>> CAR is initialized. The issue is that FSP is designed to have >>> FspTempRamInit() to do both microcode loading and CAR initialization. >>> >> >> Ah yes of course. Most unfortunate. >> >> Maybe we could have ifdtool or the Makefile put all the microcode in a >> single big blob somewhere else in u-boot.rom so that the whole thing >> can be passed to the FSP. In that case I think the FSP will select the >> correct one > > Maybe an easy way to handle this is to create a special microcode > section in u-boot.lds and just include all these header files in a > single array in the section. This does not need ifdtool or device tree > support. I'd prefer to keep it in the device tree if we can. It is easier to read. For non-FSP targets this works fine and I live in hope that Intel might improve the FSP interface such that the single microcode blob is not necessary. > >> I also wonder if it is possible to load the microcode in our own code >> and pass nothing to the FSP? I think I tried that though and it hung. > > Yes, according to FSP external architecture spec, the microcode is a > must. The reason is that on some processors without microcode update > the CAR initialization will fail. That's really terrible - we can barely even start the CPU without a microcode update. > >> The FSP design leaves much to b desired. >> >>>> The existing microcode approach (with ifdtool adding a pointer to the >>>> microcode) is a work-around for the FSP problem. Now that Bin has >>>> solved this I'd be keen to remove it an just use device tree. >>>> Regards, Simon
Hi Simon, On 06/07 18:58, Simon Glass wrote: > Hi Bin, > > On 5 June 2015 at 19:30, Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Simon, > > > > On Sat, Jun 6, 2015 at 12:17 AM, Simon Glass <sjg@chromium.org> wrote: > >> Hi Bin, > >> > >> On 4 June 2015 at 20:03, Bin Meng <bmeng.cn@gmail.com> wrote: > >>> Hi Simon, > >>> > >>> On Fri, Jun 5, 2015 at 2:31 AM, Simon Glass <sjg@chromium.org> wrote: > >>>> Hi, > >>>> > >>>> On 4 June 2015 at 10:27, Andrew Bradford <andrew@bradfordembedded.com> wrote: > >>>>> > >>>>> Hi Bin, > >>>>> > >>>>> On 06/04 22:21, Bin Meng wrote: > >>>>> > Hi Simon, > >>>>> > > >>>>> > On Thu, Jun 4, 2015 at 8:12 PM, Bin Meng <bmeng.cn@gmail.com> wrote: > >>>>> > > This is a temparory hacking for testing U-Boot on a newer version > >>>>> > > MinnowMax board. > >>>>> > > > >>>>> > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > >>>>> > > --- > >>>>> > > > >>>>> > > arch/x86/dts/minnowmax.dts | 2 +- > >>>>> > > 1 file changed, 1 insertion(+), 1 deletion(-) > >>>>> > > > >>>>> > > diff --git a/arch/x86/dts/minnowmax.dts b/arch/x86/dts/minnowmax.dts > >>>>> > > index 7103bc5..9e1fcfc 100644 > >>>>> > > --- a/arch/x86/dts/minnowmax.dts > >>>>> > > +++ b/arch/x86/dts/minnowmax.dts > >>>>> > > @@ -101,7 +101,7 @@ > >>>>> > > > >>>>> > > microcode { > >>>>> > > update@0 { > >>>>> > > -#include "microcode/m0130673322.dtsi" > >>>>> > > +#include "microcode/m0130679901.dtsi" > >>>>> > > }; > >>>>> > > }; > >>>>> > > > >>>>> > > -- > >>>>> > > >>>>> > Saket confirmed these two patches resolved the boot problem he saw. So > >>>>> > we will need think about how to better support such scenario that > >>>>> > different revision board with different stepping CPUs. Could be > >>>>> > multiple microcodes in one U-Boot image, or still one microcode with > >>>>> > some #ifdef #endif. Note FSP has the capability to accept multiple > >>>>> > microcodes as parameters in the FspTempRamInit(), but right now U-Boot > >>>>> > only specifies one. How do you think? > >>>>> > >>>>> Why not just have a minnowmax common dtsi and then top level dts files > >>>>> for each revision of the board containing the ways they are different > >>>>> (such as microcode)? > >>>> > >>>> My preference would be to include all the microcode needed by the > >>>> board and then pass the correct one to the FSP. Now that we can access > >>>> the device tree that should be possible. There is logic in > >>>> ./arch/x86/cpu/ivybridge/microcode_intel.c to do this but it may need > >>>> a bit of refactorng. > >>>> > >>> > >>> The device tree still cannot be accessed in that early phase before > >>> CAR is initialized. The issue is that FSP is designed to have > >>> FspTempRamInit() to do both microcode loading and CAR initialization. > >>> > >> > >> Ah yes of course. Most unfortunate. > >> > >> Maybe we could have ifdtool or the Makefile put all the microcode in a > >> single big blob somewhere else in u-boot.rom so that the whole thing > >> can be passed to the FSP. In that case I think the FSP will select the > >> correct one > > > > Maybe an easy way to handle this is to create a special microcode > > section in u-boot.lds and just include all these header files in a > > single array in the section. This does not need ifdtool or device tree > > support. > > I'd prefer to keep it in the device tree if we can. It is easier to > read. For non-FSP targets this works fine and I live in hope that > Intel might improve the FSP interface such that the single microcode > blob is not necessary. Are you saying that then for a board that could have more than one microcode used that the device tree for said board would actually list two different dtsi microcode files and then have a handler that is able to pass both to the FSP? I don't get the impression that Intel is going to improve the FSP for Baytrail at this point. I'd like to get Bin's microcode patch included into u-boot so it can be used with newer stepping E3800 parts, what's the best way I can help? Thanks! -Andrew
Hi Andrew, On 29 June 2015 at 08:36, Andrew Bradford <andrew@bradfordembedded.com> wrote: > Hi Simon, > > On 06/07 18:58, Simon Glass wrote: >> Hi Bin, >> >> On 5 June 2015 at 19:30, Bin Meng <bmeng.cn@gmail.com> wrote: >> > Hi Simon, >> > >> > On Sat, Jun 6, 2015 at 12:17 AM, Simon Glass <sjg@chromium.org> wrote: >> >> Hi Bin, >> >> >> >> On 4 June 2015 at 20:03, Bin Meng <bmeng.cn@gmail.com> wrote: >> >>> Hi Simon, >> >>> >> >>> On Fri, Jun 5, 2015 at 2:31 AM, Simon Glass <sjg@chromium.org> wrote: >> >>>> Hi, >> >>>> >> >>>> On 4 June 2015 at 10:27, Andrew Bradford <andrew@bradfordembedded.com> wrote: >> >>>>> >> >>>>> Hi Bin, >> >>>>> >> >>>>> On 06/04 22:21, Bin Meng wrote: >> >>>>> > Hi Simon, >> >>>>> > >> >>>>> > On Thu, Jun 4, 2015 at 8:12 PM, Bin Meng <bmeng.cn@gmail.com> wrote: >> >>>>> > > This is a temparory hacking for testing U-Boot on a newer version >> >>>>> > > MinnowMax board. >> >>>>> > > >> >>>>> > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >> >>>>> > > --- >> >>>>> > > >> >>>>> > > arch/x86/dts/minnowmax.dts | 2 +- >> >>>>> > > 1 file changed, 1 insertion(+), 1 deletion(-) >> >>>>> > > >> >>>>> > > diff --git a/arch/x86/dts/minnowmax.dts b/arch/x86/dts/minnowmax.dts >> >>>>> > > index 7103bc5..9e1fcfc 100644 >> >>>>> > > --- a/arch/x86/dts/minnowmax.dts >> >>>>> > > +++ b/arch/x86/dts/minnowmax.dts >> >>>>> > > @@ -101,7 +101,7 @@ >> >>>>> > > >> >>>>> > > microcode { >> >>>>> > > update@0 { >> >>>>> > > -#include "microcode/m0130673322.dtsi" >> >>>>> > > +#include "microcode/m0130679901.dtsi" >> >>>>> > > }; >> >>>>> > > }; >> >>>>> > > >> >>>>> > > -- >> >>>>> > >> >>>>> > Saket confirmed these two patches resolved the boot problem he saw. So >> >>>>> > we will need think about how to better support such scenario that >> >>>>> > different revision board with different stepping CPUs. Could be >> >>>>> > multiple microcodes in one U-Boot image, or still one microcode with >> >>>>> > some #ifdef #endif. Note FSP has the capability to accept multiple >> >>>>> > microcodes as parameters in the FspTempRamInit(), but right now U-Boot >> >>>>> > only specifies one. How do you think? >> >>>>> >> >>>>> Why not just have a minnowmax common dtsi and then top level dts files >> >>>>> for each revision of the board containing the ways they are different >> >>>>> (such as microcode)? >> >>>> >> >>>> My preference would be to include all the microcode needed by the >> >>>> board and then pass the correct one to the FSP. Now that we can access >> >>>> the device tree that should be possible. There is logic in >> >>>> ./arch/x86/cpu/ivybridge/microcode_intel.c to do this but it may need >> >>>> a bit of refactorng. >> >>>> >> >>> >> >>> The device tree still cannot be accessed in that early phase before >> >>> CAR is initialized. The issue is that FSP is designed to have >> >>> FspTempRamInit() to do both microcode loading and CAR initialization. >> >>> >> >> >> >> Ah yes of course. Most unfortunate. >> >> >> >> Maybe we could have ifdtool or the Makefile put all the microcode in a >> >> single big blob somewhere else in u-boot.rom so that the whole thing >> >> can be passed to the FSP. In that case I think the FSP will select the >> >> correct one >> > >> > Maybe an easy way to handle this is to create a special microcode >> > section in u-boot.lds and just include all these header files in a >> > single array in the section. This does not need ifdtool or device tree >> > support. >> >> I'd prefer to keep it in the device tree if we can. It is easier to >> read. For non-FSP targets this works fine and I live in hope that >> Intel might improve the FSP interface such that the single microcode >> blob is not necessary. > > Are you saying that then for a board that could have more than one > microcode used that the device tree for said board would actually list > two different dtsi microcode files and then have a handler that is able > to pass both to the FSP? > > I don't get the impression that Intel is going to improve the FSP for > Baytrail at this point. > > I'd like to get Bin's microcode patch included into u-boot so it can be > used with newer stepping E3800 parts, what's the best way I can help? > My understanding is that you can pass several microcode blobs to the FSP and it will load the correct one (as Bin says above). If that is untrue then this will not work. My preference is that we modify ifdtool to support collecting the microcode blobs together and put them into a single place in the ROM. This could be run from the Makefile as another ifdtool step. It would find the various microcode nodes, extract each blob of binary data, pack them together and put them in a specified place in ROM. Regards, Simon
Hi, On 29 June 2015 at 11:57, Simon Glass <sjg@chromium.org> wrote: > Hi Andrew, > > On 29 June 2015 at 08:36, Andrew Bradford <andrew@bradfordembedded.com> wrote: >> Hi Simon, >> >> On 06/07 18:58, Simon Glass wrote: >>> Hi Bin, >>> >>> On 5 June 2015 at 19:30, Bin Meng <bmeng.cn@gmail.com> wrote: >>> > Hi Simon, >>> > >>> > On Sat, Jun 6, 2015 at 12:17 AM, Simon Glass <sjg@chromium.org> wrote: >>> >> Hi Bin, >>> >> >>> >> On 4 June 2015 at 20:03, Bin Meng <bmeng.cn@gmail.com> wrote: >>> >>> Hi Simon, >>> >>> >>> >>> On Fri, Jun 5, 2015 at 2:31 AM, Simon Glass <sjg@chromium.org> wrote: >>> >>>> Hi, >>> >>>> >>> >>>> On 4 June 2015 at 10:27, Andrew Bradford <andrew@bradfordembedded.com> wrote: >>> >>>>> >>> >>>>> Hi Bin, >>> >>>>> >>> >>>>> On 06/04 22:21, Bin Meng wrote: >>> >>>>> > Hi Simon, >>> >>>>> > >>> >>>>> > On Thu, Jun 4, 2015 at 8:12 PM, Bin Meng <bmeng.cn@gmail.com> wrote: >>> >>>>> > > This is a temparory hacking for testing U-Boot on a newer version >>> >>>>> > > MinnowMax board. >>> >>>>> > > >>> >>>>> > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >>> >>>>> > > --- >>> >>>>> > > >>> >>>>> > > arch/x86/dts/minnowmax.dts | 2 +- >>> >>>>> > > 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>>>> > > >>> >>>>> > > diff --git a/arch/x86/dts/minnowmax.dts b/arch/x86/dts/minnowmax.dts >>> >>>>> > > index 7103bc5..9e1fcfc 100644 >>> >>>>> > > --- a/arch/x86/dts/minnowmax.dts >>> >>>>> > > +++ b/arch/x86/dts/minnowmax.dts >>> >>>>> > > @@ -101,7 +101,7 @@ >>> >>>>> > > >>> >>>>> > > microcode { >>> >>>>> > > update@0 { >>> >>>>> > > -#include "microcode/m0130673322.dtsi" >>> >>>>> > > +#include "microcode/m0130679901.dtsi" >>> >>>>> > > }; >>> >>>>> > > }; >>> >>>>> > > >>> >>>>> > > -- >>> >>>>> > >>> >>>>> > Saket confirmed these two patches resolved the boot problem he saw. So >>> >>>>> > we will need think about how to better support such scenario that >>> >>>>> > different revision board with different stepping CPUs. Could be >>> >>>>> > multiple microcodes in one U-Boot image, or still one microcode with >>> >>>>> > some #ifdef #endif. Note FSP has the capability to accept multiple >>> >>>>> > microcodes as parameters in the FspTempRamInit(), but right now U-Boot >>> >>>>> > only specifies one. How do you think? >>> >>>>> >>> >>>>> Why not just have a minnowmax common dtsi and then top level dts files >>> >>>>> for each revision of the board containing the ways they are different >>> >>>>> (such as microcode)? >>> >>>> >>> >>>> My preference would be to include all the microcode needed by the >>> >>>> board and then pass the correct one to the FSP. Now that we can access >>> >>>> the device tree that should be possible. There is logic in >>> >>>> ./arch/x86/cpu/ivybridge/microcode_intel.c to do this but it may need >>> >>>> a bit of refactorng. >>> >>>> >>> >>> >>> >>> The device tree still cannot be accessed in that early phase before >>> >>> CAR is initialized. The issue is that FSP is designed to have >>> >>> FspTempRamInit() to do both microcode loading and CAR initialization. >>> >>> >>> >> >>> >> Ah yes of course. Most unfortunate. >>> >> >>> >> Maybe we could have ifdtool or the Makefile put all the microcode in a >>> >> single big blob somewhere else in u-boot.rom so that the whole thing >>> >> can be passed to the FSP. In that case I think the FSP will select the >>> >> correct one >>> > >>> > Maybe an easy way to handle this is to create a special microcode >>> > section in u-boot.lds and just include all these header files in a >>> > single array in the section. This does not need ifdtool or device tree >>> > support. >>> >>> I'd prefer to keep it in the device tree if we can. It is easier to >>> read. For non-FSP targets this works fine and I live in hope that >>> Intel might improve the FSP interface such that the single microcode >>> blob is not necessary. >> >> Are you saying that then for a board that could have more than one >> microcode used that the device tree for said board would actually list >> two different dtsi microcode files and then have a handler that is able >> to pass both to the FSP? >> >> I don't get the impression that Intel is going to improve the FSP for >> Baytrail at this point. >> >> I'd like to get Bin's microcode patch included into u-boot so it can be >> used with newer stepping E3800 parts, what's the best way I can help? >> > > My understanding is that you can pass several microcode blobs to the > FSP and it will load the correct one (as Bin says above). If that is > untrue then this will not work. > > My preference is that we modify ifdtool to support collecting the > microcode blobs together and put them into a single place in the ROM. > This could be run from the Makefile as another ifdtool step. It would > find the various microcode nodes, extract each blob of binary data, > pack them together and put them in a specified place in ROM. Any thoughts on this? I don't actually have a newer board to test with. But I suspect that no one has both an old and a new board. Regards, Simon
Hi Simon, On Thu, Aug 6, 2015 at 10:02 PM, Simon Glass <sjg@chromium.org> wrote: > Hi, > > On 29 June 2015 at 11:57, Simon Glass <sjg@chromium.org> wrote: >> Hi Andrew, >> >> On 29 June 2015 at 08:36, Andrew Bradford <andrew@bradfordembedded.com> wrote: >>> Hi Simon, >>> >>> On 06/07 18:58, Simon Glass wrote: >>>> Hi Bin, >>>> >>>> On 5 June 2015 at 19:30, Bin Meng <bmeng.cn@gmail.com> wrote: >>>> > Hi Simon, >>>> > >>>> > On Sat, Jun 6, 2015 at 12:17 AM, Simon Glass <sjg@chromium.org> wrote: >>>> >> Hi Bin, >>>> >> >>>> >> On 4 June 2015 at 20:03, Bin Meng <bmeng.cn@gmail.com> wrote: >>>> >>> Hi Simon, >>>> >>> >>>> >>> On Fri, Jun 5, 2015 at 2:31 AM, Simon Glass <sjg@chromium.org> wrote: >>>> >>>> Hi, >>>> >>>> >>>> >>>> On 4 June 2015 at 10:27, Andrew Bradford <andrew@bradfordembedded.com> wrote: >>>> >>>>> >>>> >>>>> Hi Bin, >>>> >>>>> >>>> >>>>> On 06/04 22:21, Bin Meng wrote: >>>> >>>>> > Hi Simon, >>>> >>>>> > >>>> >>>>> > On Thu, Jun 4, 2015 at 8:12 PM, Bin Meng <bmeng.cn@gmail.com> wrote: >>>> >>>>> > > This is a temparory hacking for testing U-Boot on a newer version >>>> >>>>> > > MinnowMax board. >>>> >>>>> > > >>>> >>>>> > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >>>> >>>>> > > --- >>>> >>>>> > > >>>> >>>>> > > arch/x86/dts/minnowmax.dts | 2 +- >>>> >>>>> > > 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>>> > > >>>> >>>>> > > diff --git a/arch/x86/dts/minnowmax.dts b/arch/x86/dts/minnowmax.dts >>>> >>>>> > > index 7103bc5..9e1fcfc 100644 >>>> >>>>> > > --- a/arch/x86/dts/minnowmax.dts >>>> >>>>> > > +++ b/arch/x86/dts/minnowmax.dts >>>> >>>>> > > @@ -101,7 +101,7 @@ >>>> >>>>> > > >>>> >>>>> > > microcode { >>>> >>>>> > > update@0 { >>>> >>>>> > > -#include "microcode/m0130673322.dtsi" >>>> >>>>> > > +#include "microcode/m0130679901.dtsi" >>>> >>>>> > > }; >>>> >>>>> > > }; >>>> >>>>> > > >>>> >>>>> > > -- >>>> >>>>> > >>>> >>>>> > Saket confirmed these two patches resolved the boot problem he saw. So >>>> >>>>> > we will need think about how to better support such scenario that >>>> >>>>> > different revision board with different stepping CPUs. Could be >>>> >>>>> > multiple microcodes in one U-Boot image, or still one microcode with >>>> >>>>> > some #ifdef #endif. Note FSP has the capability to accept multiple >>>> >>>>> > microcodes as parameters in the FspTempRamInit(), but right now U-Boot >>>> >>>>> > only specifies one. How do you think? >>>> >>>>> >>>> >>>>> Why not just have a minnowmax common dtsi and then top level dts files >>>> >>>>> for each revision of the board containing the ways they are different >>>> >>>>> (such as microcode)? >>>> >>>> >>>> >>>> My preference would be to include all the microcode needed by the >>>> >>>> board and then pass the correct one to the FSP. Now that we can access >>>> >>>> the device tree that should be possible. There is logic in >>>> >>>> ./arch/x86/cpu/ivybridge/microcode_intel.c to do this but it may need >>>> >>>> a bit of refactorng. >>>> >>>> >>>> >>> >>>> >>> The device tree still cannot be accessed in that early phase before >>>> >>> CAR is initialized. The issue is that FSP is designed to have >>>> >>> FspTempRamInit() to do both microcode loading and CAR initialization. >>>> >>> >>>> >> >>>> >> Ah yes of course. Most unfortunate. >>>> >> >>>> >> Maybe we could have ifdtool or the Makefile put all the microcode in a >>>> >> single big blob somewhere else in u-boot.rom so that the whole thing >>>> >> can be passed to the FSP. In that case I think the FSP will select the >>>> >> correct one >>>> > >>>> > Maybe an easy way to handle this is to create a special microcode >>>> > section in u-boot.lds and just include all these header files in a >>>> > single array in the section. This does not need ifdtool or device tree >>>> > support. >>>> >>>> I'd prefer to keep it in the device tree if we can. It is easier to >>>> read. For non-FSP targets this works fine and I live in hope that >>>> Intel might improve the FSP interface such that the single microcode >>>> blob is not necessary. >>> >>> Are you saying that then for a board that could have more than one >>> microcode used that the device tree for said board would actually list >>> two different dtsi microcode files and then have a handler that is able >>> to pass both to the FSP? >>> >>> I don't get the impression that Intel is going to improve the FSP for >>> Baytrail at this point. >>> >>> I'd like to get Bin's microcode patch included into u-boot so it can be >>> used with newer stepping E3800 parts, what's the best way I can help? >>> >> >> My understanding is that you can pass several microcode blobs to the >> FSP and it will load the correct one (as Bin says above). If that is >> untrue then this will not work. >> >> My preference is that we modify ifdtool to support collecting the >> microcode blobs together and put them into a single place in the ROM. >> This could be run from the Makefile as another ifdtool step. It would >> find the various microcode nodes, extract each blob of binary data, >> pack them together and put them in a specified place in ROM. > > Any thoughts on this? I don't actually have a newer board to test > with. But I suspect that no one has both an old and a new board. > Actually I was working on a patch series for this but suspended. I will rebase and send out the series tomorrow. Regards, Bin
Hi Simon, On 08/06 08:02, Simon Glass wrote: > Hi, > > On 29 June 2015 at 11:57, Simon Glass <sjg@chromium.org> wrote: > > Hi Andrew, > > > > On 29 June 2015 at 08:36, Andrew Bradford <andrew@bradfordembedded.com> wrote: > >> Hi Simon, > >> > >> On 06/07 18:58, Simon Glass wrote: > >>> Hi Bin, > >>> > >>> On 5 June 2015 at 19:30, Bin Meng <bmeng.cn@gmail.com> wrote: > >>> > Hi Simon, > >>> > > >>> > On Sat, Jun 6, 2015 at 12:17 AM, Simon Glass <sjg@chromium.org> wrote: > >>> >> Hi Bin, > >>> >> > >>> >> On 4 June 2015 at 20:03, Bin Meng <bmeng.cn@gmail.com> wrote: > >>> >>> Hi Simon, > >>> >>> > >>> >>> On Fri, Jun 5, 2015 at 2:31 AM, Simon Glass <sjg@chromium.org> wrote: > >>> >>>> Hi, > >>> >>>> > >>> >>>> On 4 June 2015 at 10:27, Andrew Bradford <andrew@bradfordembedded.com> wrote: > >>> >>>>> > >>> >>>>> Hi Bin, > >>> >>>>> > >>> >>>>> On 06/04 22:21, Bin Meng wrote: > >>> >>>>> > Hi Simon, > >>> >>>>> > > >>> >>>>> > On Thu, Jun 4, 2015 at 8:12 PM, Bin Meng <bmeng.cn@gmail.com> wrote: > >>> >>>>> > > This is a temparory hacking for testing U-Boot on a newer version > >>> >>>>> > > MinnowMax board. > >>> >>>>> > > > >>> >>>>> > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > >>> >>>>> > > --- > >>> >>>>> > > > >>> >>>>> > > arch/x86/dts/minnowmax.dts | 2 +- > >>> >>>>> > > 1 file changed, 1 insertion(+), 1 deletion(-) > >>> >>>>> > > > >>> >>>>> > > diff --git a/arch/x86/dts/minnowmax.dts b/arch/x86/dts/minnowmax.dts > >>> >>>>> > > index 7103bc5..9e1fcfc 100644 > >>> >>>>> > > --- a/arch/x86/dts/minnowmax.dts > >>> >>>>> > > +++ b/arch/x86/dts/minnowmax.dts > >>> >>>>> > > @@ -101,7 +101,7 @@ > >>> >>>>> > > > >>> >>>>> > > microcode { > >>> >>>>> > > update@0 { > >>> >>>>> > > -#include "microcode/m0130673322.dtsi" > >>> >>>>> > > +#include "microcode/m0130679901.dtsi" > >>> >>>>> > > }; > >>> >>>>> > > }; > >>> >>>>> > > > >>> >>>>> > > -- > >>> >>>>> > > >>> >>>>> > Saket confirmed these two patches resolved the boot problem he saw. So > >>> >>>>> > we will need think about how to better support such scenario that > >>> >>>>> > different revision board with different stepping CPUs. Could be > >>> >>>>> > multiple microcodes in one U-Boot image, or still one microcode with > >>> >>>>> > some #ifdef #endif. Note FSP has the capability to accept multiple > >>> >>>>> > microcodes as parameters in the FspTempRamInit(), but right now U-Boot > >>> >>>>> > only specifies one. How do you think? > >>> >>>>> > >>> >>>>> Why not just have a minnowmax common dtsi and then top level dts files > >>> >>>>> for each revision of the board containing the ways they are different > >>> >>>>> (such as microcode)? > >>> >>>> > >>> >>>> My preference would be to include all the microcode needed by the > >>> >>>> board and then pass the correct one to the FSP. Now that we can access > >>> >>>> the device tree that should be possible. There is logic in > >>> >>>> ./arch/x86/cpu/ivybridge/microcode_intel.c to do this but it may need > >>> >>>> a bit of refactorng. > >>> >>>> > >>> >>> > >>> >>> The device tree still cannot be accessed in that early phase before > >>> >>> CAR is initialized. The issue is that FSP is designed to have > >>> >>> FspTempRamInit() to do both microcode loading and CAR initialization. > >>> >>> > >>> >> > >>> >> Ah yes of course. Most unfortunate. > >>> >> > >>> >> Maybe we could have ifdtool or the Makefile put all the microcode in a > >>> >> single big blob somewhere else in u-boot.rom so that the whole thing > >>> >> can be passed to the FSP. In that case I think the FSP will select the > >>> >> correct one > >>> > > >>> > Maybe an easy way to handle this is to create a special microcode > >>> > section in u-boot.lds and just include all these header files in a > >>> > single array in the section. This does not need ifdtool or device tree > >>> > support. > >>> > >>> I'd prefer to keep it in the device tree if we can. It is easier to > >>> read. For non-FSP targets this works fine and I live in hope that > >>> Intel might improve the FSP interface such that the single microcode > >>> blob is not necessary. > >> > >> Are you saying that then for a board that could have more than one > >> microcode used that the device tree for said board would actually list > >> two different dtsi microcode files and then have a handler that is able > >> to pass both to the FSP? > >> > >> I don't get the impression that Intel is going to improve the FSP for > >> Baytrail at this point. > >> > >> I'd like to get Bin's microcode patch included into u-boot so it can be > >> used with newer stepping E3800 parts, what's the best way I can help? > >> > > > > My understanding is that you can pass several microcode blobs to the > > FSP and it will load the correct one (as Bin says above). If that is > > untrue then this will not work. > > > > My preference is that we modify ifdtool to support collecting the > > microcode blobs together and put them into a single place in the ROM. > > This could be run from the Makefile as another ifdtool step. It would > > find the various microcode nodes, extract each blob of binary data, > > pack them together and put them in a specified place in ROM. > > Any thoughts on this? I don't actually have a newer board to test > with. But I suspect that no one has both an old and a new board. I think it's a good idea, I just haven't had any time to look at it, sorry. My custom E3800 based boards are all D0 stepping, I believe, so I've just been using Bin's D0 stepping patch for the work I've been doing with them. Thanks, Andrew
diff --git a/arch/x86/dts/minnowmax.dts b/arch/x86/dts/minnowmax.dts index 7103bc5..9e1fcfc 100644 --- a/arch/x86/dts/minnowmax.dts +++ b/arch/x86/dts/minnowmax.dts @@ -101,7 +101,7 @@ microcode { update@0 { -#include "microcode/m0130673322.dtsi" +#include "microcode/m0130679901.dtsi" }; };
This is a temparory hacking for testing U-Boot on a newer version MinnowMax board. Signed-off-by: Bin Meng <bmeng.cn@gmail.com> --- arch/x86/dts/minnowmax.dts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)