On 19.05.20 02:07, Furquan Shaikh wrote:
> On Sun, May 17, 2020 at 2:01 PM Nico Huber <nico.h@gmx.de> wrote:
>> I told people that I had something similar in mind. But actually, I
>> don't like it very much. I fear, if we move things aside, they can be
>> left behind and we might soon have to discuss platform deprecation
>> again.
>
> Having both old and new allocators gives us the option to move the
> chipsets/boards to the older one in case issues are identified even
> later on and not in the first pass of testing. So, I like the idea of
> having both allocators in the tree at least for a while.

ok, let's do this.

> As mentioned
> above, the old allocator is currently limited to only the AMD chipsets
> that are known to have breakages and require more than a quick fix:
> https://review.coreboot.org/c/coreboot/+/41444. The other chipsets
> that were identified as problematic last week have all been fixed
> since: https://review.coreboot.org/c/coreboot/+/41374.
>
> Another reason why I like having both in the tree is it serves as a
> quick A/B test without anyone having to revert series of patches just
> to test the old allocator.

That's a great idea. I will likely pick up the opportunity to add more,
optional things to test.

>> It would also be harder to make changes in generic code like
>> device/pci_device when one has to be sure that both allocators work
>> fine.
>
> For the most part, the rest of the code should not really be aware of
> the internals of any of the allocators. (In practice it is not always
> true). I understand that we had some assumptions in find_pci_tolm()
> and how invalid resources are handled. With the following changes:
> https://review.coreboot.org/c/coreboot/+/41524, those assumptions
> should no longer be required. We just need a set of rules saying what
> goes into the allocator and what is the state coming out.

Let's try it. We can put this in Documentation/ once we agree on the
rules.

INPUT
-----

The allocator processes, per domain sub-tree, all resources with
IORESOURCE_IO or IORESOURCE_MEM set. From here on, these are
simply referred to as *resources*.

Resources with the IORESOURCE_FIXED bit set are referred to as
*fixed resources*, resources with the IORESOURCE_BRIDGE bit set
as *bridge resources*, and resources with the IORESOURCE_ASSIGNED
bit set as *assigned resources*.

Resources with IORESOURCE_IO set must not have IORESOURCE_MEM set.

Resources that should not be changed by the allocator must be
fixed resources.

Fixed resources must be assigned resources and have a valid `base`
address pre-assigned.

A fixed resource must not overlap with any other fixed resource
within the same address space (IO / MEM), unless either of them
is a bridge resource.

A fixed bridge resource must not overlap with any other fixed
resource within the same address space and on the same bus (i.e.
the same device or its siblings), unless exactly one of them has
IORESOURCE_SUBTRACTIVE set.

OUTPUT
------

All device attributes beside non-fixed resources are left unaltered.