Re: [Qemu-ppc] [PATCH for-2.12 v3 07/11] spapr: introduce an 'irq_base' number

[Greg Kurz]

On Fri, 10 Nov 2017 15:20:13 +0000
Cédric Le Goater <address@hidden> wrote:

> 'irq_base' is a base IRQ number which lets us allocate only the subset
> of the IRQ numbers used on the sPAPR platform. It is sync with the
> ICSState 'offset' attribute and this is slightly redundant. We could
> also choose to waste some extra bytes (512) and allocate the whole
> number space. To be discussed.
> 
> But more important, it removes a dependency on the ICSState object of
> the sPAPR machine which is required for XIVE.
> 
> Signed-off-by: Cédric Le Goater <address@hidden>
> ---
>  hw/ppc/spapr.c         | 7 ++++---
>  include/hw/ppc/spapr.h | 1 +
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index bf0e5b4f815b..1cbbd7715a85 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2362,6 +2362,7 @@ static void ppc_spapr_init(MachineState *machine)
>      /* Initialize the IRQ allocator */
>      spapr->nr_irqs  = XICS_IRQS_SPAPR;
>      spapr->irq_map  = bitmap_new(spapr->nr_irqs);
> +    spapr->irq_base = XICS_IRQ_BASE;
> 

Since this is a constant value, do we really need a machine-level value ?

Especially now that all the code that needs it is in spapr.c, I guess it
can directly use the macro, no ?

>      /* Set up Interrupt Controller before we create the VCPUs */
>      xics_system_init(machine, spapr->nr_irqs, &error_fatal);
> @@ -3630,7 +3631,7 @@ static void spapr_irq_free_block_2_11(XICSFabric *xi, 
> int irq, int num)
>  static bool spapr_irq_test(XICSFabric *xi, int irq)
>  {
>      sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
> -    int srcno = irq - spapr->ics->offset;
> +    int srcno = irq - spapr->irq_base;
>  
>      return test_bit(srcno, spapr->irq_map);
>  }
> @@ -3656,13 +3657,13 @@ static int spapr_irq_alloc_block(XICSFabric *xi, int 
> count, int align)
>      }
>  
>      bitmap_set(spapr->irq_map, srcno, count);
> -    return srcno + spapr->ics->offset;
> +    return srcno + spapr->irq_base;
>  }
>  
>  static void spapr_irq_free_block(XICSFabric *xi, int irq, int num)
>  {
>      sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
> -    int srcno = irq - spapr->ics->offset;
> +    int srcno = irq - spapr->irq_base;
>  
>      bitmap_clear(spapr->irq_map, srcno, num);
>  }
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 023436c32b2a..200667dcff9d 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -82,6 +82,7 @@ struct sPAPRMachineState {
>      int32_t nr_irqs;
>      unsigned long *irq_map;
>      unsigned long *irq_map_ref;
> +    uint32_t irq_base;
>      ICSState *ics;
>      sPAPRRTCState rtc;
>