[coreboot] PC Engines apu2 boot regression

[Aaron Durbin]

On Fri, May 4, 2018 at 9:23 AM, Piotr Król <piotr.krol at 3mdeb.com> 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
>