The IDE unplug function needs to reset the entire PCI device, to make
sure all state is initialized to defaults. This is done by calling
pci_device_reset, which resets not only the chip specific registers, but
also all PCI state. This fixes "unplug" in a Xen HVM domU with the
modular legacy xenlinux PV drivers.

Commit ee358e919e38 ("hw/ide/piix: Convert reset handler to
DeviceReset") changed the way how the the disks are unplugged. Prior
this commit the PCI device remained unchanged. After this change,
piix_ide_reset is exercised after the "unplug" command, which was not
the case prior that commit. This function resets the command register.
As a result the ata_piix driver inside the domU will see a disabled PCI
device. The generic PCI code will reenable the PCI device. On the qemu
side, this runs pci_default_write_config/pci_update_mappings. Here a
changed address is returned by pci_bar_address, this is the address
which was truncated in piix_ide_reset. In case of a Xen HVM domU, the
address changes from 0xc120 to 0xc100. This truncation was a bug in
piix_ide_reset, which was fixed in commit 230dfd9257 ("hw/ide/piix:
properly initialize the BMIBA register"). If pci_xen_ide_unplug had used
pci_device_reset, the PCI registers would have been properly reset, and
commit ee358e919e38 would have not introduced a regression for this
specific domU environment.

While the unplug is supposed to hide the IDE disks, the changed BMIBA
address broke the UHCI device. In case the domU has an USB tablet
configured, to recive absolute pointer coordinates for the GUI, it will
cause a hang during device discovery of the partly discovered USB hid
device. Reading the USBSTS word size register will fail. The access ends
up in the QEMU piix-bmdma device, instead of the expected uhci device.
Here a byte size request is expected, and a value of ~0 is returned. As
a result the UCHI driver sees an error state in the register, and turns
off the UHCI controller.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 hw/i386/xen/xen_platform.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
index 57f1d742c1..17457ff3de 100644
--- a/hw/i386/xen/xen_platform.c
+++ b/hw/i386/xen/xen_platform.c
@@ -164,8 +164,9 @@ static void pci_unplug_nics(PCIBus *bus)
  *
  * [1] https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/misc/hvm-emulated-unplug.pandoc
  */
-static void pci_xen_ide_unplug(DeviceState *dev, bool aux)
+static void pci_xen_ide_unplug(PCIDevice *d, bool aux)
 {
+    DeviceState *dev = DEVICE(d);
     PCIIDEState *pci_ide;
     int i;
     IDEDevice *idedev;
@@ -195,7 +196,7 @@ static void pci_xen_ide_unplug(DeviceState *dev, bool aux)
             blk_unref(blk);
         }
     }
-    device_cold_reset(dev);
+    pci_device_reset(d);
 }
 
 static void unplug_disks(PCIBus *b, PCIDevice *d, void *opaque)
@@ -210,7 +211,7 @@ static void unplug_disks(PCIBus *b, PCIDevice *d, void *opaque)
 
     switch (pci_get_word(d->config + PCI_CLASS_DEVICE)) {
     case PCI_CLASS_STORAGE_IDE:
-        pci_xen_ide_unplug(DEVICE(d), aux);
+        pci_xen_ide_unplug(d, aux);
         break;
 
     case PCI_CLASS_STORAGE_SCSI:

On 20/07/2023 08:29, Olaf Hering wrote:
> The IDE unplug function needs to reset the entire PCI device, to make
> sure all state is initialized to defaults. This is done by calling
> pci_device_reset, which resets not only the chip specific registers, but
> also all PCI state. This fixes "unplug" in a Xen HVM domU with the
> modular legacy xenlinux PV drivers.
> 
> Commit ee358e919e38 ("hw/ide/piix: Convert reset handler to
> DeviceReset") changed the way how the the disks are unplugged. Prior
> this commit the PCI device remained unchanged. After this change,
> piix_ide_reset is exercised after the "unplug" command, which was not
> the case prior that commit. This function resets the command register.
> As a result the ata_piix driver inside the domU will see a disabled PCI
> device. The generic PCI code will reenable the PCI device. On the qemu
> side, this runs pci_default_write_config/pci_update_mappings. Here a
> changed address is returned by pci_bar_address, this is the address
> which was truncated in piix_ide_reset. In case of a Xen HVM domU, the
> address changes from 0xc120 to 0xc100. This truncation was a bug in
> piix_ide_reset, which was fixed in commit 230dfd9257 ("hw/ide/piix:
> properly initialize the BMIBA register"). If pci_xen_ide_unplug had used
> pci_device_reset, the PCI registers would have been properly reset, and
> commit ee358e919e38 would have not introduced a regression for this
> specific domU environment.
> 
> While the unplug is supposed to hide the IDE disks, the changed BMIBA
> address broke the UHCI device. In case the domU has an USB tablet
> configured, to recive absolute pointer coordinates for the GUI, it will
> cause a hang during device discovery of the partly discovered USB hid
> device. Reading the USBSTS word size register will fail. The access ends
> up in the QEMU piix-bmdma device, instead of the expected uhci device.
> Here a byte size request is expected, and a value of ~0 is returned. As
> a result the UCHI driver sees an error state in the register, and turns
> off the UHCI controller.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
>   hw/i386/xen/xen_platform.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 

Reviewed-by: Paul Durrant <paul@xen.org>