Re: [PATCH v10 3/4] powerpc/powernv: Override pcibios_default_alignment() to force PCI devices to be page aligned
On Sat, Apr 15, 2017 at 5:06 PM, Benjamin Herrenschmidtwrote: > On Sat, 2017-04-15 at 11:36 -0500, Bjorn Helgaas wrote: >> > I agree in principle. I'm surprised that PowerPC is the only one >> > interested here though, what about other platforms who want to use >> > KVM and PCI pass-through and use Linux to assign BARs ? >> >> If I understand correctly, the problem is with BARs smaller than a >> page, and this happens more on PowerPC because larger page sizes are >> more common there. > > Yes, it happens "more". That doesn't mean it doesn't happen at all on > others :-) Anyway, I'm not objecting, just surprised. Yeah, I agree. Maybe the devices interesting for pass-through tend to have BARs of least 4KB? Those would never be a problem on x86. But I have absolutely no data either way.
Re: [PATCH v10 3/4] powerpc/powernv: Override pcibios_default_alignment() to force PCI devices to be page aligned
On Sat, 2017-04-15 at 11:36 -0500, Bjorn Helgaas wrote: > > I agree in principle. I'm surprised that PowerPC is the only one > > interested here though, what about other platforms who want to use > > KVM and PCI pass-through and use Linux to assign BARs ? > > If I understand correctly, the problem is with BARs smaller than a > page, and this happens more on PowerPC because larger page sizes are > more common there. Yes, it happens "more". That doesn't mean it doesn't happen at all on others :-) Anyway, I'm not objecting, just surprised. Cheers, Ben.
Re: [PATCH v10 3/4] powerpc/powernv: Override pcibios_default_alignment() to force PCI devices to be page aligned
On Fri, Apr 14, 2017 at 4:52 PM, Benjamin Herrenschmidtwrote: > On Fri, 2017-04-14 at 10:58 -0500, Bjorn Helgaas wrote: >> On Mon, Apr 10, 2017 at 07:58:13PM +0800, Yongji Xie wrote: >> > This overrides pcibios_default_alignment() to set default alignment >> > to PAGE_SIZE for all PCI devices on PowerNV platform. Thus sub-page >> > BARs would not share a page and could be mapped into guest when >> > VFIO >> > passthrough them. >> >> Thanks for doing this. This looks like a reasonable strategy to me, >> but it would be good to get a powerpc ack for it. > > I agree in principle. I'm surprised that PowerPC is the only one > interested here though, what about other platforms who want to use > KVM and PCI pass-through and use Linux to assign BARs ? If I understand correctly, the problem is with BARs smaller than a page, and this happens more on PowerPC because larger page sizes are more common there. Bjorn
Re: [PATCH v10 3/4] powerpc/powernv: Override pcibios_default_alignment() to force PCI devices to be page aligned
On Fri, 2017-04-14 at 10:58 -0500, Bjorn Helgaas wrote: > On Mon, Apr 10, 2017 at 07:58:13PM +0800, Yongji Xie wrote: > > This overrides pcibios_default_alignment() to set default alignment > > to PAGE_SIZE for all PCI devices on PowerNV platform. Thus sub-page > > BARs would not share a page and could be mapped into guest when > > VFIO > > passthrough them. > > Thanks for doing this. This looks like a reasonable strategy to me, > but it would be good to get a powerpc ack for it. I agree in principle. I'm surprised that PowerPC is the only one interested here though, what about other platforms who want to use KVM and PCI pass-through and use Linux to assign BARs ? Cheers, Ben. > > Signed-off-by: Yongji Xie> > --- > > arch/powerpc/include/asm/machdep.h|2 ++ > > arch/powerpc/kernel/pci-common.c |8 > > arch/powerpc/platforms/powernv/pci-ioda.c |7 +++ > > 3 files changed, 17 insertions(+) > > > > diff --git a/arch/powerpc/include/asm/machdep.h > > b/arch/powerpc/include/asm/machdep.h > > index 5011b69..a82c192 100644 > > --- a/arch/powerpc/include/asm/machdep.h > > +++ b/arch/powerpc/include/asm/machdep.h > > @@ -173,6 +173,8 @@ struct machdep_calls { > > /* Called after scan and before resource survey */ > > void (*pcibios_fixup_phb)(struct pci_controller *hose); > > > > + resource_size_t (*pcibios_default_alignment)(struct > > pci_dev *); > > + > > #ifdef CONFIG_PCI_IOV > > void (*pcibios_fixup_sriov)(struct pci_dev *pdev); > > resource_size_t (*pcibios_iov_resource_alignment)(struct > > pci_dev *, int resno); > > diff --git a/arch/powerpc/kernel/pci-common.c > > b/arch/powerpc/kernel/pci-common.c > > index ffda24a..ceda574 100644 > > --- a/arch/powerpc/kernel/pci-common.c > > +++ b/arch/powerpc/kernel/pci-common.c > > @@ -233,6 +233,14 @@ void pcibios_reset_secondary_bus(struct > > pci_dev *dev) > > pci_reset_secondary_bus(dev); > > } > > > > +resource_size_t pcibios_default_alignment(struct pci_dev *pdev) > > +{ > > + if (ppc_md.pcibios_default_alignment) > > + return ppc_md.pcibios_default_alignment(pdev); > > + > > + return 0; > > +} > > + > > #ifdef CONFIG_PCI_IOV > > resource_size_t pcibios_iov_resource_alignment(struct pci_dev > > *pdev, int resno) > > { > > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c > > b/arch/powerpc/platforms/powernv/pci-ioda.c > > index e367382..354c852 100644 > > --- a/arch/powerpc/platforms/powernv/pci-ioda.c > > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c > > @@ -3297,6 +3297,11 @@ static void pnv_pci_setup_bridge(struct > > pci_bus *bus, unsigned long type) > > } > > } > > > > +static resource_size_t pnv_pci_default_alignment(struct pci_dev > > *pdev) > > +{ > > + return PAGE_SIZE; > > +} > > + > > #ifdef CONFIG_PCI_IOV > > static resource_size_t pnv_pci_iov_resource_alignment(struct > > pci_dev *pdev, > > int resno) > > @@ -3830,6 +3835,8 @@ static void __init > > pnv_pci_init_ioda_phb(struct device_node *np, > > hose->controller_ops = > > pnv_pci_ioda_controller_ops; > > } > > > > + ppc_md.pcibios_default_alignment = > > pnv_pci_default_alignment; > > + > > #ifdef CONFIG_PCI_IOV > > ppc_md.pcibios_fixup_sriov = > > pnv_pci_ioda_fixup_iov_resources; > > ppc_md.pcibios_iov_resource_alignment = > > pnv_pci_iov_resource_alignment; > > -- > > 1.7.9.5 > >
Re: [PATCH v10 3/4] powerpc/powernv: Override pcibios_default_alignment() to force PCI devices to be page aligned
On Mon, Apr 10, 2017 at 07:58:13PM +0800, Yongji Xie wrote: > This overrides pcibios_default_alignment() to set default alignment > to PAGE_SIZE for all PCI devices on PowerNV platform. Thus sub-page > BARs would not share a page and could be mapped into guest when VFIO > passthrough them. Thanks for doing this. This looks like a reasonable strategy to me, but it would be good to get a powerpc ack for it. > Signed-off-by: Yongji Xie> --- > arch/powerpc/include/asm/machdep.h|2 ++ > arch/powerpc/kernel/pci-common.c |8 > arch/powerpc/platforms/powernv/pci-ioda.c |7 +++ > 3 files changed, 17 insertions(+) > > diff --git a/arch/powerpc/include/asm/machdep.h > b/arch/powerpc/include/asm/machdep.h > index 5011b69..a82c192 100644 > --- a/arch/powerpc/include/asm/machdep.h > +++ b/arch/powerpc/include/asm/machdep.h > @@ -173,6 +173,8 @@ struct machdep_calls { > /* Called after scan and before resource survey */ > void (*pcibios_fixup_phb)(struct pci_controller *hose); > > + resource_size_t (*pcibios_default_alignment)(struct pci_dev *); > + > #ifdef CONFIG_PCI_IOV > void (*pcibios_fixup_sriov)(struct pci_dev *pdev); > resource_size_t (*pcibios_iov_resource_alignment)(struct pci_dev *, int > resno); > diff --git a/arch/powerpc/kernel/pci-common.c > b/arch/powerpc/kernel/pci-common.c > index ffda24a..ceda574 100644 > --- a/arch/powerpc/kernel/pci-common.c > +++ b/arch/powerpc/kernel/pci-common.c > @@ -233,6 +233,14 @@ void pcibios_reset_secondary_bus(struct pci_dev *dev) > pci_reset_secondary_bus(dev); > } > > +resource_size_t pcibios_default_alignment(struct pci_dev *pdev) > +{ > + if (ppc_md.pcibios_default_alignment) > + return ppc_md.pcibios_default_alignment(pdev); > + > + return 0; > +} > + > #ifdef CONFIG_PCI_IOV > resource_size_t pcibios_iov_resource_alignment(struct pci_dev *pdev, int > resno) > { > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c > b/arch/powerpc/platforms/powernv/pci-ioda.c > index e367382..354c852 100644 > --- a/arch/powerpc/platforms/powernv/pci-ioda.c > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c > @@ -3297,6 +3297,11 @@ static void pnv_pci_setup_bridge(struct pci_bus *bus, > unsigned long type) > } > } > > +static resource_size_t pnv_pci_default_alignment(struct pci_dev *pdev) > +{ > + return PAGE_SIZE; > +} > + > #ifdef CONFIG_PCI_IOV > static resource_size_t pnv_pci_iov_resource_alignment(struct pci_dev *pdev, > int resno) > @@ -3830,6 +3835,8 @@ static void __init pnv_pci_init_ioda_phb(struct > device_node *np, > hose->controller_ops = pnv_pci_ioda_controller_ops; > } > > + ppc_md.pcibios_default_alignment = pnv_pci_default_alignment; > + > #ifdef CONFIG_PCI_IOV > ppc_md.pcibios_fixup_sriov = pnv_pci_ioda_fixup_iov_resources; > ppc_md.pcibios_iov_resource_alignment = pnv_pci_iov_resource_alignment; > -- > 1.7.9.5 >