[commit] r5976 - trunk/src/cpu/amd/car

[repository service]

Author: sduplichan
Date: Tue Oct 19 23:08:11 2010
New Revision: 5976
URL: https://tracker.coreboot.org/trac/coreboot/changeset/5976

Log:
For AMD family 10h processors, msr c0010058 is always programmed
for 256 buses, even if fewer are configured. This patch lets msr
c0010058 programming use the configured bus count, CONFIG_MMCONF_BUS_NUMBER.

Signed-off-by: Scott Duplichan 
Acked-by: Myles Watson

Modified:
   trunk/src/cpu/amd/car/cache_as_ram.inc

Modified: trunk/src/cpu/amd/car/cache_as_ram.inc
==============================================================================
--- trunk/src/cpu/amd/car/cache_as_ram.inc	Tue Oct 19 17:25:06 2010	(r5975)
+++ trunk/src/cpu/amd/car/cache_as_ram.inc	Tue Oct 19 23:08:11 2010	(r5976)
@@ -132,14 +132,34 @@
    wrmsr

#if CONFIG_MMCONF_SUPPORT
-	/* Set MMIO config space BAR. */
-	movl	$MSR_MCFG_BASE, %ecx
-	rdmsr
-	andl	$(~(0xfff00000 | (0xf << 2))), %eax
-	orl	$((CONFIG_MMCONF_BASE_ADDRESS & 0xfff00000)), %eax
-	orl	$((8 << 2) | (1 << 0)), %eax
-	andl	$(~(0x0000ffff)), %edx
-	orl	$(CONFIG_MMCONF_BASE_ADDRESS >> 32), %edx
+   #if (CONFIG_MMCONF_BASE_ADDRESS > 0xFFFFFFFF)
+   #error "MMCONF_BASE_ADDRESS too big"
+   #elif (CONFIG_MMCONF_BASE_ADDRESS & 0xFFFFF)
+   #error "MMCONF_BASE_ADDRESS not 1MB aligned"
+   #endif
+	movl	$0, %edx
+	movl	$((CONFIG_MMCONF_BASE_ADDRESS) | (1 << 0)), %eax
+   #if (CONFIG_MMCONF_BUS_NUMBER == 1)
+   #elif (CONFIG_MMCONF_BUS_NUMBER == 2)
+	orl	$(1 << 2), %eax
+   #elif (CONFIG_MMCONF_BUS_NUMBER == 4)
+	orl	$(2 << 2), %eax	
+   #elif (CONFIG_MMCONF_BUS_NUMBER == 8)
+	orl	$(3 << 2), %eax
+   #elif (CONFIG_MMCONF_BUS_NUMBER == 16)
+	orl	$(4 << 2), %eax
+   #elif (CONFIG_MMCONF_BUS_NUMBER == 32)
+	orl	$(5 << 2), %eax
+   #elif (CONFIG_MMCONF_BUS_NUMBER == 64)
+	orl	$(6 << 2), %eax
+   #elif (CONFIG_MMCONF_BUS_NUMBER == 128)
+	orl	$(7 << 2), %eax
+   #elif (CONFIG_MMCONF_BUS_NUMBER == 256)
+	orl	$(8 << 2), %eax
+   #else
+	#error "bad MMCONF_BUS_NUMBER value"
+   #endif
+	movl	$(0xc0010058), %ecx
    wrmsr
 #endif

[Warren Turkal]

On Tue, Oct 19, 2010 at 2:08 PM, repository service  wrote:
> #if (CONFIG_MMCONF_BASE_ADDRESS > 0xFFFFFFFF)
> #error "MMCONF_BASE_ADDRESS too big"
> #elif (CONFIG_MMCONF_BASE_ADDRESS & 0xFFFFF)
> #error "MMCONF_BASE_ADDRESS not 1MB aligned"
> #endif
> movl    $0, %edx
> movl    $((CONFIG_MMCONF_BASE_ADDRESS) | (1 << 0)), %eax
> #if (CONFIG_MMCONF_BUS_NUMBER == 1)

If you insist on doing it this way, it might be better to change the
above 2 lines to:
  movl $CONFIG_MMCONF_BASE_ADDRESS
#if (CONFIG_MMCONF_BUS_NUMBER == 1)
  orl $(1 << 0), %eax
> #elif (CONFIG_MMCONF_BUS_NUMBER == 2)
> orl     $(1 << 2), %eax

However, why not just:
  movl $CONFIG_MMCONF_BASE_ADDRESS
  orl $CONFIG_MMCONF_BUS_NUMBER, %eax"
and no CPP conditionals since all those shifted numbers appear to be
the binary representations of the numbers you are testing for?
> #elif (CONFIG_MMCONF_BUS_NUMBER == 4)
> orl     $(2 << 2), %eax
> #elif (CONFIG_MMCONF_BUS_NUMBER == 8)
> orl     $(3 << 2), %eax
> #elif (CONFIG_MMCONF_BUS_NUMBER == 16)
> orl     $(4 << 2), %eax
> #elif (CONFIG_MMCONF_BUS_NUMBER == 32)
> orl     $(5 << 2), %eax
> #elif (CONFIG_MMCONF_BUS_NUMBER == 64)
> orl     $(6 << 2), %eax
> #elif (CONFIG_MMCONF_BUS_NUMBER == 128)
> orl     $(7 << 2), %eax
> #elif (CONFIG_MMCONF_BUS_NUMBER == 256)
> orl     $(8 << 2), %eax
> #else
> #error "bad MMCONF_BUS_NUMBER value"
> #endif
> movl    $(0xc0010058), %ecx

Thanks,
wt

[Scott Duplichan]

-----Original Message-----
From: [e-mail] [mailto:[e-mail]] On Behalf Of Warren Turkal
Sent: Wednesday, October 20, 2010 03:20 AM
To: [e-mail]
Subject: Re: [coreboot] [commit] r5976 - trunk/src/cpu/amd/car

]On Tue, Oct 19, 2010 at 2:08 PM, repository service  wrote:
]> +   #if (CONFIG_MMCONF_BASE_ADDRESS > 0xFFFFFFFF)
]> +   #error "MMCONF_BASE_ADDRESS too big"
]> +   #elif (CONFIG_MMCONF_BASE_ADDRESS & 0xFFFFF)
]> +   #error "MMCONF_BASE_ADDRESS not 1MB aligned"
]> +   #endif
]> +       movl    $0, %edx
]> +       movl    $((CONFIG_MMCONF_BASE_ADDRESS) | (1 << 0)), %eax
]> +   #if (CONFIG_MMCONF_BUS_NUMBER == 1)
]
]If you insist on doing it this way, it might be better to change the
]above 2 lines to:
]  movl $CONFIG_MMCONF_BASE_ADDRESS
]#if (CONFIG_MMCONF_BUS_NUMBER == 1)
]  orl $(1 << 0), %eax

Hello Warren, thanks for the suggestions. Bit zero is the enable bit,
so it must be set unconditionally. That is why I included it in the
movl. For the next question, I believe lack of readability is a 
factor. That is why so much effort went into attempts to improve it.
Bit positions 2-5 must be set to the base 2 logarithm of
CONFIG_MMCONF_BUS_NUMBER. I could not find a straightforward and
readable method of finding the base2 log at either assembly time or
at run time. What would have been nice is:
   movl $(log2(CONFIG_MMCONF_BUS_NUMBER) << 2), %eax
But gas has no base 2 log function and could not figure out how to
write a macro that returns a constant.

Thanks,
Scott

]
]> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 2)
]> +       orl     $(1 << 2), %eax
]
]However, why not just:
]  movl $CONFIG_MMCONF_BASE_ADDRESS
]  orl $CONFIG_MMCONF_BUS_NUMBER, %eax"
]and no CPP conditionals since all those shifted numbers appear to be
]the binary representations of the numbers you are testing for?
]
]> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 4)
]> +       orl     $(2 << 2), %eax
]> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 8)
]> +       orl     $(3 << 2), %eax
]> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 16)
]> +       orl     $(4 << 2), %eax
]> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 32)
]> +       orl     $(5 << 2), %eax
]> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 64)
]> +       orl     $(6 << 2), %eax
]> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 128)
]> +       orl     $(7 << 2), %eax
]> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 256)
]> +       orl     $(8 << 2), %eax
]> +   #else
]> +       #error "bad MMCONF_BUS_NUMBER value"
]> +   #endif
]> +       movl    $(0xc0010058), %ecx
]
]Thanks,
]wt

[xdrudis]

On Wed, Oct 20, 2010 at 02:20:12PM -0500, Scott Duplichan wrote:
> movl $(log2(CONFIG_MMCONF_BUS_NUMBER) << 2), %eax

I have no idea of gas, but  log2(x) for integer x > 0
is the position  of the leftmost 1 in the binary representation
of x (position 0=rightmost).

int log2(unsigned int x) {
   for(int b=sizeof(unsigned int)-1,b>=0,b--) {
      if (x&(1<<b)) {
         return b;
      }
   }
   //x==0 
   return -infinity; //???
}

Something like (for x<2^32 , tail recursive and imagining gas syntax)

.macro log2size x,sizeofx
  .if x & (1<<(sizeofx-1))
     sizeofx -1
  .else
     log2size x,sizeofx-1
  .end
.endm

.macro log2 x
  .if x
     log2size x,32
  .else
     -infinity ???
  .end
.endm

I'm not claiming this is efficient, but does it need to be ?

[xdrudis]

On Wed, Oct 20, 2010 at 02:20:12PM -0500, Scott Duplichan wrote:
> But gas has no base 2 log function and could not figure out how to
> write a macro that returns a constant.

Sorry, ignore my previous post . I misread and answered without thinking. 
It was about gas, not about log2. I'm surprised 
that a macro can't return a constant.

[Peter Stuge]

Scott Duplichan wrote:
> a straightforward and readable method of finding the base2 log at
> either assembly time or at run time.

bsf or maybe rather bsr could be used to do it pretty easily, but I
like having the compile time "perparsing" as a sort of sanity check.
I wouldn't want to get rid of that.

//Peter

[Scott Duplichan]

-----Original Message-----
From: [e-mail] [mailto:[e-mail]] On Behalf Of Peter Stuge
Sent: Thursday, October 21, 2010 03:50 AM
To: [e-mail]
Subject: Re: [coreboot] [commit] r5976 - trunk/src/cpu/amd/car

]Scott Duplichan wrote:
]> a straightforward and readable method of finding the base2 log at
]> either assembly time or at run time.
]
]bsf or maybe rather bsr could be used to do it pretty easily, but I
]like having the compile time "perparsing" as a sort of sanity check.
]I wouldn't want to get rid of that.

Oh yes, you are correct. I forgot about those instructions.

]//Peter