On 30.05.14 11:34, Alexey Kardashevskiy wrote:
Currently SPAPR PHB keeps track of all allocated MSI (here and below
MSI stands for both MSI and MSIX) interrupt because
XICS used to be unable to reuse interrupts. This is a problem for
dynamic MSI reconfiguration which happens when guest reloads a driver
or performs PCI hotplug. Another problem is that the existing
implementation can enable MSI on 32 devices maximum
(SPAPR_MSIX_MAX_DEVS=32) and there is no good reason for that.
This makes use of new XICS ability to reuse interrupts.
This reorganizes MSI information storage in sPAPRPHBState. Instead of
static array of 32 descriptors (one per a PCI function), this patch adds
a GHashTable when @config_addr is a key and (first_irq, num) pair is
a value. GHashTable can dynamically grow and shrink so the initial limit
of 32 devices is gone.
This changes migration stream as @msi_table was a static array while new
@msi_devs is a dynamic hash table. This adds temporary array which is
used for migration, it is populated in "spapr_pci"::pre_save() callback
and expanded into the hash table in post_load() callback. Since
the destination side does not know the number of MSI-enabled devices
in advance and cannot pre-allocate the temporary array to receive
migration state, this makes use of new VMSTATE_STRUCT_VARRAY_ALLOC macro
which allocates the array automatically.
This resets the MSI configuration space when interrupts are released by
the ibm,change-msi RTAS call.
This fixed traces to be more informative.
This changes vmstate_spapr_pci_msi name from "...lsi" to "...msi" which
was incorrect by accident. As the internal representation changed,
thus bumps migration version number.
Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
---
Changes:
v3:
* used GHashTable
* implemented temporary MSI state array for migration
---
hw/ppc/spapr_pci.c | 195 +++++++++++++++++++++++++-------------------
include/hw/pci-host/spapr.h | 21 +++--
trace-events | 5 +-
3 files changed, 129 insertions(+), 92 deletions(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index a2f9677..48c9aad 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -220,36 +220,12 @@ static void rtas_write_pci_config(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
}
/*
- * Find an entry with config_addr or returns the empty one if not found AND
- * alloc_new is set.
- * At the moment the msi_table entries are never released so there is
- * no point to look till the end of the list if we need to find the free entry.
- */
-static int spapr_msicfg_find(sPAPRPHBState *phb, uint32_t config_addr,
- bool alloc_new)
-{
- int i;
-
- for (i = 0; i < SPAPR_MSIX_MAX_DEVS; ++i) {
- if (!phb->msi_table[i].nvec) {
- break;
- }
- if (phb->msi_table[i].config_addr == config_addr) {
- return i;
- }
- }
- if ((i < SPAPR_MSIX_MAX_DEVS) && alloc_new) {
- trace_spapr_pci_msi("Allocating new MSI config", i, config_addr);
- return i;
- }
-
- return -1;
-}
-
-/*
* Set MSI/MSIX message data.
* This is required for msi_notify()/msix_notify() which
* will write at the addresses via spapr_msi_write().
+ *
+ * If hwaddr == 0, all entries will have .data == first_irq i.e.
+ * table will be reset.
*/
static void spapr_msi_setmsg(PCIDevice *pdev, hwaddr addr, bool msix,
unsigned first_irq, unsigned req_num)
@@ -263,9 +239,12 @@ static void spapr_msi_setmsg(PCIDevice *pdev, hwaddr addr,
bool msix,
return;
}
- for (i = 0; i < req_num; ++i, ++msg.data) {
+ for (i = 0; i < req_num; ++i) {
msix_set_message(pdev, i, msg);
trace_spapr_pci_msi_setup(pdev->name, i, msg.address);
+ if (addr) {
+ ++msg.data;
+ }
}
}
@@ -280,9 +259,12 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPREnvironment *spapr,
unsigned int req_num = rtas_ld(args, 4); /* 0 == remove all */
unsigned int seq_num = rtas_ld(args, 5);
unsigned int ret_intr_type;
- int ndev, irq, max_irqs = 0;
+ unsigned int irq, max_irqs = 0, num = 0;
sPAPRPHBState *phb = NULL;
PCIDevice *pdev = NULL;
+ bool msix = false;
+ spapr_pci_msi *msi;
+ int *config_addr_key;
switch (func) {
case RTAS_CHANGE_MSI_FN:
@@ -310,13 +292,18 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
/* Releasing MSIs */
if (!req_num) {
- ndev = spapr_msicfg_find(phb, config_addr, false);
- if (ndev < 0) {
- trace_spapr_pci_msi("MSI has not been enabled", -1, config_addr);
+ msi = (spapr_pci_msi *) g_hash_table_lookup(phb->msi, &config_addr);
+ if (!msi) {
+ trace_spapr_pci_msi("Releasing wrong config", config_addr);
rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
return;
}
- trace_spapr_pci_msi("Released MSIs", ndev, config_addr);
+
+ xics_free(spapr->icp, msi->first_irq, msi->num);
+ spapr_msi_setmsg(pdev, 0, msix, 0, num);
+ g_hash_table_remove(phb->msi, &config_addr);
Are you sure this doesn't have to be the exact same element? That
pointer is to the stack, not to the element.
+
+ trace_spapr_pci_msi("Released MSIs", config_addr);
rtas_st(rets, 0, RTAS_OUT_SUCCESS);
rtas_st(rets, 1, 0);
return;
@@ -324,15 +311,6 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
/* Enabling MSI */
- /* Find a device number in the map to add or reuse the existing one */
- ndev = spapr_msicfg_find(phb, config_addr, true);
- if (ndev >= SPAPR_MSIX_MAX_DEVS || ndev < 0) {
- error_report("No free entry for a new MSI device");
- rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
- return;
- }
- trace_spapr_pci_msi("Configuring MSI", ndev, config_addr);
-
/* Check if the device supports as many IRQs as requested */
if (ret_intr_type == RTAS_TYPE_MSI) {
max_irqs = msi_nr_vectors_allocated(pdev);
@@ -340,48 +318,47 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
max_irqs = pdev->msix_entries_nr;
}
if (!max_irqs) {
- error_report("Requested interrupt type %d is not enabled for
device#%d",
- ret_intr_type, ndev);
+ error_report("Requested interrupt type %d is not enabled for device
%x",
+ ret_intr_type, config_addr);
rtas_st(rets, 0, -1); /* Hardware error */
return;
}
/* Correct the number if the guest asked for too many */
if (req_num > max_irqs) {
+ trace_spapr_pci_msi_retry(config_addr, req_num, max_irqs);
req_num = max_irqs;
+ irq = 0; /* to avoid misleading trace */
+ goto out;
}
- /* Check if there is an old config and MSI number has not changed */
- if (phb->msi_table[ndev].nvec && (req_num != phb->msi_table[ndev].nvec)) {
- /* Unexpected behaviour */
- error_report("Cannot reuse MSI config for device#%d", ndev);
+ /* Allocate MSIs */
+ irq = xics_alloc_block(spapr->icp, 0, req_num, false,
+ ret_intr_type == RTAS_TYPE_MSI);
+ if (!irq) {
+ error_report("Cannot allocate MSIs for device %x", config_addr);
rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
return;
}
- /* There is no cached config, allocate MSIs */
- if (!phb->msi_table[ndev].nvec) {
- irq = xics_alloc_block(spapr->icp, 0, req_num, false,
- ret_intr_type == RTAS_TYPE_MSI);
- if (irq < 0) {
- error_report("Cannot allocate MSIs for device#%d", ndev);
- rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
- return;
- }
- phb->msi_table[ndev].irq = irq;
- phb->msi_table[ndev].nvec = req_num;
- phb->msi_table[ndev].config_addr = config_addr;
- }
-
/* Setup MSI/MSIX vectors in the device (via cfgspace or MSIX BAR) */
spapr_msi_setmsg(pdev, spapr->msi_win_addr, ret_intr_type ==
RTAS_TYPE_MSIX,
- phb->msi_table[ndev].irq, req_num);
+ irq, req_num);
+ /* Add MSI device to cache */
+ msi = g_new(spapr_pci_msi, 1);
+ msi->first_irq = irq;
+ msi->num = req_num;
+ config_addr_key = g_new(int, 1);
gint?
Alex
