Furquan Shaikh 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:
(2 comments)
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 249: res->flags |= IORESOURCE_ASSIGNED;
Makes sense. […]
As commented below, I think we should still set IORESOURCE_ASSIGNED. It means that the resource allocator has actually assigned a value to its base -- either the value which corresponds to address space allocated to it or limit if the resource needs to be disabled.
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 […]
Revisiting this and pci_set_resource(), this is what I am thinking:
- At the end of resource allocation, all resources that have been assigned base and limit, should have IORESOURCE_ASSIGNED flag set. - This should be true for even the resources that request a size of 0. When a resource requests size 0, resource allocator should set the base = limit to ensure that it is properly disabled. Since,the resource allocator has actually assigned base (even though it it set to disabled), IORESOURCE_ASSIGNED flag should be set.
This should just work with how pci_set_resource() is written. And it is also helpful in case we really encounter a situation where pci_set_resource() is called without IORESOURCE_ASSIGNED being set, so that it can complain and highlight the issue.