From: Greg Kurz <groug@kaod.org>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: "Jason Wang" <jasowang@redhat.com>,
"Riku Voipio" <riku.voipio@iki.fi>,
qemu-devel@nongnu.org, "Laurent Vivier" <laurent@vivier.eu>,
qemu-ppc@nongnu.org, clg@kaod.org,
"Marc-André Lureau" <marcandre.lureau@redhat.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
philmd@redhat.com
Subject: Re: [PATCH v2 09/33] spapr: Clarify and fix handling of nr_irqs
Date: Fri, 27 Sep 2019 09:53:59 +0200 [thread overview]
Message-ID: <20190927095359.4e07ea70@bahia.lan> (raw)
In-Reply-To: <20190927055028.11493-10-david@gibson.dropbear.id.au>
On Fri, 27 Sep 2019 15:50:04 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:
> Both the XICS and XIVE interrupt backends have a "nr-irqs" property, but
> it means slightly different things. For XICS (or, strictly, the ICS) it
> indicates the number of "real" external IRQs. Those start at XICS_IRQ_BASE
> (0x1000) and don't include the special IPI vector. For XIVE, however, it
> includes the whole IRQ space, including XIVE's many IPI vectors.
>
> The spapr code currently doesn't handle this sensibly, with the
> nr_irqs value in SpaprIrq having different meanings depending on the
> backend. We fix this by renaming nr_irqs to nr_xirqs and making it
> always indicate just the number of external irqs, adjusting the value
> we pass to XIVE accordingly. We also move to using common constants
> in most of the irq configurations, to make it clearer that the IRQ
> space looks the same to the guest (and emulated devices), even if the
> backend is different.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
Cedric and I have already given an R-b for this one.
Reviewed-by: Greg Kurz <groug@kaod.org>
> hw/ppc/spapr_irq.c | 53 ++++++++++++++------------------------
> include/hw/ppc/spapr_irq.h | 19 +++++++++-----
> 2 files changed, 31 insertions(+), 41 deletions(-)
>
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 8c26fa2d1e..3207b6bd01 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -92,7 +92,7 @@ static void spapr_irq_init_kvm(SpaprMachineState *spapr,
> * XICS IRQ backend.
> */
>
> -static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs,
> +static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_xirqs,
> Error **errp)
> {
> Object *obj;
> @@ -102,7 +102,7 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs,
> object_property_add_child(OBJECT(spapr), "ics", obj, &error_abort);
> object_property_add_const_link(obj, ICS_PROP_XICS, OBJECT(spapr),
> &error_fatal);
> - object_property_set_int(obj, nr_irqs, "nr-irqs", &error_fatal);
> + object_property_set_int(obj, nr_xirqs, "nr-irqs", &error_fatal);
> object_property_set_bool(obj, true, "realized", &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> @@ -234,13 +234,9 @@ static void spapr_irq_init_kvm_xics(SpaprMachineState *spapr, Error **errp)
> }
> }
>
> -#define SPAPR_IRQ_XICS_NR_IRQS 0x1000
> -#define SPAPR_IRQ_XICS_NR_MSIS \
> - (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI)
> -
> SpaprIrq spapr_irq_xics = {
> - .nr_irqs = SPAPR_IRQ_XICS_NR_IRQS,
> - .nr_msis = SPAPR_IRQ_XICS_NR_MSIS,
> + .nr_xirqs = SPAPR_NR_XIRQS,
> + .nr_msis = SPAPR_NR_MSIS,
> .ov5 = SPAPR_OV5_XIVE_LEGACY,
>
> .init = spapr_irq_init_xics,
> @@ -260,7 +256,7 @@ SpaprIrq spapr_irq_xics = {
> /*
> * XIVE IRQ backend.
> */
> -static void spapr_irq_init_xive(SpaprMachineState *spapr, int nr_irqs,
> +static void spapr_irq_init_xive(SpaprMachineState *spapr, int nr_xirqs,
> Error **errp)
> {
> uint32_t nr_servers = spapr_max_server_number(spapr);
> @@ -268,7 +264,7 @@ static void spapr_irq_init_xive(SpaprMachineState *spapr, int nr_irqs,
> int i;
>
> dev = qdev_create(NULL, TYPE_SPAPR_XIVE);
> - qdev_prop_set_uint32(dev, "nr-irqs", nr_irqs);
> + qdev_prop_set_uint32(dev, "nr-irqs", nr_xirqs + SPAPR_XIRQ_BASE);
> /*
> * 8 XIVE END structures per CPU. One for each available priority
> */
> @@ -308,7 +304,7 @@ static qemu_irq spapr_qirq_xive(SpaprMachineState *spapr, int irq)
> {
> SpaprXive *xive = spapr->xive;
>
> - if (irq >= xive->nr_irqs) {
> + if ((irq < SPAPR_XIRQ_BASE) || (irq >= xive->nr_irqs)) {
> return NULL;
> }
>
> @@ -404,17 +400,9 @@ static void spapr_irq_init_kvm_xive(SpaprMachineState *spapr, Error **errp)
> }
> }
>
> -/*
> - * XIVE uses the full IRQ number space. Set it to 8K to be compatible
> - * with XICS.
> - */
> -
> -#define SPAPR_IRQ_XIVE_NR_IRQS 0x2000
> -#define SPAPR_IRQ_XIVE_NR_MSIS (SPAPR_IRQ_XIVE_NR_IRQS - SPAPR_IRQ_MSI)
> -
> SpaprIrq spapr_irq_xive = {
> - .nr_irqs = SPAPR_IRQ_XIVE_NR_IRQS,
> - .nr_msis = SPAPR_IRQ_XIVE_NR_MSIS,
> + .nr_xirqs = SPAPR_NR_XIRQS,
> + .nr_msis = SPAPR_NR_MSIS,
> .ov5 = SPAPR_OV5_XIVE_EXPLOIT,
>
> .init = spapr_irq_init_xive,
> @@ -450,18 +438,18 @@ static SpaprIrq *spapr_irq_current(SpaprMachineState *spapr)
> &spapr_irq_xive : &spapr_irq_xics;
> }
>
> -static void spapr_irq_init_dual(SpaprMachineState *spapr, int nr_irqs,
> +static void spapr_irq_init_dual(SpaprMachineState *spapr, int nr_xirqs,
> Error **errp)
> {
> Error *local_err = NULL;
>
> - spapr_irq_xics.init(spapr, spapr_irq_xics.nr_irqs, &local_err);
> + spapr_irq_xics.init(spapr, spapr_irq_xics.nr_xirqs, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> }
>
> - spapr_irq_xive.init(spapr, spapr_irq_xive.nr_irqs, &local_err);
> + spapr_irq_xive.init(spapr, spapr_irq_xive.nr_xirqs, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> @@ -586,12 +574,9 @@ static const char *spapr_irq_get_nodename_dual(SpaprMachineState *spapr)
> /*
> * Define values in sync with the XIVE and XICS backend
> */
> -#define SPAPR_IRQ_DUAL_NR_IRQS 0x2000
> -#define SPAPR_IRQ_DUAL_NR_MSIS (SPAPR_IRQ_DUAL_NR_IRQS - SPAPR_IRQ_MSI)
> -
> SpaprIrq spapr_irq_dual = {
> - .nr_irqs = SPAPR_IRQ_DUAL_NR_IRQS,
> - .nr_msis = SPAPR_IRQ_DUAL_NR_MSIS,
> + .nr_xirqs = SPAPR_NR_XIRQS,
> + .nr_msis = SPAPR_NR_MSIS,
> .ov5 = SPAPR_OV5_XIVE_BOTH,
>
> .init = spapr_irq_init_dual,
> @@ -693,10 +678,10 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
> spapr_irq_msi_init(spapr, spapr->irq->nr_msis);
> }
>
> - spapr->irq->init(spapr, spapr->irq->nr_irqs, errp);
> + spapr->irq->init(spapr, spapr->irq->nr_xirqs, errp);
>
> spapr->qirqs = qemu_allocate_irqs(spapr->irq->set_irq, spapr,
> - spapr->irq->nr_irqs);
> + spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE);
> }
>
> int spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp)
> @@ -804,11 +789,11 @@ int spapr_irq_find(SpaprMachineState *spapr, int num, bool align, Error **errp)
> return first + ics->offset;
> }
>
> -#define SPAPR_IRQ_XICS_LEGACY_NR_IRQS 0x400
> +#define SPAPR_IRQ_XICS_LEGACY_NR_XIRQS 0x400
>
> SpaprIrq spapr_irq_xics_legacy = {
> - .nr_irqs = SPAPR_IRQ_XICS_LEGACY_NR_IRQS,
> - .nr_msis = SPAPR_IRQ_XICS_LEGACY_NR_IRQS,
> + .nr_xirqs = SPAPR_IRQ_XICS_LEGACY_NR_XIRQS,
> + .nr_msis = SPAPR_IRQ_XICS_LEGACY_NR_XIRQS,
> .ov5 = SPAPR_OV5_XIVE_LEGACY,
>
> .init = spapr_irq_init_xics,
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index 5db305165c..a8f9a2ab11 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -16,13 +16,18 @@
> * IRQ range offsets per device type
> */
> #define SPAPR_IRQ_IPI 0x0
> -#define SPAPR_IRQ_EPOW 0x1000 /* XICS_IRQ_BASE offset */
> -#define SPAPR_IRQ_HOTPLUG 0x1001
> -#define SPAPR_IRQ_VIO 0x1100 /* 256 VIO devices */
> -#define SPAPR_IRQ_PCI_LSI 0x1200 /* 32+ PHBs devices */
>
> -#define SPAPR_IRQ_MSI 0x1300 /* Offset of the dynamic range covered
> - * by the bitmap allocator */
> +#define SPAPR_XIRQ_BASE XICS_IRQ_BASE /* 0x1000 */
> +#define SPAPR_IRQ_EPOW (SPAPR_XIRQ_BASE + 0x0000)
> +#define SPAPR_IRQ_HOTPLUG (SPAPR_XIRQ_BASE + 0x0001)
> +#define SPAPR_IRQ_VIO (SPAPR_XIRQ_BASE + 0x0100) /* 256 VIO devices */
> +#define SPAPR_IRQ_PCI_LSI (SPAPR_XIRQ_BASE + 0x0200) /* 32+ PHBs devices */
> +
> +/* Offset of the dynamic range covered by the bitmap allocator */
> +#define SPAPR_IRQ_MSI (SPAPR_XIRQ_BASE + 0x0300)
> +
> +#define SPAPR_NR_XIRQS 0x1000
> +#define SPAPR_NR_MSIS (SPAPR_XIRQ_BASE + SPAPR_NR_XIRQS - SPAPR_IRQ_MSI)
>
> typedef struct SpaprMachineState SpaprMachineState;
>
> @@ -32,7 +37,7 @@ int spapr_irq_msi_alloc(SpaprMachineState *spapr, uint32_t num, bool align,
> void spapr_irq_msi_free(SpaprMachineState *spapr, int irq, uint32_t num);
>
> typedef struct SpaprIrq {
> - uint32_t nr_irqs;
> + uint32_t nr_xirqs;
> uint32_t nr_msis;
> uint8_t ov5;
>
next prev parent reply other threads:[~2019-09-27 7:55 UTC|newest]
Thread overview: 83+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-27 5:49 [PATCH v2 00/33] spapr: IRQ subsystem cleanup David Gibson
2019-09-27 5:49 ` [PATCH v2 01/33] xics: Minor fixes for XICSFabric interface David Gibson
2019-09-27 7:17 ` Greg Kurz
2019-09-27 5:49 ` [PATCH v2 02/33] xics: Eliminate 'reject', 'resend' and 'eoi' class hooks David Gibson
2019-09-27 5:49 ` [PATCH v2 03/33] xics: Rename misleading ics_simple_*() functions David Gibson
2019-09-27 5:49 ` [PATCH v2 04/33] xics: Eliminate reset hook David Gibson
2019-09-27 7:19 ` Greg Kurz
2019-09-27 5:50 ` [PATCH v2 05/33] xics: Merge TYPE_ICS_BASE and TYPE_ICS_SIMPLE classes David Gibson
2019-09-27 5:50 ` [PATCH v2 06/33] xics: Create sPAPR specific ICS subtype David Gibson
2019-09-27 7:22 ` Greg Kurz
2019-09-27 5:50 ` [PATCH v2 07/33] spapr: Fold spapr_phb_lsi_qirq() into its single caller David Gibson
2019-09-27 5:50 ` [PATCH v2 08/33] spapr: Replace spapr_vio_qirq() helper with spapr_vio_irq_pulse() helper David Gibson
2019-09-27 5:50 ` [PATCH v2 09/33] spapr: Clarify and fix handling of nr_irqs David Gibson
2019-09-27 7:53 ` Greg Kurz [this message]
2019-09-27 7:58 ` David Gibson
2019-09-27 5:50 ` [PATCH v2 10/33] spapr: Eliminate nr_irqs parameter to SpaprIrq::init David Gibson
2019-09-27 7:57 ` Greg Kurz
2019-09-27 5:50 ` [PATCH v2 11/33] spapr: Fix indexing of XICS irqs David Gibson
2019-09-27 5:50 ` [PATCH v2 12/33] spapr: Simplify spapr_qirq() handling David Gibson
2019-09-27 5:50 ` [PATCH v2 13/33] spapr: Eliminate SpaprIrq:get_nodename method David Gibson
2019-09-27 5:50 ` [PATCH v2 14/33] spapr: Remove unhelpful tracepoints from spapr_irq_free_xics() David Gibson
2019-09-27 5:50 ` [PATCH v2 15/33] spapr: Handle freeing of multiple irqs in frontend only David Gibson
2019-09-27 5:50 ` [PATCH v2 16/33] spapr, xics, xive: Better use of assert()s on irq claim/free paths David Gibson
2019-09-27 5:50 ` [PATCH v2 17/33] xive: Improve irq claim/free path David Gibson
2019-09-27 8:40 ` Greg Kurz
2019-09-30 1:39 ` David Gibson
2019-09-27 5:50 ` [PATCH v2 18/33] spapr: Use less cryptic representation of which irq backends are supported David Gibson
2019-09-27 5:50 ` [PATCH v2 19/33] spapr: Eliminate SpaprIrq::init hook David Gibson
2019-09-27 5:50 ` [PATCH v2 20/33] spapr, xics, xive: Introduce SpaprInterruptController QOM interface David Gibson
2019-09-27 9:52 ` Greg Kurz
2019-09-30 5:24 ` Cédric Le Goater
2019-09-27 5:50 ` [PATCH v2 21/33] spapr, xics, xive: Move cpu_intc_create from SpaprIrq to SpaprInterruptController David Gibson
2019-09-27 10:16 ` Greg Kurz
2019-09-30 1:49 ` David Gibson
2019-09-30 5:28 ` Cédric Le Goater
2019-09-30 6:14 ` David Gibson
2019-09-30 10:13 ` Cédric Le Goater
2019-10-01 2:31 ` David Gibson
2019-10-01 5:43 ` Cédric Le Goater
2019-10-01 6:47 ` David Gibson
2019-10-01 7:41 ` Cédric Le Goater
2019-10-01 8:11 ` David Gibson
2019-10-01 11:43 ` Cédric Le Goater
2019-10-02 1:11 ` David Gibson
2019-09-30 2:37 ` David Gibson
2019-09-30 5:30 ` Cédric Le Goater
2019-09-27 5:50 ` [PATCH v2 22/33] spapr, xics, xive: Move irq claim and free " David Gibson
2019-09-27 12:16 ` Greg Kurz
2019-09-30 2:39 ` David Gibson
2019-09-30 5:36 ` Cédric Le Goater
2019-09-30 5:33 ` Cédric Le Goater
2019-09-27 5:50 ` [PATCH v2 23/33] spapr: Formalize notion of active interrupt controller David Gibson
2019-09-27 14:16 ` Greg Kurz
2019-09-30 5:39 ` Cédric Le Goater
2019-09-27 5:50 ` [PATCH v2 24/33] spapr, xics, xive: Move set_irq from SpaprIrq to SpaprInterruptController David Gibson
2019-09-27 14:27 ` Greg Kurz
2019-09-30 2:41 ` David Gibson
2019-09-30 7:22 ` Greg Kurz
2019-09-30 8:28 ` David Gibson
2019-09-30 5:48 ` Cédric Le Goater
2019-09-27 5:50 ` [PATCH v2 25/33] spapr, xics, xive: Move print_info " David Gibson
2019-09-27 14:31 ` Greg Kurz
2019-09-27 5:50 ` [PATCH v2 26/33] spapr, xics, xive: Move dt_populate " David Gibson
2019-09-27 14:38 ` Greg Kurz
2019-09-30 5:51 ` Cédric Le Goater
2019-09-27 5:50 ` [PATCH v2 27/33] spapr, xics, xive: Match signatures for XICS and XIVE KVM connect routines David Gibson
2019-09-27 14:49 ` Greg Kurz
2019-09-30 5:52 ` Cédric Le Goater
2019-09-27 5:50 ` [PATCH v2 28/33] spapr: Remove SpaprIrq::init_kvm hook David Gibson
2019-09-27 15:04 ` Greg Kurz
2019-09-30 5:55 ` Cédric Le Goater
2019-09-27 5:50 ` [PATCH v2 29/33] spapr, xics, xive: Move SpaprIrq::reset hook logic into activate/deactivate David Gibson
2019-09-30 6:11 ` Cédric Le Goater
2019-09-30 8:25 ` David Gibson
2019-09-30 19:29 ` Cédric Le Goater
2019-10-01 3:07 ` David Gibson
2019-09-27 5:50 ` [PATCH v2 30/33] spapr, xics, xive: Move SpaprIrq::post_load hook to backends David Gibson
2019-09-27 5:50 ` [PATCH v2 31/33] spapr: Remove SpaprIrq::nr_msis David Gibson
2019-09-27 15:17 ` Greg Kurz
2019-09-27 5:50 ` [PATCH v2 32/33] spapr: Move SpaprIrq::nr_xirqs to SpaprMachineClass David Gibson
2019-09-27 15:22 ` Greg Kurz
2019-09-30 2:44 ` David Gibson
2019-09-27 5:50 ` [PATCH v2 33/33] spapr: Remove last pieces of SpaprIrq David Gibson
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=20190927095359.4e07ea70@bahia.lan \
--to=groug@kaod.org \
--cc=clg@kaod.org \
--cc=david@gibson.dropbear.id.au \
--cc=jasowang@redhat.com \
--cc=laurent@vivier.eu \
--cc=marcandre.lureau@redhat.com \
--cc=pbonzini@redhat.com \
--cc=philmd@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
--cc=riku.voipio@iki.fi \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.