I tried that and it worked! Thants! Mark Beihoffer Dragonfly Networks [email protected] [email protected] (612)508-5128
On Tue, Oct 19, 2010 at 2:03 PM, <[email protected]> wrote: > Send coreboot mailing list submissions to > [email protected] > > 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 > [email protected] > > You can reach the person managing the list at > [email protected] > > When replying, please edit your Subject line so it is more specific > than "Re: Contents of coreboot digest..." > > > Today's Topics: > > 1. Re: [PATCH] AMD F10h: set MMCONF bus count according > toconfigured value (Scott Duplichan) > 2. Re: [PATCH] AMD F10h: set MMCONF bus count > accordingtoconfigured value (Myles Watson) > 3. Re: [PATCH] AMD F10h: set MMCONF bus count according > toconfigured value (Carl-Daniel Hailfinger) > 4. Re: [PATCH] AMD F10h: set MMCONF bus count according > toconfigured value (Scott Duplichan) > 5. Re: [PATCH] AMD F10h: set MMCONF bus count according > toconfigured value (Myles Watson) > 6. Re: [PATCH] AMD F10h: set MMCONF bus count according > toconfigured value (Scott Duplichan) > 7. 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" <[email protected]> > To: "'Arne Georg Gleditsch'" <[email protected]> > Cc: 'Coreboot' <[email protected]> > Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count > according toconfigured value > Message-ID: <51ac17b3fd284fe6ba8547d65ac3c...@m3a78> > Content-Type: text/plain; charset="us-ascii" > > -----Original Message----- > From: [email protected] [mailto:[email protected]] > 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" <[email protected]> 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 <[email protected]> > > 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" <[email protected]> > To: "'Scott Duplichan'" <[email protected]>, "'Arne Georg Gleditsch'" > <[email protected]> > Cc: 'Coreboot' <[email protected]> > Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count > accordingtoconfigured value > Message-ID: <7abe7f9d18144867b79b98a8c6407...@chimp> > 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 <[email protected]> > 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 <[email protected]> > To: Scott Duplichan <[email protected]> > Cc: 'Arne Georg Gleditsch' <[email protected]>, 'Coreboot' > <[email protected]> > Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count > according toconfigured value > Message-ID: <[email protected]> > 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 > > -- > http://www.hailfinger.org/ > > > > > ------------------------------ > > Message: 4 > Date: Tue, 19 Oct 2010 13:34:20 -0500 > From: "Scott Duplichan" <[email protected]> > To: "'Carl-Daniel Hailfinger'" <[email protected]> > Cc: 'Arne Georg Gleditsch' <[email protected]>, 'Coreboot' > <[email protected]> > Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count > according toconfigured value > Message-ID: <e22bfe2842734c9d87b56fb2388a7...@m3a78> > Content-Type: text/plain; charset="us-ascii" > > -----Original Message----- > From: [email protected] [mailto:[email protected]] > 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 <[email protected]> > To: Scott Duplichan <[email protected]> > Cc: Coreboot <[email protected]> > Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count > according toconfigured value > Message-ID: > <[email protected]> > Content-Type: text/plain; charset=ISO-8859-1 > > On Tue, Oct 19, 2010 at 12:34 PM, Scott Duplichan <[email protected]> > wrote: > > -----Original Message----- > > From: [email protected] [mailto: > [email protected]] 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" <[email protected]> > To: "'Myles Watson'" <[email protected]> > Cc: 'Coreboot' <[email protected]> > Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count > according toconfigured value > Message-ID: <6d1ad5eccb184c52b72a0f9a7ce30...@m3a78> > 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 <[email protected] > > 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: > > -------------- next part -------------- > An embedded and charset-unspecified text was scrubbed... > Name: mmconf-patch-1.txt > URL: < > http://www.coreboot.org/pipermail/coreboot/attachments/20101019/1122fa41/attachment-0001.txt > > > > ------------------------------ > > Message: 7 > Date: Tue, 19 Oct 2010 13:03:13 -0600 > From: "Myles Watson" <[email protected]> > To: "'Scott Duplichan'" <[email protected]> > Cc: 'Coreboot' <[email protected]> > Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count > according toconfigured value > Message-ID: <07f0b79d477647709f8349c9b2053...@chimp> > Content-Type: text/plain; charset="us-ascii" > > > 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 <[email protected] > Acked-by: Myles Watson <[email protected]> > > Thanks, > Myles > > > > > > ------------------------------ > > _______________________________________________ > coreboot mailing list > [email protected] > http://www.coreboot.org/mailman/listinfo/coreboot > > End of coreboot Digest, Vol 68, Issue 108 > ***************************************** >
-- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
