[PATCH] v3 Resource allocator to v2

[Myles Watson]

This patch series makes v2 more similar to v3.  It splits resource
allocation into read, compute, allocate.  There is no more mutual recursion.

They go in this order:
1. white.diff
2. read_first.diff
3. domain_resources.diff
4. constraint.diff
5. compute_allocate.diff

It is boot tested on SimNOW and qemu.

domain_resources needs to be expanded to include other southbridges and
domains.  I just wanted early feedback rather than late.

Signed-off-by: Myles Watson

Thanks,
Myles

[Myles Watson]

On Fri, May 22, 2009 at 4:17 PM, Myles Watson  wrote:
> This patch series makes v2 more similar to v3.  It splits resource
> allocation into read, compute, allocate.  There is no more mutual recursion.
>
> They go in this order:
> white.diff
> read_first.diff
> domain_resources.diff
> constraint.diff
> compute_allocate.diff
>
> It is boot tested on SimNOW and qemu.

Now it is boot tested on Tyan s2892 as well.  There are a few changes
for multiple links.

Signed-off-by: Myles Watson

Carl-Daniel,

I think you can use this to fix your PCI config space problem.  If you
declare that BAR as a fixed resource, assigned, but not stored, I
think it will work.

Thanks,
Myles

[ron minnich]

We really need this improvement and boot testing is a good step.

Myles, thanks for this; it's a huge jump in our code base.

Acked-by: Ronald G. Minnich

So much good stuff has come in in the last 2 months in v2; quite impressive!

ron

[Myles Watson]

> Myles, thanks for this; it's a huge jump in our code base.

Thanks :)
> Acked-by: Ronald G. Minnich

I am holding off on the commit for now.  I don't want to break anyone's
board.

Remaining issues:
1. Right now I'm testing it with a reserved area for the IOAPIC in the
southbridge and a reserved area in the lpc/ISA function of the southbridge.

The downside of this is that I have to touch every southbridge to add it.  I
think it is the right place, though.

2. Kontron will break because I removed its special case from the resource
allocator.  I think with the new resource allocator it will be easy to fix,
but I'd like a Kontron tester.

Comments?

Thanks,
Myles

[Stefan Reinauer]

On 28.05.2009 17:32 Uhr, Myles Watson wrote:
> Remaining issues:
> Right now I'm testing it with a reserved area for the IOAPIC in the
>
> southbridge and a reserved area in the lpc/ISA function of the southbridge.
>
> The downside of this is that I have to touch every southbridge to add it.  I
> think it is the right place, though.

Not so much of a downside, since this is a fix for a real issue, and
puts the code where it (in my opinion) belongs.
> Kontron will break because I removed its special case from the resource
>
> allocator.  I think with the new resource allocator it will be easy to fix,
> but I'd like a Kontron tester.

I'll be glad to test... Can you suggest a fix or give a pointer on how
to fix this? If we can get rid of the PCIe hole handling hack, I'm all
for checking this in.

[Myles Watson]

On Thu, May 28, 2009 at 10:15 AM, Stefan Reinauer  wrote:
> On 28.05.2009 17:32 Uhr, Myles Watson wrote:
> > Remaining issues:
> > Right now I'm testing it with a reserved area for the IOAPIC in the
> >
> > southbridge and a reserved area in the lpc/ISA function of the southbridge.
> >
> > The downside of this is that I have to touch every southbridge to add it.  I
> > think it is the right place, though.
>
> Not so much of a downside, since this is a fix for a real issue, and
> puts the code where it (in my opinion) belongs.

Good.  I agree.
> > Kontron will break because I removed its special case from the resource
> >
> > allocator.  I think with the new resource allocator it will be easy to fix,
> > but I'd like a Kontron tester.
>
> I'll be glad to test... Can you suggest a fix or give a pointer on how
> to fix this? If we can get rid of the PCIe hole handling hack, I'm all
> for checking this in.

I think there are a couple of possible fixes depending on what the
real problem is.

If the problem is that it needs to be at a fixed address, allocate it
at the top (as high as you can before the IOAPIC) of the address
space, and mark it fixed.

I guess there are too many other possibilities for why you needed to
implement it that way, but I'd be happy to help you solve the problem.

Thanks,
Myles

[Stefan Reinauer]

On 28.05.2009 18:23 Uhr, Myles Watson wrote:
> > > Kontron will break because I removed its special case from the resource
> > >
> > > allocator.  I think with the new resource allocator it will be easy to fix,
> > > but I'd like a Kontron tester.
> >
> > I'll be glad to test... Can you suggest a fix or give a pointer on how
> > to fix this? If we can get rid of the PCIe hole handling hack, I'm all
> > for checking this in.
>
> I think there are a couple of possible fixes depending on what the
> real problem is.
>
> If the problem is that it needs to be at a fixed address, allocate it
> at the top (as high as you can before the IOAPIC) of the address
> space, and mark it fixed.

It's basically really just a fixed address memory space of a certain
fixed size that needs to be kept free by the resource allocator, very
similar to the way it needs to be done for the IOAPIC and Local APIC.

Since the PCIe BAR is part of the northbridge, I tried to tell the
resource allocator to do this in the northbridge.c code, but I couldn't
seem to figure out a way to make the (old) allocator honor it.

Stefan

[Myles Watson]

> It's basically really just a fixed address memory space of a certain fixed
> size that needs to be kept free by the resource allocator, very similar to
> the way it needs to be done for the IOAPIC and Local APIC.

Great.  That makes it easy.
> Since the PCIe BAR is part of the northbridge, I tried to tell the resource
> allocator to do this in the northbridge.c code, but I couldn't seem to
> figure out a way to make the (old) allocator honor it.

There was no check for fixed resources before.  There was a #warning
in compute_allocate_resources about it.  It just set the limits below
0xfec00000, and as long as your fixed resources were above that you
were fine.

The gotcha here is that I haven't implemented fixed resources in the
middle of the rest of the PCI memory address space.  If you declare a
fixed region 0xf4000000-0xf4000007 you can no longer use
0xf4000008-0xfec00000.  All of the PCI BARs have to fit on one side
(above or below) the fixed regions.

I don't think that's much of an issue, but it is there.

Thanks,
Myles

[Myles Watson]

On Wed, May 27, 2009 at 7:01 PM, ron minnich  wrote:
> Acked-by: Ronald G. Minnich

Rev 4394.

Thanks,
Myles

[Bao, Zheng]

In SB600, the IOAPIC is allocated in sb600_sm.c. Is there any conflict
with sb600_lpc.c?

Zheng

________________________________________
From: [e-mail]
[mailto:[e-mail]] On Behalf Of Myles Watson
Sent: Friday, July 03, 2009 2:59 AM
To: ron minnich
Cc: [e-mail]
Subject: Re: [coreboot] [PATCH] v3 Resource allocator to v2

On Wed, May 27, 2009 at 7:01 PM, ron minnich  wrote:

Acked-by: Ronald G. Minnich

Rev 4394.

Thanks,
Myles

[Myles Watson]

> In SB600, the IOAPIC is allocated in sb600_sm.c. Is there any conflict
> with sb600_lpc.c?

It was just for backward compatibility.  I put a fixed allocation at
0xfec00000 in most boards because that's where it used to be.  If there's
nothing that needs to be protected there, you can remove it.  You'd probably
need one for the local APICs for the processors, though.

Sorry for the confusion.

Thanks,
Myles

r4394 breaks on hardware (was: Re: [PATCH] v3 Resource allocator to v2)

[Uwe Hermann]

On Thu, Jul 02, 2009 at 12:58:57PM -0600, Myles Watson wrote:
> On Wed, May 27, 2009 at 7:01 PM, ron minnich  wrote:
> > Acked-by: Ronald G. Minnich
>
> Rev 4394.

This patch seems to cause trouble, at least on the Kontron board where I
tested recent trunk, but probably on other systems also I'd guess.

r4393 works fine, r4394 is the first broken revision.

What I see is a module not loading correctly (r8169), and a hard hang
later in the boot process (after blacklisting the r8169 module). It
seems the hang is caused by intel_agp or agpgart, not sure.

Attached is a log from the Kontron board using both r4393 and r4394.
The diff shows quite a lot of resource allocation changes, not sure
which of those is relevant and is causing the problems.

Uwe.

r4394 breaks on hardware (was: Re: [PATCH] v3 Resource allocator to v2)

[Myles Watson]

> > Rev 4394.
>
> This patch seems to cause trouble, at least on the Kontron board where I
> tested recent trunk, but probably on other systems also I'd guess.

It could be, but the problem you're seeing is Kontron-specific.

Adding PCIe enhanced config space BAR 0xf0000000-0xf4000000.

This BAR is added in a way that the allocator doesn't understand.  That was
done on purpose because the old allocator didn't avoid fixed resources.
There are several proposed fixes, but it isn't clear what the best one is.

You can chime in.  Here are some possible solutions:
1. Let the allocator place the BAR.
    Care would need to be taken when the BAR was set and the value would
have to be passed to the ACPI code.

2. Mark the resource as fixed and let the allocator avoid it.
    Either you lose a significant portion of the address space or you
have to move it.  I think you could move it to 0xf8000000-0xfc000000.

3. Add a PCI hole option to the resource allocator and manually allocate
things that you want to live there.
> r4393 works fine, r4394 is the first broken revision.

Sorry about that.

Thanks,
Myles

r4394 breaks on hardware (was: Re: [PATCH] v3 Resource allocator to v2)

[Uwe Hermann]

On Sat, Jul 04, 2009 at 07:41:09PM -0600, Myles Watson wrote:
> > > Rev 4394.
> >
> > This patch seems to cause trouble, at least on the Kontron board where I
> > tested recent trunk, but probably on other systems also I'd guess.
>
> It could be, but the problem you're seeing is Kontron-specific.

Hm, possible, but then there are other issues in the new code I guess.
Here are two dumps of r4393/r4394 from the MS-6178 (i810) board.

The first one works fine and shows a FILO menu (on serial), the second
one contains lots of "!! Resource didn't fit !!" lines among other
things, and I see a totally garbled or blank FILO screen (on serial) and
it seems to hang at that point.
> Adding PCIe enhanced config space BAR 0xf0000000-0xf4000000.
>
> This BAR is added in a way that the allocator doesn't understand.  That was
> done on purpose because the old allocator didn't avoid fixed resources.
> There are several proposed fixes, but it isn't clear what the best one is.
>
> You can chime in.

Not really, I don't know enough about this stuff to post useful comments, sorry.
> Let the allocator place the BAR.
> Care would need to be taken when the BAR was set and the value would
>
> have to be passed to the ACPI code.
> Mark the resource as fixed and let the allocator avoid it.
> Either you lose a significant portion of the address space or you
>
> have to move it.  I think you could move it to 0xf8000000-0xfc000000.
> Add a PCI hole option to the resource allocator and manually allocate
>
> things that you want to live there.

Uwe.

r4394 breaks on hardware (was: Re: [PATCH] v3 Resource allocator to v2)

[Myles Watson]

> -----Original Message-----
> From: Uwe Hermann [mailto:[e-mail]]
> Sent: Sunday, July 05, 2009 5:14 PM
> To: Myles Watson
> Cc: 'ron minnich'; [e-mail]
> Subject: Re: r4394 breaks on hardware (was: Re: [coreboot] [PATCH] v3
> Resource allocator to v2)
>
> On Sat, Jul 04, 2009 at 07:41:09PM -0600, Myles Watson wrote:
> > > > Rev 4394.
> > >
> > > This patch seems to cause trouble, at least on the Kontron board where
>
> I
> > > tested recent trunk, but probably on other systems also I'd guess.
> >
> > It could be, but the problem you're seeing is Kontron-specific.
>
> Hm, possible, but then there are other issues in the new code I guess.
> Here are two dumps of r4393/r4394 from the MS-6178 (i810) board.

Thanks for testing.  That breakage is different.
> The first one works fine and shows a FILO menu (on serial), the second
> one contains lots of "!! Resource didn't fit !!" lines among other
> things, and I see a totally garbled or blank FILO screen (on serial) and
> it seems to hang at that point.

PNP: 002e.6 resource base 0 size 8 align 3 gran 3 limit 7ff flags 100
index 60
    PNP: 002e.6 resource base 0 size 1 align 0 gran 0 limit 0 flags 400
index 70

The problem here is that these two resources are not fixed.  Since the limit
is set to 7ff on 2e.6 @ 60, the resource allocator tries to fit everything
into 0-7ff.  If you make this resource fixed, or remove them, the problem
will disappear.

In your old log, both of those resources end up at 0, which is not good.

Thanks,

Myles
> > You can chime in.
>
> Not really, I don't know enough about this stuff to post useful comments,
> sorry.

The point of the new allocator was to make it easier to debug and fix
resource allocation problems.  Hopefully it is less magical than it was, but
I'm happy to help fix all the problems that come up.

Thanks,
Myles

r4394 breaks on hardware (was: Re: [PATCH] v3 Resource allocator to v2)

[Myles Watson]

> PNP: 002e.6 resource base 0 size 8 align 3 gran 3 limit 7ff flags 100
> index 60
>    PNP: 002e.6 resource base 0 size 1 align 0 gran 0 limit 0 flags 400
> index 70
>
> The problem here is that these two resources are not fixed.  Since the
> limit
> is set to 7ff on 2e.6 @ 60, the resource allocator tries to fit everything
> into 0-7ff.  If you make this resource fixed, or remove them, the problem
> will disappear.

It looks like 2e.6 is Consumer IR, which is a TODO in the Config.lb.  Could
you try changing it to:

device pnp 2e.6 off end

I think that will fix it for you.  Otherwise you could fill in the correct
values if you want it to be enabled.  If that doesn't work, send me the log
and we'll look again.

Thanks,
Myles

r4394 breaks on hardware (was: Re: [PATCH] v3 Resource allocator to v2)

[Uwe Hermann]

On Mon, Jul 06, 2009 at 09:25:27AM -0600, Myles Watson wrote:
> > PNP: 002e.6 resource base 0 size 8 align 3 gran 3 limit 7ff flags 100
> > index 60
> >    PNP: 002e.6 resource base 0 size 1 align 0 gran 0 limit 0 flags 400
> > index 70
> >
> > The problem here is that these two resources are not fixed.  Since the
> > limit
> > is set to 7ff on 2e.6 @ 60, the resource allocator tries to fit everything
> > into 0-7ff.  If you make this resource fixed, or remove them, the problem
> > will disappear.
>
> It looks like 2e.6 is Consumer IR, which is a TODO in the Config.lb.  Could
> you try changing it to:
>
> device pnp 2e.6 off end

Yep, this fixes it indeed, thanks. Fix committed in r4423.

I attached a diff before/after the fix for reference.

Uwe.

r4394 breaks on hardware (was: Re: [PATCH] v3 Resource allocator to v2)

[Myles Watson]

> > It looks like 2e.6 is Consumer IR, which is a TODO in the Config.lb.
>
> Could
> > you try changing it to:
> >
> > device pnp 2e.6 off end
>
> Yep, this fixes it indeed, thanks. Fix committed in r4423.

Great!  Thanks for testing it.
> I attached a diff before/after the fix for reference.

Looks good.

Thanks,
Myles

r4394 breaks on hardware (was: Re: [PATCH] v3 Resource allocator to v2)

[ron minnich]

On Sat, Jul 4, 2009 at 6:41 PM, Myles Watson wrote:
> Mark the resource as fixed and let the allocator avoid it.
>
> Either you lose a significant portion of the address space or you
> have to move it.  I think you could move it to 0xf8000000-0xfc000000.

this seems the most reasonable to me. But we'll have to see what
patrick and stefan say.

ron

[Myles Watson]

On Wed, May 27, 2009 at 4:25 PM, Myles Watson  wrote:
> On Fri, May 22, 2009 at 4:17 PM, Myles Watson  wrote:
> > This patch series makes v2 more similar to v3.  It splits resource
> > allocation into read, compute, allocate.  There is no more mutual recursion.
> >
> > They go in this order:
> > white.diff
> > read_first.diff
> > domain_resources.diff

3.a domain_functions.diff
   3.b reserved_regions.diff
> > constraint.diff
> > compute_allocate.diff

I've added reserved regions for the IOAPICs and flash parts, and
removed duplicate pci_domain_read_resources and pci_domain_scan_bus
functions.

I know I've missed some southbridges.  The ones I did were the ones
that already had reserved regions because it was simpler to figure
those out.  If you don't see your southbridge in this list, you could
add it or let me know what you think should be reserved.

grep Index patches/reserved_regions.diff
Index: cbv2/src/southbridge/amd/sb600/sb600_lpc.c
Index: cbv2/src/southbridge/broadcom/bcm5785/bcm5785_lpc.c
Index: cbv2/src/southbridge/broadcom/bcm5785/bcm5785_sb_pci_main.c
Index: cbv2/src/southbridge/intel/esb6300/esb6300_lpc.c
Index: cbv2/src/southbridge/intel/i3100/i3100_lpc.c
Index: cbv2/src/southbridge/intel/i82801dbm/i82801dbm_lpc.c
Index: cbv2/src/southbridge/intel/i82801xx/i82801xx_lpc.c
Index: cbv2/src/southbridge/sis/sis966/sis966_lpc.c
Index: cbv2/src/southbridge/intel/i82801ca/i82801ca_lpc.c
Index: cbv2/src/southbridge/intel/i82801er/i82801er_lpc.c
Index: cbv2/src/southbridge/intel/i82371eb/i82371eb_isa.c

Here's the rest from the earlier patch.
        grep Index patches/domain_resources.diff | grep south
Index: cbv2/src/southbridge/intel/i82371eb/i82371eb_isa.c
Index: cbv2/src/southbridge/amd/amd8111/amd8111_lpc.c
Index: cbv2/src/southbridge/intel/i82801gx/i82801gx_lpc.c
Index: cbv2/src/southbridge/nvidia/ck804/ck804_lpc.c
Index: cbv2/src/southbridge/nvidia/mcp55/mcp55_lpc.c
Index: cbv2/src/southbridge/nvidia/ck804/ck804_pci.c
Index: cbv2/src/southbridge/nvidia/mcp55/mcp55_pci.c

Signed-off-by: Myles Watson

Thanks,
Myles

[Carl-Daniel Hailfinger]

On 28.05.2009 00:25, Myles Watson wrote:
> On Fri, May 22, 2009 at 4:17 PM, Myles Watson  wrote:
> > This patch series makes v2 more similar to v3.  It splits resource
> > allocation into read, compute, allocate.  There is no more mutual recursion.
> >
> > They go in this order:
> > white.diff
> > read_first.diff
> > domain_resources.diff
> > constraint.diff
> > compute_allocate.diff
> >
> > It is boot tested on SimNOW and qemu.
>
> Now it is boot tested on Tyan s2892 as well.  There are a few changes
> for multiple links.
>
> Signed-off-by: Myles Watson
>
> Carl-Daniel,
>
> I think you can use this to fix your PCI config space problem.  If you
> declare that BAR as a fixed resource, assigned, but not stored, I
> think it will work.

Wow, that's indeed neat. Thanks! I'll try as soon as I have access to
the hardware again.

Regards,
Carl-Daniel