[dpdk-dev] [PATCH] bus/pci: fix unexpected resource mapping override
Zhang, Qi Z
qi.z.zhang at intel.com
Wed Oct 3 15:03:34 CEST 2018
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas at monjalon.net]
> Sent: Sunday, September 30, 2018 4:53 PM
> To: Zhang, Qi Z <qi.z.zhang at intel.com>
> Cc: dev at dpdk.org; Burakov, Anatoly <anatoly.burakov at intel.com>; Yigit,
> Ferruh <ferruh.yigit at intel.com>; geoffrey.lv at gmail.com;
> ajit.khaparde at broadcom.com; stable at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] bus/pci: fix unexpected resource mapping
> 29/09/2018 08:43, Zhang, Qi Z:
> > From: Thomas Monjalon [mailto:thomas at monjalon.net]
> > >
> > > Hi,
> > >
> > > 03/09/2018 10:40, Qi Zhang:
> > > > When scanning an already plugged device, the virtual address of
> > > > mapped PCI resource in rte_pci_device will be overridden with 0,
> > > > that may cause driver does not work correctly.
> > >
> > > Why is it overridden with 0?
> > > Can we try to fix the root cause?
> > From my view this is place to fix the issue: "scan an already probed device
> will corrupt the PCI resource map"
> > Another option is "to prevent scan an already probed device", this can be
> implemented by adding some check before bus->scan in rte_dev_hotplug_add
> but I'm not prefer for this solution, because it's better to keep bus->scan's
> I don't understand why we are currently changing an already scanned device in
OK, this need to be figured out, due to hotplug, bus scan is unpredictable, so between two scans, something could happen
device maybe be unbound then bound with a different driver, or vf number is changed, so device information need to be updated.
but I'm not sure if resource mapping address ( read from /sys/bus/pci/devices/<pci addr>/resource) is possible be changed or not.
Though I don't have evidence that it is possible, but the patch respect the original assumption that it is possible, so I keep memmove here.
But I will not be surprise if It should be removed and the assumption is not correct.
> We could check the PCI address is known at the beginning and stop here, even
> before allocating a new rte_pci_device.
Yes we could check this at beginning, which means we need to figure out the rte_device by a pci address , then call rte_dev_is_probed(dev), I think that require another iterate on rte_pci_bus->device_list.
So, the benefit of a lazy check is we could merge this iterate with the iterate for device information update, and I don't have strong option for both options
> Why trying to override with this memmove?
comment in my first segment.
More information about the dev