qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v5 09/11] pci bridge dev: change msi property ty


From: Marcel Apfelbaum
Subject: Re: [Qemu-devel] [PATCH v5 09/11] pci bridge dev: change msi property type
Date: Sun, 15 May 2016 16:25:59 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0

On 05/06/2016 07:20 AM, Cao jin wrote:
 From bit to enum OnOffAuto.

cc: Michael S. Tsirkin <address@hidden>
cc: Markus Armbruster <address@hidden>
cc: Marcel Apfelbaum <address@hidden>
Signed-off-by: Cao jin <address@hidden>
---

Actually, I am not quite sure this device need this change, RFC.


Well, it already has the 'msi' property, so we may want to make it standard 
'OnOffAuto'.
One problem I can see is the change of semantics. Until now msi=on means 
'auto'. From now on
it means 'force msi=on', fail otherwise. If I got this right,  old machines 
having msi=on
will failed to start on platforms with msibroken=true, right?

Maybe we should preserve the semantics for old machines? (this patch does not 
actually
affect the semantics, but patch 11/11 should, otherwise why change it to 
OnOffAuto, right? )

Thanks,
Marcel


  hw/pci-bridge/pci_bridge_dev.c | 14 ++++++++------
  1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c
index 32f4daa..9e31f0e 100644
--- a/hw/pci-bridge/pci_bridge_dev.c
+++ b/hw/pci-bridge/pci_bridge_dev.c
@@ -41,9 +41,10 @@ struct PCIBridgeDev {

      MemoryRegion bar;
      uint8_t chassis_nr;
-#define PCI_BRIDGE_DEV_F_MSI_REQ 0
-#define PCI_BRIDGE_DEV_F_SHPC_REQ 1
+#define PCI_BRIDGE_DEV_F_SHPC_REQ 0
      uint32_t flags;
+
+    OnOffAuto msi;
  };
  typedef struct PCIBridgeDev PCIBridgeDev;

@@ -65,7 +66,7 @@ static int pci_bridge_dev_initfn(PCIDevice *dev)
          }
      } else {
          /* MSI is not applicable without SHPC */
-        bridge_dev->flags &= ~(1 << PCI_BRIDGE_DEV_F_MSI_REQ);
+        bridge_dev->msi = ON_OFF_AUTO_OFF;
      }

      err = slotid_cap_init(dev, 0, bridge_dev->chassis_nr, 0);
@@ -73,7 +74,8 @@ static int pci_bridge_dev_initfn(PCIDevice *dev)
          goto slotid_error;
      }

-    if ((bridge_dev->flags & (1 << PCI_BRIDGE_DEV_F_MSI_REQ)) &&
+    if ((bridge_dev->msi == ON_OFF_AUTO_AUTO ||
+                bridge_dev->msi == ON_OFF_AUTO_ON) &&
          msi_nonbroken) {
          err = msi_init(dev, 0, 1, true, true);
          if (err < 0) {
@@ -146,8 +148,8 @@ static Property pci_bridge_dev_properties[] = {
                      /* Note: 0 is not a legal chassis number. */
      DEFINE_PROP_UINT8(PCI_BRIDGE_DEV_PROP_CHASSIS_NR, PCIBridgeDev, 
chassis_nr,
                        0),
-    DEFINE_PROP_BIT(PCI_BRIDGE_DEV_PROP_MSI, PCIBridgeDev, flags,
-                    PCI_BRIDGE_DEV_F_MSI_REQ, true),
+    DEFINE_PROP_ON_OFF_AUTO(PCI_BRIDGE_DEV_PROP_MSI, PCIBridgeDev, msi,
+                            ON_OFF_AUTO_AUTO),
      DEFINE_PROP_BIT(PCI_BRIDGE_DEV_PROP_SHPC, PCIBridgeDev, flags,
                      PCI_BRIDGE_DEV_F_SHPC_REQ, true),
      DEFINE_PROP_END_OF_LIST(),





reply via email to

[Prev in Thread] Current Thread [Next in Thread]