Re: [PATCH RFC v4 03/21] PCI: Enable bridge's I/O and MEM access for hotplugged devices

[Sergey Miroshnichenko <s.miroshnichenko@yadro.com>]

On 3/26/19 10:13 PM, Bjorn Helgaas wrote:
> On Mon, Mar 11, 2019 at 04:31:04PM +0300, Sergey Miroshnichenko wrote:
>> After updating the bridge window resources, the PCI_COMMAND_IO and
>> PCI_COMMAND_MEMORY bits of the bridge must be addressed as well.
>>
>> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
>> ---
>>  drivers/pci/pci.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 895201d4c9e6..69898fe5255e 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -1622,6 +1622,14 @@ static void pci_enable_bridge(struct pci_dev *dev)
>>  		pci_enable_bridge(bridge);
>>  
>>  	if (pci_is_enabled(dev)) {
>> +		int i, bars = 0;
>> +
>> +		for (i = PCI_BRIDGE_RESOURCES; i < DEVICE_COUNT_RESOURCE; i++) {
>> +			if (dev->resource[i].flags & (IORESOURCE_MEM | IORESOURCE_IO))
>> +				bars |= (1 << i);
>> +		}
>> +		do_pci_enable_device(dev, bars);
> 
> In what situation is this needed, exactly?  This code already exists
> in pci_enable_device_flags().  Why isn't that enough?
> 
> I guess maybe there's some case where we enable the bridge, then
> assign bridge windows, then enable a downstream device?
> 
> Does this fix a bug with current hotplug?
> 

Sure, this change was implemented because of the issue: pci_enable_device_flags() returns
early if the device is already pci_is_enabled(), so if a bridge was already enabled before
the hotplug event, but without MEM and/or IO being set, these bits will not be set even if
a new device wants them.

I've chosen the pci_enable_bridge() for this snippet because it recursively updates all
the parent bridges.

Serge

>>  		if (!dev->is_busmaster)
>>  			pci_set_master(dev);
>>  		mutex_unlock(&dev->enable_mutex);
>> -- 
>> 2.20.1
>>