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

[Xen-changelog] [qemu-xen master] spapr_pci: fix MSI/MSIX selection



commit 80277d7cd5522e8dbfd68413b1a935d9f19be8e8
Author:     Greg Kurz <groug@xxxxxxxx>
AuthorDate: Fri Jan 26 23:25:24 2018 +0100
Commit:     Michael Roth <mdroth@xxxxxxxxxxxxxxxxxx>
CommitDate: Thu Feb 1 15:21:29 2018 -0600

    spapr_pci: fix MSI/MSIX selection
    
    In various place we don't correctly check if the device supports MSI or
    MSI-X. This can cause devices to be advertised with MSI support, even
    if they only support MSI-X (like virtio-pci-* devices for example):
    
                    ethernet@0 {
                            ibm,req#msi = <0x1>; <--- wrong!
                        .
                        ibm,loc-code = "qemu_virtio-net-pci:0000:00:00.0";
                        .
                        ibm,req#msi-x = <0x3>;
                    };
    
    Worse, this can also cause the "ibm,change-msi" RTAS call to corrupt the
    PCI status and cause migration to fail:
    
      qemu-system-ppc64: get_pci_config_device: Bad config data: i=0x6
        read: 0 device: 10 cmask: 10 wmask: 0 w1cmask:0
                                  ^^
               PCI_STATUS_CAP_LIST bit which is assumed to be constant
    
    This patch changes spapr_populate_pci_child_dt() to properly check for
    MSI support using msi_present(): this ensures that PCIDevice::msi_cap
    was set by msi_init() and that msi_nr_vectors_allocated() will look at
    the right place in the config space.
    
    Checking PCIDevice::msix_entries_nr is enough for MSI-X but let's add
    a call to msix_present() there as well for consistency.
    
    It also changes rtas_ibm_change_msi() to select the appropriate MSI
    type in Function 1 instead of always selecting plain MSI. This new
    behaviour is compliant with LoPAPR 1.1, as described in "Table 71.
    ibm,change-msi Argument Call Buffer":
    
      Function 1: If Number Outputs is equal to 3, request to set to a new
               number of MSIs (including set to 0).
               If the â??ibm,change-msix-capableâ?? property exists and Number
               Outputs is equal to 4, request is to set to a new number of
               MSI or MSI-X (platform choice) interrupts (including set to
               0).
    
    Since MSI is the the platform default (LoPAPR 6.2.3 MSI Option), let's
    check for MSI support first.
    
    And finally, it checks the input parameters are valid, as described in
    LoPAPR 1.1 "R1â??7.3.10.5.1â??3":
    
      For the MSI option: The platform must return a Status of -3 (Parameter
      error) from ibm,change-msi, with no change in interrupt assignments if
      the PCI configuration address does not support MSI and Function 3 was
      requested (that is, the â??ibm,req#msiâ?? property must exist for the PCI
      configuration address in order to use Function 3), or does not support
      MSI-X and Function 4 is requested (that is, the â??ibm,req#msi-xâ?? 
property
      must exist for the PCI configuration address in order to use Function 4),
      or if neither MSIs nor MSI-Xs are supported and Function 1 is requested.
    
    This ensures that the ret_intr_type variable contains a valid MSI type
    for this device, and that spapr_msi_setmsg() won't corrupt the PCI status.
    
    Signed-off-by: Greg Kurz <groug@xxxxxxxx>
    Reviewed-by: Alexey Kardashevskiy <aik@xxxxxxxxx>
    Signed-off-by: David Gibson <david@xxxxxxxxxxxxxxxxxxxxx>
    (cherry picked from commit 9cbe305b60cc49cfcd134765b85c28be95b1b57d)
    Signed-off-by: Michael Roth <mdroth@xxxxxxxxxxxxxxxxxx>
---
 hw/ppc/spapr_pci.c | 61 +++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 42 insertions(+), 19 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 5a3122a..a1929ab 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -280,13 +280,42 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, 
sPAPRMachineState *spapr,
     int *config_addr_key;
     Error *err = NULL;
 
+    /* Fins sPAPRPHBState */
+    phb = spapr_pci_find_phb(spapr, buid);
+    if (phb) {
+        pdev = spapr_pci_find_dev(spapr, buid, config_addr);
+    }
+    if (!phb || !pdev) {
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
     switch (func) {
-    case RTAS_CHANGE_MSI_FN:
     case RTAS_CHANGE_FN:
-        ret_intr_type = RTAS_TYPE_MSI;
+        if (msi_present(pdev)) {
+            ret_intr_type = RTAS_TYPE_MSI;
+        } else if (msix_present(pdev)) {
+            ret_intr_type = RTAS_TYPE_MSIX;
+        } else {
+            rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+            return;
+        }
+        break;
+    case RTAS_CHANGE_MSI_FN:
+        if (msi_present(pdev)) {
+            ret_intr_type = RTAS_TYPE_MSI;
+        } else {
+            rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+            return;
+        }
         break;
     case RTAS_CHANGE_MSIX_FN:
-        ret_intr_type = RTAS_TYPE_MSIX;
+        if (msix_present(pdev)) {
+            ret_intr_type = RTAS_TYPE_MSIX;
+        } else {
+            rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+            return;
+        }
         break;
     default:
         error_report("rtas_ibm_change_msi(%u) is not implemented", func);
@@ -294,16 +323,6 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, 
sPAPRMachineState *spapr,
         return;
     }
 
-    /* Fins sPAPRPHBState */
-    phb = spapr_pci_find_phb(spapr, buid);
-    if (phb) {
-        pdev = spapr_pci_find_dev(spapr, buid, config_addr);
-    }
-    if (!phb || !pdev) {
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
     msi = (spapr_pci_msi *) g_hash_table_lookup(phb->msi, &config_addr);
 
     /* Releasing MSIs */
@@ -1286,13 +1305,17 @@ static void spapr_populate_pci_child_dt(PCIDevice *dev, 
void *fdt, int offset,
     _FDT(fdt_setprop_cell(fdt, offset, "#size-cells",
                           RESOURCE_CELLS_SIZE));
 
-    max_msi = msi_nr_vectors_allocated(dev);
-    if (max_msi) {
-        _FDT(fdt_setprop_cell(fdt, offset, "ibm,req#msi", max_msi));
+    if (msi_present(dev)) {
+        max_msi = msi_nr_vectors_allocated(dev);
+        if (max_msi) {
+            _FDT(fdt_setprop_cell(fdt, offset, "ibm,req#msi", max_msi));
+        }
     }
-    max_msix = dev->msix_entries_nr;
-    if (max_msix) {
-        _FDT(fdt_setprop_cell(fdt, offset, "ibm,req#msi-x", max_msix));
+    if (msix_present(dev)) {
+        max_msix = dev->msix_entries_nr;
+        if (max_msix) {
+            _FDT(fdt_setprop_cell(fdt, offset, "ibm,req#msi-x", max_msix));
+        }
     }
 
     populate_resource_props(dev, &rp);
--
generated by git-patchbot for /home/xen/git/qemu-xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.