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 <da...@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é <phi...@redhat.com>
> void (*print_info)(SpaprMachineState *spapr, Monitor *mon);
> void (*dt_populate)(SpaprMachineState *spapr, uint32_t nr_servers,
> void *fdt, uint32_t phandle);
>
