| Message ID | 1421288887-7765-4-git-send-email-weiyang@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Thu, 2015-01-15 at 10:27 +0800, Wei Yang wrote: > The alignment of PF's IOV BAR is designed to be the individual size of a > VF's BAR size. This works fine for many platforms, but on PowerNV platform > it needs some change. > > The original alignment works, since at sizing and assigning stage the > requirement is from an individual VF's BAR size instead of the PF's IOV > BAR. This is the reason for the original code to just retrieve the > individual VF BAR size as the alignment. > > On PowerNV platform, it is required to align the whole PF IOV BAR to a > hardware segment. Based on this fact, the alignment of PF's IOV BAR should > be calculated seperately. > > This patch introduces a weak pcibios_iov_resource_alignment() interface, > which gives platform a chance to implement specific method to calculate > the PF's IOV BAR alignment. While the patch is probably fine, I find the above explanation quite confusing :) From my memory (vague now) of the scheme we put in place, we need to practically reserve a portion of address space that corresponds to VF_size * Number_of_PEs. IE, it's not just the alignment that has constraints but also the size that need to be allocated. Now I suppose if we make the alignment to be the size of the M64 window and if the core also bounces the allocated size to the alignment boundary, then we are fine, but that should be explained. Cheers, Ben. > Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com> > --- > drivers/pci/iov.c | 11 ++++++++++- > include/linux/pci.h | 3 +++ > 2 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c > index 933d8cc..5f48201 100644 > --- a/drivers/pci/iov.c > +++ b/drivers/pci/iov.c > @@ -556,6 +556,12 @@ int pci_iov_resource_bar(struct pci_dev *dev, int resno) > 4 * (resno - PCI_IOV_RESOURCES); > } > > +resource_size_t __weak pcibios_iov_resource_alignment(struct pci_dev *dev, > + int resno, resource_size_t align) > +{ > + return align; > +} > + > /** > * pci_sriov_resource_alignment - get resource alignment for VF BAR > * @dev: the PCI device > @@ -570,12 +576,15 @@ resource_size_t pci_sriov_resource_alignment(struct pci_dev *dev, int resno) > { > struct resource tmp; > int reg = pci_iov_resource_bar(dev, resno); > + resource_size_t align; > > if (!reg) > return 0; > > __pci_read_base(dev, pci_bar_unknown, &tmp, reg); > - return resource_alignment(&tmp); > + align = resource_alignment(&tmp); > + > + return pcibios_iov_resource_alignment(dev, resno, align); > } > > /** > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 74ef944..ae7a7ea 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1163,6 +1163,9 @@ unsigned char pci_bus_max_busnr(struct pci_bus *bus); > void pci_setup_bridge(struct pci_bus *bus); > resource_size_t pcibios_window_alignment(struct pci_bus *bus, > unsigned long type); > +resource_size_t pcibios_iov_resource_alignment(struct pci_dev *dev, > + int resno, > + resource_size_t align); > > #define PCI_VGA_STATE_CHANGE_BRIDGE (1 << 0) > #define PCI_VGA_STATE_CHANGE_DECODES (1 << 1) -- 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 Tue, Feb 10, 2015 at 11:32:59AM +1100, Benjamin Herrenschmidt wrote: >On Thu, 2015-01-15 at 10:27 +0800, Wei Yang wrote: >> The alignment of PF's IOV BAR is designed to be the individual size of a >> VF's BAR size. This works fine for many platforms, but on PowerNV platform >> it needs some change. >> >> The original alignment works, since at sizing and assigning stage the >> requirement is from an individual VF's BAR size instead of the PF's IOV >> BAR. This is the reason for the original code to just retrieve the >> individual VF BAR size as the alignment. >> >> On PowerNV platform, it is required to align the whole PF IOV BAR to a >> hardware segment. Based on this fact, the alignment of PF's IOV BAR should >> be calculated seperately. >> >> This patch introduces a weak pcibios_iov_resource_alignment() interface, >> which gives platform a chance to implement specific method to calculate >> the PF's IOV BAR alignment. > >While the patch is probably fine, I find the above explanation quite >confusing :) > I will try to make it more clear. >>From my memory (vague now) of the scheme we put in place, we need to >practically reserve a portion of address space that corresponds to >VF_size * Number_of_PEs. IE, it's not just the alignment that has >constraints but also the size that need to be allocated. > >Now I suppose if we make the alignment to be the size of the M64 >window and if the core also bounces the allocated size to the >alignment boundary, then we are fine, but that should be explained. > The purpose of this patch is to give a chance to different archs to calculate the alignment of PF's IOV BAR. How about I move the detailed explanation on powernv platform in the following patch? And focus on what this patch does in this log? >Cheers, >Ben. > > >> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com> >> --- >> drivers/pci/iov.c | 11 ++++++++++- >> include/linux/pci.h | 3 +++ >> 2 files changed, 13 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c >> index 933d8cc..5f48201 100644 >> --- a/drivers/pci/iov.c >> +++ b/drivers/pci/iov.c >> @@ -556,6 +556,12 @@ int pci_iov_resource_bar(struct pci_dev *dev, int resno) >> 4 * (resno - PCI_IOV_RESOURCES); >> } >> >> +resource_size_t __weak pcibios_iov_resource_alignment(struct pci_dev *dev, >> + int resno, resource_size_t align) >> +{ >> + return align; >> +} >> + >> /** >> * pci_sriov_resource_alignment - get resource alignment for VF BAR >> * @dev: the PCI device >> @@ -570,12 +576,15 @@ resource_size_t pci_sriov_resource_alignment(struct pci_dev *dev, int resno) >> { >> struct resource tmp; >> int reg = pci_iov_resource_bar(dev, resno); >> + resource_size_t align; >> >> if (!reg) >> return 0; >> >> __pci_read_base(dev, pci_bar_unknown, &tmp, reg); >> - return resource_alignment(&tmp); >> + align = resource_alignment(&tmp); >> + >> + return pcibios_iov_resource_alignment(dev, resno, align); >> } >> >> /** >> diff --git a/include/linux/pci.h b/include/linux/pci.h >> index 74ef944..ae7a7ea 100644 >> --- a/include/linux/pci.h >> +++ b/include/linux/pci.h >> @@ -1163,6 +1163,9 @@ unsigned char pci_bus_max_busnr(struct pci_bus *bus); >> void pci_setup_bridge(struct pci_bus *bus); >> resource_size_t pcibios_window_alignment(struct pci_bus *bus, >> unsigned long type); >> +resource_size_t pcibios_iov_resource_alignment(struct pci_dev *dev, >> + int resno, >> + resource_size_t align); >> >> #define PCI_VGA_STATE_CHANGE_BRIDGE (1 << 0) >> #define PCI_VGA_STATE_CHANGE_DECODES (1 << 1) >
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c index 933d8cc..5f48201 100644 --- a/drivers/pci/iov.c +++ b/drivers/pci/iov.c @@ -556,6 +556,12 @@ int pci_iov_resource_bar(struct pci_dev *dev, int resno) 4 * (resno - PCI_IOV_RESOURCES); } +resource_size_t __weak pcibios_iov_resource_alignment(struct pci_dev *dev, + int resno, resource_size_t align) +{ + return align; +} + /** * pci_sriov_resource_alignment - get resource alignment for VF BAR * @dev: the PCI device @@ -570,12 +576,15 @@ resource_size_t pci_sriov_resource_alignment(struct pci_dev *dev, int resno) { struct resource tmp; int reg = pci_iov_resource_bar(dev, resno); + resource_size_t align; if (!reg) return 0; __pci_read_base(dev, pci_bar_unknown, &tmp, reg); - return resource_alignment(&tmp); + align = resource_alignment(&tmp); + + return pcibios_iov_resource_alignment(dev, resno, align); } /** diff --git a/include/linux/pci.h b/include/linux/pci.h index 74ef944..ae7a7ea 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1163,6 +1163,9 @@ unsigned char pci_bus_max_busnr(struct pci_bus *bus); void pci_setup_bridge(struct pci_bus *bus); resource_size_t pcibios_window_alignment(struct pci_bus *bus, unsigned long type); +resource_size_t pcibios_iov_resource_alignment(struct pci_dev *dev, + int resno, + resource_size_t align); #define PCI_VGA_STATE_CHANGE_BRIDGE (1 << 0) #define PCI_VGA_STATE_CHANGE_DECODES (1 << 1)
The alignment of PF's IOV BAR is designed to be the individual size of a VF's BAR size. This works fine for many platforms, but on PowerNV platform it needs some change. The original alignment works, since at sizing and assigning stage the requirement is from an individual VF's BAR size instead of the PF's IOV BAR. This is the reason for the original code to just retrieve the individual VF BAR size as the alignment. On PowerNV platform, it is required to align the whole PF IOV BAR to a hardware segment. Based on this fact, the alignment of PF's IOV BAR should be calculated seperately. This patch introduces a weak pcibios_iov_resource_alignment() interface, which gives platform a chance to implement specific method to calculate the PF's IOV BAR alignment. Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com> --- drivers/pci/iov.c | 11 ++++++++++- include/linux/pci.h | 3 +++ 2 files changed, 13 insertions(+), 1 deletion(-)