Patchwork [RESEND,v3] xen: implement unplug protocol in xen_platform

login
register
mail settings
Submitter Stefano Stabellini
Date July 18, 2011, 4:07 p.m.
Message ID <1311005222-31060-1-git-send-email-stefano.stabellini@eu.citrix.com>
Download mbox | patch
Permalink /patch/105333/
State New
Headers show

Comments

Stefano Stabellini - July 18, 2011, 4:07 p.m.
From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

The unplug protocol is necessary to support PV drivers in the guest: the
drivers expect to be able to "unplug" emulated disks and nics before
initializing the Xen PV interfaces.
It is responsibility of the guest to make sure that the unplug is done
before the emulated devices or the PV interface start to be used.

We use pci_for_each_device to walk the PCI bus, identify the devices and
disks that we want to disable and dynamically unplug them.

Changes in v2:

- use PCI_CLASS constants;

- replace pci_unplug_device with qdev_unplug;

- do not import hw/ide/internal.h in xen_platform.c;


Changes in v3:

- introduce piix3-ide-xen, that support hot-unplug;

- move the unplug code to hw/ide/piix.c;

- just call qdev_unplug from xen_platform.c to unplug the IDE disks;

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 hw/ide.h          |    1 +
 hw/ide/piix.c     |   44 ++++++++++++++++++++++++++++++++++++++++++++
 hw/pc_piix.c      |    6 +++++-
 hw/xen_platform.c |   43 ++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 92 insertions(+), 2 deletions(-)
Alexander Graf - July 27, 2011, 12:44 p.m.
On 07/18/2011 06:07 PM, stefano.stabellini@eu.citrix.com wrote:
> From: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
>
> The unplug protocol is necessary to support PV drivers in the guest: the
> drivers expect to be able to "unplug" emulated disks and nics before
> initializing the Xen PV interfaces.
> It is responsibility of the guest to make sure that the unplug is done
> before the emulated devices or the PV interface start to be used.
>
> We use pci_for_each_device to walk the PCI bus, identify the devices and
> disks that we want to disable and dynamically unplug them.
>
> Changes in v2:
>
> - use PCI_CLASS constants;
>
> - replace pci_unplug_device with qdev_unplug;
>
> - do not import hw/ide/internal.h in xen_platform.c;
>
>
> Changes in v3:
>
> - introduce piix3-ide-xen, that support hot-unplug;
>
> - move the unplug code to hw/ide/piix.c;
>
> - just call qdev_unplug from xen_platform.c to unplug the IDE disks;
>
> Signed-off-by: Stefano Stabellini<stefano.stabellini@eu.citrix.com>

Kevin, please ack.


Alex
Kevin Wolf - July 27, 2011, 12:58 p.m.
Am 27.07.2011 14:44, schrieb Alexander Graf:
> On 07/18/2011 06:07 PM, stefano.stabellini@eu.citrix.com wrote:
>> From: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
>>
>> The unplug protocol is necessary to support PV drivers in the guest: the
>> drivers expect to be able to "unplug" emulated disks and nics before
>> initializing the Xen PV interfaces.
>> It is responsibility of the guest to make sure that the unplug is done
>> before the emulated devices or the PV interface start to be used.
>>
>> We use pci_for_each_device to walk the PCI bus, identify the devices and
>> disks that we want to disable and dynamically unplug them.
>>
>> Changes in v2:
>>
>> - use PCI_CLASS constants;
>>
>> - replace pci_unplug_device with qdev_unplug;
>>
>> - do not import hw/ide/internal.h in xen_platform.c;
>>
>>
>> Changes in v3:
>>
>> - introduce piix3-ide-xen, that support hot-unplug;
>>
>> - move the unplug code to hw/ide/piix.c;
>>
>> - just call qdev_unplug from xen_platform.c to unplug the IDE disks;
>>
>> Signed-off-by: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
> 
> Kevin, please ack.

Trivial rebase of the version I already acked.

Acked-by: Kevin Wolf <kwolf@redhat.com>
Alexander Graf - July 27, 2011, 1:26 p.m.
On 07/27/2011 02:58 PM, Kevin Wolf wrote:
> Am 27.07.2011 14:44, schrieb Alexander Graf:
>> On 07/18/2011 06:07 PM, stefano.stabellini@eu.citrix.com wrote:
>>> From: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
>>>
>>> The unplug protocol is necessary to support PV drivers in the guest: the
>>> drivers expect to be able to "unplug" emulated disks and nics before
>>> initializing the Xen PV interfaces.
>>> It is responsibility of the guest to make sure that the unplug is done
>>> before the emulated devices or the PV interface start to be used.
>>>
>>> We use pci_for_each_device to walk the PCI bus, identify the devices and
>>> disks that we want to disable and dynamically unplug them.
>>>
>>> Changes in v2:
>>>
>>> - use PCI_CLASS constants;
>>>
>>> - replace pci_unplug_device with qdev_unplug;
>>>
>>> - do not import hw/ide/internal.h in xen_platform.c;
>>>
>>>
>>> Changes in v3:
>>>
>>> - introduce piix3-ide-xen, that support hot-unplug;
>>>
>>> - move the unplug code to hw/ide/piix.c;
>>>
>>> - just call qdev_unplug from xen_platform.c to unplug the IDE disks;
>>>
>>> Signed-off-by: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
>> Kevin, please ack.
> Trivial rebase of the version I already acked.

Thanks :). Applied to the xen-next branch.


Alex

Patch

diff --git a/hw/ide.h b/hw/ide.h
index 34d9394..a490cbb 100644
--- a/hw/ide.h
+++ b/hw/ide.h
@@ -13,6 +13,7 @@  ISADevice *isa_ide_init(int iobase, int iobase2, int isairq,
 /* ide-pci.c */
 void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table,
                          int secondary_ide_enabled);
+PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
 PCIDevice *pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
 PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
 void vt82c686b_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 84f72b0..f527dbd 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -149,6 +149,42 @@  static int pci_piix_ide_initfn(PCIDevice *dev)
     return 0;
 }
 
+static int pci_piix3_xen_ide_unplug(DeviceState *dev)
+{
+    PCIDevice *pci_dev;
+    PCIIDEState *pci_ide;
+    DriveInfo *di;
+    int i = 0;
+
+    pci_dev = DO_UPCAST(PCIDevice, qdev, dev);
+    pci_ide = DO_UPCAST(PCIIDEState, dev, pci_dev);
+
+    for (; i < 3; i++) {
+        di = drive_get_by_index(IF_IDE, i);
+        if (di != NULL && di->bdrv != NULL && !di->bdrv->removable) {
+            DeviceState *ds = bdrv_get_attached(di->bdrv);
+            if (ds) {
+                bdrv_detach(di->bdrv, ds);
+            }
+            bdrv_close(di->bdrv);
+            pci_ide->bus[di->bus].ifs[di->unit].bs = NULL;
+            drive_put_ref(di);
+        }
+    }
+    qdev_reset_all(&(pci_ide->dev.qdev));
+    return 0;
+}
+
+PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
+{
+    PCIDevice *dev;
+
+    dev = pci_create_simple(bus, devfn, "piix3-ide-xen");
+    dev->qdev.info->unplug = pci_piix3_xen_ide_unplug;
+    pci_ide_create_devs(dev, hd_table);
+    return dev;
+}
+
 /* hd_table must contain 4 block drivers */
 /* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
 PCIDevice *pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
@@ -182,6 +218,14 @@  static PCIDeviceInfo piix_ide_info[] = {
         .device_id    = PCI_DEVICE_ID_INTEL_82371SB_1,
         .class_id     = PCI_CLASS_STORAGE_IDE,
     },{
+        .qdev.name    = "piix3-ide-xen",
+        .qdev.size    = sizeof(PCIIDEState),
+        .qdev.no_user = 1,
+        .init         = pci_piix_ide_initfn,
+        .vendor_id    = PCI_VENDOR_ID_INTEL,
+        .device_id    = PCI_DEVICE_ID_INTEL_82371SB_1,
+        .class_id     = PCI_CLASS_STORAGE_IDE,
+    },{
         .qdev.name    = "piix4-ide",
         .qdev.size    = sizeof(PCIIDEState),
         .qdev.no_user = 1,
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index c5c16b4..40b73ea 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -155,7 +155,11 @@  static void pc_init1(ram_addr_t ram_size,
     ide_drive_get(hd, MAX_IDE_BUS);
     if (pci_enabled) {
         PCIDevice *dev;
-        dev = pci_piix3_ide_init(pci_bus, hd, piix3_devfn + 1);
+        if (xen_enabled()) {
+            dev = pci_piix3_xen_ide_init(pci_bus, hd, piix3_devfn + 1);
+        } else {
+            dev = pci_piix3_ide_init(pci_bus, hd, piix3_devfn + 1);
+        }
         idebus[0] = qdev_get_child_bus(&dev->qdev, "ide.0");
         idebus[1] = qdev_get_child_bus(&dev->qdev, "ide.1");
     } else {
diff --git a/hw/xen_platform.c b/hw/xen_platform.c
index b167eee..a271369 100644
--- a/hw/xen_platform.c
+++ b/hw/xen_platform.c
@@ -76,6 +76,35 @@  static void log_writeb(PCIXenPlatformState *s, char val)
 }
 
 /* Xen Platform, Fixed IOPort */
+#define UNPLUG_ALL_IDE_DISKS 1
+#define UNPLUG_ALL_NICS 2
+#define UNPLUG_AUX_IDE_DISKS 4
+
+static void unplug_nic(PCIBus *b, PCIDevice *d)
+{
+    if (pci_get_word(d->config + PCI_CLASS_DEVICE) ==
+            PCI_CLASS_NETWORK_ETHERNET) {
+        qdev_unplug(&(d->qdev));
+    }
+}
+
+static void pci_unplug_nics(PCIBus *bus)
+{
+    pci_for_each_device(bus, 0, unplug_nic);
+}
+
+static void unplug_disks(PCIBus *b, PCIDevice *d)
+{
+    if (pci_get_word(d->config + PCI_CLASS_DEVICE) ==
+            PCI_CLASS_STORAGE_IDE) {
+        qdev_unplug(&(d->qdev));
+    }
+}
+
+static void pci_unplug_disks(PCIBus *bus)
+{
+    pci_for_each_device(bus, 0, unplug_disks);
+}
 
 static void platform_fixed_ioport_writew(void *opaque, uint32_t addr, uint32_t val)
 {
@@ -83,10 +112,22 @@  static void platform_fixed_ioport_writew(void *opaque, uint32_t addr, uint32_t v
 
     switch (addr - XEN_PLATFORM_IOPORT) {
     case 0:
-        /* TODO: */
         /* Unplug devices.  Value is a bitmask of which devices to
            unplug, with bit 0 the IDE devices, bit 1 the network
            devices, and bit 2 the non-primary-master IDE devices. */
+        if (val & UNPLUG_ALL_IDE_DISKS) {
+            DPRINTF("unplug disks\n");
+            qemu_aio_flush();
+            bdrv_flush_all();
+            pci_unplug_disks(s->pci_dev.bus);
+        }
+        if (val & UNPLUG_ALL_NICS) {
+            DPRINTF("unplug nics\n");
+            pci_unplug_nics(s->pci_dev.bus);
+        }
+        if (val & UNPLUG_AUX_IDE_DISKS) {
+            DPRINTF("unplug auxiliary disks not supported\n");
+        }
         break;
     case 2:
         switch (val) {