| Message ID | 1421288887-7765-15-git-send-email-weiyang@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Thu, Jan 15, 2015 at 10:28:04AM +0800, Wei Yang wrote: > On PowrNV platform, resource position in M64 implies the PE# the resource > belongs to. In some particular case, adjustment of a resource is necessary > to locate it to a correct position in M64. > > This patch introduces a function to shift the 'real' PF IOV BAR address > according to an offset. > > Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com> > --- > arch/powerpc/platforms/powernv/pci-ioda.c | 31 +++++++++++++++++++++++++++++ > 1 file changed, 31 insertions(+) > > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c > index 8bad2b0..62bb2eb 100644 > --- a/arch/powerpc/platforms/powernv/pci-ioda.c > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c > @@ -14,6 +14,7 @@ > #include <linux/kernel.h> > #include <linux/pci.h> > #include <linux/crash_dump.h> > +#include <linux/pci_regs.h> > #include <linux/debugfs.h> > #include <linux/delay.h> > #include <linux/string.h> > @@ -749,6 +750,36 @@ static unsigned int pnv_ioda_dma_weight(struct pci_dev *dev) > return 10; > } > > +#ifdef CONFIG_PCI_IOV > +static void pnv_pci_vf_resource_shift(struct pci_dev *dev, int offset) > +{ > + struct pci_dn *pdn = pci_get_pdn(dev); > + int i; > + struct resource *res; > + resource_size_t size; > + > + if (!dev->is_physfn) > + return; > + > + for (i = PCI_IOV_RESOURCES; i <= PCI_IOV_RESOURCE_END; i++) { > + res = &dev->resource[i]; > + if (!res->flags || !res->parent) > + continue; > + > + if (!pnv_pci_is_mem_pref_64(res->flags)) > + continue; > + > + dev_info(&dev->dev, " Shifting VF BAR %pR to\n", res); > + size = pci_iov_resource_size(dev, i); > + res->start += size*offset; It seems like you should adjust res->end, too. Am I missing something? And I'm not sure it's safe to move the resource here, because if we move it outside the bounds of the parent, we'll corrupt the resource tree. Maybe we're safe for some reason here, but it requires more analysis than I've done to prove it. > + > + dev_info(&dev->dev, " %pR\n", res); > + pci_update_resource(dev, i); > + } > + pdn->max_vfs -= offset; > +} > +#endif /* CONFIG_PCI_IOV */ > + > #if 0 > static struct pnv_ioda_pe *pnv_ioda_setup_dev_PE(struct pci_dev *dev) > { > -- > 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 -- 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
On Fri, Jan 30, 2015 at 05:08:03PM -0600, Bjorn Helgaas wrote: >On Thu, Jan 15, 2015 at 10:28:04AM +0800, Wei Yang wrote: >> On PowrNV platform, resource position in M64 implies the PE# the resource >> belongs to. In some particular case, adjustment of a resource is necessary >> to locate it to a correct position in M64. >> >> This patch introduces a function to shift the 'real' PF IOV BAR address >> according to an offset. >> >> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com> >> --- >> arch/powerpc/platforms/powernv/pci-ioda.c | 31 +++++++++++++++++++++++++++++ >> 1 file changed, 31 insertions(+) >> >> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c >> index 8bad2b0..62bb2eb 100644 >> --- a/arch/powerpc/platforms/powernv/pci-ioda.c >> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c >> @@ -14,6 +14,7 @@ >> #include <linux/kernel.h> >> #include <linux/pci.h> >> #include <linux/crash_dump.h> >> +#include <linux/pci_regs.h> >> #include <linux/debugfs.h> >> #include <linux/delay.h> >> #include <linux/string.h> >> @@ -749,6 +750,36 @@ static unsigned int pnv_ioda_dma_weight(struct pci_dev *dev) >> return 10; >> } >> >> +#ifdef CONFIG_PCI_IOV >> +static void pnv_pci_vf_resource_shift(struct pci_dev *dev, int offset) >> +{ >> + struct pci_dn *pdn = pci_get_pdn(dev); >> + int i; >> + struct resource *res; >> + resource_size_t size; >> + >> + if (!dev->is_physfn) >> + return; >> + >> + for (i = PCI_IOV_RESOURCES; i <= PCI_IOV_RESOURCE_END; i++) { >> + res = &dev->resource[i]; >> + if (!res->flags || !res->parent) >> + continue; >> + >> + if (!pnv_pci_is_mem_pref_64(res->flags)) >> + continue; >> + >> + dev_info(&dev->dev, " Shifting VF BAR %pR to\n", res); >> + size = pci_iov_resource_size(dev, i); >> + res->start += size*offset; > >It seems like you should adjust res->end, too. Am I missing something? > Bjorn, below is the mail I didn't manage to send yesterday. ------------------------------------------------------------------------------ No, what I am doing here is a little tricky. After reserve extra space for the IOV BAR, this range is marked down in the resource tree. And this function is called in pnv_pci_sriov_enable(), which is called when driver wants to enable the VF. How much the size this function want to shift depends on the PE number range is assigned in this turn. (The PE number range is recorded in the pdn->offset for the start number.) So we don't dear to extend the end address, otherwise it will overlap other devices' space. Or "Shrink" is more accurate than "Shift", we just "Shift" the start address instead of the end address. The consequence is the IOV BAR is told be sit on a new place. Previously I think this has no problem, while taking a close look at it, there is a potential problem. The real size of the IOV BAR is in hardware itself. This means when we "Shift" the start address of the IOV BAR, the end address is also "Shift". If the end address is out of the space we have reserved, the MMIO transaction in this range will be seen by two devices. (If this my understanding of this part is not correct, please let me know.) So what I need to fix is make sure after the shift, the real end address(start + total_vf * size) is still in the range of the extended IOV BAR range. Because we have reserved more space for the IOV BAR, we have some extra space to shift the IOV BAR. One thing I am not sure is could the real end address be (start + num_vf * size)? >And I'm not sure it's safe to move the resource here, because if we move it >outside the bounds of the parent, we'll corrupt the resource tree. Maybe >we're safe for some reason here, but it requires more analysis than I've >done to prove it. > >> + >> + dev_info(&dev->dev, " %pR\n", res); >> + pci_update_resource(dev, i); >> + } >> + pdn->max_vfs -= offset; >> +} >> +#endif /* CONFIG_PCI_IOV */ >> + >> #if 0 >> static struct pnv_ioda_pe *pnv_ioda_setup_dev_PE(struct pci_dev *dev) >> { >> -- >> 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
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c index 8bad2b0..62bb2eb 100644 --- a/arch/powerpc/platforms/powernv/pci-ioda.c +++ b/arch/powerpc/platforms/powernv/pci-ioda.c @@ -14,6 +14,7 @@ #include <linux/kernel.h> #include <linux/pci.h> #include <linux/crash_dump.h> +#include <linux/pci_regs.h> #include <linux/debugfs.h> #include <linux/delay.h> #include <linux/string.h> @@ -749,6 +750,36 @@ static unsigned int pnv_ioda_dma_weight(struct pci_dev *dev) return 10; } +#ifdef CONFIG_PCI_IOV +static void pnv_pci_vf_resource_shift(struct pci_dev *dev, int offset) +{ + struct pci_dn *pdn = pci_get_pdn(dev); + int i; + struct resource *res; + resource_size_t size; + + if (!dev->is_physfn) + return; + + for (i = PCI_IOV_RESOURCES; i <= PCI_IOV_RESOURCE_END; i++) { + res = &dev->resource[i]; + if (!res->flags || !res->parent) + continue; + + if (!pnv_pci_is_mem_pref_64(res->flags)) + continue; + + dev_info(&dev->dev, " Shifting VF BAR %pR to\n", res); + size = pci_iov_resource_size(dev, i); + res->start += size*offset; + + dev_info(&dev->dev, " %pR\n", res); + pci_update_resource(dev, i); + } + pdn->max_vfs -= offset; +} +#endif /* CONFIG_PCI_IOV */ + #if 0 static struct pnv_ioda_pe *pnv_ioda_setup_dev_PE(struct pci_dev *dev) {
On PowrNV platform, resource position in M64 implies the PE# the resource belongs to. In some particular case, adjustment of a resource is necessary to locate it to a correct position in M64. This patch introduces a function to shift the 'real' PF IOV BAR address according to an offset. Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com> --- arch/powerpc/platforms/powernv/pci-ioda.c | 31 +++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)