Re: [PATCH for-5.0 v5 09/23] ppc/xive: Implement the XivePresenter interface

[Greg Kurz]

On Fri, 15 Nov 2019 17:24:22 +0100
Cédric Le Goater <address@hidden> wrote:

> Each XIVE Router model, sPAPR and PowerNV, now implements the 'match_nvt'
> handler of the XivePresenter QOM interface. This is simply moving code
> and taking into account the new API.
> 
> To be noted that the xive_router_get_tctx() helper is not used anymore
> when doing CAM matching and will be removed later on after other changes.
> 
> The XIVE presenter model is still too simple for the PowerNV machine
> and the CAM matching algo is not correct on multichip system. Subsequent
> patches will introduce more changes to scan all chips of the system.
> 
> Signed-off-by: Cédric Le Goater <address@hidden>
> ---
>  hw/intc/pnv_xive.c   | 41 +++++++++++++++++++++++++++++++++++
>  hw/intc/spapr_xive.c | 49 ++++++++++++++++++++++++++++++++++++++++++
>  hw/intc/xive.c       | 51 ++++++--------------------------------------
>  3 files changed, 97 insertions(+), 44 deletions(-)
> 
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index a394331ddd6a..087cbfbaad48 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -372,6 +372,45 @@ static int pnv_xive_get_eas(XiveRouter *xrtr, uint8_t 
> blk, uint32_t idx,
>      return pnv_xive_vst_read(xive, VST_TSEL_IVT, blk, idx, eas);
>  }
>  
> +static int pnv_xive_match_nvt(XivePresenter *xptr, uint8_t format,
> +                              uint8_t nvt_blk, uint32_t nvt_idx,
> +                              bool cam_ignore, uint8_t priority,
> +                              uint32_t logic_serv, XiveTCTXMatch *match)
> +{
> +    CPUState *cs;
> +    int count = 0;
> +
> +    CPU_FOREACH(cs) {
> +        PowerPCCPU *cpu = POWERPC_CPU(cs);
> +        XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
> +        int ring;
> +

I guess it's ok not to check tctx here because the powernv machine type
doesn't support cpu hotplug.

LGTM despite the non-strict CamelCase type :)

Reviewed-by: Greg Kurz <address@hidden>

> +        /*
> +         * Check the thread context CAM lines and record matches.
> +         */
> +        ring = xive_presenter_tctx_match(xptr, tctx, format, nvt_blk, 
> nvt_idx,
> +                                         cam_ignore, logic_serv);
> +        /*
> +         * Save the context and follow on to catch duplicates, that we
> +         * don't support yet.
> +         */
> +        if (ring != -1) {
> +            if (match->tctx) {
> +                qemu_log_mask(LOG_GUEST_ERROR, "XIVE: already found a "
> +                              "thread context NVT %x/%x\n",
> +                              nvt_blk, nvt_idx);
> +                return -1;
> +            }
> +
> +            match->ring = ring;
> +            match->tctx = tctx;
> +            count++;
> +        }
> +    }
> +
> +    return count;
> +}
> +
>  static XiveTCTX *pnv_xive_get_tctx(XiveRouter *xrtr, CPUState *cs)
>  {
>      PowerPCCPU *cpu = POWERPC_CPU(cs);
> @@ -1810,6 +1849,7 @@ static void pnv_xive_class_init(ObjectClass *klass, 
> void *data)
>      PnvXScomInterfaceClass *xdc = PNV_XSCOM_INTERFACE_CLASS(klass);
>      XiveRouterClass *xrc = XIVE_ROUTER_CLASS(klass);
>      XiveNotifierClass *xnc = XIVE_NOTIFIER_CLASS(klass);
> +    XivePresenterClass *xpc = XIVE_PRESENTER_CLASS(klass);
>  
>      xdc->dt_xscom = pnv_xive_dt_xscom;
>  
> @@ -1825,6 +1865,7 @@ static void pnv_xive_class_init(ObjectClass *klass, 
> void *data)
>      xrc->get_tctx = pnv_xive_get_tctx;
>  
>      xnc->notify = pnv_xive_notify;
> +    xpc->match_nvt  = pnv_xive_match_nvt;
>  };
>  
>  static const TypeInfo pnv_xive_info = {
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 729246e906c9..bb3b2dfdb77f 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -405,6 +405,52 @@ static XiveTCTX *spapr_xive_get_tctx(XiveRouter *xrtr, 
> CPUState *cs)
>      return spapr_cpu_state(cpu)->tctx;
>  }
>  
> +static int spapr_xive_match_nvt(XivePresenter *xptr, uint8_t format,
> +                                uint8_t nvt_blk, uint32_t nvt_idx,
> +                                bool cam_ignore, uint8_t priority,
> +                                uint32_t logic_serv, XiveTCTXMatch *match)
> +{
> +    CPUState *cs;
> +    int count = 0;
> +
> +    CPU_FOREACH(cs) {
> +        PowerPCCPU *cpu = POWERPC_CPU(cs);
> +        XiveTCTX *tctx = spapr_cpu_state(cpu)->tctx;
> +        int ring;
> +
> +        /*
> +         * Skip partially initialized vCPUs. This can happen when
> +         * vCPUs are hotplugged.
> +         */
> +        if (!tctx) {
> +            continue;
> +        }
> +
> +        /*
> +         * Check the thread context CAM lines and record matches.
> +         */
> +        ring = xive_presenter_tctx_match(xptr, tctx, format, nvt_blk, 
> nvt_idx,
> +                                         cam_ignore, logic_serv);
> +        /*
> +         * Save the matching thread interrupt context and follow on to
> +         * check for duplicates which are invalid.
> +         */
> +        if (ring != -1) {
> +            if (match->tctx) {
> +                qemu_log_mask(LOG_GUEST_ERROR, "XIVE: already found a thread 
> "
> +                              "context NVT %x/%x\n", nvt_blk, nvt_idx);
> +                return -1;
> +            }
> +
> +            match->ring = ring;
> +            match->tctx = tctx;
> +            count++;
> +        }
> +    }
> +
> +    return count;
> +}
> +
>  static const VMStateDescription vmstate_spapr_xive_end = {
>      .name = TYPE_SPAPR_XIVE "/end",
>      .version_id = 1,
> @@ -684,6 +730,7 @@ static void spapr_xive_class_init(ObjectClass *klass, 
> void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      XiveRouterClass *xrc = XIVE_ROUTER_CLASS(klass);
>      SpaprInterruptControllerClass *sicc = SPAPR_INTC_CLASS(klass);
> +    XivePresenterClass *xpc = XIVE_PRESENTER_CLASS(klass);
>  
>      dc->desc    = "sPAPR XIVE Interrupt Controller";
>      dc->props   = spapr_xive_properties;
> @@ -708,6 +755,8 @@ static void spapr_xive_class_init(ObjectClass *klass, 
> void *data)
>      sicc->print_info = spapr_xive_print_info;
>      sicc->dt = spapr_xive_dt;
>      sicc->post_load = spapr_xive_post_load;
> +
> +    xpc->match_nvt  = spapr_xive_match_nvt;
>  }
>  
>  static const TypeInfo spapr_xive_info = {
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 344bb3f3bc4b..da6196ca958f 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -1428,51 +1428,14 @@ static bool xive_presenter_match(XiveRouter *xrtr, 
> uint8_t format,
>                                   bool cam_ignore, uint8_t priority,
>                                   uint32_t logic_serv, XiveTCTXMatch *match)
>  {
> -    CPUState *cs;
> +    XivePresenter *xptr = XIVE_PRESENTER(xrtr);
> +    XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
> +    int count;
>  
> -    /*
> -     * TODO (PowerNV): handle chip_id overwrite of block field for
> -     * hardwired CAM compares
> -     */
> -
> -    CPU_FOREACH(cs) {
> -        XiveTCTX *tctx = xive_router_get_tctx(xrtr, cs);
> -        int ring;
> -
> -        /*
> -         * Skip partially initialized vCPUs. This can happen when
> -         * vCPUs are hotplugged.
> -         */
> -        if (!tctx) {
> -            continue;
> -        }
> -
> -        /*
> -         * HW checks that the CPU is enabled in the Physical Thread
> -         * Enable Register (PTER).
> -         */
> -
> -        /*
> -         * Check the thread context CAM lines and record matches. We
> -         * will handle CPU exception delivery later
> -         */
> -        ring = xive_presenter_tctx_match(XIVE_PRESENTER(xrtr), tctx, format,
> -                                         nvt_blk, nvt_idx,
> -                                         cam_ignore, logic_serv);
> -        /*
> -         * Save the context and follow on to catch duplicates, that we
> -         * don't support yet.
> -         */
> -        if (ring != -1) {
> -            if (match->tctx) {
> -                qemu_log_mask(LOG_GUEST_ERROR, "XIVE: already found a thread 
> "
> -                              "context NVT %x/%x\n", nvt_blk, nvt_idx);
> -                return false;
> -            }
> -
> -            match->ring = ring;
> -            match->tctx = tctx;
> -        }
> +    count = xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore,
> +                           priority, logic_serv, match);
> +    if (count < 0) {
> +        return false;
>      }
>  
>      if (!match->tctx) {