Re: [PATCH] PCI: Refresh offset/stride after NumVFs is written

[Wei Yang <weiyang@linux.vnet.ibm.com>]

On Tue, Nov 25, 2014 at 10:01:08AM +1100, Gavin Shan wrote:
>On Sat, Nov 22, 2014 at 10:52:39AM +0800, Wei Yang wrote:
>>According to SR-IOV spec sec 3.3.9, 3.3.10, the NumVFs setting change will
>>affect the offset and stride. Current implementation doesn't refresh the
>>offset/stride cached in pci_sriov structure.
>>
>>This patch introduces a wrapper pci_iov_set_numvfs(), which refresh these two
>>value after NumVFs is written.
>>
>>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>>---
>> drivers/pci/iov.c |   17 +++++++++++++----
>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>
>>diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
>>index 4d109c0..c7010c5 100644
>>--- a/drivers/pci/iov.c
>>+++ b/drivers/pci/iov.c
>>@@ -31,6 +31,15 @@ static inline u8 virtfn_devfn(struct pci_dev *dev, int id)
>> 		dev->sriov->stride * id) & 0xff;
>> }
>>
>>+static inline void pci_iov_set_numvfs(struct pci_dev *dev, int nr_virtfn)
>>+{
>>+	struct pci_sriov *iov = dev->sriov;
>>+
>>+	pci_write_config_word(dev, iov->pos + PCI_SRIOV_NUM_VF, nr_virtfn);
>
>I'm suspecting writing to PCI_SRIOV_NUM_VF would take some time to take
>effect.
>
>>+	pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_OFFSET, &iov->offset);
>>+	pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_STRIDE, &iov->stride);
>>+}
>>+
>> static struct pci_bus *virtfn_add_bus(struct pci_bus *bus, int busnr)
>> {
>> 	struct pci_bus *child;
>>@@ -243,7 +252,7 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
>> 			return rc;
>> 	}
>>
>>-	pci_write_config_word(dev, iov->pos + PCI_SRIOV_NUM_VF, nr_virtfn);
>>+	pci_iov_set_numvfs(dev, nr_virtfn);
>> 	iov->ctrl |= PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE;
>> 	pci_cfg_access_lock(dev);
>> 	pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl);
>>@@ -272,7 +281,7 @@ failed:
>> 	iov->ctrl &= ~(PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE);
>> 	pci_cfg_access_lock(dev);
>> 	pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl);
>>-	pci_write_config_word(dev, iov->pos + PCI_SRIOV_NUM_VF, 0);
>>+	pci_iov_set_numvfs(dev, 0);
>> 	ssleep(1);
>
>The 1 second delay here might be for waiting VFs to be ready.
>

Hmm... so add this ssleep() in pci_iov_set_numvfs() would be better?

>> 	pci_cfg_access_unlock(dev);
>>
>>@@ -303,7 +312,7 @@ static void sriov_disable(struct pci_dev *dev)
>> 		sysfs_remove_link(&dev->dev.kobj, "dep_link");
>>
>> 	iov->num_VFs = 0;
>>-	pci_write_config_word(dev, iov->pos + PCI_SRIOV_NUM_VF, 0);
>>+	pci_iov_set_numvfs(dev, 0);
>> }
>>
>> static int sriov_init(struct pci_dev *dev, int pos)
>>@@ -439,7 +448,7 @@ static void sriov_restore_state(struct pci_dev *dev)
>> 		pci_update_resource(dev, i);
>>
>> 	pci_write_config_dword(dev, iov->pos + PCI_SRIOV_SYS_PGSIZE, iov->pgsz);
>>-	pci_write_config_word(dev, iov->pos + PCI_SRIOV_NUM_VF, iov->num_VFs);
>>+	pci_iov_set_numvfs(dev, iov->num_VFs);
>> 	pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl);
>> 	if (iov->ctrl & PCI_SRIOV_CTRL_VFE)
>> 		msleep(100);
>
>Here's another delay if PCI_SRIOV_CTRL_VFE isn't set previously.

The logic here will not be changed.

>
>Thanks,
>Gavin
>
>>-- 
>>1.7.9.5
>>
>>--
>>To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>>the body of a message to majordomo@vger.kernel.org
>>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

-- 
Richard Yang
Help you, Help me