<div dir="ltr">Oh, I thought you are not comfortable with the "Patch v12 10/21 PCI: Consider additional PF's IOV BAR alignment ..."<div><br></div><div>V14 is ready to send which is based on v4.0-rc1. </div></div><div class="gmail_extra"><br><div class="gmail_quote">2015-03-19 23:08 GMT+08:00 Bjorn Helgaas <span dir="ltr"><<a href="mailto:bhelgaas@google.com" target="_blank">bhelgaas@google.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Thu, Mar 12, 2015 at 09:15:17AM +0800, Wei Yang wrote:<br>
> On Wed, Mar 11, 2015 at 08:55:07AM -0500, Bjorn Helgaas wrote:<br>
> >On Wed, Mar 04, 2015 at 01:19:07PM +0800, Wei Yang wrote:<br>
> >> On PHB3, PF IOV BAR will be covered by M64 window to have better PE<br>
> >> isolation.  The total_pe number is usually different from total_VFs, which<br>
> >> can lead to a conflict between MMIO space and the PE number.<br>
> >><br>
> >> For example, if total_VFs is 128 and total_pe is 256, the second half of<br>
> >> M64 window will be part of other PCI device, which may already belong<br>
> >> to other PEs.<br>
> >><br>
> >> Prevent the conflict by reserving additional space for the PF IOV BAR,<br>
> >> which is total_pe number of VF's BAR size.<br>
> >><br>
> >> [bhelgaas: make dev_printk() output more consistent, index resource[]<br>
> >> conventionally]<br>
> >> Signed-off-by: Wei Yang <<a href="mailto:weiyang@linux.vnet.ibm.com">weiyang@linux.vnet.ibm.com</a>><br>
> >> ---<br>
> >>  arch/powerpc/include/asm/machdep.h        |    4 ++<br>
> >>  arch/powerpc/include/asm/pci-bridge.h     |    3 ++<br>
> >>  arch/powerpc/kernel/pci-common.c          |    5 +++<br>
> >>  arch/powerpc/kernel/pci-hotplug.c         |    4 ++<br>
> >>  arch/powerpc/platforms/powernv/pci-ioda.c |   61 +++++++++++++++++++++++++++++<br>
> >>  5 files changed, 77 insertions(+)<br>
> >><br>
> >> diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h<br>
> >> index c8175a3..965547c 100644<br>
> >> --- a/arch/powerpc/include/asm/machdep.h<br>
> >> +++ b/arch/powerpc/include/asm/machdep.h<br>
> >> @@ -250,6 +250,10 @@ struct machdep_calls {<br>
> >>    /* Reset the secondary bus of bridge */<br>
> >>    void  (*pcibios_reset_secondary_bus)(struct pci_dev *dev);<br>
> >><br>
> >> +#ifdef CONFIG_PCI_IOV<br>
> >> +  void (*pcibios_fixup_sriov)(struct pci_bus *bus);<br>
> >> +#endif /* CONFIG_PCI_IOV */<br>
> >> +<br>
> >>    /* Called to shutdown machine specific hardware not already controlled<br>
> >>     * by other drivers.<br>
> >>     */<br>
> >> diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h<br>
> >> index 513f8f2..de11de7 100644<br>
> >> --- a/arch/powerpc/include/asm/pci-bridge.h<br>
> >> +++ b/arch/powerpc/include/asm/pci-bridge.h<br>
> >> @@ -175,6 +175,9 @@ struct pci_dn {<br>
> >>  #define IODA_INVALID_PE           (-1)<br>
> >>  #ifdef CONFIG_PPC_POWERNV<br>
> >>    int     pe_number;<br>
> >> +#ifdef CONFIG_PCI_IOV<br>
> >> +  u16     max_vfs;                /* number of VFs IOV BAR expended */<br>
> >> +#endif /* CONFIG_PCI_IOV */<br>
> >>  #endif<br>
> >>    struct list_head child_list;<br>
> >>    struct list_head list;<br>
> >> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c<br>
> >> index 8203101..022e9fe 100644<br>
> >> --- a/arch/powerpc/kernel/pci-common.c<br>
> >> +++ b/arch/powerpc/kernel/pci-common.c<br>
> >> @@ -1646,6 +1646,11 @@ void pcibios_scan_phb(struct pci_controller *hose)<br>
> >>    if (ppc_md.pcibios_fixup_phb)<br>
> >>            ppc_md.pcibios_fixup_phb(hose);<br>
> >><br>
> >> +#ifdef CONFIG_PCI_IOV<br>
> >> +  if (ppc_md.pcibios_fixup_sriov)<br>
> >> +          ppc_md.pcibios_fixup_sriov(bus);<br>
> >> +#endif /* CONFIG_PCI_IOV */<br>
> ><br>
> >Here, and ...<br>
> ><br>
> >> +<br>
> >>    /* Configure PCI Express settings */<br>
> >>    if (bus && !pci_has_flag(PCI_PROBE_ONLY)) {<br>
> >>            struct pci_bus *child;<br>
> >> diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c<br>
> >> index 5b78917..7d238ae 100644<br>
> >> --- a/arch/powerpc/kernel/pci-hotplug.c<br>
> >> +++ b/arch/powerpc/kernel/pci-hotplug.c<br>
> >> @@ -94,6 +94,10 @@ void pcibios_add_pci_devices(struct pci_bus * bus)<br>
> >>             */<br>
> >>            slotno = PCI_SLOT(PCI_DN(dn->child)->devfn);<br>
> >>            pci_scan_slot(bus, PCI_DEVFN(slotno, 0));<br>
> >> +#ifdef CONFIG_PCI_IOV<br>
> >> +          if (ppc_md.pcibios_fixup_sriov)<br>
> >> +                  ppc_md.pcibios_fixup_sriov(bus);<br>
> >> +#endif /* CONFIG_PCI_IOV */<br>
> ><br>
> >here, you have the same code.  It's good that we now do it for hot-added<br>
> >devices as well as those present at boot.  But it's bad that it happens in<br>
> >two different paths.<br>
> ><br>
> >Isn't there some way we can unify this so the same path is used for the<br>
> >initial pcibios_scan_phb() and also the hot-add case?  Maybe call<br>
> >pcibios_fixup_sriov() from pcibios_add_device()?<br>
> ><br>
><br>
><br>
> This is a very good suggestion. I have changed this and works fine.<br>
<br>
</div></div>I was expecting a v14 series with this change.  Is it coming, or are you<br>
waiting for something else from me?<br>
<div class="HOEnZb"><div class="h5">_______________________________________________<br>
Linuxppc-dev mailing list<br>
<a href="mailto:Linuxppc-dev@lists.ozlabs.org">Linuxppc-dev@lists.ozlabs.org</a><br>
<a href="https://lists.ozlabs.org/listinfo/linuxppc-dev" target="_blank">https://lists.ozlabs.org/listinfo/linuxppc-dev</a></div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature">Richard Yang<br>Help You, Help Me</div>
</div>