Re: [PATCH v1] xen-platform: do full PCI reset during unplug of IDE devices

[Paul Durrant <xadimgnik@xxxxxxxxx>]

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@xxxxxxxxx>
> ---
>   hw/i386/xen/xen_platform.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
Reviewed-by: Paul Durrant <paul@xxxxxxx>