| Message ID | 1309260558-3332-1-git-send-email-stefano.stabellini@eu.citrix.com |
|---|---|
| State | New |
| Headers | show |
On 06/28/2011 01:29 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. $ ~/git/qemu/scripts/checkpatch.pl * ERROR: space required after that ',' (ctx:VxV) #158: FILE: hw/ide/piix.c:240: + },{ ^ total: 1 errors, 0 warnings, 130 lines checked I definitely want to see an ack from Kevin here first though. Alex
Am 28.06.2011 13:29, schrieb stefano.stabellini@eu.citrix.com: > 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 | 41 +++++++++++++++++++++++++++++++++++++++++ > hw/pc_piix.c | 6 +++++- > hw/xen_platform.c | 43 ++++++++++++++++++++++++++++++++++++++++++- > 4 files changed, 89 insertions(+), 2 deletions(-) > > 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 c349644..50ab7c5 100644 > --- a/hw/ide/piix.c > +++ b/hw/ide/piix.c > @@ -167,6 +167,42 @@ static int pci_piix4_ide_initfn(PCIDevice *dev) > return pci_piix_ide_initfn(d); > } > > +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; Have you tested if this is enough if the guest tries to continue using the device? I don't know of any case where it's not sufficient, just trying to make sure that it's really true in practice. > + 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; Can't this be moved into the PCIDeviceInfo now that we have a separate one for Xen? Kevin
On 06/30/2011 02:16 PM, Kevin Wolf wrote: > Am 30.06.2011 13:45, schrieb Alexander Graf: >> On 06/28/2011 01:29 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. >> $ ~/git/qemu/scripts/checkpatch.pl * >> ERROR: space required after that ',' (ctx:VxV) >> #158: FILE: hw/ide/piix.c:240: >> + },{ >> ^ >> >> total: 1 errors, 0 warnings, 130 lines checked > I think checkpatch.pl should be fixed in this case. We do have this > pattern all over the place in qemu and I don't see why it's bad. Hrm. I tend to agree. I'll post a patch :). Alex
Am 30.06.2011 13:45, schrieb Alexander Graf: > On 06/28/2011 01:29 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. > > $ ~/git/qemu/scripts/checkpatch.pl * > ERROR: space required after that ',' (ctx:VxV) > #158: FILE: hw/ide/piix.c:240: > + },{ > ^ > > total: 1 errors, 0 warnings, 130 lines checked I think checkpatch.pl should be fixed in this case. We do have this pattern all over the place in qemu and I don't see why it's bad. > I definitely want to see an ack from Kevin here first though. The approach looks good enough for me, unless someone has a better idea on how to do it cleanly. I commented on the implementation. Kevin
On Thu, 30 Jun 2011, Kevin Wolf wrote: > > +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; > > Have you tested if this is enough if the guest tries to continue using > the device? I don't know of any case where it's not sufficient, just > trying to make sure that it's really true in practice. The purpose of this is to "hide" the disk from the guest. The unplug is supposed to happen *before* the guest enumerates the IDE disks; it is responsibility of the guest to make sure of it. I tested it with Linux PV on HVM drivers, and Linux doesn't see the emulated disk after the unplug, as it should be. > > + 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; > > Can't this be moved into the PCIDeviceInfo now that we have a separate > one for Xen? No because it would be overridden by the default pci unplug function, that is not what we want in this case.
Am 30.06.2011 16:16, schrieb Stefano Stabellini: > On Thu, 30 Jun 2011, Kevin Wolf wrote: >>> +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; >> >> Have you tested if this is enough if the guest tries to continue using >> the device? I don't know of any case where it's not sufficient, just >> trying to make sure that it's really true in practice. > > The purpose of this is to "hide" the disk from the guest. The unplug is > supposed to happen *before* the guest enumerates the IDE disks; it is > responsibility of the guest to make sure of it. > I tested it with Linux PV on HVM drivers, and Linux doesn't see the > emulated disk after the unplug, as it should be. Yeah. What I meant is that we should make sure that a misbehaving guest, which just keeps on playing with the IDE ports anyway, can't crash qemu. A quick review suggests that it is the case, but testing it anyway would be better. >>> + 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; >> >> Can't this be moved into the PCIDeviceInfo now that we have a separate >> one for Xen? > > No because it would be overridden by the default pci unplug function, > that is not what we want in this case. Okay. I'm not familiar with that code, so I'll just trust you there. Kevin
On Fri, 1 Jul 2011, Kevin Wolf wrote: > Am 30.06.2011 16:16, schrieb Stefano Stabellini: > > On Thu, 30 Jun 2011, Kevin Wolf wrote: > >>> +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; > >> > >> Have you tested if this is enough if the guest tries to continue using > >> the device? I don't know of any case where it's not sufficient, just > >> trying to make sure that it's really true in practice. > > > > The purpose of this is to "hide" the disk from the guest. The unplug is > > supposed to happen *before* the guest enumerates the IDE disks; it is > > responsibility of the guest to make sure of it. > > I tested it with Linux PV on HVM drivers, and Linux doesn't see the > > emulated disk after the unplug, as it should be. > > Yeah. What I meant is that we should make sure that a misbehaving guest, > which just keeps on playing with the IDE ports anyway, can't crash qemu. > A quick review suggests that it is the case, but testing it anyway would > be better. I see what you mean: I tested it, a guest cannot crash Qemu.
On Fri, 1 Jul 2011, Stefano Stabellini wrote: > On Fri, 1 Jul 2011, Kevin Wolf wrote: > > Am 30.06.2011 16:16, schrieb Stefano Stabellini: > > > On Thu, 30 Jun 2011, Kevin Wolf wrote: > > >>> +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; > > >> > > >> Have you tested if this is enough if the guest tries to continue using > > >> the device? I don't know of any case where it's not sufficient, just > > >> trying to make sure that it's really true in practice. > > > > > > The purpose of this is to "hide" the disk from the guest. The unplug is > > > supposed to happen *before* the guest enumerates the IDE disks; it is > > > responsibility of the guest to make sure of it. > > > I tested it with Linux PV on HVM drivers, and Linux doesn't see the > > > emulated disk after the unplug, as it should be. > > > > Yeah. What I meant is that we should make sure that a misbehaving guest, > > which just keeps on playing with the IDE ports anyway, can't crash qemu. > > A quick review suggests that it is the case, but testing it anyway would > > be better. > > I see what you mean: I tested it, a guest cannot crash Qemu. > ping?
Am 15.07.2011 12:34, schrieb Stefano Stabellini: > On Fri, 1 Jul 2011, Stefano Stabellini wrote: >> On Fri, 1 Jul 2011, Kevin Wolf wrote: >>> Am 30.06.2011 16:16, schrieb Stefano Stabellini: >>>> On Thu, 30 Jun 2011, Kevin Wolf wrote: >>>>>> +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; >>>>> >>>>> Have you tested if this is enough if the guest tries to continue using >>>>> the device? I don't know of any case where it's not sufficient, just >>>>> trying to make sure that it's really true in practice. >>>> >>>> The purpose of this is to "hide" the disk from the guest. The unplug is >>>> supposed to happen *before* the guest enumerates the IDE disks; it is >>>> responsibility of the guest to make sure of it. >>>> I tested it with Linux PV on HVM drivers, and Linux doesn't see the >>>> emulated disk after the unplug, as it should be. >>> >>> Yeah. What I meant is that we should make sure that a misbehaving guest, >>> which just keeps on playing with the IDE ports anyway, can't crash qemu. >>> A quick review suggests that it is the case, but testing it anyway would >>> be better. >> >> I see what you mean: I tested it, a guest cannot crash Qemu. >> > > ping? I thought Alex had already merged it. I'm pretty sure that I stated somewhere that the patch is okay for me now. In case I didn't: Acked-by: Kevin Wolf <kwolf@redhat.com>
On 15.07.2011, at 12:52, Kevin Wolf wrote: > Am 15.07.2011 12:34, schrieb Stefano Stabellini: >> On Fri, 1 Jul 2011, Stefano Stabellini wrote: >>> On Fri, 1 Jul 2011, Kevin Wolf wrote: >>>> Am 30.06.2011 16:16, schrieb Stefano Stabellini: >>>>> On Thu, 30 Jun 2011, Kevin Wolf wrote: >>>>>>> +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; >>>>>> >>>>>> Have you tested if this is enough if the guest tries to continue using >>>>>> the device? I don't know of any case where it's not sufficient, just >>>>>> trying to make sure that it's really true in practice. >>>>> >>>>> The purpose of this is to "hide" the disk from the guest. The unplug is >>>>> supposed to happen *before* the guest enumerates the IDE disks; it is >>>>> responsibility of the guest to make sure of it. >>>>> I tested it with Linux PV on HVM drivers, and Linux doesn't see the >>>>> emulated disk after the unplug, as it should be. >>>> >>>> Yeah. What I meant is that we should make sure that a misbehaving guest, >>>> which just keeps on playing with the IDE ports anyway, can't crash qemu. >>>> A quick review suggests that it is the case, but testing it anyway would >>>> be better. >>> >>> I see what you mean: I tested it, a guest cannot crash Qemu. >>> >> >> ping? > > I thought Alex had already merged it. I'm pretty sure that I stated > somewhere that the patch is okay for me now. In case I didn't: > > Acked-by: Kevin Wolf <kwolf@redhat.com> Ah, must have missed it :). Thanks. Stefano, could you please rebase? The patch doesn't apply cleanly anymore. Alex
On Mon, 18 Jul 2011, Alexander Graf wrote: > On 15.07.2011, at 12:52, Kevin Wolf wrote: > > > Am 15.07.2011 12:34, schrieb Stefano Stabellini: > >> On Fri, 1 Jul 2011, Stefano Stabellini wrote: > >>> On Fri, 1 Jul 2011, Kevin Wolf wrote: > >>>> Am 30.06.2011 16:16, schrieb Stefano Stabellini: > >>>>> On Thu, 30 Jun 2011, Kevin Wolf wrote: > >>>>>>> +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; > >>>>>> > >>>>>> Have you tested if this is enough if the guest tries to continue using > >>>>>> the device? I don't know of any case where it's not sufficient, just > >>>>>> trying to make sure that it's really true in practice. > >>>>> > >>>>> The purpose of this is to "hide" the disk from the guest. The unplug is > >>>>> supposed to happen *before* the guest enumerates the IDE disks; it is > >>>>> responsibility of the guest to make sure of it. > >>>>> I tested it with Linux PV on HVM drivers, and Linux doesn't see the > >>>>> emulated disk after the unplug, as it should be. > >>>> > >>>> Yeah. What I meant is that we should make sure that a misbehaving guest, > >>>> which just keeps on playing with the IDE ports anyway, can't crash qemu. > >>>> A quick review suggests that it is the case, but testing it anyway would > >>>> be better. > >>> > >>> I see what you mean: I tested it, a guest cannot crash Qemu. > >>> > >> > >> ping? > > > > I thought Alex had already merged it. I'm pretty sure that I stated > > somewhere that the patch is okay for me now. In case I didn't: > > > > Acked-by: Kevin Wolf <kwolf@redhat.com> > > Ah, must have missed it :). Thanks. > > Stefano, could you please rebase? The patch doesn't apply cleanly anymore. OK, I'll send it again based on your latest xen-next branch.
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 c349644..50ab7c5 100644 --- a/hw/ide/piix.c +++ b/hw/ide/piix.c @@ -167,6 +167,42 @@ static int pci_piix4_ide_initfn(PCIDevice *dev) return pci_piix_ide_initfn(d); } +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) @@ -197,6 +233,11 @@ static PCIDeviceInfo piix_ide_info[] = { .no_hotplug = 1, .init = pci_piix3_ide_initfn, },{ + .qdev.name = "piix3-ide-xen", + .qdev.size = sizeof(PCIIDEState), + .qdev.no_user = 1, + .init = pci_piix3_ide_initfn, + },{ .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 8dbeb0c..b59adcc 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) {