Re: [PATCH v9 08/30] PCI: Update pci_host_bridge bus resource

[Yijing Wang]

On 2015/4/8 6:42, Bjorn Helgaas wrote:
> On Fri, Apr 03, 2015 at 05:25:42PM +0800, Yijing Wang wrote:
>> From: Yijing Wang <wangyijing0307@xxxxxxxxx>
>>
>> Sometimes, the bus resource start number is not equal to
>> root bus number. For example, in pci_scan_bus(), we always
>> add the default bus resource which start bus number is 0,
>> but the root bus number callers given may != 0, so
>> we need to update pci_host_bridge bus resource, because we
>> would check whether host bridge bus resoruce is confict
>> in later patch.
> 
> It's true that pci_scan_bus() always inserts [bus 00-ff].  But I think
> that's completely bogus.  The caller of pci_scan_bus() supplies a root bus
> number X.  Any bus numbers below X are useless as far as this host bridge
> is concerned, and it's pointless to include them in the range inserted by
> pci_scan_bus().

Agree, I think it's better to use the [bus start - FF] instead of [00-FF].

> 
> I think we'd be better off if we forced every pci_scan_bus() caller to
> supply a real non-overlapping bus number range.  We probably can't do that
> easily because the arch knows the beginning bus number, but some don't know
> how to figure out the ending bus number.

It seems to have some problems in pci_scan_bus() if we have more than one pci host bridge
in a domain. Because we add the default busn_res[00-FF] to resource list in pci_scan_bus(),
and pass the resource list to pci_create_root_bus().  The pci root bus would
consume the bus range from the start to the 0xFF, and request it from the per-domain resource[00-FF].
So if we have another pci host bridge in this domain, and pass another start bus number, it would
fail when the root bus try to request bus resource from per-domain resource. Now only several archs
call pci_scan_bus(), I guess they didn't run the code in above case, so we didn't find
the bus report.

> 
> Do we have a per-domain structure that tracks the bus numbers in use in the
> domain?  It seems like if we had one, we could use that to approximate the
> end.  For example, if the arch scans three root buses, at bus 00, bus 40,
> and bus 80, we could start with the first one at [bus 00-ff], and when we
> scan the one at bus 40, we could either report a conflict (if the bus 00
> tree included a bus 40) or reduce the first range to [bus 00-3f].

Per-domain resource pci_domain_busn_res is what we are looking for.
For simplicity, we could report a conflict and fail when we find a conflict bus
resource in system.


>>  	resource_list_for_each_entry(window, resources)
>> -		if (window->res->flags & IORESOURCE_BUS)
>> +		if (window->res->flags & IORESOURCE_BUS) {
>> +			if (bus > window->res->start)
>> +				window->res->start = bus;
> 
> I see what you're trying to do here, but I think this is the wrong place to
> do it.  I'd rather figure out a way to insert something other than
> busn_resource in pci_scan_bus().  That probably means we need to
> dynamically allocate a new busn_res struct.

Allocate a new busn_res maybe unnecessary, the bus resource we added to resource list
and passed to pci_create_root_bus() will never be used again after we call
pci_bus_insert_busn_res(), we may could consider to use local busn_res in pci_scan_bus().

Thanks!
Yijing.

> 
>>  			return;
>> +		}
>>  
>>  	pr_info(
>>  	 "No busn resource found for pci%04x:%02x, will use [bus %02x-ff]\n",
>> -- 
>> 1.7.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> .
> 


-- 
Thanks!
Yijing

--
To unsubscribe from this list: send the line "unsubscribe linux-alpha" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html