I think I see what is happening from the following explanation. http://sourceware.org/binutils/docs-2.20/as/Symbol-Value.html#Symbol-Value
Kinda odd that the the symbol's value is only set to 0 after the operators are applied to the symbol. I think this is why the file both assembles without issue and the separate orl instruction works where the "$(REAL_XIP_ROM_BASE | MTRR_TYPE_WRBACK)" does not work. Does this sound correct? Thanks, wt On Sun, Oct 3, 2010 at 9:54 AM, Warren Turkal <[email protected]> wrote: > I see that is the change that fixed it. However, I'm still not sure I > understand the fix completely. When CONFIG_TINY_BOOTBLOCK is defined > and AUTO_XIP_ROM_BASE is not defined, we get the following line (line > number 159 or so) in build/mainboard/via/vt8454c/crt0.s: > movl $AUTO_XIP_ROM_BASE, %eax > > How does that assemble without an error? > > Is the AUTO_XIP_ROM_BASE set to some value (presumably 0) by the > assembler for some reason that I don't see? For reference, here is the > equivalent line from crt0.disasm: > 100 0115 B8000000 >---->-------movl>---$REAL_XIP_ROM_BASE, %eax > > Thanks, > wt > > On Sun, Oct 3, 2010 at 9:31 AM, Warren Turkal <[email protected]> wrote: >> Is this what your orl change fixed? >> >> wt >> >> On Sun, Oct 3, 2010 at 6:34 AM, Stefan Reinauer >> <[email protected]> wrote: >>> On 10/3/10 11:24 AM, Warren Turkal wrote: >>>> *ping* I really need an ack or nack on this. >>> >>> NACK.. Still the wrong way, it just blindly comments out an arbitrary >>> piece of code. >>> >>> However, the code has been fixed already in the repo. >>> >>> Stefan >>>> Thanks, >>>> wt >>>> >>>> On Sat, Oct 2, 2010 at 1:59 AM, Warren Turkal <[email protected]> >>>> wrote: >>>>> VIA/AMD experts, >>>>> >>>>> This patch get's the via/vt8454c back to building. However, I am not >>>>> sure if the code that is being #ifdef'ed out will actually ever be used >>>>> on a via platform. The code comes straight from the amd CAR >>>>> implementation. A couple of questions are raised by this: >>>>> 1) Should we just delete the code from the via file instead of this >>>>> patch? >>>>> 2) Should the amd and via CAR code be integrated into one file? Maybe >>>>> just portions of the files if not the whole files? >>>>> >>>>> Also, another happy side effect of this change is that all the c7 boards >>>>> seem to build with tiny bootblocks. Would everyone be ok with my making >>>>> that change? >>>>> >>>>> Thanks, >>>>> wt >>>>> 8<---------------------------------------------------------------------- >>>>> The execute-in-place (XIP) config options need to be set in order to get >>>>> XIP functionality, so it needs to be excluded when those settings are >>>>> not set. >>>>> >>>>> Signed-off-by: Warren Turkal <[email protected]> >>>>> --- >>>>> src/cpu/via/car/cache_as_ram.inc | 4 ++++ >>>>> 1 files changed, 4 insertions(+), 0 deletions(-) >>>>> >>>>> diff --git a/src/cpu/via/car/cache_as_ram.inc >>>>> b/src/cpu/via/car/cache_as_ram.inc >>>>> index be00fe3..d18ac3a 100644 >>>>> --- a/src/cpu/via/car/cache_as_ram.inc >>>>> +++ b/src/cpu/via/car/cache_as_ram.inc >>>>> @@ -85,6 +85,8 @@ clear_fixed_var_mtrr_out: >>>>> movl $(~(CacheSize - 1) | 0x800), %eax >>>>> wrmsr >>>>> >>>>> +#if defined(CONFIG_XIP_ROM_SIZE) && defined(CONFIG_XIP_ROM_BASE) >>>>> + >>>>> #if defined(CONFIG_TINY_BOOTBLOCK) && CONFIG_TINY_BOOTBLOCK >>>>> #define REAL_XIP_ROM_BASE AUTO_XIP_ROM_BASE >>>>> #else >>>>> @@ -106,6 +108,8 @@ clear_fixed_var_mtrr_out: >>>>> movl $(~(CONFIG_XIP_ROM_SIZE - 1) | 0x800), %eax >>>>> wrmsr >>>>> >>>>> +#endif /* CONFIG_XIP_ROM_SIZE && CONFIG_XIP_ROM_BASE */ >>>>> + >>>>> /* Set the default memory type and enable fixed and variable >>>>> MTRRs. */ >>>>> /* TODO: Or also enable fixed MTRRs? Bug in the code? */ >>>>> movl $MTRRdefType_MSR, %ecx >>>>> -- >>>>> 1.7.1 >>>>> >>>>> >>> >>> >>> -- >>> coreboot mailing list: [email protected] >>> http://www.coreboot.org/mailman/listinfo/coreboot >>> >> > -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
