[Skiboot] [PATCH] Add in new OPAL call to flush the L2 and L3 caches.

[Rashmica Gupta]

On Fri, 2018-11-02 at 13:22 +1100, Alistair Popple wrote:
> Hi Rashmica,
> 
> > > +	}
> > > +	start_time = tb_to_msecs(mftb());
> > > +	while ((val & L2_PRD_PURGE_CMD_REG_BUSY) && !(timeout =
> > > time_expired(start_time))) {
> > 
> > Can we please not have '=' in a condition?
> 
> ...
> 
> > ... and ditch this check. And the time_expired() helper (which does
> > not
> > seem to help much anyway). And the local @timeout variable. And '='
> > in
> > the condition :)
> 
> Take a look at the tb_compare() function (and perhaps how others use
> it). I 
> think that probably does what you want.

:P it sure does!

> 
> - Alistair
> 
> > > +
> > > +	/* We have to clear the trigger bit ourselves */
> > > +	val &= ~L2_PRD_PURGE_CMD_TRIGGER;
> > > +	rc = xscom_write(chip_id, addr, val);
> > > +	if (rc)
> > > +		prlog(PR_ERR, "FLUSH L2 on core 0x%x: XSCOM write
> > > failed %i\n",
> > > core_id, rc); +	return 0;
> > 
> > Why not return rc?
> > 
> > > +
> > > +}
> > > +
> > > +static int flush_l3_caches(uint32_t chip_id, uint32_t core_id)
> > > +{
> > > +	int rc, timeout = 0;
> > > +	unsigned long start_time;
> > > +	uint64_t val = L3_PRD_PURGE_REQ;
> > > +	uint64_t addr = XSCOM_ADDR_P9_EX(core_id, L3_PRD_PURGE_REG);
> > > +
> > > +	rc = xscom_write_mask(chip_id, addr, L3_PRD_PURGE_REQ,
> > > L3_PRD_PURGE_REQ);
> > 
> > Similar to L2CAC_FLUSH question here:
> > 
> >  putspy pu.ex EXP.L3.L3_MISC.L3CERRS.L3_PRD_PURGE_TTYPE FULL_PURGE
> > -all
> > 
> >  putspy pu.ex EXP.L3.L3_MISC.L3CERRS.L3_PRD_PURGE_REQ ON -all
> > 
> > 
> > There is L3_PRD_PURGE_REQ but no FULL_PURGE.
> > 
> > > +	if (rc) {
> > > +		prlog(PR_ERR, "FLUSH L3 on core 0x%x: XSCOM write_mask
> > > failed %i
> 
> \n",
> > > core_id, rc);
> > 
> > return rc?
> > 
> > > +	}
> > > +
> > > +	/* Trigger bit is automatically set to zero when flushing is
> > > done*/
> > > +	start_time = tb_to_msecs(mftb());
> > > +	while ((val & L3_PRD_PURGE_REQ) && !(timeout = 
> 
> time_expired(start_time)
> > > )) { +		rc = xscom_read(chip_id, addr, &val);
> > > +		if (rc) {
> > > +			prlog(PR_ERR, "FLUSH L3 on core 0x%x: XSCOM
> > > read failed %i\n",
> > > core_id, rc); +			break;
> > > +		}
> > > +	}
> > > +	if (timeout) {
> > > +		prlog(PR_ERR, "FLUSH L3 on core 0x%x timed out %i\n",
> > > core_id, 
> 
> rc);
> > > +		return OPAL_BUSY;
> > > +	}
> > > +
> > > +	return 0;
> > 
> > return rc?
> > 
> > > +}
> > > +
> > > +int flush_caches(void)
> > 
> > OPAL calls return int64_t. And the handlers are static so there is
> > no
> > reason to export the symbol via include/cpu.h below.
> > 
> > > +{
> > > +	int rc = 0;
> > > +	struct cpu_thread *t;
> > > +	uint64_t chip_id, core_id, prev_core_id = 0xdeadbeef;
> > > +
> > > +	if ((mfspr(SPR_PVR) & PVR_TYPE_P9) != PVR_TYPE_P9)
> > > +		return OPAL_UNSUPPORTED;
> > > +
> > > +	for_each_cpu(t) {
> > > +		/* Only need to do it once per core chiplet */
> > > +		core_id = pir_to_core_id(t->pir);
> > > +		if (prev_core_id == core_id)
> > > +			continue;
> > > +		prev_core_id = core_id;
> > > +		chip_id = t->chip_id;
> > 
> > The chip_id local variable seems pretty useless, could just use t-
> > >chip_id.
> > 
> > > +
> > > +		rc |= flush_l2_caches(chip_id, core_id);
> > > +		rc |= flush_l3_caches(chip_id, core_id);
> > > +	}
> > > +
> > > +	return rc;
> > > +}
> > > +
> > > +
> > 
> > Extra empty line?
> > 
> > > +opal_call(OPAL_CLEAR_CACHE, flush_caches, 0);
> > > diff --git a/include/cpu.h b/include/cpu.h
> > > index 2fe47982..04c862c5 100644
> > > --- a/include/cpu.h
> > > +++ b/include/cpu.h
> > > @@ -329,4 +329,6 @@ int dctl_set_special_wakeup(struct cpu_thread
> > > *t);
> > > 
> > >  int dctl_clear_special_wakeup(struct cpu_thread *t);
> > >  int dctl_core_is_gated(struct cpu_thread *t);
> > > 
> > > +extern int flush_caches(void);
> > > +
> > > 
> > >  #endif /* __CPU_H */
> > > 
> > > diff --git a/include/opal-api.h b/include/opal-api.h
> > > index 5f397c8e..c24838d2 100644
> > > --- a/include/opal-api.h
> > > +++ b/include/opal-api.h
> > > @@ -226,7 +226,8 @@
> > > 
> > >  #define OPAL_NX_COPROC_INIT			167
> > >  #define OPAL_NPU_SET_RELAXED_ORDER		168
> > >  #define OPAL_NPU_GET_RELAXED_ORDER		169
> > > 
> > > -#define OPAL_LAST				169
> > > +#define OPAL_CLEAR_CACHE			170
> > 
> > s/clear/purge/. The scoms calls it "purge", let's stick to this
> > name.
> > 
> > > +#define OPAL_LAST				170
> > > 
> > >  #define QUIESCE_HOLD			1 /* Spin all calls at
> > > entry */
> > >  #define QUIESCE_REJECT			2 /* Fail all calls
> > > with OPAL_BUSY */
> 
>