pci: clean all funcs when hot-removing multifunc device

Quoted 5 lines · Amos Kong

'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:

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

Quoted 106 lines

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<[email protected]>

---
  drivers/pci/hotplug/acpiphp_glue.c | 27
  ++++++++++++++++++---------
  1 files changed, 18 insertions(+), 9 deletions(-)

drivers/pci/hotplug/acpiphp_glue.c +2 -0

index a70fa89..3b86d1a 100644
@@ -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))

drivers/pci/hotplug/acpiphp_glue.c +16 -9

@@ -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);
  		}
  	}

----- Original Message -----

Quoted 7 lines

Quoted 5 lines · Amos Kong

'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.

Quoted 5 lines

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.

Quoted 127 lines

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

Quoted 104 lines

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<[email protected]>

---
  drivers/pci/hotplug/acpiphp_glue.c | 27
  ++++++++++++++++++---------
  1 files changed, 18 insertions(+), 9 deletions(-)

drivers/pci/hotplug/acpiphp_glue.c +2 -0

index a70fa89..3b86d1a 100644
@@ -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))

drivers/pci/hotplug/acpiphp_glue.c +0 -0

@@ -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);

Quoted 4 lines

Quoted 4 lines

- pci_dev_put(pdev);
}
}

--
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

The size of bios.bin compiled from seabios

original: 128K
only apply patch1:  256K
only apply patch2:  128K

patch1: add 6 slot(only slot6 has 8 funcs) to the table can hotplug/hot-remove a multifunc device to slot 6 successfully

patch2: add 31 slot(with 8 funcs) to the table
could not boot up guest.
I found there is a special process for large bios.bin in qemu, problem maybe exist here, I'm driving into it...

qemu/hw/pc.c:
void pc_memory_init(...

    ....
    /* map the last 128KB of the BIOS in ISA space */
    isa_bios_size = bios_size;
    if (isa_bios_size > (128 * 1024))
        isa_bios_size = 128 * 1024;