| Message ID | BLU437-SMTP35F2A21CF948E46F4979B9BFCD0@phx.gbl |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Bin, On 25 May 2015 at 08:36, Bin Meng <bmeng.cn@gmail.com> wrote: > Writing 0xcb to I/O port 0xb2 (Advanced Power Management Control) causes > U-Boot to hang on QEMU q35 target. Disable the writing for QEMU targets. > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > --- > > arch/x86/cpu/coreboot/coreboot.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/arch/x86/cpu/coreboot/coreboot.c b/arch/x86/cpu/coreboot/coreboot.c > index 4cdd0d4..2234cf0 100644 > --- a/arch/x86/cpu/coreboot/coreboot.c > +++ b/arch/x86/cpu/coreboot/coreboot.c > @@ -85,10 +85,6 @@ void board_final_cleanup(void) > wrmsrl(MTRR_PHYS_MASK_MSR(top_mtrr), 0); > mtrr_close(&state); > } > - > - /* Issue SMI to Coreboot to lock down ME and registers */ > - printf("Finalizing Coreboot\n"); > - outb(0xcb, 0xb2); > } > > void panic_puts(const char *str) > -- > 1.8.2.1 > But how does this run with coreboot on platforms that need it? Should this be controlled by a device tree settings, perhaps? Regards, Simon
Hi Simon, On Wed, May 27, 2015 at 11:13 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Bin, > > On 25 May 2015 at 08:36, Bin Meng <bmeng.cn@gmail.com> wrote: >> Writing 0xcb to I/O port 0xb2 (Advanced Power Management Control) causes >> U-Boot to hang on QEMU q35 target. Disable the writing for QEMU targets. >> >> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >> --- >> >> arch/x86/cpu/coreboot/coreboot.c | 4 ---- >> 1 file changed, 4 deletions(-) >> >> diff --git a/arch/x86/cpu/coreboot/coreboot.c b/arch/x86/cpu/coreboot/coreboot.c >> index 4cdd0d4..2234cf0 100644 >> --- a/arch/x86/cpu/coreboot/coreboot.c >> +++ b/arch/x86/cpu/coreboot/coreboot.c >> @@ -85,10 +85,6 @@ void board_final_cleanup(void) >> wrmsrl(MTRR_PHYS_MASK_MSR(top_mtrr), 0); >> mtrr_close(&state); >> } >> - >> - /* Issue SMI to Coreboot to lock down ME and registers */ >> - printf("Finalizing Coreboot\n"); >> - outb(0xcb, 0xb2); >> } >> >> void panic_puts(const char *str) >> -- >> 1.8.2.1 >> > > But how does this run with coreboot on platforms that need it? Should > this be controlled by a device tree settings, perhaps? > I am not sure if the lock down should be done by the coreboot in the first place. IMHO U-Boot as the coreboot payload should not touch such low-level stuff. Regards, Bin
Hi Bin, On 26 May 2015 at 21:50, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Wed, May 27, 2015 at 11:13 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi Bin, >> >> On 25 May 2015 at 08:36, Bin Meng <bmeng.cn@gmail.com> wrote: >>> Writing 0xcb to I/O port 0xb2 (Advanced Power Management Control) causes >>> U-Boot to hang on QEMU q35 target. Disable the writing for QEMU targets. >>> >>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >>> --- >>> >>> arch/x86/cpu/coreboot/coreboot.c | 4 ---- >>> 1 file changed, 4 deletions(-) >>> >>> diff --git a/arch/x86/cpu/coreboot/coreboot.c b/arch/x86/cpu/coreboot/coreboot.c >>> index 4cdd0d4..2234cf0 100644 >>> --- a/arch/x86/cpu/coreboot/coreboot.c >>> +++ b/arch/x86/cpu/coreboot/coreboot.c >>> @@ -85,10 +85,6 @@ void board_final_cleanup(void) >>> wrmsrl(MTRR_PHYS_MASK_MSR(top_mtrr), 0); >>> mtrr_close(&state); >>> } >>> - >>> - /* Issue SMI to Coreboot to lock down ME and registers */ >>> - printf("Finalizing Coreboot\n"); >>> - outb(0xcb, 0xb2); >>> } >>> >>> void panic_puts(const char *str) >>> -- >>> 1.8.2.1 >>> >> >> But how does this run with coreboot on platforms that need it? Should >> this be controlled by a device tree settings, perhaps? >> > > I am not sure if the lock down should be done by the coreboot in the > first place. IMHO U-Boot as the coreboot payload should not touch such > low-level stuff. Well it is telling coreboot that it is about to boot the kernel. I think this is an x86 thing and we should support it. Regards, Simon
Hi Simon, On Wed, May 27, 2015 at 11:37 PM, Simon Glass <sjg@chromium.org> wrote: > Hi Bin, > > On 26 May 2015 at 21:50, Bin Meng <bmeng.cn@gmail.com> wrote: >> Hi Simon, >> >> On Wed, May 27, 2015 at 11:13 AM, Simon Glass <sjg@chromium.org> wrote: >>> Hi Bin, >>> >>> On 25 May 2015 at 08:36, Bin Meng <bmeng.cn@gmail.com> wrote: >>>> Writing 0xcb to I/O port 0xb2 (Advanced Power Management Control) causes >>>> U-Boot to hang on QEMU q35 target. Disable the writing for QEMU targets. >>>> >>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >>>> --- >>>> >>>> arch/x86/cpu/coreboot/coreboot.c | 4 ---- >>>> 1 file changed, 4 deletions(-) >>>> >>>> diff --git a/arch/x86/cpu/coreboot/coreboot.c b/arch/x86/cpu/coreboot/coreboot.c >>>> index 4cdd0d4..2234cf0 100644 >>>> --- a/arch/x86/cpu/coreboot/coreboot.c >>>> +++ b/arch/x86/cpu/coreboot/coreboot.c >>>> @@ -85,10 +85,6 @@ void board_final_cleanup(void) >>>> wrmsrl(MTRR_PHYS_MASK_MSR(top_mtrr), 0); >>>> mtrr_close(&state); >>>> } >>>> - >>>> - /* Issue SMI to Coreboot to lock down ME and registers */ >>>> - printf("Finalizing Coreboot\n"); >>>> - outb(0xcb, 0xb2); >>>> } >>>> >>>> void panic_puts(const char *str) >>>> -- >>>> 1.8.2.1 >>>> >>> >>> But how does this run with coreboot on platforms that need it? Should >>> this be controlled by a device tree settings, perhaps? >>> >> >> I am not sure if the lock down should be done by the coreboot in the >> first place. IMHO U-Boot as the coreboot payload should not touch such >> low-level stuff. > > Well it is telling coreboot that it is about to boot the kernel. I > think this is an x86 thing and we should support it. > What happens if coreboot directly boot into the kernel? Does coreboot do the same 'outb(0xcb, 0xb2)' thing before jumping to kernel? My understanding is that coreboot payload is to enhance coreboot's functionality as a system BIOS (either having legacy BIOS interface with the help of a SeaBIOS payload, or having UEFI interface with the help of a TinaoCore payload). coreboot itself does the mainboard initialization and payload does something that is not hardware dependent. Regards, Bin
Hi Bin, On 27 May 2015 at 10:17, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Wed, May 27, 2015 at 11:37 PM, Simon Glass <sjg@chromium.org> wrote: >> Hi Bin, >> >> On 26 May 2015 at 21:50, Bin Meng <bmeng.cn@gmail.com> wrote: >>> Hi Simon, >>> >>> On Wed, May 27, 2015 at 11:13 AM, Simon Glass <sjg@chromium.org> wrote: >>>> Hi Bin, >>>> >>>> On 25 May 2015 at 08:36, Bin Meng <bmeng.cn@gmail.com> wrote: >>>>> Writing 0xcb to I/O port 0xb2 (Advanced Power Management Control) causes >>>>> U-Boot to hang on QEMU q35 target. Disable the writing for QEMU targets. >>>>> >>>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >>>>> --- >>>>> >>>>> arch/x86/cpu/coreboot/coreboot.c | 4 ---- >>>>> 1 file changed, 4 deletions(-) >>>>> >>>>> diff --git a/arch/x86/cpu/coreboot/coreboot.c b/arch/x86/cpu/coreboot/coreboot.c >>>>> index 4cdd0d4..2234cf0 100644 >>>>> --- a/arch/x86/cpu/coreboot/coreboot.c >>>>> +++ b/arch/x86/cpu/coreboot/coreboot.c >>>>> @@ -85,10 +85,6 @@ void board_final_cleanup(void) >>>>> wrmsrl(MTRR_PHYS_MASK_MSR(top_mtrr), 0); >>>>> mtrr_close(&state); >>>>> } >>>>> - >>>>> - /* Issue SMI to Coreboot to lock down ME and registers */ >>>>> - printf("Finalizing Coreboot\n"); >>>>> - outb(0xcb, 0xb2); >>>>> } >>>>> >>>>> void panic_puts(const char *str) >>>>> -- >>>>> 1.8.2.1 >>>>> >>>> >>>> But how does this run with coreboot on platforms that need it? Should >>>> this be controlled by a device tree settings, perhaps? >>>> >>> >>> I am not sure if the lock down should be done by the coreboot in the >>> first place. IMHO U-Boot as the coreboot payload should not touch such >>> low-level stuff. >> >> Well it is telling coreboot that it is about to boot the kernel. I >> think this is an x86 thing and we should support it. >> > > What happens if coreboot directly boot into the kernel? Does coreboot > do the same 'outb(0xcb, 0xb2)' thing before jumping to kernel? My Kind of, although in that case it is just a function call within coreboot. > understanding is that coreboot payload is to enhance coreboot's > functionality as a system BIOS (either having legacy BIOS interface > with the help of a SeaBIOS payload, or having UEFI interface with the > help of a TinaoCore payload). coreboot itself does the mainboard > initialization and payload does something that is not hardware > dependent. Right, but we need to tell coreboot that we have finished with the ME / registers, etc. It can't lock them before calling U-Boot since we may adjust them. Regards, Simon
Hi Simon, On Thu, May 28, 2015 at 1:46 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Bin, > > On 27 May 2015 at 10:17, Bin Meng <bmeng.cn@gmail.com> wrote: >> Hi Simon, >> >> On Wed, May 27, 2015 at 11:37 PM, Simon Glass <sjg@chromium.org> wrote: >>> Hi Bin, >>> >>> On 26 May 2015 at 21:50, Bin Meng <bmeng.cn@gmail.com> wrote: >>>> Hi Simon, >>>> >>>> On Wed, May 27, 2015 at 11:13 AM, Simon Glass <sjg@chromium.org> wrote: >>>>> Hi Bin, >>>>> >>>>> On 25 May 2015 at 08:36, Bin Meng <bmeng.cn@gmail.com> wrote: >>>>>> Writing 0xcb to I/O port 0xb2 (Advanced Power Management Control) causes >>>>>> U-Boot to hang on QEMU q35 target. Disable the writing for QEMU targets. >>>>>> >>>>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >>>>>> --- >>>>>> >>>>>> arch/x86/cpu/coreboot/coreboot.c | 4 ---- >>>>>> 1 file changed, 4 deletions(-) >>>>>> >>>>>> diff --git a/arch/x86/cpu/coreboot/coreboot.c b/arch/x86/cpu/coreboot/coreboot.c >>>>>> index 4cdd0d4..2234cf0 100644 >>>>>> --- a/arch/x86/cpu/coreboot/coreboot.c >>>>>> +++ b/arch/x86/cpu/coreboot/coreboot.c >>>>>> @@ -85,10 +85,6 @@ void board_final_cleanup(void) >>>>>> wrmsrl(MTRR_PHYS_MASK_MSR(top_mtrr), 0); >>>>>> mtrr_close(&state); >>>>>> } >>>>>> - >>>>>> - /* Issue SMI to Coreboot to lock down ME and registers */ >>>>>> - printf("Finalizing Coreboot\n"); >>>>>> - outb(0xcb, 0xb2); >>>>>> } >>>>>> >>>>>> void panic_puts(const char *str) >>>>>> -- >>>>>> 1.8.2.1 >>>>>> >>>>> >>>>> But how does this run with coreboot on platforms that need it? Should >>>>> this be controlled by a device tree settings, perhaps? >>>>> >>>> >>>> I am not sure if the lock down should be done by the coreboot in the >>>> first place. IMHO U-Boot as the coreboot payload should not touch such >>>> low-level stuff. >>> >>> Well it is telling coreboot that it is about to boot the kernel. I >>> think this is an x86 thing and we should support it. >>> >> >> What happens if coreboot directly boot into the kernel? Does coreboot >> do the same 'outb(0xcb, 0xb2)' thing before jumping to kernel? My > > Kind of, although in that case it is just a function call within coreboot. > >> understanding is that coreboot payload is to enhance coreboot's >> functionality as a system BIOS (either having legacy BIOS interface >> with the help of a SeaBIOS payload, or having UEFI interface with the >> help of a TinaoCore payload). coreboot itself does the mainboard >> initialization and payload does something that is not hardware >> dependent. > > Right, but we need to tell coreboot that we have finished with the ME > / registers, etc. It can't lock them before calling U-Boot since we > may adjust them. > Will use a config option in the device tree in v2. Regards, Bin
Hi Bin, On 2 June 2015 at 08:15, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Thu, May 28, 2015 at 1:46 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi Bin, >> >> On 27 May 2015 at 10:17, Bin Meng <bmeng.cn@gmail.com> wrote: >>> Hi Simon, >>> >>> On Wed, May 27, 2015 at 11:37 PM, Simon Glass <sjg@chromium.org> wrote: >>>> Hi Bin, >>>> >>>> On 26 May 2015 at 21:50, Bin Meng <bmeng.cn@gmail.com> wrote: >>>>> Hi Simon, >>>>> >>>>> On Wed, May 27, 2015 at 11:13 AM, Simon Glass <sjg@chromium.org> wrote: >>>>>> Hi Bin, >>>>>> >>>>>> On 25 May 2015 at 08:36, Bin Meng <bmeng.cn@gmail.com> wrote: >>>>>>> Writing 0xcb to I/O port 0xb2 (Advanced Power Management Control) causes >>>>>>> U-Boot to hang on QEMU q35 target. Disable the writing for QEMU targets. >>>>>>> >>>>>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >>>>>>> --- >>>>>>> >>>>>>> arch/x86/cpu/coreboot/coreboot.c | 4 ---- >>>>>>> 1 file changed, 4 deletions(-) >>>>>>> >>>>>>> diff --git a/arch/x86/cpu/coreboot/coreboot.c b/arch/x86/cpu/coreboot/coreboot.c >>>>>>> index 4cdd0d4..2234cf0 100644 >>>>>>> --- a/arch/x86/cpu/coreboot/coreboot.c >>>>>>> +++ b/arch/x86/cpu/coreboot/coreboot.c >>>>>>> @@ -85,10 +85,6 @@ void board_final_cleanup(void) >>>>>>> wrmsrl(MTRR_PHYS_MASK_MSR(top_mtrr), 0); >>>>>>> mtrr_close(&state); >>>>>>> } >>>>>>> - >>>>>>> - /* Issue SMI to Coreboot to lock down ME and registers */ >>>>>>> - printf("Finalizing Coreboot\n"); >>>>>>> - outb(0xcb, 0xb2); >>>>>>> } >>>>>>> >>>>>>> void panic_puts(const char *str) >>>>>>> -- >>>>>>> 1.8.2.1 >>>>>>> >>>>>> >>>>>> But how does this run with coreboot on platforms that need it? Should >>>>>> this be controlled by a device tree settings, perhaps? >>>>>> >>>>> >>>>> I am not sure if the lock down should be done by the coreboot in the >>>>> first place. IMHO U-Boot as the coreboot payload should not touch such >>>>> low-level stuff. >>>> >>>> Well it is telling coreboot that it is about to boot the kernel. I >>>> think this is an x86 thing and we should support it. >>>> >>> >>> What happens if coreboot directly boot into the kernel? Does coreboot >>> do the same 'outb(0xcb, 0xb2)' thing before jumping to kernel? My >> >> Kind of, although in that case it is just a function call within coreboot. >> >>> understanding is that coreboot payload is to enhance coreboot's >>> functionality as a system BIOS (either having legacy BIOS interface >>> with the help of a SeaBIOS payload, or having UEFI interface with the >>> help of a TinaoCore payload). coreboot itself does the mainboard >>> initialization and payload does something that is not hardware >>> dependent. >> >> Right, but we need to tell coreboot that we have finished with the ME >> / registers, etc. It can't lock them before calling U-Boot since we >> may adjust them. >> > > Will use a config option in the device tree in v2. OK thanks. When you resend can you please rebase on x86/master? Regards, Simon
Hi Simon, On Tue, Jun 2, 2015 at 10:20 PM, Simon Glass <sjg@chromium.org> wrote: > Hi Bin, > > On 2 June 2015 at 08:15, Bin Meng <bmeng.cn@gmail.com> wrote: >> Hi Simon, >> >> On Thu, May 28, 2015 at 1:46 AM, Simon Glass <sjg@chromium.org> wrote: >>> Hi Bin, >>> >>> On 27 May 2015 at 10:17, Bin Meng <bmeng.cn@gmail.com> wrote: >>>> Hi Simon, >>>> >>>> On Wed, May 27, 2015 at 11:37 PM, Simon Glass <sjg@chromium.org> wrote: >>>>> Hi Bin, >>>>> >>>>> On 26 May 2015 at 21:50, Bin Meng <bmeng.cn@gmail.com> wrote: >>>>>> Hi Simon, >>>>>> >>>>>> On Wed, May 27, 2015 at 11:13 AM, Simon Glass <sjg@chromium.org> wrote: >>>>>>> Hi Bin, >>>>>>> >>>>>>> On 25 May 2015 at 08:36, Bin Meng <bmeng.cn@gmail.com> wrote: >>>>>>>> Writing 0xcb to I/O port 0xb2 (Advanced Power Management Control) causes >>>>>>>> U-Boot to hang on QEMU q35 target. Disable the writing for QEMU targets. >>>>>>>> >>>>>>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >>>>>>>> --- >>>>>>>> >>>>>>>> arch/x86/cpu/coreboot/coreboot.c | 4 ---- >>>>>>>> 1 file changed, 4 deletions(-) >>>>>>>> >>>>>>>> diff --git a/arch/x86/cpu/coreboot/coreboot.c b/arch/x86/cpu/coreboot/coreboot.c >>>>>>>> index 4cdd0d4..2234cf0 100644 >>>>>>>> --- a/arch/x86/cpu/coreboot/coreboot.c >>>>>>>> +++ b/arch/x86/cpu/coreboot/coreboot.c >>>>>>>> @@ -85,10 +85,6 @@ void board_final_cleanup(void) >>>>>>>> wrmsrl(MTRR_PHYS_MASK_MSR(top_mtrr), 0); >>>>>>>> mtrr_close(&state); >>>>>>>> } >>>>>>>> - >>>>>>>> - /* Issue SMI to Coreboot to lock down ME and registers */ >>>>>>>> - printf("Finalizing Coreboot\n"); >>>>>>>> - outb(0xcb, 0xb2); >>>>>>>> } >>>>>>>> >>>>>>>> void panic_puts(const char *str) >>>>>>>> -- >>>>>>>> 1.8.2.1 >>>>>>>> >>>>>>> >>>>>>> But how does this run with coreboot on platforms that need it? Should >>>>>>> this be controlled by a device tree settings, perhaps? >>>>>>> >>>>>> >>>>>> I am not sure if the lock down should be done by the coreboot in the >>>>>> first place. IMHO U-Boot as the coreboot payload should not touch such >>>>>> low-level stuff. >>>>> >>>>> Well it is telling coreboot that it is about to boot the kernel. I >>>>> think this is an x86 thing and we should support it. >>>>> >>>> >>>> What happens if coreboot directly boot into the kernel? Does coreboot >>>> do the same 'outb(0xcb, 0xb2)' thing before jumping to kernel? My >>> >>> Kind of, although in that case it is just a function call within coreboot. >>> >>>> understanding is that coreboot payload is to enhance coreboot's >>>> functionality as a system BIOS (either having legacy BIOS interface >>>> with the help of a SeaBIOS payload, or having UEFI interface with the >>>> help of a TinaoCore payload). coreboot itself does the mainboard >>>> initialization and payload does something that is not hardware >>>> dependent. >>> >>> Right, but we need to tell coreboot that we have finished with the ME >>> / registers, etc. It can't lock them before calling U-Boot since we >>> may adjust them. >>> >> >> Will use a config option in the device tree in v2. > > OK thanks. When you resend can you please rebase on x86/master? > Yes, already rebased :-) Regards, Bin
diff --git a/arch/x86/cpu/coreboot/coreboot.c b/arch/x86/cpu/coreboot/coreboot.c index 4cdd0d4..2234cf0 100644 --- a/arch/x86/cpu/coreboot/coreboot.c +++ b/arch/x86/cpu/coreboot/coreboot.c @@ -85,10 +85,6 @@ void board_final_cleanup(void) wrmsrl(MTRR_PHYS_MASK_MSR(top_mtrr), 0); mtrr_close(&state); } - - /* Issue SMI to Coreboot to lock down ME and registers */ - printf("Finalizing Coreboot\n"); - outb(0xcb, 0xb2); } void panic_puts(const char *str)
Writing 0xcb to I/O port 0xb2 (Advanced Power Management Control) causes U-Boot to hang on QEMU q35 target. Disable the writing for QEMU targets. Signed-off-by: Bin Meng <bmeng.cn@gmail.com> --- arch/x86/cpu/coreboot/coreboot.c | 4 ---- 1 file changed, 4 deletions(-)