[Qemu-devel] [PATCH v2] hw/pci-bridge/pcie_pci_bridge: properly handle MSI unavailability case

Aleksandr Bezzubikov posted 1 patch 6 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1506295318-10479-1-git-send-email-zuban32s@gmail.com
Test checkpatch passed
Test docker passed
Test s390x passed
hw/pci-bridge/pcie_pci_bridge.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
[Qemu-devel] [PATCH v2] hw/pci-bridge/pcie_pci_bridge: properly handle MSI unavailability case
Posted by Aleksandr Bezzubikov 6 years, 7 months ago
QEMU with the pcie-pci-bridge device crashes if the guest board doesn't support MSI,
e.g. 'qemu-system-ppc64 -M prep -device pcie-pci-bridge'.
This is caused by wrong pcie-pci-bridge instantiation error handling. This patch fixes this issue
by falling back to legacy INTx if MSI is not available.
Also set the bridge's 'msi' property default value to 'auto' in order to trigger errors 
only when user explicitly set msi=on.

v2:
rewrite the commit message

Reported-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Aleksandr Bezzubikov <zuban32s@gmail.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
 hw/pci-bridge/pcie_pci_bridge.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/hw/pci-bridge/pcie_pci_bridge.c b/hw/pci-bridge/pcie_pci_bridge.c
index 9aa5cc3..da562fe 100644
--- a/hw/pci-bridge/pcie_pci_bridge.c
+++ b/hw/pci-bridge/pcie_pci_bridge.c
@@ -65,10 +65,18 @@ static void pcie_pci_bridge_realize(PCIDevice *d, Error **errp)
         goto aer_error;
     }
 
+    Error *local_err = NULL;
     if (pcie_br->msi != ON_OFF_AUTO_OFF) {
-        rc = msi_init(d, 0, 1, true, true, errp);
+        rc = msi_init(d, 0, 1, true, true, &local_err);
         if (rc < 0) {
-            goto msi_error;
+            assert(rc == -ENOTSUP);
+            if (pcie_br->msi != ON_OFF_AUTO_ON) {
+                error_free(local_err);
+            } else {
+                /* failed to satisfy user's explicit request for MSI */
+                error_propagate(errp, local_err);
+                goto msi_error;
+            }
         }
     }
     pci_register_bar(d, 0, PCI_BASE_ADDRESS_SPACE_MEMORY |
@@ -81,7 +89,7 @@ aer_error:
 pm_error:
     pcie_cap_exit(d);
 cap_error:
-    shpc_free(d);
+    shpc_cleanup(d, &pcie_br->shpc_bar);
 error:
     pci_bridge_exitfn(d);
 }
@@ -98,7 +106,9 @@ static void pcie_pci_bridge_reset(DeviceState *qdev)
 {
     PCIDevice *d = PCI_DEVICE(qdev);
     pci_bridge_reset(qdev);
-    msi_reset(d);
+    if (msi_present(d)) {
+        msi_reset(d);
+    }
     shpc_reset(d);
 }
 
@@ -106,12 +116,14 @@ static void pcie_pci_bridge_write_config(PCIDevice *d,
         uint32_t address, uint32_t val, int len)
 {
     pci_bridge_write_config(d, address, val, len);
-    msi_write_config(d, address, val, len);
+    if (msi_present(d)) {
+        msi_write_config(d, address, val, len);
+    }
     shpc_cap_write_config(d, address, val, len);
 }
 
 static Property pcie_pci_bridge_dev_properties[] = {
-        DEFINE_PROP_ON_OFF_AUTO("msi", PCIEPCIBridge, msi, ON_OFF_AUTO_ON),
+        DEFINE_PROP_ON_OFF_AUTO("msi", PCIEPCIBridge, msi, ON_OFF_AUTO_AUTO),
         DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.7.4


Re: [Qemu-devel] [PATCH v2] hw/pci-bridge/pcie_pci_bridge: properly handle MSI unavailability case
Posted by Thomas Huth 6 years, 7 months ago
On 25.09.2017 01:21, Aleksandr Bezzubikov wrote:
> QEMU with the pcie-pci-bridge device crashes if the guest board doesn't support MSI,
> e.g. 'qemu-system-ppc64 -M prep -device pcie-pci-bridge'.
> This is caused by wrong pcie-pci-bridge instantiation error handling. This patch fixes this issue
> by falling back to legacy INTx if MSI is not available.
> Also set the bridge's 'msi' property default value to 'auto' in order to trigger errors 
> only when user explicitly set msi=on.
> 
> v2:
> rewrite the commit message
> 
> Reported-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Aleksandr Bezzubikov <zuban32s@gmail.com>
> Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
> ---
>  hw/pci-bridge/pcie_pci_bridge.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)

This also fixes the issue that I've seen with qemu-system-mips64el today:

$ mips64el-softmmu/qemu-system-mips64el -M malta -nographic -S -device
pcie-pci-bridge -bios pc-bios/bios.bin
qemu-system-mips64el: memory.c:1699: memory_region_finalize:
 Assertion `!mr->container' failed.
Aborted (core dumped)

So feel free to add:

Tested-by: Thomas Huth <thuth@redhat.com>