[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 03/19] xics: Disintricate allocation and type
From: |
Greg Kurz |
Subject: |
Re: [Qemu-devel] [PATCH v3 03/19] xics: Disintricate allocation and type setting of interrupts |
Date: |
Fri, 18 Jan 2019 12:47:47 +0100 |
On Thu, 17 Jan 2019 18:14:46 +0100
Greg Kurz <address@hidden> wrote:
> The current code assumes that an interrupt is allocated as soon as its
> type is set to MSI or LSI. PHB hotplug will require to be able to set
> the type of an interrupt before actually allocating it.
>
> Disintricate type setting from allocation by using another flag bit for
> the latter. Introduce a new ics_claim_irq() function for allocation. The
> behavior of aborting if the same irq gets allocated twice is kept.
>
> ics_set_irq_type() now only sets the type to MSI or LSI. It doesn't bring
> anything to abort if the type was already set before. Drop the assert
> and XICS_FLAGS_IRQ_MASK on the way.
>
With some more testing (and thinking :) I now realize that the type of
the irq should survive PHB unplug, which is not the case with this patch.
This calls for a v4.
> Older QEMUs don't know about XICS_FLAGS_IRQ_CLAIMED. In order to safely
> handle incoming migration, we must fix the irq flags. This is done at
> post load thanks to a compat property. We don't need to do anything for
> backward migration since older machines only care for the irq type.
>
> Signed-off-by: Greg Kurz <address@hidden>
> ---
> hw/core/machine.c | 4 ++++
> hw/intc/xics.c | 28 +++++++++++++++++++++++++---
> hw/ppc/pnv_psi.c | 1 +
> hw/ppc/spapr_irq.c | 4 +---
> include/hw/ppc/xics.h | 8 ++++++--
> 5 files changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 1a0a9ab1117a..536a34092367 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -25,6 +25,10 @@
>
> GlobalProperty hw_compat_3_1[] = {
> {
> + .driver = "ics-base",
> + .property = "has-claimed-flag",
> + .value = "off",
> + },{
> .driver = "pcie-root-port",
> .property = "x-speed",
> .value = "2_5",
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index 16e8ffa2aaf7..82cf04548907 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -75,7 +75,7 @@ void ics_pic_print_info(ICSState *ics, Monitor *mon)
> for (i = 0; i < ics->nr_irqs; i++) {
> ICSIRQState *irq = ics->irqs + i;
>
> - if (!(irq->flags & XICS_FLAGS_IRQ_MASK)) {
> + if (!(irq->flags & XICS_FLAGS_IRQ_CLAIMED)) {
> continue;
> }
> monitor_printf(mon, " %4x %s %02x %02x\n",
> @@ -662,6 +662,22 @@ static int ics_base_dispatch_post_load(void *opaque, int
> version_id)
> ICSState *ics = opaque;
> ICSStateClass *info = ICS_BASE_GET_CLASS(ics);
>
> + if (!ics->has_claimed_flag) {
> + int i;
> +
> + for (i = 0; i < ics->nr_irqs; i++) {
> + ICSIRQState *irq = ics->irqs + i;
> +
> + /*
> + * For older machines, allocating the irq and setting its type is
> + * the same thing.
> + */
> + if (irq->flags & (XICS_FLAGS_IRQ_LSI| XICS_FLAGS_IRQ_MSI)) {
> + irq->flags |= XICS_FLAGS_IRQ_CLAIMED;
> + }
> + }
> + }
> +
> if (info->post_load) {
> return info->post_load(ics, version_id);
> }
> @@ -702,6 +718,7 @@ static const VMStateDescription vmstate_ics_base = {
>
> static Property ics_base_properties[] = {
> DEFINE_PROP_UINT32("nr-irqs", ICSState, nr_irqs, 0),
> + DEFINE_PROP_BOOL("has-claimed-flag", ICSState, has_claimed_flag, true),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> @@ -741,10 +758,15 @@ ICPState *xics_icp_get(XICSFabric *xi, int server)
> return xic->icp_get(xi, server);
> }
>
> -void ics_set_irq_type(ICSState *ics, int srcno, bool lsi)
> +void ics_claim_irq(ICSState *ics, int srcno)
> {
> - assert(!(ics->irqs[srcno].flags & XICS_FLAGS_IRQ_MASK));
> + assert(ICS_IRQ_FREE(ics, srcno));
> +
> + ics->irqs[srcno].flags |= XICS_FLAGS_IRQ_CLAIMED;
> +}
>
> +void ics_set_irq_type(ICSState *ics, int srcno, bool lsi)
> +{
> ics->irqs[srcno].flags |=
> lsi ? XICS_FLAGS_IRQ_LSI : XICS_FLAGS_IRQ_MSI;
> }
> diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
> index 8ced09506321..ced34e1119dc 100644
> --- a/hw/ppc/pnv_psi.c
> +++ b/hw/ppc/pnv_psi.c
> @@ -488,6 +488,7 @@ static void pnv_psi_realize(DeviceState *dev, Error
> **errp)
>
> for (i = 0; i < ics->nr_irqs; i++) {
> ics_set_irq_type(ics, i, true);
> + ics_claim_irq(ics, i);
> }
>
> psi->qirqs = qemu_allocate_irqs(ics_simple_set_irq, ics, ics->nr_irqs);
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 1da7a32348fc..86c712d15382 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -125,9 +125,6 @@ error:
> error_propagate(errp, local_err);
> }
>
> -#define ICS_IRQ_FREE(ics, srcno) \
> - (!((ics)->irqs[(srcno)].flags & (XICS_FLAGS_IRQ_MASK)))
> -
> static int spapr_irq_claim_xics(sPAPRMachineState *spapr, int irq, bool lsi,
> Error **errp)
> {
> @@ -146,6 +143,7 @@ static int spapr_irq_claim_xics(sPAPRMachineState *spapr,
> int irq, bool lsi,
> }
>
> ics_set_irq_type(ics, irq - ics->offset, lsi);
> + ics_claim_irq(ics, irq - ics->offset);
> return 0;
> }
>
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index fad786e8b22d..e3be5cc663c5 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -131,6 +131,7 @@ struct ICSState {
> /*< public >*/
> uint32_t nr_irqs;
> uint32_t offset;
> + bool has_claimed_flag;
> ICSIRQState *irqs;
> XICSFabric *xics;
> };
> @@ -153,13 +154,15 @@ struct ICSIRQState {
> #define XICS_STATUS_PRESENTED 0x10
> #define XICS_STATUS_QUEUED 0x20
> uint8_t status;
> -/* (flags & XICS_FLAGS_IRQ_MASK) == 0 means the interrupt is not allocated */
> #define XICS_FLAGS_IRQ_LSI 0x1
> #define XICS_FLAGS_IRQ_MSI 0x2
> -#define XICS_FLAGS_IRQ_MASK 0x3
> +#define XICS_FLAGS_IRQ_CLAIMED 0x4
> uint8_t flags;
> };
>
> +#define ICS_IRQ_FREE(ics, srcno) \
> + (!((ics)->irqs[(srcno)].flags & (XICS_FLAGS_IRQ_CLAIMED)))
> +
> struct XICSFabric {
> Object parent;
> };
> @@ -193,6 +196,7 @@ void ics_simple_write_xive(ICSState *ics, int nr, int
> server,
> void ics_simple_set_irq(void *opaque, int srcno, int val);
> void ics_kvm_set_irq(void *opaque, int srcno, int val);
>
> +void ics_claim_irq(ICSState *ics, int srcno);
> void ics_set_irq_type(ICSState *ics, int srcno, bool lsi);
> void icp_pic_print_info(ICPState *icp, Monitor *mon);
> void ics_pic_print_info(ICSState *ics, Monitor *mon);
>
- [Qemu-devel] [PATCH v3 00/19] spapr: Add support for PHB hotplug, Greg Kurz, 2019/01/17
- [Qemu-devel] [PATCH v3 01/19] ppc: Move spapr-related prototypes from xics.h into a seperate header file, Greg Kurz, 2019/01/17
- [Qemu-devel] [PATCH v3 02/19] spapr: Rename xics to intc in interrupt controller agnostic code, Greg Kurz, 2019/01/17
- [Qemu-devel] [PATCH v3 03/19] xics: Disintricate allocation and type setting of interrupts, Greg Kurz, 2019/01/17
- [Qemu-devel] [PATCH v3 04/19] spapr/xive: Don't set irq type in spapr_xive_irq_claim(), Greg Kurz, 2019/01/17
- [Qemu-devel] [PATCH v3 05/19] spapr: Set irq type in a dedicated function, Greg Kurz, 2019/01/17
- [Qemu-devel] [PATCH v3 06/19] spapr: Identify LSIs of all possible PHBs at machine init, Greg Kurz, 2019/01/17