-----Original Message-----
From: Thomas Monjalon [mailto:thomas@monjalon.net]
Sent: Sunday, September 30, 2018 4:53 PM
To: Zhang, Qi Z <redacted>
Cc: dev@dpdk.org; Burakov, Anatoly <redacted>; Yigit,
Ferruh [off-list ref]; geoffrey.lv@gmail.com;
ajit.khaparde@broadcom.com; stable@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] bus/pci: fix unexpected resource mapping
override

29/09/2018 08:43, Zhang, Qi Z:

quoted

From: Thomas Monjalon [mailto:thomas@monjalon.net]

quoted

Hi,

03/09/2018 10:40, Qi Zhang:

quoted

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"

quoted

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
independency.

I don't understand why we are currently changing an already scanned device in
pci_scan_one.

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.