On Thu, Mar 17, 2016 at 05:03:46PM -0300, Guilherme G. Piccoli wrote:
>The domain/PHB field of PCI addresses has its value obtained from a
>global variable, incremented each time a new domain (represented by
>struct pci_controller) is added on the system. The domain addition
>process happens during boot or due to PCI device hotplug.
>
>As recent kernels are using predictable naming for network interfaces,
>the network stack is more tied to PCI naming. This can be a problem in
>hotplug scenarios, because PCI addresses will change if devices are
>removed and then re-added. This situation seems unusual, but it can
>happen if a user wants to replace a NIC without rebooting the machine,
>for example.
>
>This patch changes the way PCI domain values are generated: now, we use
>device-tree properties to assign fixed PHB numbers to PCI addresses
>when available (meaning pSeries and PowerNV cases). We also use a bitmap
>to allow dynamic PHB numbering when device-tree properties are not
>used. This bitmap keeps track of used PHB numbers and if a PHB is
>released (by hotplug operations for example), it allows the reuse of
>this PHB number, avoiding PCI address to change in case of device remove
>and re-add soon after. No functional changes were introduced.
>
>Signed-off-by: Guilherme G. Piccoli <gpiccoli@xxxxxxxxxxxxxxxxxx>
Apart from below minor issues to be fixed, it looks good to me.
Reviewed-by: Gavin Shan <gwshan@xxxxxxxxxxxxxxxxxx>
>---
> arch/powerpc/kernel/pci-common.c | 41 +++++++++++++++++++++++++++++++++++++---
> 1 file changed, 38 insertions(+), 3 deletions(-)
>
>diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
>index 0f7a60f..8777614 100644
>--- a/arch/powerpc/kernel/pci-common.c
>+++ b/arch/powerpc/kernel/pci-common.c
>@@ -44,8 +44,11 @@
> static DEFINE_SPINLOCK(hose_spinlock);
> LIST_HEAD(hose_list);
>
>-/* XXX kill that some day ... */
>-static int global_phb_number; /* Global phb counter */
>+/* For dynamic PHB numbering on get_phb_number(): max number of PHBs. */
>+#define MAX_PHBS 8192
>+
>+/* For dynamic PHB numbering: used/free PHBs tracking bitmap. */
>+static DECLARE_BITMAP(phb_bitmap, MAX_PHBS);
>
> /* ISA Memory physical address */
> resource_size_t isa_mem_base;
>@@ -64,6 +67,33 @@ struct dma_map_ops *get_pci_dma_ops(void)
> }
> EXPORT_SYMBOL(get_pci_dma_ops);
>
>+static int get_phb_number(struct device_node *dn)
>+{
>+ const __be64 *prop64;
>+ const __be32 *regs;
>+ int phb_id = 0;
>+
>+ /* try fixed PHB numbering first, by checking archs and reading
>+ * the respective device-tree property. */
>+ if (machine_is(pseries)) {
>+ regs = of_get_property(dn, "reg", NULL);
>+ if (regs)
>+ return (int)(be32_to_cpu(regs[1]) & 0xFFFF);
>+ } else {
>+ if (machine_is(powernv)) {
>+ prop64 = of_get_property(dn, "ibm,opal-phbid", NULL);
>+ if (prop64)
>+ return (int)(be64_to_cpup(prop64) & 0xFFFF);
>+ }
>+ }
>+
The nested statements can be merged to one with "else if (machine_is(powernv))".
>+ /* if not pSeries nor PowerNV, fallback to dynamic PHB numbering */
>+ phb_id = find_first_zero_bit(phb_bitmap, MAX_PHBS);
>+ BUG_ON(phb_id >= MAX_PHBS); /* reached maximum number of PHBs */
>+ set_bit(phb_id, phb_bitmap);
>+ return phb_id;
It's possible another CPU sets this bit before current CPU updates it, which
will cause same domain number used by two PHBs though it's very rare. I guess
it might be worthwhile to check if the bit is reserved by somebody else to
avoid the issue.
>+}
>+
> struct pci_controller *pcibios_alloc_controller(struct device_node *dev)
> {
> struct pci_controller *phb;
>@@ -72,7 +102,7 @@ struct pci_controller *pcibios_alloc_controller(struct device_node *dev)
> if (phb == NULL)
> return NULL;
> spin_lock(&hose_spinlock);
>- phb->global_number = global_phb_number++;
>+ phb->global_number = get_phb_number(dev);
> list_add_tail(&phb->list_node, &hose_list);
> spin_unlock(&hose_spinlock);
> phb->dn = dev;
>@@ -94,6 +124,11 @@ EXPORT_SYMBOL_GPL(pcibios_alloc_controller);
> void pcibios_free_controller(struct pci_controller *phb)
> {
> spin_lock(&hose_spinlock);
>+
>+ /* clear bit of phb_bitmap to allow reuse of this phb number */
>+ if (phb->global_number < MAX_PHBS)
>+ clear_bit(phb->global_number, phb_bitmap);
>+
> list_del(&phb->list_node);
> spin_unlock(&hose_spinlock);
>
Thanks,
Gavin
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
