* [PATCH] pci: clean all funcs when hot-removing multifunc device
[not found] <1315976141-6684-1-git-send-email-akong@redhat.com>
@ 2011-09-14 4:56 ` Amos Kong
2011-09-15 19:03 ` Bjorn Helgaas
1 sibling, 0 replies; 6+ messages in thread
From: Amos Kong @ 2011-09-14 4:56 UTC (permalink / raw)
To: linux-pci; +Cc: jbarnes, mst, alex williamson, mtosatti, kvm
CC: kvm@vger.kernel.org
----- Original Message -----
'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().
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@redhat.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] pci: clean all funcs when hot-removing multifunc device
[not found] <4E7045C3.6040604@jp.fujitsu.com>
@ 2011-09-14 8:13 ` Amos Kong
2011-09-14 11:45 ` Amos Kong
0 siblings, 1 reply; 6+ messages in thread
From: Amos Kong @ 2011-09-14 8:13 UTC (permalink / raw)
To: Kenji Kaneshige; +Cc: linux-pci, jbarnes, mst, alex williamson, mtosatti, kvm
----- 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@redhat.com>
> > ---
> > 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);
> > }
> > }
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pci: clean all funcs when hot-removing multifunc device
2011-09-14 8:13 ` Amos Kong
@ 2011-09-14 11:45 ` Amos Kong
2011-09-15 1:59 ` Kevin O'Connor
0 siblings, 1 reply; 6+ messages in thread
From: Amos Kong @ 2011-09-14 11:45 UTC (permalink / raw)
To: Kenji Kaneshige
Cc: linux-pci, jbarnes, mst, alex williamson, mtosatti, kvm, seabios
[-- Attachment #1: Type: text/plain, Size: 5970 bytes --]
----- 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@redhat.com>
> > > ---
> > > 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: patch1.txt --]
[-- Type: text/plain, Size: 2325 bytes --]
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)
[-- Attachment #3: patch2.txt --]
[-- Type: text/plain, Size: 3968 bytes --]
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)
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] pci: clean all funcs when hot-removing multifunc device
2011-09-14 11:45 ` Amos Kong
@ 2011-09-15 1:59 ` Kevin O'Connor
0 siblings, 0 replies; 6+ messages in thread
From: Kevin O'Connor @ 2011-09-15 1:59 UTC (permalink / raw)
To: Amos Kong
Cc: Kenji Kaneshige, linux-pci, jbarnes, mst, alex williamson,
mtosatti, kvm, seabios
On Wed, Sep 14, 2011 at 07:45:59AM -0400, Amos Kong wrote:
> 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;
This is probably a regression since seabios commit 87b533bf. Prior to
that commit, seabios did not mark the early 32bit initialization code
as init code. However, a side effect of marking that code
(handle_post) as init code is that it is more likely the linker could
place the code at an address less than 0xe0000.
I'm guesing the patch below (just a hack) would cover up the issue.
-Kevin
--- a/src/post.c
+++ b/src/post.c
@@ -336,7 +336,7 @@ reloc_init(void)
// Start of Power On Self Test (POST) - the BIOS initilization phase.
// This function does the setup needed for code relocation, and then
// invokes the relocation and main setup code.
-void VISIBLE32INIT
+void VISIBLE32FLAT
handle_post(void)
{
debug_serial_setup();
@@ -356,6 +356,14 @@ handle_post(void)
// Allow writes to modify bios area (0xf0000)
make_bios_writable();
+
+ void handle_post2(void);
+ handle_post2();
+}
+
+void VISIBLE32INIT
+handle_post2(void)
+{
HaveRunPost = 1;
// Detect ram and setup internal malloc.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pci: clean all funcs when hot-removing multifunc device
[not found] <1315976141-6684-1-git-send-email-akong@redhat.com>
2011-09-14 4:56 ` [PATCH] pci: clean all funcs when hot-removing multifunc device Amos Kong
@ 2011-09-15 19:03 ` Bjorn Helgaas
2011-09-15 23:56 ` Kenji Kaneshige
1 sibling, 1 reply; 6+ messages in thread
From: Bjorn Helgaas @ 2011-09-15 19:03 UTC (permalink / raw)
To: Amos Kong
Cc: linux-pci, jbarnes, mst, alex.williamson, mtosatti, kvm,
Kenji Kaneshige, Kevin O'Connor, seabios
On Tue, Sep 13, 2011 at 10:55 PM, Amos Kong <akong@redhat.com> 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@redhat.com>
> ---
> 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pci: clean all funcs when hot-removing multifunc device
2011-09-15 19:03 ` Bjorn Helgaas
@ 2011-09-15 23:56 ` Kenji Kaneshige
0 siblings, 0 replies; 6+ messages in thread
From: Kenji Kaneshige @ 2011-09-15 23:56 UTC (permalink / raw)
To: Bjorn Helgaas, Amos Kong
Cc: linux-pci, jbarnes, mst, alex.williamson, mtosatti, kvm,
Kevin O'Connor, seabios
(2011/09/16 4:03), Bjorn Helgaas wrote:
> On Tue, Sep 13, 2011 at 10:55 PM, Amos Kong<akong@redhat.com> 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.
No objection about fixing Linux.
>
> 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.
It should be pci_get_slot() instead. Note that it needs
corresponding pci_dev_put().
Regards,
Kenji Kaneshige
>
> 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@redhat.com>
>> ---
>> 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@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-09-15 23:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1315976141-6684-1-git-send-email-akong@redhat.com>
2011-09-14 4:56 ` [PATCH] pci: clean all funcs when hot-removing multifunc device Amos Kong
2011-09-15 19:03 ` Bjorn Helgaas
2011-09-15 23:56 ` Kenji Kaneshige
[not found] <4E7045C3.6040604@jp.fujitsu.com>
2011-09-14 8:13 ` Amos Kong
2011-09-14 11:45 ` Amos Kong
2011-09-15 1:59 ` Kevin O'Connor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).