On Wed, 2013-03-20 at 10:30 -0500, Aaron Durbin wrote:
On Wed, Mar 20, 2013 at 1:10 AM, Kyösti Mälkki kyosti.malkki@gmail.com wrote:
On Tue, 2013-03-19 at 23:16 -0500, Aaron Durbin wrote:
Hi corebooters,
I'm trying to understand the reason for the existence of IORESOURCE_UMA_FB. Is this to allow one to carve out an uncacheable MTRR region for the UMA framebuffer? If so, why was that ram added as cacheable to begin with?
Thanks for the help. Full disclosure: I'd like to get rid of it and handle these concepts in a different manner.
-Aaron
Hi Aaron
Reasons are in the poor implementation of variable MTRRs and choice of defined IORESOURCE flags.
Variable MTRR routine causes excessive use of MTRRs when the cacheable resources do not add to powers of 2. Try describing 3 GiB - 128 MiB cacheable memory, and current variable MTRR routines might use 5 MTRRs for that (2048+1024+512+256+128 MiB).
Understood. That 128MiB aligned UMA area met the 64MiB alignment minimum in the current code which led to sub-optimal variable MTRR usage. If I understand correctly, the only way for this scheme to work is to include 1 large resource as IORESOURCE_CACHEABLE and have a smaller IORESOURCE_UMA_FB resource. Correct?
I am not sure if you want some regions for SMM/TSEG as un-cacheable or not. I believe you could use a new type of memory resource: IORESOURCE_RESERVE | IORESOURCE_CACHEABLE
When programming MTRRs these would merge with any adjacent ram_resource(), but would be exluced from OS in coreboot table. But as that variable MTRR routine is so bad, any IORESOURCE_CACHEABLE resources must be added in order and they must not overlap.
I did quite a few changes on this topic last summer to fix issues with AMD boards with 4GB or more RAM. I believe I received enough change resistance to not touch MTRR further.
I'm not sure what 4GiB or more of RAM has to do w/ the non-optimal variable MTRR usage. However, I've run into this very issue recently (use too many MTRRs). Why wasn't a more suitable IO hole used? i.e. largest memory address below 4GiB is 2^N + UMA memory size. Were you then getting bit on the upper end (cacheable area above 4GiB) ?
There may be issues with granularity of TOLM and UMA/framebuffer registers so you do not get to select the optimal base for UMA and avoid use of un-cacheable MTRR hole. For AMD there are some hardware straps involved in setting UMA/frame-buffer base and I think the decision on TOLM is done in vendorcode.
Also see: http://review.coreboot.org/#/c/1431
Thanks for the context. I'm looking into removing IORESOURCE_IGNORE_MTRR as that is the wrong layer to deal with OS reserved address space. However, I wanted to properly handle the UMA_FB type as well. Longer term, I think we can get rid of UMA_FB too, but that will require a smarter MTRR algorithm.
Thanks again.
-Aaron
Use of IGNORE_MTRR allows you to place a resource within a previously added ram_resource(), that was used for bad_ram_resource() for sandybridge. You could get rid of that if variable MTRR routines were improved to split and merge regions the same way coreboot table does.
As for UMA_FB, I think renaming would be in order but I believe the need to carve out un-cacheable hole remains -- the base of un-cacheable region remains to be sub-optimal for some cases. Of course, if the region is un-cacheable by other rule already, this would not use additional MTRR.
Kyösti
On Wed, Mar 20, 2013 at 1:54 PM, Kyösti Mälkki kyosti.malkki@gmail.com wrote:
On Wed, 2013-03-20 at 10:30 -0500, Aaron Durbin wrote:
On Wed, Mar 20, 2013 at 1:10 AM, Kyösti Mälkki kyosti.malkki@gmail.com wrote:
On Tue, 2013-03-19 at 23:16 -0500, Aaron Durbin wrote:
Hi corebooters,
I'm trying to understand the reason for the existence of IORESOURCE_UMA_FB. Is this to allow one to carve out an uncacheable MTRR region for the UMA framebuffer? If so, why was that ram added as cacheable to begin with?
Thanks for the help. Full disclosure: I'd like to get rid of it and handle these concepts in a different manner.
-Aaron
Hi Aaron
Reasons are in the poor implementation of variable MTRRs and choice of defined IORESOURCE flags.
Variable MTRR routine causes excessive use of MTRRs when the cacheable resources do not add to powers of 2. Try describing 3 GiB - 128 MiB cacheable memory, and current variable MTRR routines might use 5 MTRRs for that (2048+1024+512+256+128 MiB).
Understood. That 128MiB aligned UMA area met the 64MiB alignment minimum in the current code which led to sub-optimal variable MTRR usage. If I understand correctly, the only way for this scheme to work is to include 1 large resource as IORESOURCE_CACHEABLE and have a smaller IORESOURCE_UMA_FB resource. Correct?
I am not sure if you want some regions for SMM/TSEG as un-cacheable or not. I believe you could use a new type of memory resource: IORESOURCE_RESERVE | IORESOURCE_CACHEABLE
When programming MTRRs these would merge with any adjacent ram_resource(), but would be exluced from OS in coreboot table. But as that variable MTRR routine is so bad, any IORESOURCE_CACHEABLE resources must be added in order and they must not overlap.
Correct. You and I are on the same page. I have 95% of such a change. I was just trying to figure out how to eliminate the UMA_FB resource type as well. Doing that actually requires a smarter MTRR algorithm as you noted.
I did quite a few changes on this topic last summer to fix issues with AMD boards with 4GB or more RAM. I believe I received enough change resistance to not touch MTRR further.
I'm not sure what 4GiB or more of RAM has to do w/ the non-optimal variable MTRR usage. However, I've run into this very issue recently (use too many MTRRs). Why wasn't a more suitable IO hole used? i.e. largest memory address below 4GiB is 2^N + UMA memory size. Were you then getting bit on the upper end (cacheable area above 4GiB) ?
There may be issues with granularity of TOLM and UMA/framebuffer registers so you do not get to select the optimal base for UMA and avoid use of un-cacheable MTRR hole. For AMD there are some hardware straps involved in setting UMA/frame-buffer base and I think the decision on TOLM is done in vendorcode.
Are you familiar with the granularity constraints? I assume we could fix AGESA if that's the vendor code you are referring to. Anyway, just a thought.
Also see: http://review.coreboot.org/#/c/1431
Thanks for the context. I'm looking into removing IORESOURCE_IGNORE_MTRR as that is the wrong layer to deal with OS reserved address space. However, I wanted to properly handle the UMA_FB type as well. Longer term, I think we can get rid of UMA_FB too, but that will require a smarter MTRR algorithm.
Thanks again.
-Aaron
Use of IGNORE_MTRR allows you to place a resource within a previously added ram_resource(), that was used for bad_ram_resource() for sandybridge. You could get rid of that if variable MTRR routines were improved to split and merge regions the same way coreboot table does.
Agreed.
As for UMA_FB, I think renaming would be in order but I believe the need to carve out un-cacheable hole remains -- the base of un-cacheable region remains to be sub-optimal for some cases. Of course, if the region is un-cacheable by other rule already, this would not use additional MTRR.
That assumes the default MTRR type is UC. We can flip the problem on its head and make the default WB. There are other games that can be played. But that's for another day.
-Aaron
-
Aaron Durbin -
Kyösti Mälkki