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:
(1 comment)
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);
First, I don't see how `base == limit` implies or disables anything. Please explain.
I think what you are looking for is limit < base so that it is disabled as per the PCI Specification?
But please tell me how pci_set_resource() does the right thing for a non-
bridge resource. I just don't see it.
What is the definition of right thing for a non-bridge resource to be disabled? Again as per PCI specification, there are separate enable control bits that decide when to enable the decoding for I/O and mem for non-bridge resource. So, how do you really define that a resource is disabled?
My argument here is that we are bringing in the semantics of PCI devices into the resource allocator. I understand that we are dealing only with PCI devices right now, but both the old and new resource allocators are written without really making assumptions about the kind of devices that are requesting these resources. In that case, we need to come up with a notion of saying how you mark a device as invalid/disabled.
* Setting IORESOURCE_ASSIGNED indicates that the resource for the device has been considered by the resource allocator and assigned some base value.
* Setting base = limit indicates that the resource could not really be allocated any space. If you are concerned about resources requesting a size of 1,we can set base = limit and limit = base - 1 and set size = 0. I think we need to set the semantics of how resource allocator indicates to rest of the drivers that a resource is disabled.
Now, how pci_set_resource() uses it is upto that driver. For a bridge resource, it can set base and limit as per the specification. For a non-bridge resource, it can check size as 0 and decide for itself that IO and mem decoding should not be enabled. But, that shouldn't be done as part of the allocator.