On 02-Oct-18 1:35 PM, Jeff Guo wrote:
This patch implements the ops for the PCI bus sigbus handler. It finds the
PCI device that is being hot-unplugged and calls the relevant ops of the
hot-unplug handler to handle the hot-unplug failure of the device.

Signed-off-by: Jeff Guo <jia....@intel.com>
Acked-by: Shaopeng He <shaopeng...@intel.com>
---
v12->v11:
no change.
---
  drivers/bus/pci/pci_common.c | 53 ++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 53 insertions(+)

diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index d286234..f313fe9 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -405,6 +405,36 @@ pci_find_device(const struct rte_device *start, 
rte_dev_cmp_t cmp,
        return NULL;
  }
+/**
+ * find the device which encounter the failure, by iterate over all device on
+ * PCI bus to check if the memory failure address is located in the range
+ * of the BARs of the device.
+ */
+static struct rte_pci_device *
+pci_find_device_by_addr(const void *failure_addr)
+{
+       struct rte_pci_device *pdev = NULL;
+       int i;
+
+       FOREACH_DEVICE_ON_PCIBUS(pdev) {
+               for (i = 0; i != RTE_DIM(pdev->mem_resource); i++) {
+                       if ((uint64_t)(uintptr_t)failure_addr >=
+                           (uint64_t)(uintptr_t)pdev->mem_resource[i].addr &&
+                           (uint64_t)(uintptr_t)failure_addr <
+                           (uint64_t)(uintptr_t)pdev->mem_resource[i].addr +
+                           pdev->mem_resource[i].len) {

You must *really* dislike local variables :) Suggested rewriting:

const void *start, *end;
size_t len;

start = pdev->mem_resource[i].addr;
len = pdev->mem_resource[i].len;
end = RTE_PTR_ADD(start, len);

if (failure_addr >= start && failure_addr < end) {
        ...
}

I think this is way clearer.

+                               RTE_LOG(INFO, EAL, "Failure address "
+                                       "%16.16"PRIx64" belongs to "
+                                       "device %s!\n",
+                                       (uint64_t)(uintptr_t)failure_addr,
+                                       pdev->device.name);

I feel like this should have DEBUG level, rather than INFO. Alternatively, if you really feel like this should be at level INFO, the message should be reworded because the word "failure" might give the wrong impression :)

(but really, i think this is info useful for debugging purposes but not interesting generally, so it should be under DEBUG IMO)

--
Thanks,
Anatoly

Reply via email to