[coreboot] [PATCH] Streamline CPU_ADDR_BITS usage

[Uwe Hermann]

On Mon, Oct 04, 2010 at 12:27:02AM +0200, Patrick Georgi wrote:
> Am 03.10.2010 23:59, schrieb Uwe Hermann:
> > +	/*
> > +	 * Important: The code below makes a run-time decision depending on
> > +	 * whether this is a K8 or Fam10h system. Depending on which it is,
> > +	 * the CONFIG_CPU_ADDR_BITS_MASK value might be be different.
> > +	 */
> >  	movl	$MTRRphysMask_MSR(1), %ecx
> > -	movl	$0xff, %edx /* (1 << (CONFIG_CPU_ADDR_BITS - 32)) - 1 for K8 (CONFIG_CPU_ADDR_BITS = 40) */
> > +	movl	$CONFIG_CPU_ADDR_BITS_MASK, %edx /* K8 */
> >  	jmp_if_k8(wbcache_post_fam10_setup)
> > -	movl	$0xffff, %edx /* (1 << (CONFIG_CPU_ADDR_BITS - 32)) - 1 for FAM10 (CONFIG_CPU_ADDR_BITS = 48) */
> > +	movl	$CONFIG_CPU_ADDR_BITS_MASK, %edx /* Fam10h */
> >  wbcache_post_fam10_setup:
> >  	movl	$(~(CONFIG_XIP_ROM_SIZE - 1) | 0x800), %eax
> >  	wrmsr
> > +
> >  #endif /* CONFIG_XIP_ROM_SIZE && CONFIG_XIP_ROM_BASE */
> How is this supposed to work?
> "Set to the build specific value, and if it is fam10 (ie. jmp_if_k8 not
> taken), set to the build specific value again"?

Hm, seems I misunderstood how this works. I was under the impression
such a two-image solution (K8 + Fam10h code in one coreboot.rom) would
also set two different CONFIG_CPU_ADDR_BITS_MASK values, one for K8, and
one for Fam10h. And if the K8 image is running it will use
CONFIG_CPU_ADDR_BITS_MASK (=40) at runtime, but if the Fam10h image runs
it would use CONFIG_CPU_ADDR_BITS_MASK (=48) instead.
That's probably not how it would work though, it seems?

Anyway, I'll just leave this snippet alone for now. Updated patch will
follow, need to fix another kconfig-related issue brought up by Peter.

But note that the current form is also a bit dangerous. It hardcodes 40bits
for K8 and 48bits for Fam10h here unconditionally. I don't know if this
assumption is always correct for all CPUs. Using the correct per-CPU
CONFIG_CPU_ADDR_BITS_MASK would definately be safer (if this mechanism
can work here at all). Are we sure there are no K8 systems that support
CPUs with bits != 40? Are we sure there are no Fam10h CPUs with
bits != 48 (and that there never will be in the future)?


Uwe.
-- 
http://hermann-uwe.de     | http://sigrok.org
http://randomprojects.org | http://unmaintained-free-software.org