Re: [Qemu-devel] [PATCH for-2.12 v3 04/11] spapr: move current IRQ allocation under the machine

[Greg Kurz <groug@kaod.org>]

On Fri, 10 Nov 2017 15:20:10 +0000
Cédric Le Goater <clg@kaod.org> wrote:

> Use the new XICSFabric operations to handle the IRQ number allocation
> directly under the machine. These changes only move code and adapt it
> to take into account the new API which uses IRQ numbers.
> 
> On PowerNV, only provide a basic irq_test() operation. For the moment,
> there is no need for more.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---

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

>  hw/intc/trace-events |  2 --
>  hw/intc/xics.c       |  3 ++-
>  hw/intc/xics_spapr.c | 57 +++++++++-------------------------------------------
>  hw/ppc/pnv.c         | 18 +++++++++++++++++
>  hw/ppc/spapr.c       | 56 ++++++++++++++++++++++++++++++++++++++++++++++++---
>  hw/ppc/trace-events  |  2 ++
>  6 files changed, 85 insertions(+), 53 deletions(-)
> 
> diff --git a/hw/intc/trace-events b/hw/intc/trace-events
> index b86f242b0fcf..e34ecf7a16e5 100644
> --- a/hw/intc/trace-events
> +++ b/hw/intc/trace-events
> @@ -65,8 +65,6 @@ xics_ics_simple_reject(int nr, int srcno) "reject irq 0x%x [src %d]"
>  xics_ics_simple_eoi(int nr) "ics_eoi: irq 0x%x"
>  xics_alloc(int irq) "irq %d"
>  xics_alloc_block(int first, int num, bool lsi, int align) "first irq %d, %d irqs, lsi=%d, alignnum %d"
> -xics_ics_free(int src, int irq, int num) "Source#%d, first irq %d, %d irqs"
> -xics_ics_free_warn(int src, int irq) "Source#%d, irq %d is already free"
>  
>  # hw/intc/s390_flic_kvm.c
>  flic_create_device(int err) "flic: create device failed %d"
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index cc9816e7f204..2c4899f278e2 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -53,6 +53,7 @@ void icp_pic_print_info(ICPState *icp, Monitor *mon)
>  void ics_pic_print_info(ICSState *ics, Monitor *mon)
>  {
>      uint32_t i;
> +    XICSFabricClass *xic = XICS_FABRIC_GET_CLASS(ics->xics);
>  
>      monitor_printf(mon, "ICS %4x..%4x %p\n",
>                     ics->offset, ics->offset + ics->nr_irqs - 1, ics);
> @@ -64,7 +65,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 (!xic->irq_test(ics->xics, i + ics->offset)) {
>              continue;
>          }
>          monitor_printf(mon, "  %4x %s %02x %02x\n",
> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> index e8c0a1b3e903..de9e65d35247 100644
> --- a/hw/intc/xics_spapr.c
> +++ b/hw/intc/xics_spapr.c
> @@ -245,50 +245,26 @@ void xics_spapr_init(sPAPRMachineState *spapr)
>      spapr_register_hypercall(H_IPOLL, h_ipoll);
>  }
>  
> -#define ICS_IRQ_FREE(ics, srcno)   \
> -    (!((ics)->irqs[(srcno)].flags & (XICS_FLAGS_IRQ_MASK)))
> -
> -static int ics_find_free_block(ICSState *ics, int num, int alignnum)
> -{
> -    int first, i;
> -
> -    for (first = 0; first < ics->nr_irqs; first += alignnum) {
> -        if (num > (ics->nr_irqs - first)) {
> -            return -1;
> -        }
> -        for (i = first; i < first + num; ++i) {
> -            if (!ICS_IRQ_FREE(ics, i)) {
> -                break;
> -            }
> -        }
> -        if (i == (first + num)) {
> -            return first;
> -        }
> -    }
> -
> -    return -1;
> -}
> -
>  int spapr_ics_alloc(ICSState *ics, int irq_hint, bool lsi, Error **errp)
>  {
>      int irq;
> +    XICSFabricClass *xic = XICS_FABRIC_GET_CLASS(ics->xics);
>  
>      if (!ics) {
>          return -1;
>      }
>      if (irq_hint) {
> -        if (!ICS_IRQ_FREE(ics, irq_hint - ics->offset)) {
> +        if (xic->irq_test(ics->xics, irq_hint)) {
>              error_setg(errp, "can't allocate IRQ %d: already in use", irq_hint);
>              return -1;
>          }
>          irq = irq_hint;
>      } else {
> -        irq = ics_find_free_block(ics, 1, 1);
> +        irq = xic->irq_alloc_block(ics->xics, 1, 1);
>          if (irq < 0) {
>              error_setg(errp, "can't allocate IRQ: no IRQ left");
>              return -1;
>          }
> -        irq += ics->offset;
>      }
>  
>      ics_set_irq_type(ics, irq - ics->offset, lsi);
> @@ -305,6 +281,7 @@ int spapr_ics_alloc_block(ICSState *ics, int num, bool lsi,
>                            bool align, Error **errp)
>  {
>      int i, first = -1;
> +    XICSFabricClass *xic = XICS_FABRIC_GET_CLASS(ics->xics);
>  
>      if (!ics) {
>          return -1;
> @@ -320,9 +297,9 @@ int spapr_ics_alloc_block(ICSState *ics, int num, bool lsi,
>      if (align) {
>          assert((num == 1) || (num == 2) || (num == 4) ||
>                 (num == 8) || (num == 16) || (num == 32));
> -        first = ics_find_free_block(ics, num, num);
> +        first = xic->irq_alloc_block(ics->xics, num, num);
>      } else {
> -        first = ics_find_free_block(ics, num, 1);
> +        first = xic->irq_alloc_block(ics->xics, num, 1);
>      }
>      if (first < 0) {
>          error_setg(errp, "can't find a free %d-IRQ block", num);
> @@ -330,33 +307,19 @@ int spapr_ics_alloc_block(ICSState *ics, int num, bool lsi,
>      }
>  
>      for (i = first; i < first + num; ++i) {
> -        ics_set_irq_type(ics, i, lsi);
> +        ics_set_irq_type(ics, i - ics->offset, lsi);
>      }
> -    first += ics->offset;
>  
>      trace_xics_alloc_block(first, num, lsi, align);
>  
>      return first;
>  }
>  
> -static void ics_free(ICSState *ics, int srcno, int num)
> -{
> -    int i;
> -
> -    for (i = srcno; i < srcno + num; ++i) {
> -        if (ICS_IRQ_FREE(ics, i)) {
> -            trace_xics_ics_free_warn(0, i + ics->offset);
> -        }
> -        memset(&ics->irqs[i], 0, sizeof(ICSIRQState));
> -    }
> -}
> -
>  void spapr_ics_free(ICSState *ics, int irq, int num)
>  {
> -    if (ics_valid_irq(ics, irq)) {
> -        trace_xics_ics_free(0, irq, num);
> -        ics_free(ics, irq - ics->offset, num);
> -    }
> +    XICSFabricClass *xic = XICS_FABRIC_GET_CLASS(ics->xics);
> +
> +    xic->irq_free_block(ics->xics, irq, num);
>  }
>  
>  void spapr_dt_xics(int nr_servers, void *fdt, uint32_t phandle)
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index c35c439d816b..8288940ef9d7 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -1018,6 +1018,23 @@ static ICPState *pnv_icp_get(XICSFabric *xi, int pir)
>      return cpu ? ICP(cpu->intc) : NULL;
>  }
>  
> +static bool pnv_irq_test(XICSFabric *xi, int irq)
> +{
> +    PnvMachineState *pnv = POWERNV_MACHINE(xi);
> +    int i;
> +
> +    /* We don't have a IRQ allocator for the PowerNV machine yet, so
> +     * just check that the IRQ number is valid for the PSI source
> +     */
> +    for (i = 0; i < pnv->num_chips; i++) {
> +        ICSState *ics = &pnv->chips[i]->psi.ics;
> +        if (ics_valid_irq(ics, irq)) {
> +            return true;
> +        }
> +    }
> +    return false;
> +}
> +
>  static void pnv_pic_print_info(InterruptStatsProvider *obj,
>                                 Monitor *mon)
>  {
> @@ -1102,6 +1119,7 @@ static void powernv_machine_class_init(ObjectClass *oc, void *data)
>      xic->icp_get = pnv_icp_get;
>      xic->ics_get = pnv_ics_get;
>      xic->ics_resend = pnv_ics_resend;
> +    xic->irq_test = pnv_irq_test;
>      ispc->print_info = pnv_pic_print_info;
>  
>      powernv_machine_class_props_init(oc);
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 84d68f2fdbae..4bdceb45a14f 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3536,19 +3536,69 @@ static ICPState *spapr_icp_get(XICSFabric *xi, int vcpu_id)
>      return cpu ? ICP(cpu->intc) : NULL;
>  }
>  
> +#define ICS_IRQ_FREE(ics, srcno)   \
> +    (!((ics)->irqs[(srcno)].flags & (XICS_FLAGS_IRQ_MASK)))
> +
> +static int ics_find_free_block(ICSState *ics, int num, int alignnum)
> +{
> +    int first, i;
> +
> +    for (first = 0; first < ics->nr_irqs; first += alignnum) {
> +        if (num > (ics->nr_irqs - first)) {
> +            return -1;
> +        }
> +        for (i = first; i < first + num; ++i) {
> +            if (!ICS_IRQ_FREE(ics, i)) {
> +                break;
> +            }
> +        }
> +        if (i == (first + num)) {
> +            return first;
> +        }
> +    }
> +
> +    return -1;
> +}
> +
>  static bool spapr_irq_test(XICSFabric *xi, int irq)
>  {
> -    return false;
> +    sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
> +    ICSState *ics = spapr->ics;
> +    int srcno = irq - ics->offset;
> +
> +    return !ICS_IRQ_FREE(ics, srcno);
>  }
>  
>  static int spapr_irq_alloc_block(XICSFabric *xi, int count, int align)
>  {
> -    return -1;
> +    sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
> +    ICSState *ics = spapr->ics;
> +    int srcno;
> +
> +    srcno = ics_find_free_block(ics, count, align);
> +    if (srcno == -1) {
> +        return -1;
> +    }
> +
> +    return srcno + ics->offset;
>  }
>  
>  static void spapr_irq_free_block(XICSFabric *xi, int irq, int num)
>  {
> -    ;
> +    sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
> +    ICSState *ics = spapr->ics;
> +    int srcno = irq - ics->offset;
> +    int i;
> +
> +    if (ics_valid_irq(ics, irq)) {
> +        trace_spapr_irq_free(0, irq, num);
> +        for (i = srcno; i < srcno + num; ++i) {
> +            if (ICS_IRQ_FREE(ics, i)) {
> +                trace_spapr_irq_free_warn(0, i + ics->offset);
> +            }
> +            memset(&ics->irqs[i], 0, sizeof(ICSIRQState));
> +        }
> +    }
>  }
>  
>  static void spapr_pic_print_info(InterruptStatsProvider *obj,
> diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
> index 4a6a6490fa78..dc9ab4c4deb3 100644
> --- a/hw/ppc/trace-events
> +++ b/hw/ppc/trace-events
> @@ -12,6 +12,8 @@ spapr_pci_msi_retry(unsigned config_addr, unsigned req_num, unsigned max_irqs) "
>  # hw/ppc/spapr.c
>  spapr_cas_failed(unsigned long n) "DT diff buffer is too small: %ld bytes"
>  spapr_cas_continue(unsigned long n) "Copy changes to the guest: %ld bytes"
> +spapr_irq_free(int src, int irq, int num) "Source#%d, first irq %d, %d irqs"
> +spapr_irq_free_warn(int src, int irq) "Source#%d, irq %d is already free"
>  
>  # hw/ppc/spapr_hcall.c
>  spapr_cas_pvr_try(uint32_t pvr) "0x%x"