[coreboot] CBMEM Console corruption with fsp_baytrail in coreboot 4.2

[Ben Gardner]

I think I found the issue, but won't be able to test for another week.

car_get_var_ptr() assumes that the migrated base is the same as _car_data_start.
On FSP 1.0, that isn't true.  _car_data_start is 0x0c00 bytes past
migrated base.

The last two lines of car_get_var_ptr() read as follows:
   offset = (char *)var - (char *)_car_data_start;
   return &migrated_base[offset];

However, migrated base matches 0xfef00000 (DCACHE_RAM_BASE), while
_car_data_start is 0xfef00c00.
So, when the data is migrated, the migrated offset is wrong and
cbmem_console_p is fairly random.

It worked when I moved _preram_cbmem_console because that put
_car_data_start at 0xfef00000 again.
A simple fix would be to use CONFIG_DCACHE_RAM_BASE instead of
_car_data_start when calculating the base.

I think we may also need Aaron's earlier patch proposed in this
thread, as _preram_cbmem_console is not between _car_data_start and
_car_data_end.

The fix might be something like the following: (gmail ate my tabs).

What do you think?

--- a/src/cpu/x86/car.c
+++ b/src/cpu/x86/car.c
@@ -63,15 +63,15 @@ void *car_get_var_ptr(void *var)
 #if IS_ENABLED(CONFIG_PLATFORM_USES_FSP1_0)
        migrated_base=(char *)find_saved_temp_mem(
                        *(void **)CBMEM_FSP_HOB_PTR);
+       offset = (char *)var - (char *)CONFIG_DCACHE_RAM_BASE;
 #else
        migrated_base = cbmem_find(CBMEM_ID_CAR_GLOBALS);
+       offset = (char *)var - (char *)_car_start;
 #endif

        if (migrated_base == NULL)
                die( "CAR: Could not find migration base!\n");

-       offset = (char *)var - (char *)_car_start;
-
        return &migrated_base[offset];
 }


On Fri, Nov 20, 2015 at 5:11 PM, Ben Gardner <gardner.ben at gmail.com> wrote:
> On Fri, Nov 20, 2015 at 4:56 PM, Ben Gardner <gardner.ben at gmail.com> wrote:
>> Ug. I was looking at the wrong log. Time for a break.
>>
>> The output from the log was:
>> Stack: fef03fc4 or fef03fcc
>>
>> That seems to match the settings:
>> CONFIG_DCACHE_RAM_BASE=0xfef00000
>> CONFIG_DCACHE_RAM_SIZE=0x4000
>
> For what it is worth, I verified that ECX = 0xfef00000 and EDX =
> 0xfef04000 when the FSP's TempRamIinitEntry() function returns.
> The CAR global data makes more sense now, as that is mapped to the
> bottom of the temporary stack area.
> The CAR data location is correct and the stack data does indeed appear
> to have been copied correctly to 0x7ae230a0.
> I think I'm looking in the wrong area.
>
> Oh, well.  I'll look at it again after the Thanksgiving holiday.
>
> Ben