qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kurz <groug@kaod.org>
To: qemu-devel@nongnu.org
Cc: "Greg Kurz" <groug@kaod.org>,
	qemu-ppc@nongnu.org, "Cédric Le Goater" <clg@kaod.org>,
	"David Gibson" <david@gibson.dropbear.id.au>
Subject: [PATCH for-6.0 4/8] spapr/xive: Add "nr-ipis" property
Date: Fri, 20 Nov 2020 18:46:42 +0100	[thread overview]
Message-ID: <20201120174646.619395-5-groug@kaod.org> (raw)
In-Reply-To: <20201120174646.619395-1-groug@kaod.org>

The sPAPR XIVE device exposes a range of LISNs that the guest uses
for IPIs. This range is currently sized according to the highest
vCPU id, ie. spapr_max_server_number(), as obtained from the machine
through the "nr_servers" argument of the generic spapr_irq_dt() call.

This makes sense for the "ibm,interrupt-server-ranges" property of
sPAPR XICS, but certainly not for "ibm,xive-lisn-ranges". The range
should be sized to the maximum number of possible vCPUs. It happens
that spapr_max_server_number() == smp.max_cpus in practice with the
machine default settings. This might not be the case though when
VSMT is in use : we can end up with a much larger range (up to 8
times bigger) than needed and waste LISNs. But most importantly, this
lures people into thinking that IPI numbers are always equal to
vCPU ids, which is wrong and bit us recently:

https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg01476.html

Introduce an "nr-ipis" property that the machine sets to smp.max_cpus
before realizing the deice. Use that instead of "nr_servers" in
spapr_xive_dt(). Have the machine to claim the same number of IPIs
in spapr_irq_init().

This doesn't cause any guest visible change when using the machine
default settings (ie. VSMT == smp.threads).

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 include/hw/ppc/spapr_xive.h | 8 ++++++++
 hw/intc/spapr_xive.c        | 4 +++-
 hw/ppc/spapr_irq.c          | 4 +++-
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
index 7123ea07ed78..69b9fbc1b9a5 100644
--- a/include/hw/ppc/spapr_xive.h
+++ b/include/hw/ppc/spapr_xive.h
@@ -43,6 +43,14 @@ typedef struct SpaprXive {
 
     /* DT */
     gchar *nodename;
+    /*
+     * The sPAPR XIVE device needs to know how many vCPUs it
+     * might be exposed to in order to size the IPI range in
+     * "ibm,interrupt-server-ranges". It is the purpose of the
+     * "nr-ipis" property which *must* be set to a non-null
+     * value before realizing the sPAPR XIVE device.
+     */
+    uint32_t nr_ipis;
 
     /* Routing table */
     XiveEAS       *eat;
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index e4dbf9c2c409..d13a2955ce9b 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -311,6 +311,7 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp)
     /* Set by spapr_irq_init() */
     g_assert(xive->nr_irqs);
     g_assert(xive->nr_servers);
+    g_assert(xive->nr_ipis);
 
     sxc->parent_realize(dev, &local_err);
     if (local_err) {
@@ -608,6 +609,7 @@ static Property spapr_xive_properties[] = {
      */
     DEFINE_PROP_UINT32("nr-ends", SpaprXive, nr_ends_vmstate, 0),
     DEFINE_PROP_UINT32("nr-servers", SpaprXive, nr_servers, 0),
+    DEFINE_PROP_UINT32("nr-ipis", SpaprXive, nr_ipis, 0),
     DEFINE_PROP_UINT64("vc-base", SpaprXive, vc_base, SPAPR_XIVE_VC_BASE),
     DEFINE_PROP_UINT64("tm-base", SpaprXive, tm_base, SPAPR_XIVE_TM_BASE),
     DEFINE_PROP_UINT8("hv-prio", SpaprXive, hv_prio, 7),
@@ -698,7 +700,7 @@ static void spapr_xive_dt(SpaprInterruptController *intc, uint32_t nr_servers,
     /* Interrupt number ranges for the IPIs */
     uint32_t lisn_ranges[] = {
         cpu_to_be32(SPAPR_IRQ_IPI),
-        cpu_to_be32(SPAPR_IRQ_IPI + nr_servers),
+        cpu_to_be32(SPAPR_IRQ_IPI + xive->nr_ipis),
     };
     /*
      * EQ size - the sizes of pages supported by the system 4K, 64K,
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index 8c5627225636..a0fc474ecb06 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -325,12 +325,14 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
 
     if (spapr->irq->xive) {
         uint32_t nr_servers = spapr_max_server_number(spapr);
+        uint32_t max_cpus = MACHINE(spapr)->smp.max_cpus;
         DeviceState *dev;
         int i;
 
         dev = qdev_new(TYPE_SPAPR_XIVE);
         qdev_prop_set_uint32(dev, "nr-irqs", smc->nr_xirqs + SPAPR_XIRQ_BASE);
         qdev_prop_set_uint32(dev, "nr-servers", nr_servers);
+        qdev_prop_set_uint32(dev, "nr-ipis", max_cpus);
         object_property_set_link(OBJECT(dev), "xive-fabric", OBJECT(spapr),
                                  &error_abort);
         sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
@@ -338,7 +340,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
         spapr->xive = SPAPR_XIVE(dev);
 
         /* Enable the CPU IPIs */
-        for (i = 0; i < nr_servers; ++i) {
+        for (i = 0; i < max_cpus; ++i) {
             SpaprInterruptControllerClass *sicc
                 = SPAPR_INTC_GET_CLASS(spapr->xive);
 
-- 
2.26.2



  parent reply	other threads:[~2020-11-20 17:48 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-20 17:46 [PATCH for-6.0 0/8] spapr: Address the confusion between IPI numbers and vCPU ids Greg Kurz
2020-11-20 17:46 ` [PATCH for-6.0 1/8] spapr/xive: Turn some sanity checks into assertions Greg Kurz
2020-11-23  3:33   ` David Gibson
2020-11-23  8:09   ` Cédric Le Goater
2020-11-20 17:46 ` [PATCH for-6.0 2/8] spapr/xive: Introduce spapr_xive_nr_ends() Greg Kurz
2020-11-23  3:33   ` David Gibson
2020-11-25 22:43     ` Greg Kurz
2020-11-26  0:06       ` David Gibson
2020-11-23  9:46   ` Cédric Le Goater
2020-11-23 11:16     ` Greg Kurz
2020-11-24 13:54       ` Cédric Le Goater
2020-11-24 17:01         ` Greg Kurz
2020-11-24 17:56           ` Cédric Le Goater
2020-11-25  9:33             ` Greg Kurz
2020-11-25 11:34               ` Cédric Le Goater
2020-11-25 12:26                 ` Greg Kurz
2020-11-26  7:06                   ` Cédric Le Goater
2020-11-20 17:46 ` [PATCH for-6.0 3/8] spapr/xive: Add "nr-servers" property Greg Kurz
2020-11-23  3:52   ` David Gibson
2020-11-23  9:20     ` Greg Kurz
2020-11-23  9:56   ` Cédric Le Goater
2020-11-23 11:25     ` Greg Kurz
2020-11-24 14:18       ` Cédric Le Goater
2020-11-20 17:46 ` Greg Kurz [this message]
2020-11-23  4:10   ` [PATCH for-6.0 4/8] spapr/xive: Add "nr-ipis" property David Gibson
2020-11-23 10:13   ` Cédric Le Goater
2020-11-24 17:18     ` Greg Kurz
2020-11-20 17:46 ` [PATCH for-6.0 5/8] spapr/xics: Drop unused argument to xics_kvm_has_broken_disconnect() Greg Kurz
2020-11-23  4:10   ` David Gibson
2020-11-23 10:15   ` Cédric Le Goater
2020-11-20 17:46 ` [PATCH for-6.0 6/8] spapr/xics: Add "nr-servers" property Greg Kurz
2020-11-23  4:18   ` David Gibson
2020-11-23  9:39     ` Greg Kurz
2020-11-23 10:24   ` Cédric Le Goater
2020-11-20 17:46 ` [PATCH for-6.0 7/8] spapr: Drop "nr_servers" argument of the sPAPR IC activate() operation Greg Kurz
2020-11-23  4:38   ` David Gibson
2020-11-23  9:47     ` Greg Kurz
2020-11-20 17:46 ` [PATCH for-6.0 8/8] spapr: Drop "nr_servers" argument of the sPAPR IC dt() operation Greg Kurz
2020-11-23  8:04 ` [PATCH for-6.0 0/8] spapr: Address the confusion between IPI numbers and vCPU ids Cédric Le Goater
2020-11-23 10:07   ` Greg Kurz

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=20201120174646.619395-5-groug@kaod.org \
    --to=groug@kaod.org \
    --cc=clg@kaod.org \
    --cc=david@gibson.dropbear.id.au \
    --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).