On Tue, Aug 19, 2014 at 03:37:26PM -0600, Bjorn Helgaas wrote:
>On Thu, Jul 24, 2014 at 02:22:11PM +0800, Wei Yang wrote:
>> When implementing the SR-IOV on PowerNV platform, some resource reservation
>> is
>> needed for VFs which don't exist at the bootup stage. To do the match between
>> resources and VFs, the code need to get the VF's BDF in advance.
>
>Ben started explaining this whole hardware PE/VF/etc stuff to me, but it
>hasn't all sunk in yet. We need to describe it somewhere (it sounds pretty
>involved, so maybe an extended description in Documentation/ would be
>appropriate).
Yes, this is not that easy to understand the whole stuff. I'd like to write a
file in Documentation/. By scaning the directory, I am not sure which one
would be proper, the Documentation/powerpc/ would be fine?
>
>What I'm concerned about is that PCI resource assignment is a huge mess,
>and this obviously complicates it even more. That's necessary and OK, but
>I want to at least preserve the possibility that somebody could rework it
>to make it manageable, and that means we need to know what the special
>constraints of PowerNV are.
Sure, let me try my best to explain it, my English is not that good, hope it
is understandable. :-)
>
>Code question below.
>
>> In this patch, it exports the interface to retrieve VF's BDF:
>> * Make the virtfn_bus as an interface
>> * Make the virtfn_devfn as an interface
>> * rename them with more specific name
>> * code cleanup in pci_sriov_resource_alignment()
>>
>> Signed-off-by: Wei Yang <weiy...@linux.vnet.ibm.com>
>> ---
>> drivers/pci/iov.c | 26 +++++++-------------------
>> drivers/pci/pci.h | 19 -------------------
>> include/linux/pci.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 51 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
>> index cb6f247..7566238 100644
>> --- a/drivers/pci/iov.c
>> +++ b/drivers/pci/iov.c
>> @@ -19,18 +19,6 @@
>>
>> #define VIRTFN_ID_LEN 16
>>
>> -static inline u8 virtfn_bus(struct pci_dev *dev, int id)
>> -{
>> - return dev->bus->number + ((dev->devfn + dev->sriov->offset +
>> - dev->sriov->stride * id) >> 8);
>> -}
>> -
>> -static inline u8 virtfn_devfn(struct pci_dev *dev, int id)
>> -{
>> - return (dev->devfn + dev->sriov->offset +
>> - dev->sriov->stride * id) & 0xff;
>> -}
>> -
>> static struct pci_bus *virtfn_add_bus(struct pci_bus *bus, int busnr)
>> {
>> struct pci_bus *child;
>> @@ -69,7 +57,7 @@ static int virtfn_add(struct pci_dev *dev, int id, int
>> reset)
>> struct pci_bus *bus;
>>
>> mutex_lock(&iov->dev->sriov->lock);
>> - bus = virtfn_add_bus(dev->bus, virtfn_bus(dev, id));
>> + bus = virtfn_add_bus(dev->bus, pci_iov_virtfn_bus(dev, id));
>> if (!bus)
>> goto failed;
>>
>> @@ -77,7 +65,7 @@ static int virtfn_add(struct pci_dev *dev, int id, int
>> reset)
>> if (!virtfn)
>> goto failed0;
>>
>> - virtfn->devfn = virtfn_devfn(dev, id);
>> + virtfn->devfn = pci_iov_virtfn_devfn(dev, id);
>> virtfn->vendor = dev->vendor;
>> pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_DID, &virtfn->device);
>> pci_setup_device(virtfn);
>> @@ -140,8 +128,8 @@ static void virtfn_remove(struct pci_dev *dev, int id,
>> int reset)
>> struct pci_sriov *iov = dev->sriov;
>>
>> virtfn = pci_get_domain_bus_and_slot(pci_domain_nr(dev->bus),
>> - virtfn_bus(dev, id),
>> - virtfn_devfn(dev, id));
>> + pci_iov_virtfn_bus(dev, id),
>> + pci_iov_virtfn_devfn(dev, id));
>> if (!virtfn)
>> return;
>>
>> @@ -216,7 +204,7 @@ static int sriov_enable(struct pci_dev *dev, int
>> nr_virtfn)
>> iov->offset = offset;
>> iov->stride = stride;
>>
>> - if (virtfn_bus(dev, nr_virtfn - 1) > dev->bus->busn_res.end) {
>> + if (pci_iov_virtfn_bus(dev, nr_virtfn - 1) > dev->bus->busn_res.end) {
>> dev_err(&dev->dev, "SR-IOV: bus number out of range\n");
>> return -ENOMEM;
>> }
>> @@ -516,7 +504,7 @@ resource_size_t pci_sriov_resource_alignment(struct
>> pci_dev *dev, int resno)
>> if (!reg)
>> return 0;
>>
>> - __pci_read_base(dev, type, &tmp, reg);
>> + __pci_read_base(dev, type, &tmp, reg);
>> return resource_alignment(&tmp);
>> }
>>
>> @@ -546,7 +534,7 @@ int pci_iov_bus_range(struct pci_bus *bus)
>> list_for_each_entry(dev, &bus->devices, bus_list) {
>> if (!dev->is_physfn)
>> continue;
>> - busnr = virtfn_bus(dev, dev->sriov->total_VFs - 1);
>> + busnr = pci_iov_virtfn_bus(dev, dev->sriov->total_VFs - 1);
>> if (busnr > max)
>> max = busnr;
>> }
>> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
>> index 0601890..a3158b2 100644
>> --- a/drivers/pci/pci.h
>> +++ b/drivers/pci/pci.h
>> @@ -221,25 +221,6 @@ static inline int pci_ari_enabled(struct pci_bus *bus)
>> void pci_reassigndev_resource_alignment(struct pci_dev *dev);
>> void pci_disable_bridge_window(struct pci_dev *dev);
>>
>> -/* Single Root I/O Virtualization */
>> -struct pci_sriov {
>> - int pos; /* capability position */
>> - int nres; /* number of resources */
>> - u32 cap; /* SR-IOV Capabilities */
>> - u16 ctrl; /* SR-IOV Control */
>> - u16 total_VFs; /* total VFs associated with the PF */
>> - u16 initial_VFs; /* initial VFs associated with the PF */
>> - u16 num_VFs; /* number of VFs available */
>> - u16 offset; /* first VF Routing ID offset */
>> - u16 stride; /* following VF stride */
>> - u32 pgsz; /* page size for BAR alignment */
>> - u8 link; /* Function Dependency Link */
>> - u16 driver_max_VFs; /* max num VFs driver supports */
>> - struct pci_dev *dev; /* lowest numbered PF */
>> - struct pci_dev *self; /* this PF */
>> - struct mutex lock; /* lock for VF bus */
>> -};
>> -
>> #ifdef CONFIG_PCI_ATS
>> void pci_restore_ats_state(struct pci_dev *dev);
>> #else
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 466bcd1..194db52 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -245,6 +245,27 @@ struct pci_vpd;
>> struct pci_sriov;
>> struct pci_ats;
>>
>> +/* Single Root I/O Virtualization */
>> +struct pci_sriov {
>> + int pos; /* capability position */
>> + int nres; /* number of resources */
>> + u32 cap; /* SR-IOV Capabilities */
>> + u16 ctrl; /* SR-IOV Control */
>> + u16 total_VFs; /* total VFs associated with the PF */
>> + u16 initial_VFs; /* initial VFs associated with the PF */
>> + u16 num_VFs; /* number of VFs available */
>> + u16 offset; /* first VF Routing ID offset */
>> + u16 stride; /* following VF stride */
>> + u32 pgsz; /* page size for BAR alignment */
>> + u8 link; /* Function Dependency Link */
>> + u16 driver_max_VFs; /* max num VFs driver supports */
>> + struct pci_dev *dev; /* lowest numbered PF */
>> + struct pci_dev *self; /* this PF */
>> + struct mutex lock; /* lock for VF bus */
>> + struct work_struct mtask; /* VF Migration task */
>> + u8 __iomem *mstate; /* VF Migration State Array */
>> +};
>> +
>> /*
>> * The pci_dev structure is used to describe PCI devices.
>> */
>> @@ -1616,6 +1637,21 @@ int pci_ext_cfg_avail(void);
>> void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar);
>>
>> #ifdef CONFIG_PCI_IOV
>> +static inline int pci_iov_virtfn_bus(struct pci_dev *dev, int id)
>> +{
>> + if (!dev->is_physfn)
>> + return -EINVAL;
>> + return dev->bus->number + ((dev->devfn + dev->sriov->offset +
>> + dev->sriov->stride * id) >> 8);
>> +}
>> +static inline int pci_iov_virtfn_devfn(struct pci_dev *dev, int id)
>> +{
>> + if (!dev->is_physfn)
>> + return -EINVAL;
>> + return (dev->devfn + dev->sriov->offset +
>> + dev->sriov->stride * id) & 0xff;
>> +}
>
>Do these really need to be inline? If they weren't inline, we wouldn't
>have to move struct pci_sriov to the public header file.
>
Not necessary to be inline, I think.
I will rework it and hide pci_sriov from public.
>> +
>> int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn);
>> void pci_disable_sriov(struct pci_dev *dev);
>> int pci_num_vf(struct pci_dev *dev);
>> @@ -1623,6 +1659,14 @@ int pci_vfs_assigned(struct pci_dev *dev);
>> int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs);
>> int pci_sriov_get_totalvfs(struct pci_dev *dev);
>> #else
>> +static inline int pci_iov_virtfn_bus(struct pci_dev *dev, int id)
>> +{
>> + return -ENXIO;
>> +}
>> +static inline int pci_iov_virtfn_devfn(struct pci_dev *dev, int id)
>> +{
>> + return -ENXIO;
>> +}
>> static inline int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn)
>> { return -ENODEV; }
>> static inline void pci_disable_sriov(struct pci_dev *dev) { }
>> --
>> 1.7.9.5
>>
--
Richard Yang
Help you, Help me
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
