qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Alexey Kardashevskiy <aik@ozlabs.ru>
To: qemu-devel@nongnu.org
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>,
	qemu-ppc@nongnu.org, Alexander Graf <agraf@suse.de>
Subject: [Qemu-devel] [PATCH] spapr_pci: fix MSI limit
Date: Mon,  5 May 2014 00:09:48 +1000	[thread overview]
Message-ID: <1399212588-23304-1-git-send-email-aik@ozlabs.ru> (raw)

At the moment XICS does not support interrupts reuse so sPAPR PHB
implements this. sPAPRPHBState holds array of 32 spapr_pci_msi to
describe PCI config address, first MSI and number of MSIs. Once
allocated for a device, QEMU tries reusing this config until the number
of MSIs changes.

Existing SPAPR guests call ibm,change-msi in a loop until the handler
returns the requested number of vectors.

Recently introduced check for the maximum number of MSI/MSIX vectors
supported by a device only works for a device which is new for PHB's
MSI cache. If it is already there, the check is not performed which
leads to new IRQ block allocation. This happens during PCI hotplug
even when the user hot plug the same device which he just hot unplugged.

This moves the check earlier.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---

I believe the proper fix would be implementing interrupt reuse which
I posted a while ago in [PATCH 0/8] spapr: fix IOMMU and XICS/IRQs migration,
patches 2..7 (ignore first anmd last patches of that set):

xics: add flags for interrupts
xics: add find_server
xics: add pre_load() hook to ICSStateClass
xics: disable flags reset on xics reset
spapr: move interrupt allocator to xics
spapr: remove @next_irq

This would allow us to get rid of sPAPRPHBState::msi_state.

Please comment. Thanks!
---
 hw/ppc/spapr_pci.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index c052917..1db73f2 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -280,7 +280,7 @@ 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;
+    int ndev, irq, max_irqs = 0;
     sPAPRPHBState *phb = NULL;
     PCIDevice *pdev = NULL;
 
@@ -333,6 +333,23 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPREnvironment *spapr,
     }
     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);
+    } else if (ret_intr_type == RTAS_TYPE_MSIX) {
+        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);
+        rtas_st(rets, 0, -1); /* Hardware error */
+        return;
+    }
+    /* Correct the number if the guest asked for too many */
+    if (req_num > max_irqs) {
+        req_num = max_irqs;
+    }
+
     /* 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 */
@@ -343,21 +360,6 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPREnvironment *spapr,
 
     /* There is no cached config, allocate MSIs */
     if (!phb->msi_table[ndev].nvec) {
-        int max_irqs = 0;
-        if (ret_intr_type == RTAS_TYPE_MSI) {
-            max_irqs = msi_nr_vectors_allocated(pdev);
-        } else if (ret_intr_type == RTAS_TYPE_MSIX) {
-            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);
-            rtas_st(rets, 0, -1); /* Hardware error */
-            return;
-        }
-        if (req_num > max_irqs) {
-            req_num = max_irqs;
-        }
         irq = spapr_allocate_irq_block(req_num, false,
                                        ret_intr_type == RTAS_TYPE_MSI);
         if (irq < 0) {
-- 
1.8.4.rc4

             reply	other threads:[~2014-05-04 14:10 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-04 14:09 Alexey Kardashevskiy [this message]
2014-05-08 10:00 ` [Qemu-devel] [PATCH] spapr_pci: fix MSI limit Alexander Graf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1399212588-23304-1-git-send-email-aik@ozlabs.ru \
    --to=aik@ozlabs.ru \
    --cc=agraf@suse.de \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).