Re: [coreboot] coreboot Digest, Vol 68, Issue 108

[Mark Beihoffer]

I tried that and it worked! Thants!

Mark Beihoffer
Dragonfly Networks
[e-mail]
[e-mail]
[phone]

On Tue, Oct 19, 2010 at 2:03 PM,  wrote:
> Send coreboot mailing list submissions to
>        [e-mail]
>
> To subscribe or unsubscribe via the World Wide Web, visit
>        http://www.coreboot.org/mailman/listinfo/coreboot
> or, via email, send a message with subject or body 'help' to
>        [e-mail]
>
> You can reach the person managing the list at
>        [e-mail]
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of coreboot digest..."
>
> Today's Topics:
> Re: [PATCH] AMD F10h: set MMCONF bus count according
> toconfigured value (Scott Duplichan)
> Re: [PATCH] AMD F10h: set MMCONF bus count
> accordingtoconfigured value (Myles Watson)
> Re: [PATCH] AMD F10h: set MMCONF bus count according
> toconfigured value (Carl-Daniel Hailfinger)
> Re: [PATCH] AMD F10h: set MMCONF bus count        according
> toconfigured value (Scott Duplichan)
> Re: [PATCH] AMD F10h: set MMCONF bus count according
> toconfigured value (Myles Watson)
> Re: [PATCH] AMD F10h: set MMCONF bus count according
> toconfigured value (Scott Duplichan)
> Re: [PATCH] AMD F10h: set MMCONF bus count according
> toconfigured value (Myles Watson)
>
> Message: 1
> Date: Tue, 19 Oct 2010 12:47:08 -0500
> From: "Scott Duplichan" 
> To: "'Arne Georg Gleditsch'" 
> Cc: 'Coreboot' 
> Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count
>        according       toconfigured value
> Message-ID: 
> Content-Type: text/plain;       charset="us-ascii"
>
> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]]
> On Behalf Of Arne Georg Gleditsch
> Sent: Tuesday, October 19, 2010 01:51 AM
> To: Scott Duplichan
> Cc: 'Coreboot'
> Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count according
> toconfigured value
>
> ]"Scott Duplichan"  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.
> ]
> ]--
> ]                                                       Arne.
>
> Hello Arne,
>
> Thanks for the suggestion. It would certainly eliminate the awkward
> logic. On the other hand, if anyone is overriding the kconfig
> value CONFIG_MMCONF_BUS_NUMBER locally, they would have to update
> their override.
>
> Last night, I thought I would just learn about gas macros and do it
> that way. It was more difficult than I thought. I cannot even find
> examples of gas macros that use arguments. A C style macro seemed
> possible, but I found when gas processes it the needed ?: does not
> work. So I ended up with a gas macro that does not use arguments.
> The macro allows the awkwark logic and additional error checking to
> be placed in an existing include file. The new error checking was
> tested by forcing a few failing cases:
>
> Signed-off-by: Scott Duplichan
> Index: src/include/cpu/amd/mtrr.h
>
> --- src/include/cpu/amd/mtrr.h  (revision 5975)
> +++ src/include/cpu/amd/mtrr.h  (working copy)
> @@ -32,6 +32,44 @@
>  #define TOP_MEM_MASK                   0x007fffff
>  #define TOP_MEM_MASK_KB                        (TOP_MEM_MASK >> 10)
>
> +#if defined(ASSEMBLY)
>
> +.macro SET_C0010058
> #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
>
> +.endm
>
> +#endif
>
> #if !defined(__PRE_RAM__) && !defined(ASSEMBLY)
>  void amd_setup_mtrrs(void);
>  #endif
> Index: src/cpu/amd/car/cache_as_ram.inc
> ===================================================================
> --- src/cpu/amd/car/cache_as_ram.inc    (revision 5975)
> +++ src/cpu/amd/car/cache_as_ram.inc    (working copy)
> @@ -132,15 +132,7 @@
>        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
>   wrmsr
> SET_C0010058
>
> #endif
>
> CAR_FAM10_out_post_errata:
>
> Message: 2
> Date: Tue, 19 Oct 2010 12:05:48 -0600
> From: "Myles Watson" 
> To: "'Scott Duplichan'" ,     "'Arne Georg Gleditsch'"
>        
> Cc: 'Coreboot' 
> Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count
>        accordingtoconfigured value
> Message-ID: 
> Content-Type: text/plain;       charset="us-ascii"
> > Last night, I thought I would just learn about gas macros and do it
> > that way. It was more difficult than I thought. I cannot even find
> > examples of gas macros that use arguments. A C style macro seemed
> > possible, but I found when gas processes it the needed ?: does not
> > work. So I ended up with a gas macro that does not use arguments.
> > The macro allows the awkwark logic and additional error checking to
> > be placed in an existing include file. The new error checking was
> > tested by forcing a few failing cases:
> >
> > Signed-off-by: Scott Duplichan
>
> I'm sorry that I wasn't more clear.  I think it was better before.  The
> code
> looks exactly the same, but it's in a different file, which wasn't my
> intent
> at all.
>
> I like the other version much better.  I hope that your new knowledge of
> gas
> macros will come in handy in the future :)
>
> Thanks,
> Myles
>
> Message: 3
> Date: Tue, 19 Oct 2010 20:24:38 +0200
> From: Carl-Daniel Hailfinger 
> To: Scott Duplichan 
> Cc: 'Arne Georg Gleditsch' ,      'Coreboot'
>        
> Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count
>        according       toconfigured value
> Message-ID: 
> Content-Type: text/plain; charset=ISO-8859-1
>
> Hi Scott,
>
> On 19.10.2010 19:47, Scott Duplichan wrote:
> > Last night, I thought I would just learn about gas macros and do it
> > that way. It was more difficult than I thought. I cannot even find
> > examples of gas macros that use arguments.
>
> Take a look at src/cpu/amd/car/cache_as_ram.inc, specifically
> extractmask and simplemask.
>
> Regards,
> Carl-Daniel
>
> Message: 4
> Date: Tue, 19 Oct 2010 13:34:20 -0500
> From: "Scott Duplichan" 
> To: "'Carl-Daniel Hailfinger'" 
> Cc: 'Arne Georg Gleditsch' ,      'Coreboot'
>        
> Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count
>        according       toconfigured value
> Message-ID: 
> Content-Type: text/plain;       charset="us-ascii"
>
> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]]
> On Behalf Of Carl-Daniel Hailfinger
> Sent: Tuesday, October 19, 2010 01:25 PM
> To: Scott Duplichan
> Cc: 'Arne Georg Gleditsch'; 'Coreboot'
> Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count according
> toconfigured value
>
> ]Hi Scott,
> ]
> ]On 19.10.2010 19:47, Scott Duplichan wrote:
> ]> Last night, I thought I would just learn about gas macros and do it
> ]> that way. It was more difficult than I thought. I cannot even find
> ]> examples of gas macros that use arguments.
> ]
> ]Take a look at src/cpu/amd/car/cache_as_ram.inc, specifically
> ]extractmask and simplemask.
> ]
> ]Regards,
> ]Carl-Daniel
> ]
> ]--
> ]http://www.hailfinger.org/
>
> Thanks Carl-Daniel.
>
> Hello Myles,
>
> I am not so happy with the new one either. What I really wanted was a
> macro called highestSetBit. That way, the reader can guess what macro
> invocation highestSetBit (CONFIG_MMCONF_BUS_NUMBER) does without
> actually digging it out. But I could not find a way to make that work.
> Another idea was to find highestSetBit at runtime, if it could be done
> a short and readable code sequence. This would be possible with family
> 15h, where instructions such as lzcnt are supported. But family 10h
> does not have this instruction.
>
> Now that Carl-Daniel has showed how to do gas macros, how about I add a
> highestSetBit macro to amd/mtrr.h? Then everything else can be inlined?
>
> Thanks,
> Scott
>
> Message: 5
> Date: Tue, 19 Oct 2010 12:39:47 -0600
> From: Myles Watson 
> To: Scott Duplichan 
> Cc: Coreboot 
> Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count
>        according toconfigured value
> Message-ID:
>        
> Content-Type: text/plain; charset=ISO-8859-1
>
> On Tue, Oct 19, 2010 at 12:34 PM, Scott Duplichan 
> wrote:
> > -----Original Message-----
> > From: [e-mail] [mailto:
>
> [e-mail]] On Behalf Of Carl-Daniel Hailfinger
> > Sent: Tuesday, October 19, 2010 01:25 PM
> > To: Scott Duplichan
> > Cc: 'Arne Georg Gleditsch'; 'Coreboot'
> > Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count according
>
> toconfigured value
> > ]Hi Scott,
> > ]
> > ]On 19.10.2010 19:47, Scott Duplichan wrote:
> > ]> Last night, I thought I would just learn about gas macros and do it
> > ]> that way. It was more difficult than I thought. I cannot even find
> > ]> examples of gas macros that use arguments.
> > ]
> > ]Take a look at src/cpu/amd/car/cache_as_ram.inc, specifically
> > ]extractmask and simplemask.
> > ]
> > ]Regards,
> > ]Carl-Daniel
> > ]
> > ]--
> > ]http://www.hailfinger.org/
> >
> > Thanks Carl-Daniel.
> >
> > Hello Myles,
> >
> > I am not so happy with the new one either. What I really wanted was a
> > macro called highestSetBit. That way, the reader can guess what macro
> > invocation highestSetBit (CONFIG_MMCONF_BUS_NUMBER) does without
> > actually digging it out. But I could not find a way to make that work.
> > Another idea was to find highestSetBit at runtime, if it could be done
> > a short and readable code sequence. This would be possible with family
> > 15h, where instructions such as lzcnt are supported. But family 10h
> > does not have this instruction.
> >
> > Now that Carl-Daniel has showed how to do gas macros, how about I add a
> > highestSetBit macro to amd/mtrr.h? Then everything else can be inlined?
>
> I would say it isn't worth the effort, but it's up to you.  The
> problem with macros in assembly is that it's unclear which registers
> they clobber.  That was why I suggested a pre-processor macro, but
> it's not that important.
>
> Thanks,
> Myles
>
> Message: 6
> Date: Tue, 19 Oct 2010 13:56:14 -0500
> From: "Scott Duplichan" 
> To: "'Myles Watson'" 
> Cc: 'Coreboot' 
> Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count
>        according       toconfigured value
> Message-ID: 
> Content-Type: text/plain; charset="us-ascii"
>
> ]I would say it isn't worth the effort, but it's up to you.  The
> ]problem with macros in assembly is that it's unclear which registers
> ]they clobber.  That was why I suggested a pre-processor macro, but
> ]it's not that important.
> ]
> ]Thanks,
> ]Myles
>
> Hello Myles,
>
> Good point. Best would be a 'macro' that allows writing:
>   movl  highestSetBit (busn), %eax
> But that is not possible apparently. How about then, back
> to inlined code with the extra error checks:
>
> Signed-off-by: Scott Duplichan <[e-mail]
> Index: src/cpu/amd/car/cache_as_ram.inc
>
> --- src/cpu/amd/car/cache_as_ram.inc    (revision 5975)
> +++ src/cpu/amd/car/cache_as_ram.inc    (working copy)
> @@ -132,15 +132,35 @@
>        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
>   wrmsr
> #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
>
> CAR_FAM10_out_post_errata: