Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41443 )
Change subject: device: Add support for resource allocator v4 ......................................................................
Patch Set 6:
(1 comment)
pci_set_resource() sets some nasty constraints here. Not sure if we should adapt it first or handle its quirks here. On one hand, we have to always mark bridge resources to be assigned. Otherwise the bridge window won't be written (and the PCI spec explicitly mentions that we have to because there is no sane default). On the other hand, if we mark a regular resource as assigned, there are no further checks if it fits (e.g. `base + size - 1 <= limit` can be violated). So we mustn't mark non- bridge resources as assigned if allocation failed.
IMHO, the best solution would be to patch pci_set_resource() to always process bridge resources if their size is 0. If it is not 0, or it's a non-bridge resource, check the assigned status. iow.
if ((!(resource->flags & IORESOURCE_BRIDGE) || resource->size > 0) && !(resource->flags & IORESOURCE_ASSIGNED)) { complain; return; }
https://review.coreboot.org/c/coreboot/+/41443/6/src/device/resource_allocat... File src/device/resource_allocator_v4.c:
https://review.coreboot.org/c/coreboot/+/41443/6/src/device/resource_allocat... PS6, Line 272: mark_resource_invalid(resource); pci_set_resource() is odd. The old allocator set IORESOURCE_ASSIGNED on purpose on this path. We can either set it here or patch pci_set_resource() to ignore the assigned status if the resource size is 0.
Both seems logically sound to me. If the size is 0, other properties (base, limit, assigned, ...) have no real meaning. (I wouldn't call it invalid, though.)