Mark Beihoffer
Dragonfly Networks
mbeihoffer@gmail.com
mark@dragonfly-networks.com
(612)508-5128

On Tue, Oct 19, 2010 at 2:03 PM, <coreboot-request@coreboot.org> wrote:

Send coreboot mailing list submissions to
coreboot@coreboot.org

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
coreboot-request@coreboot.org

You can reach the person managing the list at
coreboot-owner@coreboot.org

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" <scott@notabs.org>
To: "'Arne Georg Gleditsch'" <arne.gleditsch@numascale.com>
Cc: 'Coreboot' <coreboot@coreboot.org>
Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count
according toconfigured value
Message-ID: <51AC17B3FD284FE6BA8547D65AC3C0C5@m3a78>
Content-Type: text/plain; charset="us-ascii"

-----Original Message-----
From: coreboot-bounces@coreboot.org [mailto:coreboot-bounces@coreboot.org] 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" <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.
]
]--
] 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 <scott@notabs.org>

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" <mylesgw@gmail.com>
To: "'Scott Duplichan'" <scott@notabs.org>, "'Arne Georg Gleditsch'"
<arne.gleditsch@numascale.com>
Cc: 'Coreboot' <coreboot@coreboot.org>
Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count
accordingtoconfigured value
Message-ID: <7ABE7F9D18144867B79B98A8C6407C8A@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 <scott@notabs.org>
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 <c-d.hailfinger.devel.2006@gmx.net>
To: Scott Duplichan <scott@notabs.org>
Cc: 'Arne Georg Gleditsch' <arne.gleditsch@numascale.com>, 'Coreboot'
<coreboot@coreboot.org>
Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count
according toconfigured value
Message-ID: <4CBDE266.2050604@gmx.net>
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" <scott@notabs.org>
To: "'Carl-Daniel Hailfinger'" <c-d.hailfinger.devel.2006@gmx.net>
Cc: 'Arne Georg Gleditsch' <arne.gleditsch@numascale.com>, 'Coreboot'
<coreboot@coreboot.org>
Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count
according toconfigured value
Message-ID: <E22BFE2842734C9D87B56FB2388A7CC4@m3a78>
Content-Type: text/plain; charset="us-ascii"

-----Original Message-----
From: coreboot-bounces@coreboot.org [mailto:coreboot-bounces@coreboot.org] 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 <mylesgw@gmail.com>
To: Scott Duplichan <scott@notabs.org>
Cc: Coreboot <coreboot@coreboot.org>
Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count
according toconfigured value
Message-ID:
<AANLkTin0CEA+NtTi_owW6ux=5bBkr2-QhSGawEwDqFx0@mail.gmail.com>
Content-Type: text/plain; charset=ISO-8859-1

On Tue, Oct 19, 2010 at 12:34 PM, Scott Duplichan <scott@notabs.org> wrote:
> -----Original Message-----
> From: coreboot-bounces@coreboot.org [mailto:coreboot-bounces@coreboot.org] 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" <scott@notabs.org>
To: "'Myles Watson'" <mylesgw@gmail.com>
Cc: 'Coreboot' <coreboot@coreboot.org>
Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count
according toconfigured value
Message-ID: <6D1AD5ECCB184C52B72A0F9A7CE30587@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 <scott@notabs.org

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" <mylesgw@gmail.com>
To: "'Scott Duplichan'" <scott@notabs.org>
Cc: 'Coreboot' <coreboot@coreboot.org>
Subject: Re: [coreboot] [PATCH] AMD F10h: set MMCONF bus count
according toconfigured value
Message-ID: <07F0B79D477647709F8349C9B2053647@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 <scott@notabs.org
Acked-by: Myles Watson <mylesgw@gmail.com>

Thanks,
Myles

------------------------------

_______________________________________________
coreboot mailing list
coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

End of coreboot Digest, Vol 68, Issue 108
*****************************************