Re: [Qemu-devel] [PATCH 3/3] xen: implement unplug protocol in xen_platform

[Kevin Wolf <kwolf@redhat.com>]

Am 20.06.2011 10:28, schrieb Alexander Graf:
> 
> On 16.06.2011, at 18:05, <stefano.stabellini@eu.citrix.com> <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.
> 
> Kevin, please check the block parts of this code.
> Michael, please check the PCI parts of this code.
> 
> Thanks :)
> 
> Alex
> 
>>
>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> ---
>> hw/xen_platform.c |   63 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 files changed, 62 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/xen_platform.c b/hw/xen_platform.c
>> index b167eee..9f8c843 100644
>> --- a/hw/xen_platform.c
>> +++ b/hw/xen_platform.c
>> @@ -34,6 +34,9 @@
>> #include "xen_backend.h"
>> #include "rwhandler.h"
>> #include "trace.h"
>> +#include "hw/ide/internal.h"

Sorry, no. :-)

This is not using a proper interface, but just a hack that depends on
the internal structure of the IDE emulation. It's going to break sooner
or later.

It seems your problem is that IDE isn't unpluggable. I'm not entirely
sure what the right solution is, maybe just adding a new xen-ide device
that is used for the Xen machine and closely resembles piix4-ide, but
can be hot-unplugged.

Kevin

>> +#include "hw/ide/pci.h"
>> +#include "hw/pci_ids.h"
>>
>> #include <xenguest.h>
>>
>> @@ -76,6 +79,54 @@ 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 int unplug_param;
>> +
>> +static void unplug_nic(PCIBus *b, PCIDevice *d)
>> +{
>> +    if (d->config[0xa] == 0 && d->config[0xb] == 2) {
>> +        pci_unplug_device(&(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 (d->config[0xa] == 1 && d->config[0xb] == 1) {
>> +        PCIIDEState *pci_ide = DO_UPCAST(PCIIDEState, dev, d);
>> +        DriveInfo *di;
>> +        int i = 0;
>> +
>> +        if (unplug_param & UNPLUG_AUX_IDE_DISKS)
>> +            i++;
>> +
>> +        for (; i < 3; i++) {
>> +            di = drive_get_by_index(IF_IDE, i); 
>> +            if (di != NULL && di->bdrv != NULL && di->bdrv->type != BDRV_TYPE_CDROM) {
>> +                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);
>> +            }
>> +        }
>> +        ide_bus_reset(&pci_ide->bus[0]);
>> +        ide_bus_reset(&pci_ide->bus[1]);
>> +    }
>> +}
>> +
>> +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 +134,20 @@ static void platform_fixed_ioport_writew(void *opaque, uint32_t addr, uint32_t v
>>
>>     switch (addr - XEN_PLATFORM_IOPORT) {
>>     case 0:
>> -        /* TODO: */
>> +        unplug_param = val;
>>         /* 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 || val & UNPLUG_AUX_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);
>> +        }
>>         break;
>>     case 2:
>>         switch (val) {
>> -- 
>> 1.7.2.3
>>
>