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

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

 



----- Original Message -----
> (2011/09/14 13:55), Amos Kong 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.
> 
> I guess your hotplug slot has only one device object (for func#0)
> in ACPI Namespace (DSDT), and guess this is why there is only one
> entry in the 'slot->funcs'. If so, what about adding device objects
> for function 1-7 to ACPI Namespace? I think most of bare-metal
> environments have such definition in ACPI Namespace. For example:

Hi Kaneshige,

I did some test, fix acpi tables can resolve this problem,
then register_slot() will be executed for all funcs, 
and each func has a entry in slot->funcs.
I will send a patch to seabios.

Thanks a lot!
Amos

> Device (P2P) { // PCI to PCI bridge
> Name (_ADR, ...) // PCI address
> Name (_HPP, ...) // Hot Plug parameter
> ...
> Device (S0F0) { // For function 0
> Name (_ADR, ...)
> Name (_SUN, ...)
> Method (_EJ0, ...)
> }
> Device (S0F1) { // For function 1
> ...
> }
> ...
> Device (S0F7) { // For function 7
> ...
> }
> }
> 
> Regards,
> Kenji Kaneshige
> 
> 
> > 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().
> >
> > drivers/pci/hotplug/acpiphp_glue.c:
> > static int disable_device(struct acpiphp_slot *slot) {
> > 	list_for_each_entry(func,&slot->funcs, sibling) {
> > 		pdev = pci_get_slot(slot->bridge->pci_bus,
> > 		       PCI_DEVFN(slot->device, func->function));
> > 		..clean code.. // those code can only be executed one time(func 0)
> >                  pci_remove_bus_device(pdev);
> > ---
> > pci_bus_add_device() is called for each func device in
> > acpiphp_glue.c:enable_device().
> > pci_remove_bus_device(pdev) is only called for func 0 in
> > acpiphp_glue.c:disable_device().
> >
> > Boot up a KVM guest, hotplug a multifunc device(8 funcs), we can
> > find it in the guest.
> > @ ls /dev/vd*
> >     vda vdb vdc vde vdf vdg vdh
> > @ lspci
> >     00:06.0 SCSI storage controller: Red Hat, Inc Virtio block
> >     device
> >     ...
> >     00:06.7 SCSI storage controller: Red Hat, Inc Virtio block
> >     device
> >
> > But func 1~7 still exist in guest after hot-removing the multifunc
> > device through qemu monitor.
> > @ lspci (00:06.0 disappeared)
> >     00:06.1 SCSI storage controller: Red Hat, Inc Virtio block
> >     device (rev ff)
> >     ...
> >     00:06.7 SCSI storage controller: Red Hat, Inc Virtio block
> >     device (rev ff)
> >                                                                       ^^^^^^^^
> > @ ls /dev/vd*
> >     vdb vdc vde vdf vdg vdh
> > @ mkfs /dev/vdb
> >     INFO: task mkfs.ext2:1784 blocked for more than 120 seconds.
> >     (task hung)
> >
> > Hotpluging multifunc of WinXp is fine.
> >
> > 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);
> >   		}
> >   	}
> >
--
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