Re: [Qemu-devel] [PATCH v3 17/19] spapr_irq: Expose the phandle of the interrupt controller

[Cédric Le Goater]

On 1/17/19 6:16 PM, Greg Kurz wrote:
> This will be used by PHB hotplug in order to create the "interrupt-map"
> property of the PHB node.
> 
> Signed-off-by: Greg Kurz <address@hidden>
> ---
>  hw/ppc/spapr_irq.c         |   44 
> +++++++++++++++++++++++++++++++++++++++++++-
>  include/hw/ppc/spapr_irq.h |    3 +++
>  2 files changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index b3e0713428ed..93243cfd0644 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -243,6 +243,11 @@ static void spapr_irq_reset_xics(sPAPRMachineState 
> *spapr, Error **errp)
>      /* TODO: create the KVM XICS device */
>  }
>  
> +static const char *spapr_irq_get_nodename_xics(sPAPRMachineState *spapr)
> +{
> +    return xics_spapr_get_nodename();

can't we just return XICS_NODENAME ?

> +}
> +
>  #define SPAPR_IRQ_XICS_NR_IRQS     0x1000
>  #define SPAPR_IRQ_XICS_NR_MSIS     \
>      (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI)
> @@ -263,6 +268,7 @@ sPAPRIrq spapr_irq_xics = {
>      .post_load   = spapr_irq_post_load_xics,
>      .reset       = spapr_irq_reset_xics,
>      .set_irq     = spapr_irq_set_irq_xics,
> +    .get_nodename = spapr_irq_get_nodename_xics,
>  };
>  
>  /*
> @@ -401,6 +407,11 @@ static void spapr_irq_set_irq_xive(void *opaque, int 
> srcno, int val)
>      xive_source_set_irq(&spapr->xive->source, srcno, val);
>  }
>  
> +static const char *spapr_irq_get_nodename_xive(sPAPRMachineState *spapr)
> +{
> +    return spapr_xive_get_nodename(spapr->xive);
> +}
> +
>  /*
>   * XIVE uses the full IRQ number space. Set it to 8K to be compatible
>   * with XICS.
> @@ -425,6 +436,7 @@ sPAPRIrq spapr_irq_xive = {
>      .post_load   = spapr_irq_post_load_xive,
>      .reset       = spapr_irq_reset_xive,
>      .set_irq     = spapr_irq_set_irq_xive,
> +    .get_nodename = spapr_irq_get_nodename_xive,
>  };
>  
>  /*
> @@ -588,6 +600,11 @@ static void spapr_irq_set_irq_dual(void *opaque, int 
> srcno, int val)
>      spapr_irq_current(spapr)->set_irq(spapr, srcno, val);
>  }
>  
> +static const char *spapr_irq_get_nodename_dual(sPAPRMachineState *spapr)
> +{
> +    return spapr_irq_current(spapr)->get_nodename(spapr);
> +}
> +
>  /*
>   * Define values in sync with the XIVE and XICS backend
>   */
> @@ -609,7 +626,8 @@ sPAPRIrq spapr_irq_dual = {
>      .cpu_intc_create = spapr_irq_cpu_intc_create_dual,
>      .post_load   = spapr_irq_post_load_dual,
>      .reset       = spapr_irq_reset_dual,
> -    .set_irq     = spapr_irq_set_irq_dual
> +    .set_irq     = spapr_irq_set_irq_dual,
> +    .get_nodename = spapr_irq_get_nodename_dual,
>  };
>  
>  /*
> @@ -680,6 +698,29 @@ void spapr_irq_dt_populate(sPAPRMachineState *spapr, 
> uint32_t nr_servers,
>      _FDT(fdt_setprop_cell(fdt, node, "phandle", phandle));
>  }
>  
> +uint32_t spapr_irq_get_phandle(sPAPRMachineState *spapr, void *fdt,
> +                               Error **errp)
> +{
> +    const char *nodename = spapr->irq->get_nodename(spapr);
> +    int phandle, offset;
> +
> +    offset = fdt_subnode_offset(fdt, 0, nodename);

if I am correct, you changed dt_populate() to return the same offset.

> +    if (offset < 0) {
> +        error_setg(errp, "Can't find node \"%s\": %s", nodename,
> +                   fdt_strerror(offset));
> +        return -1;
> +    }
> +
> +    phandle = fdt_get_phandle(fdt, offset);

if we stored the phandle when spapr_irq_dt_populate() is called, I think
things would be easier. May be under the backend ? 

C.


> +    if (phandle < 0) {
> +        error_setg(errp, "Can't get phandle of node \"%s\": %s",
> +                   nodename, fdt_strerror(phandle));
> +        return -1;
> +    }
> +
> +    return phandle;
> +}
> +
>  /*
>   * XICS legacy routines - to deprecate one day
>   */
> @@ -752,4 +793,5 @@ sPAPRIrq spapr_irq_xics_legacy = {
>      .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
>      .post_load   = spapr_irq_post_load_xics,
>      .set_irq     = spapr_irq_set_irq_xics,
> +    .get_nodename = spapr_irq_get_nodename_xics,
>  };
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index dfed6b7e9976..d44d659231a6 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -48,6 +48,7 @@ typedef struct sPAPRIrq {
>      int (*post_load)(sPAPRMachineState *spapr, int version_id);
>      void (*reset)(sPAPRMachineState *spapr, Error **errp);
>      void (*set_irq)(void *opaque, int srcno, int val);
> +    const char *(*get_nodename)(sPAPRMachineState *spapr);
>  } sPAPRIrq;
>  
>  extern sPAPRIrq spapr_irq_xics;
> @@ -65,6 +66,8 @@ int spapr_irq_post_load(sPAPRMachineState *spapr, int 
> version_id);
>  void spapr_irq_reset(sPAPRMachineState *spapr, Error **errp);
>  void spapr_irq_dt_populate(sPAPRMachineState *spapr, uint32_t nr_servers,
>                             void *fdt, uint32_t phandle);
> +uint32_t spapr_irq_get_phandle(sPAPRMachineState *spapr, void *fdt,
> +                               Error **errp);
>  
>  /*
>   * XICS legacy routines
>