1. Patchew
  2. linux
  3. [PATCH v2 0/2] coresight: Allow guests to be traced when FEAT_TRF and VHE are present
  4. View patch

[PATCH v2 2/2] coresight: Allow guests to be traced when FEAT_TRF and VHE are present

James Clark posted 2 patches 2 years, 5 months ago
There is a newer version of this series
[PATCH v2 2/2] coresight: Allow guests to be traced when FEAT_TRF and VHE are present
Posted by James Clark 2 years, 5 months ago 
Currently the userspace and kernel filters for guests are never set, so
no trace will be generated for them. Add it by writing to the guest
filters when exclude_guest isn't set. By writing either E1TRE or E0TRE,
filtering on either guest kernel or guest userspace is also supported.

Since TRFCR_EL1 access is trapped, this can't be  modified by the guest.

This change also brings exclude_host support which is difficult to add
as a separate commit without excess churn and resulting in no trace at
all.

Testing
=======

The addresses were counted with the following:

  $ perf report -D | grep -Eo 'EL2|EL1|EL0' | sort | uniq -c

Guest kernel only:

  $ perf record -e cs_etm//Gk -a -- true
    535 EL1
      1 EL2

Guest user only (0 addresses expected because the guest OS hasn't reached
userspace yet):

  $ perf record -e cs_etm//Gu -a -- true

Host kernel only:

  $  perf record -e cs_etm//Hk -a -- true
   3501 EL2

Host userspace only:

  $  perf record -e cs_etm//Hu -a -- true
    408 EL0
      1 EL2

Reviewed-by: Mark Brown <broonie@kernel.org> (sysreg)
Signed-off-by: James Clark <james.clark@arm.com>
---
 arch/arm64/tools/sysreg                       |  4 ++
 .../coresight/coresight-etm4x-core.c          | 51 ++++++++++++++++---
 drivers/hwtracing/coresight/coresight-etm4x.h |  2 +-
 drivers/hwtracing/coresight/coresight-priv.h  |  3 ++
 4 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index 6ca7db69d6c9..cae9139b6c05 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -2531,3 +2531,7 @@ EndSysreg
 Sysreg	TRFCR_EL2	3	4	1	2	1
 Fields	TRFCR_EL2
 EndSysreg
+
+Sysreg TRFCR_EL12	3	5	1	2	1
+Fields	TRFCR_ELx
+EndSysreg
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 77b0271ce6eb..6c16a14d6fbe 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -274,6 +274,18 @@ static void etm4x_prohibit_trace(struct etmv4_drvdata *drvdata)
 	cpu_prohibit_trace();
 }
 
+static u64 etm4x_get_kern_user_filter(struct etmv4_drvdata *drvdata)
+{
+	u64 trfcr = drvdata->trfcr;
+
+	if (drvdata->config.mode & ETM_MODE_EXCL_KERN)
+		trfcr &= ~TRFCR_ELx_ExTRE;
+	if (drvdata->config.mode & ETM_MODE_EXCL_USER)
+		trfcr &= ~TRFCR_ELx_E0TRE;
+
+	return trfcr;
+}
+
 /*
  * etm4x_allow_trace - Allow CPU tracing in the respective ELs,
  * as configured by the drvdata->config.mode for the current
@@ -286,18 +298,39 @@ static void etm4x_prohibit_trace(struct etmv4_drvdata *drvdata)
  */
 static void etm4x_allow_trace(struct etmv4_drvdata *drvdata)
 {
-	u64 trfcr = drvdata->trfcr;
+	u64 trfcr;
 
 	/* If the CPU doesn't support FEAT_TRF, nothing to do */
-	if (!trfcr)
+	if (!drvdata->trfcr)
 		return;
 
-	if (drvdata->config.mode & ETM_MODE_EXCL_KERN)
-		trfcr &= ~TRFCR_ELx_ExTRE;
-	if (drvdata->config.mode & ETM_MODE_EXCL_USER)
-		trfcr &= ~TRFCR_ELx_E0TRE;
+	if (drvdata->config.mode & ETM_MODE_EXCL_HOST)
+		trfcr = drvdata->trfcr & ~(TRFCR_ELx_ExTRE | TRFCR_ELx_E0TRE);
+	else
+		trfcr = etm4x_get_kern_user_filter(drvdata);
 
 	write_trfcr(trfcr);
+
+	/*
+	 * Filters for EL1 and EL0 (when running a guest) are stored in
+	 * TRFCR_EL1 so write it there for VHE. For nVHE, the filters in
+	 * have to be re-applied when switching to the guest instead.
+	 */
+	if (!is_kernel_in_hyp_mode())
+		return;
+
+	if (drvdata->config.mode & ETM_MODE_EXCL_GUEST)
+		trfcr = drvdata->trfcr & ~(TRFCR_ELx_ExTRE | TRFCR_ELx_E0TRE);
+	else
+		trfcr = etm4x_get_kern_user_filter(drvdata);
+
+	/*
+	 * TRFCR_EL1 doesn't have CX and TRFCR_EL1.TS has no effect when TS is
+	 * set in EL2 so mask them out.
+	 */
+	trfcr &= ~(TRFCR_ELx_TS_MASK | TRFCR_EL2_CX);
+
+	write_sysreg_s(trfcr, SYS_TRFCR_EL12);
 }
 
 #ifdef CONFIG_ETM4X_IMPDEF_FEATURE
@@ -655,6 +688,12 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
 	if (attr->exclude_user)
 		config->mode = ETM_MODE_EXCL_USER;
 
+	if (attr->exclude_host)
+		config->mode |= ETM_MODE_EXCL_HOST;
+
+	if (attr->exclude_guest)
+		config->mode |= ETM_MODE_EXCL_GUEST;
+
 	/* Always start from the default config */
 	etm4_set_default_config(config);
 
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index 20e2e4cb7614..3f170599822f 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -841,7 +841,7 @@ enum etm_impdef_type {
  * @s_ex_level: Secure ELs where tracing is supported.
  */
 struct etmv4_config {
-	u32				mode;
+	u64				mode;
 	u32				pe_sel;
 	u32				cfg;
 	u32				eventctrl0;
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index 767076e07970..727dd27ba800 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -39,6 +39,9 @@
 
 #define ETM_MODE_EXCL_KERN	BIT(30)
 #define ETM_MODE_EXCL_USER	BIT(31)
+#define ETM_MODE_EXCL_HOST	BIT(32)
+#define ETM_MODE_EXCL_GUEST	BIT(33)
+
 struct cs_pair_attribute {
 	struct device_attribute attr;
 	u32 lo_off;
-- 
2.34.1
Re: [PATCH v2 2/2] coresight: Allow guests to be traced when FEAT_TRF and VHE are present
Posted by Suzuki K Poulose 2 years, 4 months ago 
Hi James


On 04/09/2023 15:07, James Clark wrote:
> Currently the userspace and kernel filters for guests are never set, so
> no trace will be generated for them. Add it by writing to the guest
> filters when exclude_guest isn't set. By writing either E1TRE or E0TRE,
> filtering on either guest kernel or guest userspace is also supported.
> 
> Since TRFCR_EL1 access is trapped, this can't be  modified by the guest.
> 
> This change also brings exclude_host support which is difficult to add
> as a separate commit without excess churn and resulting in no trace at
> all.
> 
> Testing
> =======
> 
> The addresses were counted with the following:
> 
>    $ perf report -D | grep -Eo 'EL2|EL1|EL0' | sort | uniq -c
> 
> Guest kernel only:
> 
>    $ perf record -e cs_etm//Gk -a -- true
>      535 EL1
>        1 EL2
> 
> Guest user only (0 addresses expected because the guest OS hasn't reached
> userspace yet):
> 
>    $ perf record -e cs_etm//Gu -a -- true
> 
> Host kernel only:
> 
>    $  perf record -e cs_etm//Hk -a -- true
>     3501 EL2
> 
> Host userspace only:
> 
>    $  perf record -e cs_etm//Hu -a -- true
>      408 EL0
>        1 EL2
> 
> Reviewed-by: Mark Brown <broonie@kernel.org> (sysreg)
> Signed-off-by: James Clark <james.clark@arm.com>
> ---
>   arch/arm64/tools/sysreg                       |  4 ++
>   .../coresight/coresight-etm4x-core.c          | 51 ++++++++++++++++---
>   drivers/hwtracing/coresight/coresight-etm4x.h |  2 +-
>   drivers/hwtracing/coresight/coresight-priv.h  |  3 ++
>   4 files changed, 53 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
> index 6ca7db69d6c9..cae9139b6c05 100644
> --- a/arch/arm64/tools/sysreg
> +++ b/arch/arm64/tools/sysreg
> @@ -2531,3 +2531,7 @@ EndSysreg
>   Sysreg	TRFCR_EL2	3	4	1	2	1
>   Fields	TRFCR_EL2
>   EndSysreg
> +
> +Sysreg TRFCR_EL12	3	5	1	2	1
> +Fields	TRFCR_ELx
> +EndSysreg
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 77b0271ce6eb..6c16a14d6fbe 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -274,6 +274,18 @@ static void etm4x_prohibit_trace(struct etmv4_drvdata *drvdata)
>   	cpu_prohibit_trace();
>   }
>   
> +static u64 etm4x_get_kern_user_filter(struct etmv4_drvdata *drvdata)
> +{
> +	u64 trfcr = drvdata->trfcr;
> +
> +	if (drvdata->config.mode & ETM_MODE_EXCL_KERN)
> +		trfcr &= ~TRFCR_ELx_ExTRE;
> +	if (drvdata->config.mode & ETM_MODE_EXCL_USER)
> +		trfcr &= ~TRFCR_ELx_E0TRE;
> +
> +	return trfcr;
> +}
> +
>   /*
>    * etm4x_allow_trace - Allow CPU tracing in the respective ELs,
>    * as configured by the drvdata->config.mode for the current
> @@ -286,18 +298,39 @@ static void etm4x_prohibit_trace(struct etmv4_drvdata *drvdata)
>    */
>   static void etm4x_allow_trace(struct etmv4_drvdata *drvdata)
>   {
> -	u64 trfcr = drvdata->trfcr;
> +	u64 trfcr;
>   
>   	/* If the CPU doesn't support FEAT_TRF, nothing to do */
> -	if (!trfcr)
> +	if (!drvdata->trfcr)
>   		return;
>   
> -	if (drvdata->config.mode & ETM_MODE_EXCL_KERN)
> -		trfcr &= ~TRFCR_ELx_ExTRE;
> -	if (drvdata->config.mode & ETM_MODE_EXCL_USER)
> -		trfcr &= ~TRFCR_ELx_E0TRE;
> +	if (drvdata->config.mode & ETM_MODE_EXCL_HOST)
> +		trfcr = drvdata->trfcr & ~(TRFCR_ELx_ExTRE | TRFCR_ELx_E0TRE);
> +	else
> +		trfcr = etm4x_get_kern_user_filter(drvdata);
>   
>   	write_trfcr(trfcr);
> +
> +	/*
> +	 * Filters for EL1 and EL0 (when running a guest) are stored in
> +	 * TRFCR_EL1 so write it there for VHE. For nVHE, the filters in
> +	 * have to be re-applied when switching to the guest instead.
> +	 */
> +	if (!is_kernel_in_hyp_mode())
> +		return;
> +
> +	if (drvdata->config.mode & ETM_MODE_EXCL_GUEST)
> +		trfcr = drvdata->trfcr & ~(TRFCR_ELx_ExTRE | TRFCR_ELx_E0TRE);
> +	else
> +		trfcr = etm4x_get_kern_user_filter(drvdata);
> +
> +	/*
> +	 * TRFCR_EL1 doesn't have CX and TRFCR_EL1.TS has no effect when TS is
> +	 * set in EL2 so mask them out.
> +	 */
> +	trfcr &= ~(TRFCR_ELx_TS_MASK | TRFCR_EL2_CX);
> +
> +	write_sysreg_s(trfcr, SYS_TRFCR_EL12);



While I agree with the intention of the patch, I am not sure if we
should do the TRFCR_EL1 (via TRFCR_EL12) updates from CoreSight driver.

It might be much better to do it from the KVM for both VHE and nVHE.


Marc

What are your thoughts on this ?

Suzuki


>   }
>   
>   #ifdef CONFIG_ETM4X_IMPDEF_FEATURE
> @@ -655,6 +688,12 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
>   	if (attr->exclude_user)
>   		config->mode = ETM_MODE_EXCL_USER;
>   
> +	if (attr->exclude_host)
> +		config->mode |= ETM_MODE_EXCL_HOST;
> +
> +	if (attr->exclude_guest)
> +		config->mode |= ETM_MODE_EXCL_GUEST;
> +
>   	/* Always start from the default config */
>   	etm4_set_default_config(config);
>   
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index 20e2e4cb7614..3f170599822f 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -841,7 +841,7 @@ enum etm_impdef_type {
>    * @s_ex_level: Secure ELs where tracing is supported.
>    */
>   struct etmv4_config {
> -	u32				mode;
> +	u64				mode;
>   	u32				pe_sel;
>   	u32				cfg;
>   	u32				eventctrl0;
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index 767076e07970..727dd27ba800 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -39,6 +39,9 @@
>   
>   #define ETM_MODE_EXCL_KERN	BIT(30)
>   #define ETM_MODE_EXCL_USER	BIT(31)
> +#define ETM_MODE_EXCL_HOST	BIT(32)
> +#define ETM_MODE_EXCL_GUEST	BIT(33)
> +
>   struct cs_pair_attribute {
>   	struct device_attribute attr;
>   	u32 lo_off;
Re: [PATCH v2 2/2] coresight: Allow guests to be traced when FEAT_TRF and VHE are present
Posted by James Clark 2 years, 4 months ago 

On 12/09/2023 18:37, Suzuki K Poulose wrote:
> Hi James
> 
> 
> On 04/09/2023 15:07, James Clark wrote:
>> Currently the userspace and kernel filters for guests are never set, so
>> no trace will be generated for them. Add it by writing to the guest
>> filters when exclude_guest isn't set. By writing either E1TRE or E0TRE,
>> filtering on either guest kernel or guest userspace is also supported.
>>
>> Since TRFCR_EL1 access is trapped, this can't be  modified by the guest.
>>
>> This change also brings exclude_host support which is difficult to add
>> as a separate commit without excess churn and resulting in no trace at
>> all.
>>
>> Testing
>> =======
>>
>> The addresses were counted with the following:
>>
>>    $ perf report -D | grep -Eo 'EL2|EL1|EL0' | sort | uniq -c
>>
>> Guest kernel only:
>>
>>    $ perf record -e cs_etm//Gk -a -- true
>>      535 EL1
>>        1 EL2
>>
>> Guest user only (0 addresses expected because the guest OS hasn't reached
>> userspace yet):
>>
>>    $ perf record -e cs_etm//Gu -a -- true
>>
>> Host kernel only:
>>
>>    $  perf record -e cs_etm//Hk -a -- true
>>     3501 EL2
>>
>> Host userspace only:
>>
>>    $  perf record -e cs_etm//Hu -a -- true
>>      408 EL0
>>        1 EL2
>>
>> Reviewed-by: Mark Brown <broonie@kernel.org> (sysreg)
>> Signed-off-by: James Clark <james.clark@arm.com>
>> ---
>>   arch/arm64/tools/sysreg                       |  4 ++
>>   .../coresight/coresight-etm4x-core.c          | 51 ++++++++++++++++---
>>   drivers/hwtracing/coresight/coresight-etm4x.h |  2 +-
>>   drivers/hwtracing/coresight/coresight-priv.h  |  3 ++
>>   4 files changed, 53 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
>> index 6ca7db69d6c9..cae9139b6c05 100644
>> --- a/arch/arm64/tools/sysreg
>> +++ b/arch/arm64/tools/sysreg
>> @@ -2531,3 +2531,7 @@ EndSysreg
>>   Sysreg    TRFCR_EL2    3    4    1    2    1
>>   Fields    TRFCR_EL2
>>   EndSysreg
>> +
>> +Sysreg TRFCR_EL12    3    5    1    2    1
>> +Fields    TRFCR_ELx
>> +EndSysreg
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index 77b0271ce6eb..6c16a14d6fbe 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -274,6 +274,18 @@ static void etm4x_prohibit_trace(struct
>> etmv4_drvdata *drvdata)
>>       cpu_prohibit_trace();
>>   }
>>   +static u64 etm4x_get_kern_user_filter(struct etmv4_drvdata *drvdata)
>> +{
>> +    u64 trfcr = drvdata->trfcr;
>> +
>> +    if (drvdata->config.mode & ETM_MODE_EXCL_KERN)
>> +        trfcr &= ~TRFCR_ELx_ExTRE;
>> +    if (drvdata->config.mode & ETM_MODE_EXCL_USER)
>> +        trfcr &= ~TRFCR_ELx_E0TRE;
>> +
>> +    return trfcr;
>> +}
>> +
>>   /*
>>    * etm4x_allow_trace - Allow CPU tracing in the respective ELs,
>>    * as configured by the drvdata->config.mode for the current
>> @@ -286,18 +298,39 @@ static void etm4x_prohibit_trace(struct
>> etmv4_drvdata *drvdata)
>>    */
>>   static void etm4x_allow_trace(struct etmv4_drvdata *drvdata)
>>   {
>> -    u64 trfcr = drvdata->trfcr;
>> +    u64 trfcr;
>>         /* If the CPU doesn't support FEAT_TRF, nothing to do */
>> -    if (!trfcr)
>> +    if (!drvdata->trfcr)
>>           return;
>>   -    if (drvdata->config.mode & ETM_MODE_EXCL_KERN)
>> -        trfcr &= ~TRFCR_ELx_ExTRE;
>> -    if (drvdata->config.mode & ETM_MODE_EXCL_USER)
>> -        trfcr &= ~TRFCR_ELx_E0TRE;
>> +    if (drvdata->config.mode & ETM_MODE_EXCL_HOST)
>> +        trfcr = drvdata->trfcr & ~(TRFCR_ELx_ExTRE | TRFCR_ELx_E0TRE);
>> +    else
>> +        trfcr = etm4x_get_kern_user_filter(drvdata);
>>         write_trfcr(trfcr);
>> +
>> +    /*
>> +     * Filters for EL1 and EL0 (when running a guest) are stored in
>> +     * TRFCR_EL1 so write it there for VHE. For nVHE, the filters in
>> +     * have to be re-applied when switching to the guest instead.
>> +     */
>> +    if (!is_kernel_in_hyp_mode())
>> +        return;
>> +
>> +    if (drvdata->config.mode & ETM_MODE_EXCL_GUEST)
>> +        trfcr = drvdata->trfcr & ~(TRFCR_ELx_ExTRE | TRFCR_ELx_E0TRE);
>> +    else
>> +        trfcr = etm4x_get_kern_user_filter(drvdata);
>> +
>> +    /*
>> +     * TRFCR_EL1 doesn't have CX and TRFCR_EL1.TS has no effect when
>> TS is
>> +     * set in EL2 so mask them out.
>> +     */
>> +    trfcr &= ~(TRFCR_ELx_TS_MASK | TRFCR_EL2_CX);
>> +
>> +    write_sysreg_s(trfcr, SYS_TRFCR_EL12);
> 
> 
> 
> While I agree with the intention of the patch, I am not sure if we
> should do the TRFCR_EL1 (via TRFCR_EL12) updates from CoreSight driver.
> 
> It might be much better to do it from the KVM for both VHE and nVHE.
> 

For the next version of the nVHE change [1] I planned to re-write it
just in terms of the guest trfcr register value rather than
exclude_guest/exclude host. That means that it would be pretty easy to
move the write to SYS_TRFCR_EL12 to the VHE implementation of
kvm_etm_set_guest_trfcr(). So yes I can make this change I think it
makes sense.

[1]:
https://lore.kernel.org/kvmarm/20230804101317.460697-1-james.clark@arm.com/

James

> 
> Marc
> 
> What are your thoughts on this ?
> 
> Suzuki
> 
> 
>>   }
>>     #ifdef CONFIG_ETM4X_IMPDEF_FEATURE
>> @@ -655,6 +688,12 @@ static int etm4_parse_event_config(struct
>> coresight_device *csdev,
>>       if (attr->exclude_user)
>>           config->mode = ETM_MODE_EXCL_USER;
>>   +    if (attr->exclude_host)
>> +        config->mode |= ETM_MODE_EXCL_HOST;
>> +
>> +    if (attr->exclude_guest)
>> +        config->mode |= ETM_MODE_EXCL_GUEST;
>> +
>>       /* Always start from the default config */
>>       etm4_set_default_config(config);
>>   diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h
>> b/drivers/hwtracing/coresight/coresight-etm4x.h
>> index 20e2e4cb7614..3f170599822f 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
>> @@ -841,7 +841,7 @@ enum etm_impdef_type {
>>    * @s_ex_level: Secure ELs where tracing is supported.
>>    */
>>   struct etmv4_config {
>> -    u32                mode;
>> +    u64                mode;
>>       u32                pe_sel;
>>       u32                cfg;
>>       u32                eventctrl0;
>> diff --git a/drivers/hwtracing/coresight/coresight-priv.h
>> b/drivers/hwtracing/coresight/coresight-priv.h
>> index 767076e07970..727dd27ba800 100644
>> --- a/drivers/hwtracing/coresight/coresight-priv.h
>> +++ b/drivers/hwtracing/coresight/coresight-priv.h
>> @@ -39,6 +39,9 @@
>>     #define ETM_MODE_EXCL_KERN    BIT(30)
>>   #define ETM_MODE_EXCL_USER    BIT(31)
>> +#define ETM_MODE_EXCL_HOST    BIT(32)
>> +#define ETM_MODE_EXCL_GUEST    BIT(33)
>> +
>>   struct cs_pair_attribute {
>>       struct device_attribute attr;
>>       u32 lo_off;
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.