From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id AA02B1CEF2 for ; Fri, 6 Apr 2018 12:57:31 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Apr 2018 03:57:30 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,415,1517904000"; d="scan'208";a="48488893" Received: from jguo15x-mobl3.ccr.corp.intel.com (HELO [10.67.68.27]) ([10.67.68.27]) by orsmga002.jf.intel.com with ESMTP; 06 Apr 2018 03:57:27 -0700 To: "Tan, Jianfeng" , "stephen@networkplumber.org" , "Richardson, Bruce" , "Yigit, Ferruh" , "Ananyev, Konstantin" , "gaetan.rivet@6wind.com" , "Wu, Jingjing" , "thomas@monjalon.net" , "motih@mellanox.com" , "Van Haaren, Harry" References: <1522779443-1932-1-git-send-email-jia.guo@intel.com> <1522779443-1932-3-git-send-email-jia.guo@intel.com> Cc: "jblunck@infradead.org" , "shreyansh.jain@nxp.com" , "dev@dpdk.org" , "Zhang, Helin" From: "Guo, Jia" Message-ID: <336c9255-de33-cac9-eb0b-59d466f0bdc7@intel.com> Date: Fri, 6 Apr 2018 18:57:27 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH V18 2/5] bus/pci: implement handle hot unplug operation X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Apr 2018 10:57:32 -0000 On 4/4/2018 1:25 PM, Tan, Jianfeng wrote: > >> -----Original Message----- >> From: Guo, Jia >> Sent: Wednesday, April 4, 2018 2:17 AM >> To: stephen@networkplumber.org; Richardson, Bruce; Yigit, Ferruh; >> Ananyev, Konstantin; gaetan.rivet@6wind.com; Wu, Jingjing; >> thomas@monjalon.net; motih@mellanox.com; Van Haaren, Harry; Tan, >> Jianfeng >> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org; Guo, >> Jia; Zhang, Helin >> Subject: [PATCH V18 2/5] bus/pci: implement handle hot unplug operation >> >> When handle device hot unplug event, remap a dummy memory to avoid >> bus read/write error. >> >> Signed-off-by: Jeff Guo >> --- >> v16->v15; >> split patch, merge some function to be simple >> --- >> drivers/bus/pci/pci_common.c | 42 >> ++++++++++++++++++++++++++++++++++++++++ >> drivers/bus/pci/pci_common_uio.c | 33 >> +++++++++++++++++++++++++++++++ >> drivers/bus/pci/private.h | 12 ++++++++++++ >> 3 files changed, 87 insertions(+) >> >> diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c >> index 2a00f36..fa077ec 100644 >> --- a/drivers/bus/pci/pci_common.c >> +++ b/drivers/bus/pci/pci_common.c >> @@ -474,6 +474,47 @@ pci_find_device(const struct rte_device *start, >> rte_dev_cmp_t cmp, >> } >> >> static int >> +pci_handle_hot_unplug(struct rte_device *dev) >> +{ >> + struct rte_pci_device *pdev; >> + int ret; >> + >> + if (dev == NULL) >> + return -EINVAL; >> + >> + pdev = RTE_DEV_TO_PCI(dev); >> + >> + /* remap resources for devices */ >> + switch (pdev->kdrv) { >> + case RTE_KDRV_VFIO: >> +#ifdef VFIO_PRESENT >> + /* TODO */ >> +#endif > What's the difference between uio and vfio? We can just fall though? the VFIO mapping is functional different with uio, and the key is i don't implement vfio hotplug right now, so let it TODO. >> + break; >> + case RTE_KDRV_IGB_UIO: >> + case RTE_KDRV_UIO_GENERIC: >> + if (rte_eal_using_phys_addrs()) { > Why do we care about if we are using physical addresses? please check with the mapping function. >> + /* map resources for devices that use uio */ >> + ret = pci_uio_remap_resource(pdev); >> + } >> + break; >> + case RTE_KDRV_NIC_UIO: >> + ret = pci_uio_remap_resource(pdev); >> + break; >> + default: >> + RTE_LOG(DEBUG, EAL, >> + " Not managed by a supported kernel driver, skipped\n"); >> + ret = 1; > -1 for such case? thanks. >> + break; >> + } >> + >> + if (ret != 0) >> + RTE_LOG(ERR, EAL, "failed to handle hot unplug of %s", >> + pdev->name); >> + return ret; >> +} >> + >> +static int >> pci_plug(struct rte_device *dev) >> { >> return pci_probe_all_drivers(RTE_DEV_TO_PCI(dev)); >> @@ -503,6 +544,7 @@ struct rte_pci_bus rte_pci_bus = { >> .unplug = pci_unplug, >> .parse = pci_parse, >> .get_iommu_class = rte_pci_get_iommu_class, >> + .handle_hot_unplug = pci_handle_hot_unplug, >> }, >> .device_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.device_list), >> .driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list), >> diff --git a/drivers/bus/pci/pci_common_uio.c >> b/drivers/bus/pci/pci_common_uio.c >> index 54bc20b..468ade4 100644 >> --- a/drivers/bus/pci/pci_common_uio.c >> +++ b/drivers/bus/pci/pci_common_uio.c >> @@ -146,6 +146,39 @@ pci_uio_unmap(struct mapped_pci_resource >> *uio_res) >> } >> } >> >> +/* remap the PCI resource of a PCI device in private virtual memory */ >> +int >> +pci_uio_remap_resource(struct rte_pci_device *dev) > Why's this function uio specific? I think we can move it to pci_common.c. not convince because not let vfio in this patch set. >> +{ >> + int i; >> + uint64_t phaddr; >> + void *map_address; >> + >> + if (dev == NULL) >> + return -1; >> + >> + /* Map all BARs */ > s/Map/Remap > >> + for (i = 0; i != PCI_MAX_RESOURCE; i++) { >> + /* skip empty BAR */ >> + phaddr = dev->mem_resource[i].phys_addr; >> + if (phaddr == 0) >> + continue; > How about just simple: > > if (dev->mem_resource[i].phys_addr == 0) > >> + pci_unmap_resource(dev->mem_resource[i].addr, >> + (size_t)dev->mem_resource[i].len); >> + map_address = pci_map_resource( >> + dev->mem_resource[i].addr, -1, 0, >> + (size_t)dev->mem_resource[i].len, >> + MAP_ANONYMOUS | MAP_FIXED); >> + if (map_address == MAP_FAILED) { >> + RTE_LOG(ERR, EAL, >> + "Cannot remap resource for device %s\n", >> dev->name); >> + return -1; >> + } >> + } >> + >> + return 0; >> +} >> + >> static struct mapped_pci_resource * >> pci_uio_find_resource(struct rte_pci_device *dev) >> { >> diff --git a/drivers/bus/pci/private.h b/drivers/bus/pci/private.h >> index 88fa587..7a862ef 100644 >> --- a/drivers/bus/pci/private.h >> +++ b/drivers/bus/pci/private.h >> @@ -173,6 +173,18 @@ void pci_uio_free_resource(struct rte_pci_device >> *dev, >> struct mapped_pci_resource *uio_res); >> >> /** >> + * remap the pci uio resource.. >> + * >> + * @param dev >> + * Point to the struct rte pci device. >> + * @return >> + * - On success, zero. >> + * - On failure, a negative value. >> + */ >> +int >> +pci_uio_remap_resource(struct rte_pci_device *dev); > If we define > >> + >> +/** >> * Map device memory to uio resource >> * >> * This function is private to EAL. >> -- >> 2.7.4