diff mbox series

[12/20] spapr: Simplify spapr_qirq() handling

Message ID 20190925064534.19155-13-david@gibson.dropbear.id.au
State New, archived
Headers show
Series spapr: IRQ subsystem cleanups | expand

Commit Message

David Gibson Sept. 25, 2019, 6:45 a.m. UTC
Currently spapr_qirq() used to find the qemu_irq for an spapr global irq
number, redirects through the SpaprIrq::qirq method.  But the array of
qemu_irqs is allocated in the PAPR layer, not the backends, and so the
method implementations all return the same thing, just differing in the
preliminary checks they make.

So, we can remove the method, and just implement spapr_qirq() directly,
including all the relevant checks in one place.  We change all those
checks into assert()s as well, since a failure here indicates an error in
the calling code.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_irq.c         | 47 ++++++++++----------------------------
 include/hw/ppc/spapr_irq.h |  1 -
 2 files changed, 12 insertions(+), 36 deletions(-)

Comments

Cédric Le Goater Sept. 25, 2019, 7:16 a.m. UTC | #1
On 25/09/2019 08:45, David Gibson wrote:
> Currently spapr_qirq() used to find the qemu_irq for an spapr global irq
> number, redirects through the SpaprIrq::qirq method.  But the array of
> qemu_irqs is allocated in the PAPR layer, not the backends, and so the
> method implementations all return the same thing, just differing in the
> preliminary checks they make.
> 
> So, we can remove the method, and just implement spapr_qirq() directly,
> including all the relevant checks in one place.  We change all those
> checks into assert()s as well, since a failure here indicates an error in
> the calling code.

nice,
 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

C.

> ---
>  hw/ppc/spapr_irq.c         | 47 ++++++++++----------------------------
>  include/hw/ppc/spapr_irq.h |  1 -
>  2 files changed, 12 insertions(+), 36 deletions(-)
> 
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 9a9e486eb5..038bfffff4 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -150,17 +150,6 @@ static void spapr_irq_free_xics(SpaprMachineState *spapr, int irq, int num)
>      }
>  }
>  
> -static qemu_irq spapr_qirq_xics(SpaprMachineState *spapr, int irq)
> -{
> -    ICSState *ics = spapr->ics;
> -
> -    if (ics_valid_irq(ics, irq)) {
> -        return spapr->qirqs[irq];
> -    }
> -
> -    return NULL;
> -}
> -
>  static void spapr_irq_print_info_xics(SpaprMachineState *spapr, Monitor *mon)
>  {
>      CPUState *cs;
> @@ -242,7 +231,6 @@ SpaprIrq spapr_irq_xics = {
>      .init        = spapr_irq_init_xics,
>      .claim       = spapr_irq_claim_xics,
>      .free        = spapr_irq_free_xics,
> -    .qirq        = spapr_qirq_xics,
>      .print_info  = spapr_irq_print_info_xics,
>      .dt_populate = spapr_dt_xics,
>      .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
> @@ -300,20 +288,6 @@ static void spapr_irq_free_xive(SpaprMachineState *spapr, int irq, int num)
>      }
>  }
>  
> -static qemu_irq spapr_qirq_xive(SpaprMachineState *spapr, int irq)
> -{
> -    SpaprXive *xive = spapr->xive;
> -
> -    if ((irq < SPAPR_XIRQ_BASE) || (irq >= xive->nr_irqs)) {
> -        return NULL;
> -    }
> -
> -    /* The sPAPR machine/device should have claimed the IRQ before */
> -    assert(xive_eas_is_valid(&xive->eat[irq]));
> -
> -    return spapr->qirqs[irq];
> -}
> -
>  static void spapr_irq_print_info_xive(SpaprMachineState *spapr,
>                                        Monitor *mon)
>  {
> @@ -413,7 +387,6 @@ SpaprIrq spapr_irq_xive = {
>      .init        = spapr_irq_init_xive,
>      .claim       = spapr_irq_claim_xive,
>      .free        = spapr_irq_free_xive,
> -    .qirq        = spapr_qirq_xive,
>      .print_info  = spapr_irq_print_info_xive,
>      .dt_populate = spapr_dt_xive,
>      .cpu_intc_create = spapr_irq_cpu_intc_create_xive,
> @@ -487,11 +460,6 @@ static void spapr_irq_free_dual(SpaprMachineState *spapr, int irq, int num)
>      spapr_irq_xive.free(spapr, irq, num);
>  }
>  
> -static qemu_irq spapr_qirq_dual(SpaprMachineState *spapr, int irq)
> -{
> -    return spapr_irq_current(spapr)->qirq(spapr, irq);
> -}
> -
>  static void spapr_irq_print_info_dual(SpaprMachineState *spapr, Monitor *mon)
>  {
>      spapr_irq_current(spapr)->print_info(spapr, mon);
> @@ -586,7 +554,6 @@ SpaprIrq spapr_irq_dual = {
>      .init        = spapr_irq_init_dual,
>      .claim       = spapr_irq_claim_dual,
>      .free        = spapr_irq_free_dual,
> -    .qirq        = spapr_qirq_dual,
>      .print_info  = spapr_irq_print_info_dual,
>      .dt_populate = spapr_irq_dt_populate_dual,
>      .cpu_intc_create = spapr_irq_cpu_intc_create_dual,
> @@ -700,7 +667,18 @@ void spapr_irq_free(SpaprMachineState *spapr, int irq, int num)
>  
>  qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq)
>  {
> -    return spapr->irq->qirq(spapr, irq);
> +    assert(irq >= SPAPR_XIRQ_BASE);
> +    assert(irq < (spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE));
> +
> +    if (spapr->ics) {
> +        assert(ics_valid_irq(spapr->ics, irq));
> +    }
> +    if (spapr->xive) {
> +        assert(irq < spapr->xive->nr_irqs);
> +        assert(xive_eas_is_valid(&spapr->xive->eat[irq]));
> +    }
> +
> +    return spapr->qirqs[irq];
>  }
>  
>  int spapr_irq_post_load(SpaprMachineState *spapr, int version_id)
> @@ -803,7 +781,6 @@ SpaprIrq spapr_irq_xics_legacy = {
>      .init        = spapr_irq_init_xics,
>      .claim       = spapr_irq_claim_xics,
>      .free        = spapr_irq_free_xics,
> -    .qirq        = spapr_qirq_xics,
>      .print_info  = spapr_irq_print_info_xics,
>      .dt_populate = spapr_dt_xics,
>      .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index 7e26288fcd..a4e790ef60 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -44,7 +44,6 @@ typedef struct SpaprIrq {
>      void (*init)(SpaprMachineState *spapr, Error **errp);
>      int (*claim)(SpaprMachineState *spapr, int irq, bool lsi, Error **errp);
>      void (*free)(SpaprMachineState *spapr, int irq, int num);
> -    qemu_irq (*qirq)(SpaprMachineState *spapr, int irq);
>      void (*print_info)(SpaprMachineState *spapr, Monitor *mon);
>      void (*dt_populate)(SpaprMachineState *spapr, uint32_t nr_servers,
>                          void *fdt, uint32_t phandle);
>
Greg Kurz Sept. 25, 2019, 8:30 p.m. UTC | #2
On Wed, 25 Sep 2019 16:45:26 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> Currently spapr_qirq() used to find the qemu_irq for an spapr global irq
> number, redirects through the SpaprIrq::qirq method.  But the array of
> qemu_irqs is allocated in the PAPR layer, not the backends, and so the
> method implementations all return the same thing, just differing in the
> preliminary checks they make.
> 
> So, we can remove the method, and just implement spapr_qirq() directly,
> including all the relevant checks in one place.  We change all those
> checks into assert()s as well, since a failure here indicates an error in
> the calling code.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/ppc/spapr_irq.c         | 47 ++++++++++----------------------------
>  include/hw/ppc/spapr_irq.h |  1 -
>  2 files changed, 12 insertions(+), 36 deletions(-)
> 
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 9a9e486eb5..038bfffff4 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -150,17 +150,6 @@ static void spapr_irq_free_xics(SpaprMachineState *spapr, int irq, int num)
>      }
>  }
>  
> -static qemu_irq spapr_qirq_xics(SpaprMachineState *spapr, int irq)
> -{
> -    ICSState *ics = spapr->ics;
> -
> -    if (ics_valid_irq(ics, irq)) {
> -        return spapr->qirqs[irq];
> -    }
> -
> -    return NULL;
> -}
> -
>  static void spapr_irq_print_info_xics(SpaprMachineState *spapr, Monitor *mon)
>  {
>      CPUState *cs;
> @@ -242,7 +231,6 @@ SpaprIrq spapr_irq_xics = {
>      .init        = spapr_irq_init_xics,
>      .claim       = spapr_irq_claim_xics,
>      .free        = spapr_irq_free_xics,
> -    .qirq        = spapr_qirq_xics,
>      .print_info  = spapr_irq_print_info_xics,
>      .dt_populate = spapr_dt_xics,
>      .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
> @@ -300,20 +288,6 @@ static void spapr_irq_free_xive(SpaprMachineState *spapr, int irq, int num)
>      }
>  }
>  
> -static qemu_irq spapr_qirq_xive(SpaprMachineState *spapr, int irq)
> -{
> -    SpaprXive *xive = spapr->xive;
> -
> -    if ((irq < SPAPR_XIRQ_BASE) || (irq >= xive->nr_irqs)) {
> -        return NULL;
> -    }
> -
> -    /* The sPAPR machine/device should have claimed the IRQ before */
> -    assert(xive_eas_is_valid(&xive->eat[irq]));
> -
> -    return spapr->qirqs[irq];
> -}
> -
>  static void spapr_irq_print_info_xive(SpaprMachineState *spapr,
>                                        Monitor *mon)
>  {
> @@ -413,7 +387,6 @@ SpaprIrq spapr_irq_xive = {
>      .init        = spapr_irq_init_xive,
>      .claim       = spapr_irq_claim_xive,
>      .free        = spapr_irq_free_xive,
> -    .qirq        = spapr_qirq_xive,
>      .print_info  = spapr_irq_print_info_xive,
>      .dt_populate = spapr_dt_xive,
>      .cpu_intc_create = spapr_irq_cpu_intc_create_xive,
> @@ -487,11 +460,6 @@ static void spapr_irq_free_dual(SpaprMachineState *spapr, int irq, int num)
>      spapr_irq_xive.free(spapr, irq, num);
>  }
>  
> -static qemu_irq spapr_qirq_dual(SpaprMachineState *spapr, int irq)
> -{
> -    return spapr_irq_current(spapr)->qirq(spapr, irq);
> -}
> -
>  static void spapr_irq_print_info_dual(SpaprMachineState *spapr, Monitor *mon)
>  {
>      spapr_irq_current(spapr)->print_info(spapr, mon);
> @@ -586,7 +554,6 @@ SpaprIrq spapr_irq_dual = {
>      .init        = spapr_irq_init_dual,
>      .claim       = spapr_irq_claim_dual,
>      .free        = spapr_irq_free_dual,
> -    .qirq        = spapr_qirq_dual,
>      .print_info  = spapr_irq_print_info_dual,
>      .dt_populate = spapr_irq_dt_populate_dual,
>      .cpu_intc_create = spapr_irq_cpu_intc_create_dual,
> @@ -700,7 +667,18 @@ void spapr_irq_free(SpaprMachineState *spapr, int irq, int num)
>  
>  qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq)
>  {
> -    return spapr->irq->qirq(spapr, irq);
> +    assert(irq >= SPAPR_XIRQ_BASE);
> +    assert(irq < (spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE));
> +
> +    if (spapr->ics) {
> +        assert(ics_valid_irq(spapr->ics, irq));
> +    }
> +    if (spapr->xive) {
> +        assert(irq < spapr->xive->nr_irqs);
> +        assert(xive_eas_is_valid(&spapr->xive->eat[irq]));
> +    }
> +
> +    return spapr->qirqs[irq];
>  }
>  
>  int spapr_irq_post_load(SpaprMachineState *spapr, int version_id)
> @@ -803,7 +781,6 @@ SpaprIrq spapr_irq_xics_legacy = {
>      .init        = spapr_irq_init_xics,
>      .claim       = spapr_irq_claim_xics,
>      .free        = spapr_irq_free_xics,
> -    .qirq        = spapr_qirq_xics,
>      .print_info  = spapr_irq_print_info_xics,
>      .dt_populate = spapr_dt_xics,
>      .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index 7e26288fcd..a4e790ef60 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -44,7 +44,6 @@ typedef struct SpaprIrq {
>      void (*init)(SpaprMachineState *spapr, Error **errp);
>      int (*claim)(SpaprMachineState *spapr, int irq, bool lsi, Error **errp);
>      void (*free)(SpaprMachineState *spapr, int irq, int num);
> -    qemu_irq (*qirq)(SpaprMachineState *spapr, int irq);
>      void (*print_info)(SpaprMachineState *spapr, Monitor *mon);
>      void (*dt_populate)(SpaprMachineState *spapr, uint32_t nr_servers,
>                          void *fdt, uint32_t phandle);
Philippe Mathieu-Daudé Sept. 26, 2019, 7:10 a.m. UTC | #3
On 9/25/19 8:45 AM, David Gibson wrote:
> Currently spapr_qirq() used to find the qemu_irq for an spapr global irq
> number, redirects through the SpaprIrq::qirq method.  But the array of
> qemu_irqs is allocated in the PAPR layer, not the backends, and so the
> method implementations all return the same thing, just differing in the
> preliminary checks they make.
> 
> So, we can remove the method, and just implement spapr_qirq() directly,
> including all the relevant checks in one place.  We change all those
> checks into assert()s as well, since a failure here indicates an error in
> the calling code.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr_irq.c         | 47 ++++++++++----------------------------
>  include/hw/ppc/spapr_irq.h |  1 -
>  2 files changed, 12 insertions(+), 36 deletions(-)
> 
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 9a9e486eb5..038bfffff4 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -150,17 +150,6 @@ static void spapr_irq_free_xics(SpaprMachineState *spapr, int irq, int num)
>      }
>  }
>  
> -static qemu_irq spapr_qirq_xics(SpaprMachineState *spapr, int irq)
> -{
> -    ICSState *ics = spapr->ics;
> -
> -    if (ics_valid_irq(ics, irq)) {
> -        return spapr->qirqs[irq];
> -    }
> -
> -    return NULL;
> -}
> -
>  static void spapr_irq_print_info_xics(SpaprMachineState *spapr, Monitor *mon)
>  {
>      CPUState *cs;
> @@ -242,7 +231,6 @@ SpaprIrq spapr_irq_xics = {
>      .init        = spapr_irq_init_xics,
>      .claim       = spapr_irq_claim_xics,
>      .free        = spapr_irq_free_xics,
> -    .qirq        = spapr_qirq_xics,
>      .print_info  = spapr_irq_print_info_xics,
>      .dt_populate = spapr_dt_xics,
>      .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
> @@ -300,20 +288,6 @@ static void spapr_irq_free_xive(SpaprMachineState *spapr, int irq, int num)
>      }
>  }
>  
> -static qemu_irq spapr_qirq_xive(SpaprMachineState *spapr, int irq)
> -{
> -    SpaprXive *xive = spapr->xive;
> -
> -    if ((irq < SPAPR_XIRQ_BASE) || (irq >= xive->nr_irqs)) {
> -        return NULL;
> -    }
> -
> -    /* The sPAPR machine/device should have claimed the IRQ before */
> -    assert(xive_eas_is_valid(&xive->eat[irq]));
> -
> -    return spapr->qirqs[irq];
> -}
> -
>  static void spapr_irq_print_info_xive(SpaprMachineState *spapr,
>                                        Monitor *mon)
>  {
> @@ -413,7 +387,6 @@ SpaprIrq spapr_irq_xive = {
>      .init        = spapr_irq_init_xive,
>      .claim       = spapr_irq_claim_xive,
>      .free        = spapr_irq_free_xive,
> -    .qirq        = spapr_qirq_xive,
>      .print_info  = spapr_irq_print_info_xive,
>      .dt_populate = spapr_dt_xive,
>      .cpu_intc_create = spapr_irq_cpu_intc_create_xive,
> @@ -487,11 +460,6 @@ static void spapr_irq_free_dual(SpaprMachineState *spapr, int irq, int num)
>      spapr_irq_xive.free(spapr, irq, num);
>  }
>  
> -static qemu_irq spapr_qirq_dual(SpaprMachineState *spapr, int irq)
> -{
> -    return spapr_irq_current(spapr)->qirq(spapr, irq);
> -}
> -
>  static void spapr_irq_print_info_dual(SpaprMachineState *spapr, Monitor *mon)
>  {
>      spapr_irq_current(spapr)->print_info(spapr, mon);
> @@ -586,7 +554,6 @@ SpaprIrq spapr_irq_dual = {
>      .init        = spapr_irq_init_dual,
>      .claim       = spapr_irq_claim_dual,
>      .free        = spapr_irq_free_dual,
> -    .qirq        = spapr_qirq_dual,
>      .print_info  = spapr_irq_print_info_dual,
>      .dt_populate = spapr_irq_dt_populate_dual,
>      .cpu_intc_create = spapr_irq_cpu_intc_create_dual,
> @@ -700,7 +667,18 @@ void spapr_irq_free(SpaprMachineState *spapr, int irq, int num)
>  
>  qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq)
>  {
> -    return spapr->irq->qirq(spapr, irq);
> +    assert(irq >= SPAPR_XIRQ_BASE);
> +    assert(irq < (spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE));
> +
> +    if (spapr->ics) {
> +        assert(ics_valid_irq(spapr->ics, irq));
> +    }
> +    if (spapr->xive) {
> +        assert(irq < spapr->xive->nr_irqs);
> +        assert(xive_eas_is_valid(&spapr->xive->eat[irq]));
> +    }
> +
> +    return spapr->qirqs[irq];
>  }
>  
>  int spapr_irq_post_load(SpaprMachineState *spapr, int version_id)
> @@ -803,7 +781,6 @@ SpaprIrq spapr_irq_xics_legacy = {
>      .init        = spapr_irq_init_xics,
>      .claim       = spapr_irq_claim_xics,
>      .free        = spapr_irq_free_xics,
> -    .qirq        = spapr_qirq_xics,
>      .print_info  = spapr_irq_print_info_xics,
>      .dt_populate = spapr_dt_xics,
>      .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index 7e26288fcd..a4e790ef60 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -44,7 +44,6 @@ typedef struct SpaprIrq {
>      void (*init)(SpaprMachineState *spapr, Error **errp);
>      int (*claim)(SpaprMachineState *spapr, int irq, bool lsi, Error **errp);
>      void (*free)(SpaprMachineState *spapr, int irq, int num);
> -    qemu_irq (*qirq)(SpaprMachineState *spapr, int irq);

Yay, cleanup!

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>      void (*print_info)(SpaprMachineState *spapr, Monitor *mon);
>      void (*dt_populate)(SpaprMachineState *spapr, uint32_t nr_servers,
>                          void *fdt, uint32_t phandle);
>
diff mbox series

Patch

diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index 9a9e486eb5..038bfffff4 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -150,17 +150,6 @@  static void spapr_irq_free_xics(SpaprMachineState *spapr, int irq, int num)
     }
 }
 
-static qemu_irq spapr_qirq_xics(SpaprMachineState *spapr, int irq)
-{
-    ICSState *ics = spapr->ics;
-
-    if (ics_valid_irq(ics, irq)) {
-        return spapr->qirqs[irq];
-    }
-
-    return NULL;
-}
-
 static void spapr_irq_print_info_xics(SpaprMachineState *spapr, Monitor *mon)
 {
     CPUState *cs;
@@ -242,7 +231,6 @@  SpaprIrq spapr_irq_xics = {
     .init        = spapr_irq_init_xics,
     .claim       = spapr_irq_claim_xics,
     .free        = spapr_irq_free_xics,
-    .qirq        = spapr_qirq_xics,
     .print_info  = spapr_irq_print_info_xics,
     .dt_populate = spapr_dt_xics,
     .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
@@ -300,20 +288,6 @@  static void spapr_irq_free_xive(SpaprMachineState *spapr, int irq, int num)
     }
 }
 
-static qemu_irq spapr_qirq_xive(SpaprMachineState *spapr, int irq)
-{
-    SpaprXive *xive = spapr->xive;
-
-    if ((irq < SPAPR_XIRQ_BASE) || (irq >= xive->nr_irqs)) {
-        return NULL;
-    }
-
-    /* The sPAPR machine/device should have claimed the IRQ before */
-    assert(xive_eas_is_valid(&xive->eat[irq]));
-
-    return spapr->qirqs[irq];
-}
-
 static void spapr_irq_print_info_xive(SpaprMachineState *spapr,
                                       Monitor *mon)
 {
@@ -413,7 +387,6 @@  SpaprIrq spapr_irq_xive = {
     .init        = spapr_irq_init_xive,
     .claim       = spapr_irq_claim_xive,
     .free        = spapr_irq_free_xive,
-    .qirq        = spapr_qirq_xive,
     .print_info  = spapr_irq_print_info_xive,
     .dt_populate = spapr_dt_xive,
     .cpu_intc_create = spapr_irq_cpu_intc_create_xive,
@@ -487,11 +460,6 @@  static void spapr_irq_free_dual(SpaprMachineState *spapr, int irq, int num)
     spapr_irq_xive.free(spapr, irq, num);
 }
 
-static qemu_irq spapr_qirq_dual(SpaprMachineState *spapr, int irq)
-{
-    return spapr_irq_current(spapr)->qirq(spapr, irq);
-}
-
 static void spapr_irq_print_info_dual(SpaprMachineState *spapr, Monitor *mon)
 {
     spapr_irq_current(spapr)->print_info(spapr, mon);
@@ -586,7 +554,6 @@  SpaprIrq spapr_irq_dual = {
     .init        = spapr_irq_init_dual,
     .claim       = spapr_irq_claim_dual,
     .free        = spapr_irq_free_dual,
-    .qirq        = spapr_qirq_dual,
     .print_info  = spapr_irq_print_info_dual,
     .dt_populate = spapr_irq_dt_populate_dual,
     .cpu_intc_create = spapr_irq_cpu_intc_create_dual,
@@ -700,7 +667,18 @@  void spapr_irq_free(SpaprMachineState *spapr, int irq, int num)
 
 qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq)
 {
-    return spapr->irq->qirq(spapr, irq);
+    assert(irq >= SPAPR_XIRQ_BASE);
+    assert(irq < (spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE));
+
+    if (spapr->ics) {
+        assert(ics_valid_irq(spapr->ics, irq));
+    }
+    if (spapr->xive) {
+        assert(irq < spapr->xive->nr_irqs);
+        assert(xive_eas_is_valid(&spapr->xive->eat[irq]));
+    }
+
+    return spapr->qirqs[irq];
 }
 
 int spapr_irq_post_load(SpaprMachineState *spapr, int version_id)
@@ -803,7 +781,6 @@  SpaprIrq spapr_irq_xics_legacy = {
     .init        = spapr_irq_init_xics,
     .claim       = spapr_irq_claim_xics,
     .free        = spapr_irq_free_xics,
-    .qirq        = spapr_qirq_xics,
     .print_info  = spapr_irq_print_info_xics,
     .dt_populate = spapr_dt_xics,
     .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
index 7e26288fcd..a4e790ef60 100644
--- a/include/hw/ppc/spapr_irq.h
+++ b/include/hw/ppc/spapr_irq.h
@@ -44,7 +44,6 @@  typedef struct SpaprIrq {
     void (*init)(SpaprMachineState *spapr, Error **errp);
     int (*claim)(SpaprMachineState *spapr, int irq, bool lsi, Error **errp);
     void (*free)(SpaprMachineState *spapr, int irq, int num);
-    qemu_irq (*qirq)(SpaprMachineState *spapr, int irq);
     void (*print_info)(SpaprMachineState *spapr, Monitor *mon);
     void (*dt_populate)(SpaprMachineState *spapr, uint32_t nr_servers,
                         void *fdt, uint32_t phandle);