PC Engines apu2 boot regression

[Piotr Król]

Hi Aaron,
I tried to boot PC Engines apu2 on recent master branch and discovered
that it not boot. Bisection points to:

60320182d011 console: only allow console messages after initialization

It is hard to believe that this change cause AGESA reset loop, but I
checked 3 times. After applying above commit I end up with loop:

coreboot-4.7-441-g60320182d0 Fri Mar  2 15:22:24 UTC 2018 romstage
starting...
BSP Family_Model: 00730f01
cpu_init_detectedx = 00000000
agesawrapper_amdinitreset() entry
CBFS: 'Master Header Locator' located CBFS at [200:7fffc0)
CBFS: Locating 'AGESA'
CBFS: Found @ offset 5ffdc0 size 7b0e0
CBFS: 'Master Header Locator' located CBFS at [200:7fffc0)
CBFS: Locating 'AGESA'
CBFS: Found @ offset 5ffdc0 size 7b0e0
Fch OEM config in INIT RESET Done

Any idea what can be result of such weird behavior?

Best Regards,

[Aaron Durbin]

On Fri, May 4, 2018 at 9:23 AM, Piotr Król  wrote:
> Hi Aaron,
> I tried to boot PC Engines apu2 on recent master branch and discovered
> that it not boot. Bisection points to:
>
> 60320182d011 console: only allow console messages after initialization
>
> It is hard to believe that this change cause AGESA reset loop, but I
> checked 3 times. After applying above commit I end up with loop:
>
> coreboot-4.7-441-g60320182d0 Fri Mar  2 15:22:24 UTC 2018 romstage
> starting...
> BSP Family_Model: 00730f01
> cpu_init_detectedx = 00000000
> agesawrapper_amdinitreset() entry
> CBFS: 'Master Header Locator' located CBFS at [200:7fffc0)
> CBFS: Locating 'AGESA'
> CBFS: Found @ offset 5ffdc0 size 7b0e0
> CBFS: 'Master Header Locator' located CBFS at [200:7fffc0)
> CBFS: Locating 'AGESA'
> CBFS: Found @ offset 5ffdc0 size 7b0e0
> Fch OEM config in INIT RESET Done
>
> Any idea what can be result of such weird behavior?

I think it's because we still have boards that utilize CAR_GLOBAL, but
don't handle migration of CAR globals. Those two things combined
really makes for situations that just don't work and ramstage loading
is just lucky to work on those platforms. I was originally thinking
this patch would work, but I don't think that's the case because when
I build apu2 CONFIG_EARLY_CBMEM_INIT is already set. If you revert
that patch what does your log look like? We could be recursively
entering into car_get_var_ptr() through the printk() path, but that
would require things not marked as CAR_GLOBAL being passed to that
function. There is one way to fix all of these scenarios: remove CAR
migration by transitioning everyone to postcar stub between romstage
and ramstage. That's a lot of work, though. We can try and debug
further because I'm not clear why things aren't working.

diff --git a/src/console/init.c b/src/console/init.c
index 8f71b09881..b5c2f792fa 100644
--- a/src/console/init.c
+++ b/src/console/init.c
@@ -35,7 +35,7 @@ static int console_loglevel = CONFIG_DEFAULT_CONSOLE_LOGLEVEL;

static inline int get_log_level(void)
 {
-       if (car_get_var(console_inited) == 0)
+       if (IS_ENABLED(CONFIG_EARLY_CBMEM_INIT) && !car_get_var(console_inited))
                return -1;
        if (CONSOLE_LEVEL_CONST)
                return get_console_loglevel();
@@ -78,7 +78,8 @@ asmlinkage void console_init(void)

console_hw_init();

-       car_set_var(console_inited, 1);
+       if (IS_ENABLED(CONFIG_EARLY_CBMEM_INIT))
+               car_set_var(console_inited, 1);

printk(BIOS_NOTICE, "\n\ncoreboot-%s%s %s " ENV_STRING " starting...\n",
               coreboot_version, coreboot_extra_version, coreboot_build);
> Best Regards,
>
> Piotr Król
> Embedded Systems Consultant
> https://3mdeb.com | @3mdeb_com

DDR1 K8 boot broken too (was: Re: PC Engines apu2 boot regression)

[Jonathan A. Kollasch]

On Fri, May 04, 2018 at 05:23:40PM +0200, Piotr Król wrote:
> Hi Aaron,
> I tried to boot PC Engines apu2 on recent master branch and discovered
> that it not boot. Bisection points to:
>
> 60320182d011 console: only allow console messages after initialization
>
> It is hard to believe that this change cause AGESA reset loop, but I
> checked 3 times. After applying above commit I end up with loop:

I see a similar romstage boot loop starting with this commit on
msi/ms7135 (K8N Neo3), a DDR1 K8 board.

Jonathan Kollasch

DDR1 K8 boot broken too (was: Re: PC Engines apu2 boot regression)

[Aaron Durbin]

On Fri, May 4, 2018 at 10:01 AM, Jonathan A. Kollasch
 wrote:
> On Fri, May 04, 2018 at 05:23:40PM +0200, Piotr Król wrote:
> > Hi Aaron,
> > I tried to boot PC Engines apu2 on recent master branch and discovered
> > that it not boot. Bisection points to:
> >
> > 60320182d011 console: only allow console messages after initialization
> >
> > It is hard to believe that this change cause AGESA reset loop, but I
> > checked 3 times. After applying above commit I end up with loop:
>
> I see a similar romstage boot loop starting with this commit on
> msi/ms7135 (K8N Neo3), a DDR1 K8 board.

I think it's the AMD boards and their weird config. CAR_GLOBAL is
used, but apparently these boards fail if accessing CAR_GLOBAL at the
wrong time? If you have log diffs w/ and w/o the patch it'd help with
root cause.
>     Jonathan Kollasch