diff mbox

[v10,3/4] powerpc/powernv: Override pcibios_default_alignment() to force PCI devices to be page aligned

Message ID 1491825494-19331-4-git-send-email-elohimes@gmail.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Yongji Xie April 10, 2017, 11:58 a.m. UTC
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.

Signed-off-by: Yongji Xie <elohimes@gmail.com>
---
 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(+)

Comments

Bjorn Helgaas April 14, 2017, 3:58 p.m. UTC | #1
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 <elohimes@gmail.com>
> ---
>  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
>
Benjamin Herrenschmidt April 14, 2017, 9:52 p.m. UTC | #2
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 <elohimes@gmail.com>
> > ---
> >  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
> >
Bjorn Helgaas April 15, 2017, 4:36 p.m. UTC | #3
On Fri, Apr 14, 2017 at 4:52 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> 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
Benjamin Herrenschmidt April 15, 2017, 10:06 p.m. UTC | #4
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.
Bjorn Helgaas April 17, 2017, 3:51 p.m. UTC | #5
On Sat, Apr 15, 2017 at 5:06 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> 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.
diff mbox

Patch

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;