Re: [Qemu-devel] [PATCH 01/19] ppc/xive: hardwire the Physical CAM line of the thread context

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 7609 bytes --]

On Mon, Jan 28, 2019 at 10:46:07AM +0100, Cédric Le Goater wrote:
> By default on P9, the HW CAM line (23bits) is hardwired to :
> 
>       0x000||0b1||4Bit chip number||7Bit Thread number.
> 
> When the block group mode is enabled at the controller level (PowerNV),
> the CAM line is changed for CAM compares to :
> 
>       4Bit chip number||0x001||7Bit Thread number
> 
> This will require changes in xive_presenter_tctx_match() possibly.
> This is a lowlevel functionality of the HW controller and it is not
> strictly needed. Leave it for later.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  include/hw/ppc/xive.h |  1 +
>  hw/intc/xive.c        | 53 ++++++++++++++++++++++++++++++++++++++++---
>  hw/ppc/pnv_core.c     | 16 ++++++++-----
>  3 files changed, 61 insertions(+), 9 deletions(-)
> 
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index ec3bb2aae45a..04d54e8315f7 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -319,6 +319,7 @@ typedef struct XiveTCTX {
>      qemu_irq    output;
>  
>      uint8_t     regs[XIVE_TM_RING_COUNT * XIVE_TM_RING_SIZE];
> +    uint32_t    hw_cam;

I don't love the fact that the hw_cam field is mirroring something
that's in the regs structure.  Any way you can have the property poke
the right value directly into regs?  Or else split up regs so you can
set the pieces individually.

>  } XiveTCTX;
>  
>  /*
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 2e9b8efd4342..f5642f2338de 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -469,6 +469,12 @@ static void xive_tctx_realize(DeviceState *dev, Error **errp)
>      Object *obj;
>      Error *local_err = NULL;
>  
> +    if (!tctx->hw_cam) {
> +        error_setg(errp, "XIVE: HW CAM is not set for CPU %d",
> +                   tctx->cs->cpu_index);
> +        return;
> +    }
> +
>      obj = object_property_get_link(OBJECT(dev), "cpu", &local_err);
>      if (!obj) {
>          error_propagate(errp, local_err);
> @@ -509,11 +515,17 @@ static const VMStateDescription vmstate_xive_tctx = {
>      },
>  };
>  
> +static Property  xive_tctx_properties[] = {
> +    DEFINE_PROP_UINT32("hw-cam", XiveTCTX, hw_cam, 0),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void xive_tctx_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
>      dc->desc = "XIVE Interrupt Thread Context";
> +    dc->props = xive_tctx_properties;
>      dc->realize = xive_tctx_realize;
>      dc->unrealize = xive_tctx_unrealize;
>      dc->vmsd = &vmstate_xive_tctx;
> @@ -526,8 +538,21 @@ static const TypeInfo xive_tctx_info = {
>      .class_init    = xive_tctx_class_init,
>  };
>  
> +/*
> + * The HW CAM line (23bits) is hardwired to :
> + *
> + *   0x000||0b1||4Bit chip number||7Bit Thread number.
> + */
> +static uint32_t hw_cam_line(uint8_t chip_id, uint8_t tid)
> +{
> +    return 1 << 11 | (chip_id & 0xf) << 7 | (tid & 0x7f);
> +}
> +
>  Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
>  {
> +    CPUPPCState *env = &POWERPC_CPU(cpu)->env;
> +    uint32_t pir = env->spr_cb[SPR_PIR].default_value;
> +    uint32_t hw_cam = hw_cam_line((pir >> 8) & 0xf, pir & 0x7f);
>      Error *local_err = NULL;
>      Object *obj;
>  
> @@ -535,6 +560,10 @@ 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_set_int(obj, hw_cam, "hw-cam", &local_err);
> +    if (local_err) {
> +        goto error;
> +    }
>      object_property_set_bool(obj, true, "realized", &local_err);
>      if (local_err) {
>          goto error;
> @@ -1112,14 +1141,28 @@ XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs)
>      return xrc->get_tctx(xrtr, cs);
>  }
>  
> +/*
> + * When the block grouping is enabled, the CAM line is changed to :
> + *
> + *   4Bit chip number||0x001||7Bit Thread number.
> + */
> +static bool xive_presenter_tctx_match_hw(XiveRouter *xrtr, XiveTCTX *tctx,
> +                                         uint8_t nvt_blk, uint32_t nvt_idx)
> +{
> +    /* TODO: block group support */
> +    return tctx->hw_cam == hw_cam_line(nvt_blk, nvt_idx);
> +}
> +
>  /*
>   * The thread context register words are in big-endian format.
>   */
> -static int xive_presenter_tctx_match(XiveTCTX *tctx, uint8_t format,
> +static int xive_presenter_tctx_match(XiveRouter *xrtr,
> +                                     XiveTCTX *tctx, uint8_t format,
>                                       uint8_t nvt_blk, uint32_t nvt_idx,
>                                       bool cam_ignore, uint32_t logic_serv)
>  {
>      uint32_t cam = xive_nvt_cam_line(nvt_blk, nvt_idx);
> +    uint32_t qw3w2 = xive_tctx_word2(&tctx->regs[TM_QW3_HV_PHYS]);
>      uint32_t qw2w2 = xive_tctx_word2(&tctx->regs[TM_QW2_HV_POOL]);
>      uint32_t qw1w2 = xive_tctx_word2(&tctx->regs[TM_QW1_OS]);
>      uint32_t qw0w2 = xive_tctx_word2(&tctx->regs[TM_QW0_USER]);
> @@ -1142,7 +1185,11 @@ static int xive_presenter_tctx_match(XiveTCTX *tctx, uint8_t format,
>  
>          /* F=0 & i=0: Specific NVT notification */
>  
> -        /* TODO (PowerNV) : PHYS ring */
> +        /* PHYS ring */
> +        if ((be32_to_cpu(qw3w2) & TM_QW3W2_VT) &&
> +            xive_presenter_tctx_match_hw(xrtr, tctx, nvt_blk, nvt_idx)) {
> +            return TM_QW3_HV_PHYS;
> +        }
>  
>          /* HV POOL ring */
>          if ((be32_to_cpu(qw2w2) & TM_QW2W2_VP) &&
> @@ -1199,7 +1246,7 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format,
>           * Check the thread context CAM lines and record matches. We
>           * will handle CPU exception delivery later
>           */
> -        ring = xive_presenter_tctx_match(tctx, format, nvt_blk, nvt_idx,
> +        ring = xive_presenter_tctx_match(xrtr, tctx, format, nvt_blk, nvt_idx,
>                                           cam_ignore, logic_serv);
>          /*
>           * Save the context and follow on to catch duplicates, that we
> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
> index 7c806da720c6..f035522b4ec9 100644
> --- a/hw/ppc/pnv_core.c
> +++ b/hw/ppc/pnv_core.c
> @@ -114,12 +114,6 @@ static void pnv_realize_vcpu(PowerPCCPU *cpu, PnvChip *chip, Error **errp)
>          return;
>      }
>  
> -    pcc->intc_create(chip, cpu, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        return;
> -    }
> -
>      core_pir = object_property_get_uint(OBJECT(cpu), "core-pir", &error_abort);
>  
>      /*
> @@ -129,6 +123,16 @@ static void pnv_realize_vcpu(PowerPCCPU *cpu, PnvChip *chip, Error **errp)
>       */
>      pir->default_value = core_pir + thread_index;
>  
> +    /*
> +     * On P9, the interrupt presenter needs to hardwire the PIR value
> +     * in the thread interrupt context of the CPU.
> +     */
> +    pcc->intc_create(chip, cpu, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +
>      /* Set time-base frequency to 512 MHz */
>      cpu_ppc_tb_init(env, PNV_TIMEBASE_FREQ);
>  

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]