[coreboot] [PATCH] AMD F10h: set MMCONF bus count according to configured value

Myles Watson mylesgw at gmail.com
Mon Oct 18 19:44:38 CEST 2010


On Mon, Oct 18, 2010 at 11:29 AM, Scott Duplichan <scott at notabs.org> wrote:
> 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.
>
> Tested with the mahogany_fam10 project. Does the assembler have
> a compile time operator for highest set bit or base 2 log? That
> would sure simplify this patch.
>
>
> Signed-off-by: Scott Duplichan <scott at notabs.org>
>
> Index: src/cpu/amd/car/cache_as_ram.inc
> ===================================================================
> --- src/cpu/amd/car/cache_as_ram.inc    (revision 5965)
> +++ src/cpu/amd/car/cache_as_ram.inc    (working copy)
> @@ -136,8 +136,26 @@
>        movl    $MSR_MCFG_BASE, %ecx
>        rdmsr
>        andl    $(~(0xfff00000 | (0xf << 2))), %eax
> -       orl     $((CONFIG_MMCONF_BASE_ADDRESS & 0xfff00000)), %eax
> -       orl     $((8 << 2) | (1 << 0)), %eax
I don't see how the logic below ever lets you get (8<<2)  The largest
I see is 8.

> +       orl     $((CONFIG_MMCONF_BASE_ADDRESS & 0xfff00000) | (1 << 0)), %eax
> +   #if (CONFIG_MMCONF_BUS_NUMBER == 2)
> +       orl     $1, %eax
Doesn't this just set the enable bit?

> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 4)
> +       orl     $2, %eax
> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 8)
> +       orl     $3, %eax
> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 16)
> +       orl     $4, %eax
> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 32)
> +       orl     $5, %eax
> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 64)
> +       orl     $6, %eax
> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 128)
> +       orl     $7, %eax
> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 256)
> +       orl     $8, %eax
> +   #else
> +       #error "unsupported MMCONF_BUS_NUMBER value"
> +   #endif
>        andl    $(~(0x0000ffff)), %edx
>        orl     $(CONFIG_MMCONF_BASE_ADDRESS >> 32), %edx
>        wrmsr

Could you move the ugly logic into Kconfig or a header file?  Should
there be a check with the base address alignment & size to make sure
it's a valid combination?

> Index: src/cpu/amd/car/cache_as_ram.inc
> ===================================================================
> --- src/cpu/amd/car/cache_as_ram.inc    (revision 5965)
> +++ src/cpu/amd/car/cache_as_ram.inc    (working copy)
> @@ -136,8 +136,26 @@
>        movl    $MSR_MCFG_BASE, %ecx
>        rdmsr
>        andl    $(~(0xfff00000 | (0xf << 2))), %eax
> -       orl     $((CONFIG_MMCONF_BASE_ADDRESS & 0xfff00000)), %eax
> -       orl     $((8 << 2) | (1 << 0)), %eax
> +       orl     $((CONFIG_AMD_MMCONF_REG_VAL << 2) | (1 << 0)), %eax

Thanks,
Myles




More information about the coreboot mailing list