Device assignment hotplug broken

PATCH Applied Avi Kivity 3 participants 7 messages Jun 25, 2009 – Jul 2, 2009 KVM

Share & tools

Avi Kivity

author

2009-06-25 17:23 UTC #0 lore raw

The impact of the hotplug changes on device assignment were too difficult for me to fix up during the merge, so I disabled it temporarily. Please take a look at qemu-kvm.git commit a3b371477e3.

Signature · 2 lines

I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Sheng Yang

2009-06-30 05:38 UTC #1 lore raw

Quoted 3 lines · On Friday 26 June 2009 01:24:03 Avi Kivity wrote:

The impact of the hotplug changes on device assignment were too difficult for me to fix up during the merge, so I disabled it temporarily. Please take a look at qemu-kvm.git commit a3b371477e3.

When the device assignment can come back? 4 day passed and no message from Markus Armbruster till now.

Signature · 2 lines

regards
Yang, Sheng

Markus Armbruster

2009-06-30 12:28 UTC #2 lore raw

Quoted 7 lines · Sheng Yang

Quoted 3 lines · On Friday 26 June 2009 01:24:03 Avi Kivity wrote:
The impact of the hotplug changes on device assignment were too difficult for me to fix up during the merge, so I disabled it temporarily. Please take a look at qemu-kvm.git commit a3b371477e3.
When the device assignment can come back? 4 day passed and no message from Markus Armbruster till now.

I'm having difficulties testing my patch. If it works for you, please let me know. Once I'm satisfied it works, I'll post it properly.

hw/device-assignment.c +5 -6

index e282498..88c3baf 100644
@@ -1126,9 +1126,8 @@ static int assigned_dev_register_msix_mmio(AssignedDevice *dev)
 struct PCIDevice *init_assigned_device(AssignedDevInfo *adev,
                                        const char *devaddr)
 {
-    printf("init_assigned_device: fix me please\n");
-    return NULL;
-#if 0
+    PCIBus *bus;
+    int devfn;
     int r;
     AssignedDevice *dev;
     PCIDevice *pci_dev;

@@ -1138,8 +1137,9 @@ struct PCIDevice *init_assigned_device(AssignedDevInfo *adev,
     DEBUG("Registering real physical device %s (bus=%x dev=%x func=%x)\n",
           adev->name, adev->bus, adev->dev, adev->func);
 
+    bus = pci_get_bus_devfn(&devfn, devaddr);
     pci_dev = pci_register_device(bus, adev->name,
-              sizeof(AssignedDevice), -1, assigned_dev_pci_read_config,
+              sizeof(AssignedDevice), devfn, assigned_dev_pci_read_config,
               assigned_dev_pci_write_config);
     dev = container_of(pci_dev, AssignedDevice, dev);

@@ -1203,7 +1203,6 @@ assigned_out:
 out:
     free_assigned_device(adev);
     return NULL;
-#endif
 }
 
 /*

@@ -1268,7 +1267,7 @@ void add_assigned_devices(PCIBus *bus, const char **devices, int n_devices)
             exit(1);
         }
 
-        if (!init_assigned_device(adev, bus)) {
+        if (!init_assigned_device(adev, NULL)) {
             fprintf(stderr, "Failed to initialize assigned device %s\n",
                     devices[i]);
             exit(1);

hw/pci.c +1 -1

index 618582a..0107096 100644
@@ -290,7 +290,7 @@ int pci_read_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp)
     return pci_parse_devaddr(devaddr, domp, busp, slotp);
 }
 
-static PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr)
+PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr)
 {
     int dom, bus;
     unsigned slot;

hw/pci.h +1 -0

index 7d8df83..a2c3a23 100644
@@ -239,6 +239,7 @@ int pci_read_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp);
 
 int pci_parse_host_devaddr(const char *addr, int *busp,
                            int *slotp, int *funcp);
+PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr);
 
 void pci_info(Monitor *mon);
 PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,

Signature · 1 line

1.6.2.5

Sheng Yang

2009-07-01 01:54 UTC #3 lore raw

Quoted 11 lines · On Tuesday 30 June 2009 20:28:08 Markus Armbruster wrote:

Quoted 7 lines · Sheng Yang
Quoted 3 lines · On Friday 26 June 2009 01:24:03 Avi Kivity wrote:
The impact of the hotplug changes on device assignment were too difficult for me to fix up during the merge, so I disabled it temporarily. Please take a look at qemu-kvm.git commit a3b371477e3.
When the device assignment can come back? 4 day passed and no message from Markus Armbruster till now.
I'm having difficulties testing my patch. If it works for you, please let me know. Once I'm satisfied it works, I'll post it properly.

Hi Markus

In my (limited) test, the patch works well.

Thanks!

Signature · 2 lines

regards
Yang, Sheng

Quoted 74 lines

hw/device-assignment.c +0 -0

index e282498..88c3baf 100644
@@ -1126,9 +1126,8 @@ static int

assigned_dev_register_msix_mmio(AssignedDevice *dev) struct PCIDevice *init_assigned_device(AssignedDevInfo *adev,
const char *devaddr)
{

hw/device-assignment.c +5 -6

-    printf("init_assigned_device: fix me please\n");
-    return NULL;
-#if 0
+    PCIBus *bus;
+    int devfn;
     int r;
     AssignedDevice *dev;
     PCIDevice *pci_dev;

@@ -1138,8 +1137,9 @@ struct PCIDevice
 *init_assigned_device(AssignedDevInfo *adev, DEBUG("Registering real
 physical device %s (bus=%x dev=%x func=%x)\n", adev->name, adev->bus,
 adev->dev, adev->func);
 
+    bus = pci_get_bus_devfn(&devfn, devaddr);
     pci_dev = pci_register_device(bus, adev->name,
-              sizeof(AssignedDevice), -1, assigned_dev_pci_read_config,
+              sizeof(AssignedDevice), devfn, assigned_dev_pci_read_config,
               assigned_dev_pci_write_config);
     dev = container_of(pci_dev, AssignedDevice, dev);

@@ -1203,7 +1203,6 @@ assigned_out:
 out:
     free_assigned_device(adev);
     return NULL;
-#endif
 }
 
 /*

@@ -1268,7 +1267,7 @@ void add_assigned_devices(PCIBus *bus, const char
 **devices, int n_devices) exit(1);
         }
 
-        if (!init_assigned_device(adev, bus)) {
+        if (!init_assigned_device(adev, NULL)) {
             fprintf(stderr, "Failed to initialize assigned device %s\n",
                     devices[i]);
             exit(1);

hw/pci.c +1 -1

index 618582a..0107096 100644
@@ -290,7 +290,7 @@ int pci_read_devaddr(const char *addr, int *domp, int
 *busp, unsigned *slotp) return pci_parse_devaddr(devaddr, domp, busp,
 slotp);
 }
 
-static PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr)
+PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr)
 {
     int dom, bus;

     unsigned slot;

hw/pci.h +1 -0

index 7d8df83..a2c3a23 100644
@@ -239,6 +239,7 @@ int pci_read_devaddr(const char *addr, int *domp, int
 *busp, unsigned *slotp);
 
 int pci_parse_host_devaddr(const char *addr, int *busp,
                            int *slotp, int *funcp);
+PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr);
 
 void pci_info(Monitor *mon);

PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t
did,

Markus Armbruster

2009-07-01 15:45 UTC #4 lore raw

Quoted 10 lines · Sheng Yang

Quoted 9 lines · On Tuesday 30 June 2009 20:28:08 Markus Armbruster wrote:
Quoted 7 lines · Sheng Yang
Quoted 3 lines · On Friday 26 June 2009 01:24:03 Avi Kivity wrote:
The impact of the hotplug changes on device assignment were too difficult for me to fix up during the merge, so I disabled it temporarily. Please take a look at qemu-kvm.git commit a3b371477e3.
When the device assignment can come back? 4 day passed and no message from Markus Armbruster till now.

Quoted 8 lines

Quoted 2 lines
I'm having difficulties testing my patch. If it works for you, please let me know. Once I'm satisfied it works, I'll post it properly.
Hi Markus
In my (limited) test, the patch works well.
Thanks!

Many thanks for testing this.

Markus Armbruster

2009-07-01 16:22 UTC #5 lore raw

PATCH 1/1 Fix up device assignment hotplug and re-enable it

In reply to Avi Kivity

Disabled in merge commit a3b371477e3.

Signed-off-by Markus Armbruster <[email protected]>

---
 hw/device-assignment.c |   11 +++++------
 hw/pci.c               |    2 +-
 hw/pci.h               |    1 +
 3 files changed, 7 insertions(+), 7 deletions(-)

hw/device-assignment.c +5 -6

index e282498..88c3baf 100644
@@ -1126,9 +1126,8 @@ static int assigned_dev_register_msix_mmio(AssignedDevice *dev)
 struct PCIDevice *init_assigned_device(AssignedDevInfo *adev,
                                        const char *devaddr)
 {
-    printf("init_assigned_device: fix me please\n");
-    return NULL;
-#if 0
+    PCIBus *bus;
+    int devfn;
     int r;
     AssignedDevice *dev;
     PCIDevice *pci_dev;

@@ -1138,8 +1137,9 @@ struct PCIDevice *init_assigned_device(AssignedDevInfo *adev,
     DEBUG("Registering real physical device %s (bus=%x dev=%x func=%x)\n",
           adev->name, adev->bus, adev->dev, adev->func);
 
+    bus = pci_get_bus_devfn(&devfn, devaddr);
     pci_dev = pci_register_device(bus, adev->name,
-              sizeof(AssignedDevice), -1, assigned_dev_pci_read_config,
+              sizeof(AssignedDevice), devfn, assigned_dev_pci_read_config,
               assigned_dev_pci_write_config);
     dev = container_of(pci_dev, AssignedDevice, dev);

@@ -1203,7 +1203,6 @@ assigned_out:
 out:
     free_assigned_device(adev);
     return NULL;
-#endif
 }
 
 /*

@@ -1268,7 +1267,7 @@ void add_assigned_devices(PCIBus *bus, const char **devices, int n_devices)
             exit(1);
         }
 
-        if (!init_assigned_device(adev, bus)) {
+        if (!init_assigned_device(adev, NULL)) {
             fprintf(stderr, "Failed to initialize assigned device %s\n",
                     devices[i]);
             exit(1);

hw/pci.c +1 -1

index 618582a..0107096 100644
@@ -290,7 +290,7 @@ int pci_read_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp)
     return pci_parse_devaddr(devaddr, domp, busp, slotp);
 }
 
-static PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr)
+PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr)
 {
     int dom, bus;
     unsigned slot;

hw/pci.h +1 -0

index 7d8df83..a2c3a23 100644
@@ -239,6 +239,7 @@ int pci_read_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp);
 
 int pci_parse_host_devaddr(const char *addr, int *busp,
                            int *slotp, int *funcp);
+PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr);
 
 void pci_info(Monitor *mon);
 PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,

Signature · 1 line

1.6.2.5

Avi Kivity

author Applied

2009-07-02 12:01 UTC #6 lore raw

Re: [PATCH] Fix up device assignment hotplug and re-enable it

Quoted 3 lines · Markus Armbruster

Disabled in merge commit a3b371477e3.

Applied, thanks.

Signature · 1 line

error compiling committee.c: too many arguments to function