[coreboot] [PATCH] AMD MMCONF Support
mylesgw at gmail.com
Wed Sep 8 18:04:16 CEST 2010
On Wed, Sep 8, 2010 at 9:00 AM, Arne Georg Gleditsch
<arne.gleditsch at numascale.com> wrote:
> Myles Watson <mylesgw at gmail.com> writes:
>> I'm confused why you wouldn't add the resource from the northbridge
>> code. Is there a reason to have it be a mainboard resource?
> Not really, but there's no existing infrastructure for having
> add_northbridge_resources be called except by way of
> add_mainboard_resources. As far as I can tell. I think this may
> warrant changing, but preferrably as a separate step.
>> +config MMCONF_SUPPORT
>> + bool
>> + default y
>> + depends on NORTHBRIDGE_AMD_AMDFAM10
>> You could use select for MMCONF_SUPPORT. I think it should be
>> selected in the northbridge or in the mainboard, but not both.
> MMCONF_SUPPORT is selected in the northbridge, MMCONF_SUPPORT_DEFAULT is
> selected in the mainboard config to actually activate the functionality.
> This was the way I read the existing code, perhaps this two-level
> approach is not be required? Either way, I think you want to select
> this on a mainboard-by-mainboard basis, at least initially. There is a
> potential for breakage, like the one we experienced with the nvidia
>> Instead of adding the reserved area directly to the coreboot tables,
>> you should add it before resource allocation and let the rest happen
> I'm sorry, I'm not at all familiar with the resource allocation
> framework. I tried to model this on the corresponding code for relevant
> Intel mainboards. How would you change it, precisely?
Fair question. It's been a long time since I've thought about MMCONF,
but here are a couple of ways that I think it could be done:
I. Use a hard-coded address for MMCONF only for pre-RAM
1. Add a resource to read_resources in the northbridge that gives the
size but isn't fixed
2. The resource allocator will find a good place for it
3. In set resource update the address so that future accesses work
II. Use a hard-coded fixed address always
1. Add a fixed resource to read_resources in the northbridge
2. The resource allocator will avoid it
Either way, if the area needs to end up reserved in the coreboot
tables, then the code should live there. If we need a new resource
flag that says create an entry for this, then I think that would be
the way to go. I nominate IORESOURCE_RESERVE.
I started a patch that applied over yours, but I think discussing the
way to go in higher-level terms is the right first step.
Much of your patch is unrelated to this discussion. If we split the
bug fixes into a separate patch and get that committed I'd be happy to
help make this work.
More information about the coreboot