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


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;


> 
> > 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
diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
index 08412e2..c1504d0 100644
--- a/src/acpi-dsdt.dsl
+++ b/src/acpi-dsdt.dsl
@@ -128,9 +128,9 @@ DefinitionBlock (
                 PCRM, 32,
             }
 
-#define hotplug_slot(name, nr) \
-            Device (S##name) {                    \
-               Name (_ADR, nr##0000)              \
+#define hotplug_func(name, nr, fn) \
+            Device (S##name##fn) {                \
+               Name (_ADR, nr##fn)                \
                Method (_EJ0,1) {                  \
                     Store(ShiftLeft(1, nr), B0EJ) \
                     Return (0x0)                  \
@@ -138,6 +138,16 @@ DefinitionBlock (
                Name (_SUN, name)                  \
             }
 
+#define hotplug_slot(name, nr) \
+	    hotplug_func(name, nr##0000, 0)  \
+	    hotplug_func(name, nr##0001, 1)  \
+	    hotplug_func(name, nr##0002, 2)  \
+	    hotplug_func(name, nr##0003, 3)  \
+	    hotplug_func(name, nr##0004, 4)  \
+	    hotplug_func(name, nr##0005, 5)  \
+	    hotplug_func(name, nr##0006, 6)  \
+	    hotplug_func(name, nr##0007, 7)
+
 	    hotplug_slot(1, 0x0001)
 	    hotplug_slot(2, 0x0002)
 	    hotplug_slot(3, 0x0003)
@@ -842,13 +852,22 @@ DefinitionBlock (
             Return(0x01)
         }
 
-#define gen_pci_hotplug(nr)                                       \
+#define gen_pci_hotplug_func(nr, fn)                              \
             If (And(\_SB.PCI0.PCIU, ShiftLeft(1, nr))) {          \
-                Notify(\_SB.PCI0.S##nr, 1)                        \
+                Notify(\_SB.PCI0.S##nr##fn, 1)                    \
             }                                                     \
             If (And(\_SB.PCI0.PCID, ShiftLeft(1, nr))) {          \
-                Notify(\_SB.PCI0.S##nr, 3)                        \
+                Notify(\_SB.PCI0.S##nr##fn, 3)                    \
             }
+#define gen_pci_hotplug(nr) \
+	    gen_pci_hotplug_func(nr, 0)    \
+	    gen_pci_hotplug_func(nr, 1)    \
+	    gen_pci_hotplug_func(nr, 2)    \
+	    gen_pci_hotplug_func(nr, 3)    \
+	    gen_pci_hotplug_func(nr, 4)    \
+	    gen_pci_hotplug_func(nr, 5)    \
+	    gen_pci_hotplug_func(nr, 6)    \
+	    gen_pci_hotplug_func(nr, 7)
 
         Method(_L01) {
             gen_pci_hotplug(1)
diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
index 08412e2..10b651e 100644
--- a/src/acpi-dsdt.dsl
+++ b/src/acpi-dsdt.dsl
@@ -128,9 +128,9 @@ DefinitionBlock (
                 PCRM, 32,
             }
 
-#define hotplug_slot(name, nr) \
-            Device (S##name) {                    \
-               Name (_ADR, nr##0000)              \
+#define hotplug_func(name, nr, fn) \
+            Device (S##name##fn) {                \
+               Name (_ADR, nr)                    \
                Method (_EJ0,1) {                  \
                     Store(ShiftLeft(1, nr), B0EJ) \
                     Return (0x0)                  \
@@ -138,13 +138,30 @@ DefinitionBlock (
                Name (_SUN, name)                  \
             }
 
+#define hotplug_slot(name, nr) \
+	    hotplug_func(name, nr##0000, 0)/*  \
+	    hotplug_func(name, nr##0001, 1)  \
+	    hotplug_func(name, nr##0002, 2)  \
+	    hotplug_func(name, nr##0003, 3)  \
+	    hotplug_func(name, nr##0004, 4)  \
+	    hotplug_func(name, nr##0005, 5)  \
+	    hotplug_func(name, nr##0006, 6)  \
+	    hotplug_func(name, nr##0007, 7)*/
+
 	    hotplug_slot(1, 0x0001)
 	    hotplug_slot(2, 0x0002)
 	    hotplug_slot(3, 0x0003)
 	    hotplug_slot(4, 0x0004)
 	    hotplug_slot(5, 0x0005)
-	    hotplug_slot(6, 0x0006)
-	    hotplug_slot(7, 0x0007)
+	    hotplug_func(6, 0x00060000, 0)
+	    hotplug_func(6, 0x00060001, 1)
+	    hotplug_func(6, 0x00060002, 2)
+	    hotplug_func(6, 0x00060003, 3)
+	    hotplug_func(6, 0x00060004, 4)
+	    hotplug_func(6, 0x00060005, 5)
+	    hotplug_func(6, 0x00060006, 6)
+	    hotplug_func(6, 0x00060007, 7)
+	    /*hotplug_slot(7, 0x0007)
 	    hotplug_slot(8, 0x0008)
 	    hotplug_slot(9, 0x0009)
 	    hotplug_slot(10, 0x000a)
@@ -168,7 +185,7 @@ DefinitionBlock (
 	    hotplug_slot(28, 0x001c)
 	    hotplug_slot(29, 0x001d)
 	    hotplug_slot(30, 0x001e)
-	    hotplug_slot(31, 0x001f)
+	    hotplug_slot(31, 0x001f)*/
 
             Name (_CRS, ResourceTemplate ()
             {
@@ -842,13 +859,22 @@ DefinitionBlock (
             Return(0x01)
         }
 
-#define gen_pci_hotplug(nr)                                       \
+#define gen_pci_hotplug_func(nr, fn)                              \
             If (And(\_SB.PCI0.PCIU, ShiftLeft(1, nr))) {          \
-                Notify(\_SB.PCI0.S##nr, 1)                        \
+                Notify(\_SB.PCI0.S##nr##fn, 1)                    \
             }                                                     \
             If (And(\_SB.PCI0.PCID, ShiftLeft(1, nr))) {          \
-                Notify(\_SB.PCI0.S##nr, 3)                        \
+                Notify(\_SB.PCI0.S##nr##fn, 3)                    \
             }
+#define gen_pci_hotplug(nr) \
+	    gen_pci_hotplug_func(nr, 0)/*    \
+	    gen_pci_hotplug_func(nr, 1)    \
+	    gen_pci_hotplug_func(nr, 2)    \
+	    gen_pci_hotplug_func(nr, 3)    \
+	    gen_pci_hotplug_func(nr, 4)    \
+	    gen_pci_hotplug_func(nr, 5)    \
+	    gen_pci_hotplug_func(nr, 6)    \
+	    gen_pci_hotplug_func(nr, 7)*/
 
         Method(_L01) {
             gen_pci_hotplug(1)
@@ -856,8 +882,15 @@ DefinitionBlock (
             gen_pci_hotplug(3)
             gen_pci_hotplug(4)
             gen_pci_hotplug(5)
-            gen_pci_hotplug(6)
-            gen_pci_hotplug(7)
+            gen_pci_hotplug_func(6, 0)
+	    gen_pci_hotplug_func(6, 1)
+	    gen_pci_hotplug_func(6, 2)
+	    gen_pci_hotplug_func(6, 3)
+	    gen_pci_hotplug_func(6, 4)
+	    gen_pci_hotplug_func(6, 5)
+	    gen_pci_hotplug_func(6, 6)
+	    gen_pci_hotplug_func(6, 7)
+            /*gen_pci_hotplug(7)
             gen_pci_hotplug(8)
             gen_pci_hotplug(9)
             gen_pci_hotplug(10)
@@ -881,7 +914,7 @@ DefinitionBlock (
             gen_pci_hotplug(28)
             gen_pci_hotplug(29)
             gen_pci_hotplug(30)
-            gen_pci_hotplug(31)
+            gen_pci_hotplug(31) */
 
             Return (0x01)
 

[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