On Wed, Jul 11, 2012 at 11:14 AM, Jason Baron <jbaron@xxxxxxxxxx> wrote:
> 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. Do you have "lspci -v" output and the DSDT AML handy?
I'd like to look in more detail at what we're missing.
--
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
