[PATCH v4 13/15] coresight: trbe: Save and restore state across CPU low power state

[James Clark]

On 04/11/2025 3:21 pm, Leo Yan wrote:
> From: Yabin Cui <yabinc at google.com>
> 
> Similar to ETE, TRBE may lose its context when a CPU enters low power
> state. To make things worse, if ETE is restored without TRBE being
> restored, an enabled source device with no enabled sink devices can
> cause CPU hang on some devices (e.g., Pixel 9).
> 
> The save and restore flows are described in the section K5.5 "Context
> switching" of Arm ARM (ARM DDI 0487 L.a). This commit adds save and
> restore callbacks with following the software usages defined in the
> architecture manual.
> 
> Signed-off-by: Yabin Cui <yabinc at google.com>
> Co-developed-by: Leo Yan <leo.yan at arm.com>
> Signed-off-by: Leo Yan <leo.yan at arm.com>
> ---
>   drivers/hwtracing/coresight/coresight-trbe.c | 84 +++++++++++++++++++++++++++-
>   1 file changed, 83 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index 3c82ab4394fde1e3dd4371a9b1703da4d8f6db9d..9888a153bc2f99b4826f6103bdbc12304c9c94cb 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -116,6 +116,20 @@ static int trbe_errata_cpucaps[] = {
>    */
>   #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE_SKIP_BYTES	256
>   
> +/*
> + * struct trbe_save_state: Register values representing TRBE state
> + * @trblimitr		- Trace Buffer Limit Address Register value
> + * @trbbaser		- Trace Buffer Base Register value
> + * @trbptr		- Trace Buffer Write Pointer Register value
> + * @trbsr		- Trace Buffer Status Register value
> + */
> +struct trbe_save_state {
> +	u64 trblimitr;
> +	u64 trbbaser;
> +	u64 trbptr;
> +	u64 trbsr;
> +};
> +
>   /*
>    * struct trbe_cpudata: TRBE instance specific data
>    * @trbe_flag		- TRBE dirty/access flag support
> @@ -134,6 +148,7 @@ struct trbe_cpudata {
>   	enum cs_mode mode;
>   	struct trbe_buf *buf;
>   	struct trbe_drvdata *drvdata;
> +	struct trbe_save_state save_state;
>   	DECLARE_BITMAP(errata, TRBE_ERRATA_MAX);
>   };
>   
> @@ -1189,6 +1204,71 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
>   	return IRQ_HANDLED;
>   }
>   
> +static int arm_trbe_save(struct coresight_device *csdev)
> +{
> +	struct trbe_cpudata *cpudata = dev_get_drvdata(&csdev->dev);
> +	struct trbe_save_state *state = &cpudata->save_state;
> +
> +	if (cpudata->mode == CS_MODE_DISABLED)
> +		return 0;
> +
> +	/*
> +	 * According to the section K5.5 Context switching, Arm ARM (ARM DDI
> +	 * 0487 L.a), the software usage VKHHY requires a TSB CSYNC instruction
> +	 * to ensure the program-flow trace is flushed, which has been executed
> +	 * in ETM driver.
> +	 */
> +
> +	/* Disable trace buffer unit */
> +	state->trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
> +	write_sysreg_s(state->trblimitr & ~TRBLIMITR_EL1_E, SYS_TRBLIMITR_EL1);
> +
> +	/*
> +	 * Execute a further Context synchronization event. Ensure the writes to
> +	 * memory are complete.
> +	 */
> +	trbe_drain_buffer();
> +
> +	/* Synchronize the TRBE disabling */
> +	isb();

Can we use set_trbe_disabled() here, and set_trbe_enabled() for restore 
below. There is a bit of duplication and they do the same thing except 
this version here always has trbe_drain_buffer() rather than it being 
conditional. I'm not sure if that is correct as they should both be the 
same?

set_trbe_disabled() already reads TRBLIMITR_EL1 so you can return it.

> +
> +	state->trbbaser = read_sysreg_s(SYS_TRBBASER_EL1);
> +	state->trbptr = read_sysreg_s(SYS_TRBPTR_EL1);
> +	state->trbsr = read_sysreg_s(SYS_TRBSR_EL1);
> +	return 0;
> +}
> +
> +static void arm_trbe_restore(struct coresight_device *csdev)
> +{
> +	struct trbe_cpudata *cpudata = dev_get_drvdata(&csdev->dev);
> +	struct trbe_save_state *state = &cpudata->save_state;
> +
> +	if (cpudata->mode == CS_MODE_DISABLED)
> +		return;
> +
> +	write_sysreg_s(state->trbbaser, SYS_TRBBASER_EL1);
> +	write_sysreg_s(state->trbptr, SYS_TRBPTR_EL1);
> +	write_sysreg_s(state->trbsr, SYS_TRBSR_EL1);
> +	write_sysreg_s(state->trblimitr & ~TRBLIMITR_EL1_E, SYS_TRBLIMITR_EL1);
> +
> +	/*
> +	 * According to the section K5.5 Context switching, Arm ARM (ARM DDI
> +	 * 0487 L.a), the software usage PKLXF requires a Context
> +	 * synchronization event to guarantee the Trace Buffer Unit will observe
> +	 * the new values of the System registers.
> +	 */
> +	isb();
> +
> +	/* Enable the Trace Buffer Unit */
> +	write_sysreg_s(state->trblimitr, SYS_TRBLIMITR_EL1);
> +
> +	/* Synchronize the TRBE enable event */
> +	isb();
> +
> +	if (trbe_needs_ctxt_sync_after_enable(cpudata))
> +		isb();

Especially this double isb() part shouldn't be duplicated as that could 
get broken in a refactor in the future.

> +}
> +
>   static const struct coresight_ops_sink arm_trbe_sink_ops = {
>   	.enable		= arm_trbe_enable,
>   	.disable	= arm_trbe_disable,
> @@ -1198,7 +1278,9 @@ static const struct coresight_ops_sink arm_trbe_sink_ops = {
>   };
>   
>   static const struct coresight_ops arm_trbe_cs_ops = {
> -	.sink_ops	= &arm_trbe_sink_ops,
> +	.pm_save_disable	= arm_trbe_save,
> +	.pm_restore_enable	= arm_trbe_restore,
> +	.sink_ops		= &arm_trbe_sink_ops,
>   };
>   
>   static ssize_t align_show(struct device *dev, struct device_attribute *attr, char *buf)
>