| Submitter | Scott |
|---|---|
| Date | 2010-10-18 17:29:23 |
| Message ID | <4C5D9429A9174743B3717E0AC0385999@m3a78> |
| Download | mbox | patch |
| Permalink | /patch/2138/ |
| State | Superseded |
| Headers | show |
Comments
On Mon, Oct 18, 2010 at 11:29 AM, Scott Duplichan <scott@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@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
On 18.10.2010, at 10:44, Myles Watson <mylesgw@gmail.com> wrote: > On Mon, Oct 18, 2010 at 11:29 AM, Scott Duplichan <scott@notabs.org> wrote: > >> + #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? I don't think that we should move ugly logic into Kconfig. Kconfig is for configuration, code is for logic. Stefan >
On Mon, Oct 18, 2010 at 8:06 PM, Stefan Reinauer <stefan.reinauer@coresystems.de> wrote: > > On 18.10.2010, at 10:44, Myles Watson <mylesgw@gmail.com> wrote: > >> On Mon, Oct 18, 2010 at 11:29 AM, Scott Duplichan <scott@notabs.org> wrote: >> >>> + #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? > > I don't think that we should move ugly logic into Kconfig. Kconfig is for configuration, code is for logic. Agreed. Thanks, Myles
"Scott Duplichan" <scott@notabs.org> writes: > 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. How about just renaming CONFIG_MMCONF_BUS_NUMBER to ...BUS_BITS? There's only 8 discrete values it can take anyway, and this would also make it harder to pick an unsupported value for BUS_NUMBER.
Patch
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_MMCONF_BASE_ADDRESS & 0xfff00000) | (1 << 0)), %eax + #if (CONFIG_MMCONF_BUS_NUMBER == 2) + orl $1, %eax + #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
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@notabs.org>