Re: [PATCH V3 9/9] powerpc/eeh: handle VF PE properly

From: Gavin Shan <gwshan@linux.vnet.ibm.com>
To: Wei Yang <weiyang@linux.vnet.ibm.com>
Cc: Gavin Shan <gwshan@linux.vnet.ibm.com>,
	bhelgaas@google.com, linuxppc-dev@lists.ozlabs.org,
	linux-pci@vger.kernel.org
Subject: Re: [PATCH V3 9/9] powerpc/eeh: handle VF PE properly
Date: Thu, 14 May 2015 22:30:49 +1000	[thread overview]
Message-ID: <20150514123048.GB12847@gwshan> (raw)
In-Reply-To: <20150514100250.GB12847@richard>

On Thu, May 14, 2015 at 06:02:50PM +0800, Wei Yang wrote:
>On Wed, May 13, 2015 at 11:16:30AM +1000, Gavin Shan wrote:
>>> 	 * Actually, we should remove the PCI bridges as well.
>>>@@ -416,7 +450,7 @@ static void *eeh_rmv_device(void *data, void *userdata)
>>> 	driver = eeh_pcid_get(dev);
>>> 	if (driver) {
>>> 		eeh_pcid_put(dev);
>>>-		if (driver->err_handler)
>>>+		if (removed && driver->err_handler)
>>> 			return NULL;
>>> 	}
>>>
>>>@@ -425,11 +459,21 @@ static void *eeh_rmv_device(void *data, void *userdata)
>>> 		 pci_name(dev));
>>> 	edev->bus = dev->bus;
>>> 	edev->mode |= EEH_DEV_DISCONNECTED;
>>>-	(*removed)++;
>>>-
>>>-	pci_lock_rescan_remove();
>>>-	pci_stop_and_remove_bus_device(dev);
>>>-	pci_unlock_rescan_remove();
>>>+	if (removed)
>>>+		(*removed)++;
>>>+
>>>+#ifdef CONFIG_PCI_IOV
>>>+	if (edev->mode & EEH_DEV_VF) {
>>>+		pci_iov_virtfn_remove(edev->physfn, pdn->vf_index, 0);
>>>+		edev->pdev = NULL;
>>>+		pdn->pe_number = IODA_INVALID_PE;
>>
>>Setting the PE number to invalid one seems not correct because the PE
>>is still consumed by the VF's RID.
>>
>
>In commit 781a868f3136, we introduce the check of pdn->pe_number in
>pnv_pci_dma_dev_setup(). Since VFs are create/released dynamically, we need to
>delay the bind between PE and the device. Then to avoid rebind it, we check
>the pdn->pe_number. The WARN_ON() is what you suggested.
>
>So if don't clear the pe_number here, we break that rule.
>

Ok. You choose one of the following two options:

- Remove WARN_ON() in pnv_pci_dma_dev_setup(). Actually, I don't think
  it's quite correct to update PE# to VF's pdn at this function. Instead,
  it would have been done at the SRIOV enablement backend. But it's not
  related to this patchset. You may improve it later.
- Keep the code you had: set the PE# to invalid one, and put some comments
  here as below.

  /*
   * We have to set the VF PE number to invalid one, which is required
   * to plug the VF successfully.
   */

Thanks,
Gavin