Re: [Qemu-devel] [PATCH 08/10] ppc/xive: Extend XiveTCTX with an router object pointer

[David Gibson]

On Sun, Jun 30, 2019 at 10:45:59PM +0200, Cédric Le Goater wrote:
> This is to perform lookups in the NVT table when a vCPU is dispatched
> and possibly resend interrupts.

I'm slightly confused by this one.  Aren't there multiple router
objects, each of which can deliver to any thread?  In which case what
router object is associated with a specific TCTX?

> Future XIVE chip will use a different class for the model of the
> interrupt controller. So use an 'Object *' instead of a 'XiveRouter *'.

This seems odd to me, shouldn't it be an interface pointer or
something in that case?

> Signed-off-by: Cédric Le Goater <address@hidden>

> ---
>  include/hw/ppc/xive.h |  4 +++-
>  hw/intc/xive.c        | 11 ++++++++++-
>  hw/ppc/pnv.c          |  2 +-
>  hw/ppc/spapr_irq.c    |  2 +-
>  4 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index d922524982d3..b764e1e4e6d4 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -321,6 +321,8 @@ typedef struct XiveTCTX {
>      qemu_irq    os_output;
>  
>      uint8_t     regs[XIVE_TM_RING_COUNT * XIVE_TM_RING_SIZE];
> +
> +    Object      *xrtr;
>  } XiveTCTX;
>  
>  /*
> @@ -416,7 +418,7 @@ void xive_tctx_tm_write(XiveTCTX *tctx, hwaddr offset, 
> uint64_t value,
>  uint64_t xive_tctx_tm_read(XiveTCTX *tctx, hwaddr offset, unsigned size);
>  
>  void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon);
> -Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp);
> +Object *xive_tctx_create(Object *cpu, Object *xrtr, Error **errp);
>  
>  static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
>  {
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index f7ba1c3b622f..56700681884f 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -573,6 +573,14 @@ static void xive_tctx_realize(DeviceState *dev, Error 
> **errp)
>      Object *obj;
>      Error *local_err = NULL;
>  
> +    obj = object_property_get_link(OBJECT(dev), "xrtr", &local_err);
> +    if (!obj) {
> +        error_propagate(errp, local_err);
> +        error_prepend(errp, "required link 'xrtr' not found: ");
> +        return;
> +    }
> +    tctx->xrtr = obj;
> +
>      obj = object_property_get_link(OBJECT(dev), "cpu", &local_err);
>      if (!obj) {
>          error_propagate(errp, local_err);
> @@ -657,7 +665,7 @@ static const TypeInfo xive_tctx_info = {
>      .class_init    = xive_tctx_class_init,
>  };
>  
> -Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
> +Object *xive_tctx_create(Object *cpu, Object *xrtr, Error **errp)
>  {
>      Error *local_err = NULL;
>      Object *obj;
> @@ -666,6 +674,7 @@ Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, 
> Error **errp)
>      object_property_add_child(cpu, TYPE_XIVE_TCTX, obj, &error_abort);
>      object_unref(obj);
>      object_property_add_const_link(obj, "cpu", cpu, &error_abort);
> +    object_property_add_const_link(obj, "xrtr", xrtr, &error_abort);
>      object_property_set_bool(obj, true, "realized", &local_err);
>      if (local_err) {
>          goto error;
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index b87e01e5b925..11916dc273c2 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -765,7 +765,7 @@ static void pnv_chip_power9_intc_create(PnvChip *chip, 
> PowerPCCPU *cpu,
>       * controller object is initialized afterwards. Hopefully, it's
>       * only used at runtime.
>       */
> -    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive), 
> &local_err);
> +    obj = xive_tctx_create(OBJECT(cpu), OBJECT(&chip9->xive), &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
>          return;
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index b2b01e850de8..5b3c3c50967b 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -353,7 +353,7 @@ static void 
> spapr_irq_cpu_intc_create_xive(SpaprMachineState *spapr,
>      Object *obj;
>      SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
>  
> -    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(spapr->xive), 
> &local_err);
> +    obj = xive_tctx_create(OBJECT(cpu), OBJECT(spapr->xive), &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
>          return;

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature