On Wed, Jul 11, 2012 at 09:50:19AM -0600, Bjorn Helgaas wrote:
> On Sat, Jun 23, 2012 at 1:42 AM, Yinghai Lu <yinghai@xxxxxxxxxx> wrote:
> > When system support hotplug bridge with children hotplug slots, we need to make sure
> > that parent bridge get preallocated resource so later when device is plugged into
> > children slot, those children devices will get resource allocated.
> >
> > We do not meet this problem, because for pcie hotplug card, when acpiphp is used,
> > pci_scan_bridge will set that for us when detect hotplug bit in slot cap.
> >
> > Reported-and-tested-by: Jason Baron <jbaron@xxxxxxxxxx>
> > Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx>
> > Acked-by: Jason Baron <jbaron@xxxxxxxxxx>
> > ---
> > drivers/pci/hotplug/acpiphp_glue.c | 27 ++++++++++++++++++++++++++-
> > 1 files changed, 26 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> > index ad6fd66..0f2b72d 100644
> > --- a/drivers/pci/hotplug/acpiphp_glue.c
> > +++ b/drivers/pci/hotplug/acpiphp_glue.c
> > @@ -783,6 +783,29 @@ static void acpiphp_set_acpi_region(struct acpiphp_slot *slot)
> > }
> > }
> >
> > +static void check_hotplug_bridge(struct acpiphp_slot *slot, struct pci_dev *dev)
> > +{
> > + struct acpiphp_func *func;
> > +
> > + if (!dev->subordinate)
> > + return;
> > +
> > + /* quirk, or pcie could set it already */
> > + if (dev->is_hotplug_bridge)
> > + return;
> > +
> > + if (PCI_SLOT(dev->devfn) != slot->device)
> > + return;
> > +
> > + list_for_each_entry(func, &slot->funcs, sibling) {
> > + if (PCI_FUNC(dev->devfn) == func->function) {
> > + /* check if this bridge has ejectable slots */
> > + if ((detect_ejectable_slots(func->handle) > 0))
> > + dev->is_hotplug_bridge = 1;
> > + break;
> > + }
> > + }
> > +}
> > /**
> > * enable_device - enable, configure a slot
> > * @slot: slot to be enabled
> > @@ -817,8 +840,10 @@ static int __ref enable_device(struct acpiphp_slot *slot)
> > if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
> > dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) {
> > max = pci_scan_bridge(bus, dev, max, pass);
> > - if (pass && dev->subordinate)
> > + if (pass && dev->subordinate) {
> > + check_hotplug_bridge(slot, dev);
>
> I don't like this patch because it increases the differences between
> the hotplug drivers, rather than decreasing them.
>
> For PCI Express devices, we set dev->is_hotplug_bridge in the
> pci_setup_device() path (in set_pcie_hotplug_bridge()). I think it
> would make sense to try to expand that path to also handle SHPC and
> ACPI hotplug as well. ACPI is harder because it's not PCI-specified,
> so we'd need some sort of pcibios or other optional hook.
>
> I don't have a clear picture of how this works -- if I understand
> correctly, the situation is that we have a bridge managed by acpiphp.
> That part makes sense because the bridge is on the motherboard and can
> have a DSDT device. Now we plug something into the slot below the
> bridge. I *think* this patch handles the case where this new
> hot-added thing is also a bridge managed by acpiphp. But where does
> the ACPI device for this hot-added bridge come from? It's an
> arbitrary device the BIOS knows nothing about, so it can't be in the
> DSDT.
>
So this came up while I was developing pci bridge hotplug for qemu.
Currently, there is a top level host bus (with ACPI device definitions), where
devices can be hot-plugged. What I've done is added a second level
of hotplug pci busses (again with ACPI device definitions). Thus, we can
now hotplug a bridge into the top-level bus and then devices behind it.
Effectively increasing the hot-plug space from n -> n^2.
Before the above pci patch, the devices behind the bridge would not
configure their PCI BARs properly, since there were no i/o, mem resources
assigned to the bridge. However, with the above patch in place things
work as expected.
Using the same code base I was able to do acpi hotplug on Windows 7,
which correctly configured the both the bridge window and devices behind
it on hot-plug. So currently, the above usage pattern works on Windows
7, but not on Linux.
Thanks,
-Jason
--
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
