From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f46.google.com (mail-pa0-f46.google.com [209.85.220.46]) by dpdk.org (Postfix) with ESMTP id D900F9AB7 for ; Tue, 3 Feb 2015 09:00:49 +0100 (CET) Received: by mail-pa0-f46.google.com with SMTP id lj1so93147563pab.5 for ; Tue, 03 Feb 2015 00:00:49 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=viZo3dzcaMId5RwpGNP+qJDygKfDGtAJIlvv1NO82fk=; b=A3K/wqKiT4mRnsOm/Hfn2H/fArjrFZr7nKgxV6DZRglVw62T6S1/ad7y7MhmqwZbqq zQs+zAYKEJzh5s9mXufh119mMQINrWGVGMDaytP8guEM3hQ3qIv1nz3V+3RWX+Y9cO87 XdbCLxJifdmBI+rMuMQCyKRx2yDbqOG6RqvmmCHaeCEq0FZ0xmKJkxuMEAXjiW6B6C0E RE4marucvTJ5YGrwzBrU+d6/Llg8UaVaGiZXmMvECcYuazNclZs+kxL694PvMh20spXC 9YwfntYfp/U569TriP6p/1cB2niXPc6+8/+XwtxMGMRmuhNfsqtBiahaPolqiuWVJGDD u8cQ== X-Gm-Message-State: ALoCoQk3MlCfo4/szaoO+zq/+iZmOV5VeniFi+zRvhZ/Ry+01D/Sw08C0Y6e5FmaF8VEgd/ICBvK X-Received: by 10.68.138.229 with SMTP id qt5mr995029pbb.62.1422950449110; Tue, 03 Feb 2015 00:00:49 -0800 (PST) Received: from [10.16.129.101] (napt.igel.co.jp. [219.106.231.132]) by mx.google.com with ESMTPSA id bx10sm1262186pab.25.2015.02.03.00.00.47 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 03 Feb 2015 00:00:48 -0800 (PST) Message-ID: <54D0802C.8020508@igel.co.jp> Date: Tue, 03 Feb 2015 17:00:44 +0900 From: Tetsuya Mukawa User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: "Qiu, Michael" , "dev@dpdk.org" References: <1421664027-17971-9-git-send-email-mukawa@igel.co.jp> <1422763322-13742-1-git-send-email-mukawa@igel.co.jp> <1422763322-13742-11-git-send-email-mukawa@igel.co.jp> <533710CFB86FA344BFBF2D6802E60286CD3AE6@SHSMSX101.ccr.corp.intel.com> <54D0497F.9080206@igel.co.jp> <533710CFB86FA344BFBF2D6802E60286CD3DE7@SHSMSX101.ccr.corp.intel.com> In-Reply-To: <533710CFB86FA344BFBF2D6802E60286CD3DE7@SHSMSX101.ccr.corp.intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v6 10/13] eal/pci: Cleanup pci driver initialization code X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 03 Feb 2015 08:00:50 -0000 On 2015/02/03 14:05, Qiu, Michael wrote: > On 2/3/2015 12:07 PM, Tetsuya Mukawa wrote: >> On 2015/02/03 11:35, Qiu, Michael wrote: >>> On 2/1/2015 12:02 PM, Tetsuya Mukawa wrote: >>>> - Add rte_eal_pci_close_one_dirver() >>>> The function is used for closing the specified driver and device. >>>> - Add pci_invoke_all_drivers() > [...] >>>> >>>> +#ifdef ENABLE_HOTPLUG >>>> +/* >>>> + * If vendor/device ID match, call the devuninit() function of the >>>> + * driver. >>>> + */ >>>> +int >>>> +rte_eal_pci_close_one_driver(struct rte_pci_driver *dr, >>>> + struct rte_pci_device *dev) >>>> +{ >>>> + struct rte_pci_id *id_table; >>>> + >>>> + if ((dr == NULL) || (dev == NULL)) >>>> + return -EINVAL; >>>> + >>>> + for (id_table = dr->id_table ; id_table->vendor_id != 0; id_table++) { >>>> + >>>> + /* check if device's identifiers match the driver's ones */ >>>> + if (id_table->vendor_id != dev->id.vendor_id && >>>> + id_table->vendor_id != PCI_ANY_ID) >>>> + continue; >>>> + if (id_table->device_id != dev->id.device_id && >>>> + id_table->device_id != PCI_ANY_ID) >>>> + continue; >>>> + if (id_table->subsystem_vendor_id != >>>> + dev->id.subsystem_vendor_id && >>>> + id_table->subsystem_vendor_id != PCI_ANY_ID) >>>> + continue; >>>> + if (id_table->subsystem_device_id != >>>> + dev->id.subsystem_device_id && >>>> + id_table->subsystem_device_id != PCI_ANY_ID) >>>> + continue; >>>> + >>>> + struct rte_pci_addr *loc = &dev->addr; >>>> + >>>> + RTE_LOG(DEBUG, EAL, >>>> + "PCI device "PCI_PRI_FMT" on NUMA socket %i\n", >>>> + loc->domain, loc->bus, loc->devid, >>>> + loc->function, dev->numa_node); >>>> + >>>> + RTE_LOG(DEBUG, EAL, " remove driver: %x:%x %s\n", >>>> + dev->id.vendor_id, dev->id.device_id, >>>> + dr->name); >>>> + >>>> + /* call the driver devuninit() function */ >>>> + if (dr->devuninit && (dr->devuninit(dr, dev) < 0)) >>>> + return -1; /* negative value is an error */ >>>> + >>>> + /* clear driver structure */ >>>> + dev->driver = NULL; >>>> + >>>> + if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) >>>> + /* unmap resources for devices that use igb_uio */ >>>> + pci_unmap_device(dev); >>> Hi, Tetsuya >>> >>> I have one question, as the code shows, in pci_unmap_device(), will >>> check pt_driver. >>> >>> But assume that, we are now try to detach a vfio device, after print out >>> a error message of unsupported, the does this port workable? >>> >>> I think this port will unworkable, am I right? >>> >>> But actually, we should keep it workable. >>> >>> My suggestion is to add a check in rte_eth_dev_check_detachable() for >>> pci_device port. >> Hi Michael, >> >> I appreciate your comment. >> In the function called "rte_eal_dev_detach_pdev()", >> "rte_eth_dev_check_detachable()" has been already checked. > What I mean is check the pt_driver for pci_dev in > rte_eth_dev_check_detachable(), so that hotplug framework will not > affect vfio devices, just as I reply in another mail. > > Current logic will affect vfio devices if try to detach( Not do the > really test, just the logic shows), am I right? Thanks, I've got your point. Yes, you are right. I will fix it. Tetsuya > Thanks, > Michael > >> But in the future, someone may want to reuse >> "rte_eal_pci_close_one_driver()". >> So I will add the checking like your suggestion. >> >> Thanks, >> Tetsuya >> >>> Thanks >>> Michael >>> >>>> + >>>> + return 0; >>>> + } >>>> + /* return positive value if driver is not found */ >>>> + return 1; >>>> +} >>>> +#else /* ENABLE_HOTPLUG */ >>>> +int >>>> +rte_eal_pci_close_one_driver(struct rte_pci_driver *dr __rte_unused, >>>> + struct rte_pci_device *dev __rte_unused) >>>> +{ >>>> + RTE_LOG(ERR, EAL, "Hotplug support isn't enabled\n"); >>>> + return -1; >>>> +} >>>> +#endif /* ENABLE_HOTPLUG */ >>>> + >>>> /* Init the PCI EAL subsystem */ >>>> int >>>> rte_eal_pci_init(void) >>