Re: [PATCH v2] hw/ide/piix: properly initialize the BMIBA register

[Bernhard Beschow]

Am 5. Juli 2023 10:01:21 UTC schrieb Olaf Hering <olaf@aepfle.de>:
>Tue, 4 Jul 2023 08:38:33 +0200 Paolo Bonzini <pbonzini@redhat.com>:
>
>> I agree that calling pci_device_reset() would be a better match for 
>> pci_xen_ide_unplug().
>
>This change works as well:
>
>--- 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:

Hi Olaf,

Would you mind sending this patch as well? The PIIX fix alone just fixes the 
syptom, not the underlying problem. The underlying problem is that the BAR 
isn't deactivated, and with the PIIX patch it will stay at address zero rather 
than in the USB function address range.

I think this patch should ideally land in 8.1, and IIUC bug fixes are still 
accepted for it.

Best regards,
Bernhard

>--- a/hw/ide/piix.c
>+++ b/hw/ide/piix.c
>@@ -118,7 +118,6 @@ static void piix_ide_reset(DeviceState *dev)
>     pci_set_word(pci_conf + PCI_COMMAND, 0x0000);
>     pci_set_word(pci_conf + PCI_STATUS,
>                  PCI_STATUS_DEVSEL_MEDIUM | PCI_STATUS_FAST_BACK);
>-    pci_set_byte(pci_conf + 0x20, 0x01);  /* BMIBA: 20-23h */
> }
> 
> static bool pci_piix_init_bus(PCIIDEState *d, unsigned i, Error **errp)
>
>
>Olaf