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

[Marcel Apfelbaum]

Hi Aleksandr,

On 21/09/2017 0:21, Aleksandr Bezzubikov wrote:

Signed-off-by: Aleksandr Bezzubikov <zuban...@gmail.com>
---
  hw/pci-bridge/pcie_pci_bridge.c | 24 ++++++++++++++++++------
  1 file changed, 18 insertions(+), 6 deletions(-)

Please add Eduardo's Reported-by tag and the
actual failure in the commit message.
You might even explain in the commit message that you
moved msi prop to ON_OFF_AUTO_AUTO.

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);

In that case it will fallback to legacy INTx, right?

Thanks,
Marcel

+            } 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(),
  };