Re: [Very RFC 06/46] powerpc/iov: Move VF pdev fixup into pcibios_fixup_iov()
On Thu, Nov 21, 2019 at 3:34 PM Alexey Kardashevskiy wrote:
>
>
>
> On 20/11/2019 12:28, Oliver O'Halloran wrote:
> > Move this out of the PHB's dma_dev_setup() callback and into the
> > ppc_md.pcibios_fixup_iov callback. This ensures that the VF PE's
> > pdev pointer is always valid for the whole time the device is
> > added the bus.
>
> Yeah it would be nice if dma setup did just dma stuff.
>
> > This isn't strictly required, but it's slightly a slightly more logical
>
> s/slightly a slightly/slightly (slightly)/ ? :)
>
>
> > place to do the fixup and it makes dma_dev_setup a bit simpler.
> >
> > Signed-off-by: Oliver O'Halloran
> > ---
> > arch/powerpc/platforms/powernv/pci-ioda.c | 35 +++
> > 1 file changed, 17 insertions(+), 18 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
> > b/arch/powerpc/platforms/powernv/pci-ioda.c
> > index 45f974258766..c6ea7a504e04 100644
> > --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> > @@ -2910,9 +2910,6 @@ static void pnv_pci_ioda_fixup_iov_resources(struct
> > pci_dev *pdev)
> > struct pci_dn *pdn;
> > int mul, total_vfs;
> >
> > - if (!pdev->is_physfn || pci_dev_is_added(pdev))
> > - return;
> > -
> > pdn = pci_get_pdn(pdev);
> > pdn->vfs_expanded = 0;
> > pdn->m64_single_mode = false;
> > @@ -2987,6 +2984,22 @@ static void pnv_pci_ioda_fixup_iov_resources(struct
> > pci_dev *pdev)
> > res->end = res->start - 1;
> > }
> > }
> > +
> > +static void pnv_pci_ioda_fixup_iov(struct pci_dev *pdev)
> > +{
> > + if (WARN_ON(pci_dev_is_added(pdev)))
> > + return;
> > +
> > + if (pdev->is_virtfn) {
> > + /* Fix the VF PE's pdev pointer */
> > + struct pnv_ioda_pe *pe = pnv_ioda_get_pe(pdev);
> > + pe->pdev = pdev;
> > +
> > + WARN_ON(!(pe->flags & PNV_IODA_PE_VF));
>
>
> return;
>
> > + } else if (pdev->is_physfn) {
>
>
>
> > + pnv_pci_ioda_fixup_iov_resources(pdev);
>
>
> and open code pnv_pci_ioda_fixup_iov_resources() right here?
pnv_pci_ioda_fixup_iov_resources() is pretty hairy so I'd rather keep
it as a separate function. I'd like to get rid of it entirely at some
point, but that's a problem for another day.
Re: [Very RFC 06/46] powerpc/iov: Move VF pdev fixup into pcibios_fixup_iov()
On Thu, Nov 21, 2019 at 6:48 PM Christoph Hellwig wrote:
>
> On Wed, Nov 20, 2019 at 12:28:19PM +1100, Oliver O'Halloran wrote:
> > Move this out of the PHB's dma_dev_setup() callback and into the
> > ppc_md.pcibios_fixup_iov callback. This ensures that the VF PE's
> > pdev pointer is always valid for the whole time the device is
> > added the bus.
> >
> > This isn't strictly required, but it's slightly a slightly more logical
> > place to do the fixup and it makes dma_dev_setup a bit simpler.
>
> Ok, this removes the code I commented on earlier, so I take my
> comment there back.
It is a bit weird. I'll re-order the two patches so we're not
shovelling around the fixup junk.
> > + if (pdev->is_virtfn) {
> > + /* Fix the VF PE's pdev pointer */
> > + struct pnv_ioda_pe *pe = pnv_ioda_get_pe(pdev);
> > + pe->pdev = pdev;
>
> Maybe add an empty line after the variable declaration?
ok
> > @@ -3641,20 +3654,6 @@ void pnv_pci_dma_dev_setup(struct pci_dev *pdev)
> > {
> > struct pci_controller *hose = pci_bus_to_host(pdev->bus);
> > struct pnv_phb *phb = hose->private_data;
> >
> > pnv_pci_ioda_dma_dev_setup(phb, pdev);
> > }
>
> Can you just merge pnv_pci_dma_dev_setup and pnv_pci_ioda_dma_dev_setup
> now?
Oh cool, looks like we can. I'll do that.
Re: [Very RFC 06/46] powerpc/iov: Move VF pdev fixup into pcibios_fixup_iov()
On Wed, Nov 20, 2019 at 12:28:19PM +1100, Oliver O'Halloran wrote:
> Move this out of the PHB's dma_dev_setup() callback and into the
> ppc_md.pcibios_fixup_iov callback. This ensures that the VF PE's
> pdev pointer is always valid for the whole time the device is
> added the bus.
>
> This isn't strictly required, but it's slightly a slightly more logical
> place to do the fixup and it makes dma_dev_setup a bit simpler.
Ok, this removes the code I commented on earlier, so I take my
comment there back.
> + if (pdev->is_virtfn) {
> + /* Fix the VF PE's pdev pointer */
> + struct pnv_ioda_pe *pe = pnv_ioda_get_pe(pdev);
> + pe->pdev = pdev;
Maybe add an empty line after the variable declaration?
> @@ -3641,20 +3654,6 @@ void pnv_pci_dma_dev_setup(struct pci_dev *pdev)
> {
> struct pci_controller *hose = pci_bus_to_host(pdev->bus);
> struct pnv_phb *phb = hose->private_data;
>
> pnv_pci_ioda_dma_dev_setup(phb, pdev);
> }
Can you just merge pnv_pci_dma_dev_setup and pnv_pci_ioda_dma_dev_setup
now?
Re: [Very RFC 06/46] powerpc/iov: Move VF pdev fixup into pcibios_fixup_iov()
On 20/11/2019 12:28, Oliver O'Halloran wrote:
> Move this out of the PHB's dma_dev_setup() callback and into the
> ppc_md.pcibios_fixup_iov callback. This ensures that the VF PE's
> pdev pointer is always valid for the whole time the device is
> added the bus.
Yeah it would be nice if dma setup did just dma stuff.
> This isn't strictly required, but it's slightly a slightly more logical
s/slightly a slightly/slightly (slightly)/ ? :)
> place to do the fixup and it makes dma_dev_setup a bit simpler.
>
> Signed-off-by: Oliver O'Halloran
> ---
> arch/powerpc/platforms/powernv/pci-ioda.c | 35 +++
> 1 file changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
> b/arch/powerpc/platforms/powernv/pci-ioda.c
> index 45f974258766..c6ea7a504e04 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -2910,9 +2910,6 @@ static void pnv_pci_ioda_fixup_iov_resources(struct
> pci_dev *pdev)
> struct pci_dn *pdn;
> int mul, total_vfs;
>
> - if (!pdev->is_physfn || pci_dev_is_added(pdev))
> - return;
> -
> pdn = pci_get_pdn(pdev);
> pdn->vfs_expanded = 0;
> pdn->m64_single_mode = false;
> @@ -2987,6 +2984,22 @@ static void pnv_pci_ioda_fixup_iov_resources(struct
> pci_dev *pdev)
> res->end = res->start - 1;
> }
> }
> +
> +static void pnv_pci_ioda_fixup_iov(struct pci_dev *pdev)
> +{
> + if (WARN_ON(pci_dev_is_added(pdev)))
> + return;
> +
> + if (pdev->is_virtfn) {
> + /* Fix the VF PE's pdev pointer */
> + struct pnv_ioda_pe *pe = pnv_ioda_get_pe(pdev);
> + pe->pdev = pdev;
> +
> + WARN_ON(!(pe->flags & PNV_IODA_PE_VF));
return;
> + } else if (pdev->is_physfn) {
> + pnv_pci_ioda_fixup_iov_resources(pdev);
and open code pnv_pci_ioda_fixup_iov_resources() right here?
Either way
Reviewed-by: Alexey Kardashevskiy
> + }
> +}
> #endif /* CONFIG_PCI_IOV */
>
> static void pnv_ioda_setup_pe_res(struct pnv_ioda_pe *pe,
> @@ -3641,20 +3654,6 @@ void pnv_pci_dma_dev_setup(struct pci_dev *pdev)
> {
> struct pci_controller *hose = pci_bus_to_host(pdev->bus);
> struct pnv_phb *phb = hose->private_data;
> -#ifdef CONFIG_PCI_IOV
> - struct pnv_ioda_pe *pe;
> -
> - /* Fix the VF pdn PE number */
> - if (pdev->is_virtfn) {
> - list_for_each_entry(pe, &phb->ioda.pe_list, list) {
> - if (pe->rid == ((pdev->bus->number << 8) |
> - (pdev->devfn & 0xff))) {
> - pe->pdev = pdev;
> - break;
> - }
> - }
> - }
> -#endif /* CONFIG_PCI_IOV */
>
> pnv_pci_ioda_dma_dev_setup(phb, pdev);
> }
> @@ -3945,7 +3944,7 @@ static void __init pnv_pci_init_ioda_phb(struct
> device_node *np,
> 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_fixup_sriov = pnv_pci_ioda_fixup_iov;
> ppc_md.pcibios_iov_resource_alignment = pnv_pci_iov_resource_alignment;
> ppc_md.pcibios_sriov_enable = pnv_pcibios_sriov_enable;
> ppc_md.pcibios_sriov_disable = pnv_pcibios_sriov_disable;
>
--
Alexey
[Very RFC 06/46] powerpc/iov: Move VF pdev fixup into pcibios_fixup_iov()
Move this out of the PHB's dma_dev_setup() callback and into the
ppc_md.pcibios_fixup_iov callback. This ensures that the VF PE's
pdev pointer is always valid for the whole time the device is
added the bus.
This isn't strictly required, but it's slightly a slightly more logical
place to do the fixup and it makes dma_dev_setup a bit simpler.
Signed-off-by: Oliver O'Halloran
---
arch/powerpc/platforms/powernv/pci-ioda.c | 35 +++
1 file changed, 17 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
b/arch/powerpc/platforms/powernv/pci-ioda.c
index 45f974258766..c6ea7a504e04 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -2910,9 +2910,6 @@ static void pnv_pci_ioda_fixup_iov_resources(struct
pci_dev *pdev)
struct pci_dn *pdn;
int mul, total_vfs;
- if (!pdev->is_physfn || pci_dev_is_added(pdev))
- return;
-
pdn = pci_get_pdn(pdev);
pdn->vfs_expanded = 0;
pdn->m64_single_mode = false;
@@ -2987,6 +2984,22 @@ static void pnv_pci_ioda_fixup_iov_resources(struct
pci_dev *pdev)
res->end = res->start - 1;
}
}
+
+static void pnv_pci_ioda_fixup_iov(struct pci_dev *pdev)
+{
+ if (WARN_ON(pci_dev_is_added(pdev)))
+ return;
+
+ if (pdev->is_virtfn) {
+ /* Fix the VF PE's pdev pointer */
+ struct pnv_ioda_pe *pe = pnv_ioda_get_pe(pdev);
+ pe->pdev = pdev;
+
+ WARN_ON(!(pe->flags & PNV_IODA_PE_VF));
+ } else if (pdev->is_physfn) {
+ pnv_pci_ioda_fixup_iov_resources(pdev);
+ }
+}
#endif /* CONFIG_PCI_IOV */
static void pnv_ioda_setup_pe_res(struct pnv_ioda_pe *pe,
@@ -3641,20 +3654,6 @@ void pnv_pci_dma_dev_setup(struct pci_dev *pdev)
{
struct pci_controller *hose = pci_bus_to_host(pdev->bus);
struct pnv_phb *phb = hose->private_data;
-#ifdef CONFIG_PCI_IOV
- struct pnv_ioda_pe *pe;
-
- /* Fix the VF pdn PE number */
- if (pdev->is_virtfn) {
- list_for_each_entry(pe, &phb->ioda.pe_list, list) {
- if (pe->rid == ((pdev->bus->number << 8) |
- (pdev->devfn & 0xff))) {
- pe->pdev = pdev;
- break;
- }
- }
- }
-#endif /* CONFIG_PCI_IOV */
pnv_pci_ioda_dma_dev_setup(phb, pdev);
}
@@ -3945,7 +3944,7 @@ static void __init pnv_pci_init_ioda_phb(struct
device_node *np,
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_fixup_sriov = pnv_pci_ioda_fixup_iov;
ppc_md.pcibios_iov_resource_alignment = pnv_pci_iov_resource_alignment;
ppc_md.pcibios_sriov_enable = pnv_pcibios_sriov_enable;
ppc_md.pcibios_sriov_disable = pnv_pcibios_sriov_disable;
--
2.21.0
