Re: [PATCH] pci: clean all funcs when hot-removing multifunc device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Sep 13, 2011 at 10:55 PM, Amos Kong <akong@xxxxxxxxxx> wrote:
> 'slot->funcs' is initialized in acpiphp_glue.c:register_slot() before
> hotpluging device, and only one entry(func 0) is added to it,
> no new entry will be added to the list when hotpluging devices to the slot.
> When we release the whole device, there is only one entry in the list,
> this causes func1~7 could not be released.
> I try to add entries for all hotpluged device in enable_device(), but
> it doesn't work, because 'slot->funcs' is used in many place which we only
> need to process func 0. This patch just try to clean all funcs in
> disable_device().
...
> Hotpluging multifunc of WinXp is fine.

I'm going to ignore this patch for now.  Please consider these
questions, then repost it if you still want it:

I assume you mean that Linux and WinXP are both running on top of the
same SeaBIOS, and hot-remove of a multifunction device works in WinXP
and fails in Linux.  That sounds like Linux is broken, and we should
fix it.  We might want to make a SeaBIOS change for other reasons, but
it'd still be good to fix Linux in case there are other similar
BIOSes.

Why do we need pci_scan_single_device()?  The device should have been
scanned already when it was added, and I would think that should have
set pdev->multifunction.

Your patch needs spaces around the operators in the "for" loop.

In the changelog, it would be nice to have the URL of a bugzilla where
the dmesg and DSDT are attached.

Bjorn

> Signed-off-by: Amos Kong <akong@xxxxxxxxxx>
> ---
>  drivers/pci/hotplug/acpiphp_glue.c |   27 ++++++++++++++++++---------
>  1 files changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> index a70fa89..3b86d1a 100644
> --- a/drivers/pci/hotplug/acpiphp_glue.c
> +++ b/drivers/pci/hotplug/acpiphp_glue.c
> @@ -880,6 +880,8 @@ static int disable_device(struct acpiphp_slot *slot)
>  {
>        struct acpiphp_func *func;
>        struct pci_dev *pdev;
> +       struct pci_bus *bus = slot->bridge->pci_bus;
> +       int i, num = 1;
>
>        /* is this slot already disabled? */
>        if (!(slot->flags & SLOT_ENABLED))
> @@ -893,16 +895,23 @@ static int disable_device(struct acpiphp_slot *slot)
>                        func->bridge = NULL;
>                }
>
> -               pdev = pci_get_slot(slot->bridge->pci_bus,
> -                                   PCI_DEVFN(slot->device, func->function));
> -               if (pdev) {
> -                       pci_stop_bus_device(pdev);
> -                       if (pdev->subordinate) {
> -                               disable_bridges(pdev->subordinate);
> -                               pci_disable_device(pdev);
> +               pdev = pci_scan_single_device(bus,
> +                                       PCI_DEVFN(slot->device, 0));
> +               if (!pdev)
> +                       goto err_exit;
> +               if (pdev->multifunction == 1)
> +                       num = 8;
> +                for (i=0; i<num; i++) {
> +                       pdev = pci_get_slot(bus, PCI_DEVFN(slot->device, i));
> +                       if (pdev) {
> +                               pci_stop_bus_device(pdev);
> +                               if (pdev->subordinate) {
> +                                       disable_bridges(pdev->subordinate);
> +                                       pci_disable_device(pdev);
> +                               }
> +                               pci_remove_bus_device(pdev);
> +                               pci_dev_put(pdev);
>                        }
> -                       pci_remove_bus_device(pdev);
> -                       pci_dev_put(pdev);
>                }
>        }
>
> --
> 1.7.6.1
>
> --
> 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
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux