diff mbox series

[12/19] coresight: etm4x: Cleanup secure exception level masks

Message ID 20200911084119.1080694-13-suzuki.poulose@arm.com (mailing list archive)
State New, archived
Headers show
Series coresight: Support for ETMv4.4 system instructions | expand

Commit Message

Suzuki K Poulose Sept. 11, 2020, 8:41 a.m. UTC
We rely on the ETM architecture version to decide whether
Secure EL2 is available on the CPU for excluding the level
for address comparators and viewinst main control register.
We must instead use the TRCDIDR3.EXLEVEL_S field to detect
the supported levels.

Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm4x.c | 13 +++----------
 drivers/hwtracing/coresight/coresight-etm4x.h |  6 ++++--
 2 files changed, 7 insertions(+), 12 deletions(-)

Comments

Mike Leach Sept. 18, 2020, 3:35 p.m. UTC | #1
On Fri, 11 Sep 2020 at 09:41, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>
> We rely on the ETM architecture version to decide whether
> Secure EL2 is available on the CPU for excluding the level
> for address comparators and viewinst main control register.
> We must instead use the TRCDIDR3.EXLEVEL_S field to detect
> the supported levels.
>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x.c | 13 +++----------
>  drivers/hwtracing/coresight/coresight-etm4x.h |  6 ++++--
>  2 files changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
> index 7feb57108bdc..439e9da41006 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.c
> @@ -758,7 +758,6 @@ static void etm4_init_arch_data(void *info)
>          * TRCARCHMAJ, bits[11:8] architecture major versin number
>          */
>         drvdata->arch = BMVAL(etmidr1, 4, 11);
> -       drvdata->config.arch = drvdata->arch;
>
>         /* maximum size of resources */
>         etmidr2 = etm4x_relaxed_read32(&csa, TRCIDR2);
> @@ -774,6 +773,7 @@ static void etm4_init_arch_data(void *info)
>         drvdata->ccitmin = BMVAL(etmidr3, 0, 11);
>         /* EXLEVEL_S, bits[19:16] Secure state instruction tracing */
>         drvdata->s_ex_level = BMVAL(etmidr3, 16, 19);
> +       drvdata->config.s_ex_level = drvdata->s_ex_level;
>         /* EXLEVEL_NS, bits[23:20] Non-secure state instruction tracing */
>         drvdata->ns_ex_level = BMVAL(etmidr3, 20, 23);
>
> @@ -935,16 +935,9 @@ static u64 etm4_get_ns_access_type(struct etmv4_config *config)
>  static u64 etm4_get_access_type(struct etmv4_config *config)
>  {
>         u64 access_type = etm4_get_ns_access_type(config);
> -       u64 s_hyp = (config->arch & 0x0f) >= 0x4 ? ETM_EXLEVEL_S_HYP : 0;
>
> -       /*
> -        * EXLEVEL_S, bits[11:8], don't trace anything happening
> -        * in secure state.
> -        */
> -       access_type |= (ETM_EXLEVEL_S_APP       |
> -                       ETM_EXLEVEL_S_OS        |
> -                       s_hyp                   |
> -                       ETM_EXLEVEL_S_MON);
> +       /* All supported secure ELs are excluded */
> +       access_type |= (u64)config->s_ex_level << TRCACATR_EXLEVEL_SHIFT;
>

What is the << TRCACATR_EXLEVEL_SHIFT doing here?

>         return access_type;
>  }
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index efd903688edd..407ad6647f36 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -522,6 +522,8 @@
>  /* PowerDown Control Register bits */
>  #define TRCPDCR_PU                     BIT(3)
>
> +#define TRCACATR_EXLEVEL_SHIFT         8
> +
>  /* secure state access levels - TRCACATRn */
>  #define ETM_EXLEVEL_S_APP              BIT(8)
>  #define ETM_EXLEVEL_S_OS               BIT(9)
> @@ -604,7 +606,7 @@
>   * @vmid_mask0:        VM ID comparator mask for comparator 0-3.
>   * @vmid_mask1:        VM ID comparator mask for comparator 4-7.
>   * @ext_inp:   External input selection.
> - * @arch:      ETM architecture version (for arch dependent config).
> + * @s_ex_level: Secure ELs where tracing is supported.
>   */
>  struct etmv4_config {
>         u32                             mode;
> @@ -648,7 +650,7 @@ struct etmv4_config {
>         u32                             vmid_mask0;
>         u32                             vmid_mask1;
>         u32                             ext_inp;
> -       u8                              arch;
> +       u8                              s_ex_level;
>  };
>
>  /**
> --
> 2.24.1
>

Perhaps this patch could be combined with the next patch as it
operates on the same set of flags.

Regards

Mike
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
Suzuki K Poulose Sept. 22, 2020, 10:55 a.m. UTC | #2
On 09/18/2020 04:35 PM, Mike Leach wrote:
> On Fri, 11 Sep 2020 at 09:41, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>>
>> We rely on the ETM architecture version to decide whether
>> Secure EL2 is available on the CPU for excluding the level
>> for address comparators and viewinst main control register.
>> We must instead use the TRCDIDR3.EXLEVEL_S field to detect
>> the supported levels.
>>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>   drivers/hwtracing/coresight/coresight-etm4x.c | 13 +++----------
>>   drivers/hwtracing/coresight/coresight-etm4x.h |  6 ++++--
>>   2 files changed, 7 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
>> index 7feb57108bdc..439e9da41006 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x.c
>> @@ -758,7 +758,6 @@ static void etm4_init_arch_data(void *info)
>>           * TRCARCHMAJ, bits[11:8] architecture major versin number
>>           */
>>          drvdata->arch = BMVAL(etmidr1, 4, 11);
>> -       drvdata->config.arch = drvdata->arch;
>>
>>          /* maximum size of resources */
>>          etmidr2 = etm4x_relaxed_read32(&csa, TRCIDR2);
>> @@ -774,6 +773,7 @@ static void etm4_init_arch_data(void *info)
>>          drvdata->ccitmin = BMVAL(etmidr3, 0, 11);
>>          /* EXLEVEL_S, bits[19:16] Secure state instruction tracing */
>>          drvdata->s_ex_level = BMVAL(etmidr3, 16, 19);
>> +       drvdata->config.s_ex_level = drvdata->s_ex_level;
>>          /* EXLEVEL_NS, bits[23:20] Non-secure state instruction tracing */
>>          drvdata->ns_ex_level = BMVAL(etmidr3, 20, 23);
>>
>> @@ -935,16 +935,9 @@ static u64 etm4_get_ns_access_type(struct etmv4_config *config)
>>   static u64 etm4_get_access_type(struct etmv4_config *config)
>>   {
>>          u64 access_type = etm4_get_ns_access_type(config);
>> -       u64 s_hyp = (config->arch & 0x0f) >= 0x4 ? ETM_EXLEVEL_S_HYP : 0;
>>
>> -       /*
>> -        * EXLEVEL_S, bits[11:8], don't trace anything happening
>> -        * in secure state.
>> -        */
>> -       access_type |= (ETM_EXLEVEL_S_APP       |
>> -                       ETM_EXLEVEL_S_OS        |
>> -                       s_hyp                   |
>> -                       ETM_EXLEVEL_S_MON);
>> +       /* All supported secure ELs are excluded */
>> +       access_type |= (u64)config->s_ex_level << TRCACATR_EXLEVEL_SHIFT;
>>
> 
> What is the << TRCACATR_EXLEVEL_SHIFT doing here?

The config->s_ex_level is the EXLVEL mask from the TRCIDR3 shifted to bit 0, as above.
We need to make sure that we use the mask in the correct position for TRCACATR register.
Basically, we simply exclude all the secure levels supported by the ETM.

> 
>>          return access_type;
>>   }
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
>> index efd903688edd..407ad6647f36 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
>> @@ -522,6 +522,8 @@
>>   /* PowerDown Control Register bits */
>>   #define TRCPDCR_PU                     BIT(3)
>>
>> +#define TRCACATR_EXLEVEL_SHIFT         8
>> +
>>   /* secure state access levels - TRCACATRn */
>>   #define ETM_EXLEVEL_S_APP              BIT(8)
>>   #define ETM_EXLEVEL_S_OS               BIT(9)
>> @@ -604,7 +606,7 @@
>>    * @vmid_mask0:        VM ID comparator mask for comparator 0-3.
>>    * @vmid_mask1:        VM ID comparator mask for comparator 4-7.
>>    * @ext_inp:   External input selection.
>> - * @arch:      ETM architecture version (for arch dependent config).
>> + * @s_ex_level: Secure ELs where tracing is supported.
>>    */
>>   struct etmv4_config {
>>          u32                             mode;
>> @@ -648,7 +650,7 @@ struct etmv4_config {
>>          u32                             vmid_mask0;
>>          u32                             vmid_mask1;
>>          u32                             ext_inp;
>> -       u8                              arch;
>> +       u8                              s_ex_level;
>>   };
>>
>>   /**
>> --
>> 2.24.1
>>
> 
> Perhaps this patch could be combined with the next patch as it
> operates on the same set of flags.
> 

I agree that they both deal with the same set of masks. However, functionally
they have separate purposes.

1) This patch disconnects the usage of drvdata->arch field to determine
the secure exception level mask. This is more of a correctness, as a given
v4.4 implementation may not have a Secure EL2.

2) The next patch cleans up the way we define and use all the exception level
masks, both secure and non-secure.

I understand that the subjects are quite similar. I could try to fix the
subject for this one to make (1) more explicit. This way it is more easier
to review.

Suzuki
Mike Leach Sept. 22, 2020, 12:47 p.m. UTC | #3
Hi Suzuki,

On Tue, 22 Sep 2020 at 11:51, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>
> On 09/18/2020 04:35 PM, Mike Leach wrote:
> > On Fri, 11 Sep 2020 at 09:41, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
> >>
> >> We rely on the ETM architecture version to decide whether
> >> Secure EL2 is available on the CPU for excluding the level
> >> for address comparators and viewinst main control register.
> >> We must instead use the TRCDIDR3.EXLEVEL_S field to detect
> >> the supported levels.
> >>
> >> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> >> ---
> >>   drivers/hwtracing/coresight/coresight-etm4x.c | 13 +++----------
> >>   drivers/hwtracing/coresight/coresight-etm4x.h |  6 ++++--
> >>   2 files changed, 7 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
> >> index 7feb57108bdc..439e9da41006 100644
> >> --- a/drivers/hwtracing/coresight/coresight-etm4x.c
> >> +++ b/drivers/hwtracing/coresight/coresight-etm4x.c
> >> @@ -758,7 +758,6 @@ static void etm4_init_arch_data(void *info)
> >>           * TRCARCHMAJ, bits[11:8] architecture major versin number
> >>           */
> >>          drvdata->arch = BMVAL(etmidr1, 4, 11);
> >> -       drvdata->config.arch = drvdata->arch;
> >>
> >>          /* maximum size of resources */
> >>          etmidr2 = etm4x_relaxed_read32(&csa, TRCIDR2);
> >> @@ -774,6 +773,7 @@ static void etm4_init_arch_data(void *info)
> >>          drvdata->ccitmin = BMVAL(etmidr3, 0, 11);
> >>          /* EXLEVEL_S, bits[19:16] Secure state instruction tracing */
> >>          drvdata->s_ex_level = BMVAL(etmidr3, 16, 19);
> >> +       drvdata->config.s_ex_level = drvdata->s_ex_level;
> >>          /* EXLEVEL_NS, bits[23:20] Non-secure state instruction tracing */
> >>          drvdata->ns_ex_level = BMVAL(etmidr3, 20, 23);
> >>
> >> @@ -935,16 +935,9 @@ static u64 etm4_get_ns_access_type(struct etmv4_config *config)
> >>   static u64 etm4_get_access_type(struct etmv4_config *config)
> >>   {
> >>          u64 access_type = etm4_get_ns_access_type(config);
> >> -       u64 s_hyp = (config->arch & 0x0f) >= 0x4 ? ETM_EXLEVEL_S_HYP : 0;
> >>
> >> -       /*
> >> -        * EXLEVEL_S, bits[11:8], don't trace anything happening
> >> -        * in secure state.
> >> -        */
> >> -       access_type |= (ETM_EXLEVEL_S_APP       |
> >> -                       ETM_EXLEVEL_S_OS        |
> >> -                       s_hyp                   |
> >> -                       ETM_EXLEVEL_S_MON);
> >> +       /* All supported secure ELs are excluded */
> >> +       access_type |= (u64)config->s_ex_level << TRCACATR_EXLEVEL_SHIFT;
> >>
> >
> > What is the << TRCACATR_EXLEVEL_SHIFT doing here?
>
> The config->s_ex_level is the EXLVEL mask from the TRCIDR3 shifted to bit 0, as above.
> We need to make sure that we use the mask in the correct position for TRCACATR register.
> Basically, we simply exclude all the secure levels supported by the ETM.
>

Sorry, should have been a little more explicit. This breaks the next patch!

> >
> >>          return access_type;
> >>   }
> >> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> >> index efd903688edd..407ad6647f36 100644
> >> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> >> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> >> @@ -522,6 +522,8 @@
> >>   /* PowerDown Control Register bits */
> >>   #define TRCPDCR_PU                     BIT(3)
> >>
> >> +#define TRCACATR_EXLEVEL_SHIFT         8
> >> +
> >>   /* secure state access levels - TRCACATRn */
> >>   #define ETM_EXLEVEL_S_APP              BIT(8)
> >>   #define ETM_EXLEVEL_S_OS               BIT(9)
> >> @@ -604,7 +606,7 @@
> >>    * @vmid_mask0:        VM ID comparator mask for comparator 0-3.
> >>    * @vmid_mask1:        VM ID comparator mask for comparator 4-7.
> >>    * @ext_inp:   External input selection.
> >> - * @arch:      ETM architecture version (for arch dependent config).
> >> + * @s_ex_level: Secure ELs where tracing is supported.
> >>    */
> >>   struct etmv4_config {
> >>          u32                             mode;
> >> @@ -648,7 +650,7 @@ struct etmv4_config {
> >>          u32                             vmid_mask0;
> >>          u32                             vmid_mask1;
> >>          u32                             ext_inp;
> >> -       u8                              arch;
> >> +       u8                              s_ex_level;
> >>   };
> >>
> >>   /**
> >> --
> >> 2.24.1
> >>
> >
> > Perhaps this patch could be combined with the next patch as it
> > operates on the same set of flags.
> >
>
> I agree that they both deal with the same set of masks. However, functionally
> they have separate purposes.
>
> 1) This patch disconnects the usage of drvdata->arch field to determine
> the secure exception level mask. This is more of a correctness, as a given
> v4.4 implementation may not have a Secure EL2.
>
> 2) The next patch cleans up the way we define and use all the exception level
> masks, both secure and non-secure.
>



Applying this and the next patch which moves to the bits being indexed
from 0 for a common reusable field gets you the following code
sequence:-

static u64 etm4_get_ns_access_type(struct etmv4_config *config)
{
    u64 access_type = 0;

    /*
     * EXLEVEL_NS, bits[15:12]
     * The Exception levels are:
     *   Bit[12] Exception level 0 - Application
     *   Bit[13] Exception level 1 - OS
     *   Bit[14] Exception level 2 - Hypervisor
     *   Bit[15] Never implemented
     */


[ MJL: at this point the comment is true to an extent but no longer
applies to the values #defines below - which have been shifted by the
change to the #defines to form a field that is indexed from bit 0]


    if (!is_kernel_in_hyp_mode()) {
        /* Stay away from hypervisor mode for non-VHE */
        access_type =  ETM_EXLEVEL_NS_HYP;
        if (config->mode & ETM_MODE_EXCL_KERN)
            access_type |= ETM_EXLEVEL_NS_OS;
    } else if (config->mode & ETM_MODE_EXCL_KERN) {
        access_type = ETM_EXLEVEL_NS_HYP;
    }

    if (config->mode & ETM_MODE_EXCL_USER)
        access_type |= ETM_EXLEVEL_NS_APP;

    return access_type;
}

static u64 etm4_get_access_type(struct etmv4_config *config)
{
    u64 access_type = etm4_get_ns_access_type(config);

    /* All supported secure ELs are excluded */
    access_type |= (u64)config->s_ex_level << TRCACATR_EXLEVEL_SHIFT;

[MJL: Now we are OR ing a 0 bit index based field (NS access type)
with another 0 index based field - but shifting it for some reason?]

    return access_type;
}

static u64 etm4_get_comparator_access_type(struct etmv4_config *config)
{
    return etm4_get_access_type(config) << TRCACATR_EXLEVEL_SHIFT;

[MJL: resulting in TRCACATR_EXLEVEL_SHIFT being applied twice.]
}


 Hence my assertion that this should really be one patch, otherwise
you have to add the shift in one patch and correct the problem in the
second.

Regards

Mike

> I understand that the subjects are quite similar. I could try to fix the
> subject for this one to make (1) more explicit. This way it is more easier
> to review.
>
> Suzuki
Suzuki K Poulose Sept. 30, 2020, 10:32 a.m. UTC | #4
Hi Mike,

On 09/22/2020 01:47 PM, Mike Leach wrote:
> Hi Suzuki,
>
>>>> @@ -935,16 +935,9 @@ static u64 etm4_get_ns_access_type(struct etmv4_config *config)
>>>>    static u64 etm4_get_access_type(struct etmv4_config *config)
>>>>    {
>>>>           u64 access_type = etm4_get_ns_access_type(config);
>>>> -       u64 s_hyp = (config->arch & 0x0f) >= 0x4 ? ETM_EXLEVEL_S_HYP : 0;
>>>>
>>>> -       /*
>>>> -        * EXLEVEL_S, bits[11:8], don't trace anything happening
>>>> -        * in secure state.
>>>> -        */
>>>> -       access_type |= (ETM_EXLEVEL_S_APP       |
>>>> -                       ETM_EXLEVEL_S_OS        |
>>>> -                       s_hyp                   |
>>>> -                       ETM_EXLEVEL_S_MON);
>>>> +       /* All supported secure ELs are excluded */
>>>> +       access_type |= (u64)config->s_ex_level << TRCACATR_EXLEVEL_SHIFT;
>>>>
>>>
>>> What is the << TRCACATR_EXLEVEL_SHIFT doing here?
>>
>> The config->s_ex_level is the EXLVEL mask from the TRCIDR3 shifted to bit 0, as above.
>> We need to make sure that we use the mask in the correct position for TRCACATR register.
>> Basically, we simply exclude all the secure levels supported by the ETM.
>>
> 
> Sorry, should have been a little more explicit. This breaks the next patch!
> 
>>>
>>>>           return access_type;
>>>>    }
>>>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
>>>> index efd903688edd..407ad6647f36 100644
>>>> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
>>>> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
>>>> @@ -522,6 +522,8 @@
>>>>    /* PowerDown Control Register bits */
>>>>    #define TRCPDCR_PU                     BIT(3)
>>>>
>>>> +#define TRCACATR_EXLEVEL_SHIFT         8
>>>> +
>>>>    /* secure state access levels - TRCACATRn */
>>>>    #define ETM_EXLEVEL_S_APP              BIT(8)
>>>>    #define ETM_EXLEVEL_S_OS               BIT(9)
>>>> @@ -604,7 +606,7 @@
>>>>     * @vmid_mask0:        VM ID comparator mask for comparator 0-3.
>>>>     * @vmid_mask1:        VM ID comparator mask for comparator 4-7.
>>>>     * @ext_inp:   External input selection.
>>>> - * @arch:      ETM architecture version (for arch dependent config).
>>>> + * @s_ex_level: Secure ELs where tracing is supported.
>>>>     */
>>>>    struct etmv4_config {
>>>>           u32                             mode;
>>>> @@ -648,7 +650,7 @@ struct etmv4_config {
>>>>           u32                             vmid_mask0;
>>>>           u32                             vmid_mask1;
>>>>           u32                             ext_inp;
>>>> -       u8                              arch;
>>>> +       u8                              s_ex_level;
>>>>    };
>>>>
>>>>    /**
>>>> --
>>>> 2.24.1
>>>>
>>>
>>> Perhaps this patch could be combined with the next patch as it
>>> operates on the same set of flags.
>>>
>>
>> I agree that they both deal with the same set of masks. However, functionally
>> they have separate purposes.
>>
>> 1) This patch disconnects the usage of drvdata->arch field to determine
>> the secure exception level mask. This is more of a correctness, as a given
>> v4.4 implementation may not have a Secure EL2.
>>
>> 2) The next patch cleans up the way we define and use all the exception level
>> masks, both secure and non-secure.
>>
> 
> 
> 
> Applying this and the next patch which moves to the bits being indexed
> from 0 for a common reusable field gets you the following code
> sequence:-

Good catch.

> 
> static u64 etm4_get_ns_access_type(struct etmv4_config *config)
> {
>      u64 access_type = 0;
> 
>      /*
>       * EXLEVEL_NS, bits[15:12]
>       * The Exception levels are:
>       *   Bit[12] Exception level 0 - Application
>       *   Bit[13] Exception level 1 - OS
>       *   Bit[14] Exception level 2 - Hypervisor
>       *   Bit[15] Never implemented
>       */
> 

Updated the comments.

> 
> [ MJL: at this point the comment is true to an extent but no longer
> applies to the values #defines below - which have been shifted by the
> change to the #defines to form a field that is indexed from bit 0]
> 


> 
> static u64 etm4_get_access_type(struct etmv4_config *config)
> {
>      u64 access_type = etm4_get_ns_access_type(config);
> 
>      /* All supported secure ELs are excluded */
>      access_type |= (u64)config->s_ex_level << TRCACATR_EXLEVEL_SHIFT;
> 
> [MJL: Now we are OR ing a 0 bit index based field (NS access type)
> with another 0 index based field - but shifting it for some reason?]

That was a mistake. I will drop this shift from here to make get_access_type() and
this should work.

Suzuki
diff mbox series

Patch

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 7feb57108bdc..439e9da41006 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -758,7 +758,6 @@  static void etm4_init_arch_data(void *info)
 	 * TRCARCHMAJ, bits[11:8] architecture major versin number
 	 */
 	drvdata->arch = BMVAL(etmidr1, 4, 11);
-	drvdata->config.arch = drvdata->arch;
 
 	/* maximum size of resources */
 	etmidr2 = etm4x_relaxed_read32(&csa, TRCIDR2);
@@ -774,6 +773,7 @@  static void etm4_init_arch_data(void *info)
 	drvdata->ccitmin = BMVAL(etmidr3, 0, 11);
 	/* EXLEVEL_S, bits[19:16] Secure state instruction tracing */
 	drvdata->s_ex_level = BMVAL(etmidr3, 16, 19);
+	drvdata->config.s_ex_level = drvdata->s_ex_level;
 	/* EXLEVEL_NS, bits[23:20] Non-secure state instruction tracing */
 	drvdata->ns_ex_level = BMVAL(etmidr3, 20, 23);
 
@@ -935,16 +935,9 @@  static u64 etm4_get_ns_access_type(struct etmv4_config *config)
 static u64 etm4_get_access_type(struct etmv4_config *config)
 {
 	u64 access_type = etm4_get_ns_access_type(config);
-	u64 s_hyp = (config->arch & 0x0f) >= 0x4 ? ETM_EXLEVEL_S_HYP : 0;
 
-	/*
-	 * EXLEVEL_S, bits[11:8], don't trace anything happening
-	 * in secure state.
-	 */
-	access_type |= (ETM_EXLEVEL_S_APP	|
-			ETM_EXLEVEL_S_OS	|
-			s_hyp			|
-			ETM_EXLEVEL_S_MON);
+	/* All supported secure ELs are excluded */
+	access_type |= (u64)config->s_ex_level << TRCACATR_EXLEVEL_SHIFT;
 
 	return access_type;
 }
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index efd903688edd..407ad6647f36 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -522,6 +522,8 @@ 
 /* PowerDown Control Register bits */
 #define TRCPDCR_PU			BIT(3)
 
+#define TRCACATR_EXLEVEL_SHIFT		8
+
 /* secure state access levels - TRCACATRn */
 #define ETM_EXLEVEL_S_APP		BIT(8)
 #define ETM_EXLEVEL_S_OS		BIT(9)
@@ -604,7 +606,7 @@ 
  * @vmid_mask0:	VM ID comparator mask for comparator 0-3.
  * @vmid_mask1:	VM ID comparator mask for comparator 4-7.
  * @ext_inp:	External input selection.
- * @arch:	ETM architecture version (for arch dependent config).
+ * @s_ex_level: Secure ELs where tracing is supported.
  */
 struct etmv4_config {
 	u32				mode;
@@ -648,7 +650,7 @@  struct etmv4_config {
 	u32				vmid_mask0;
 	u32				vmid_mask1;
 	u32				ext_inp;
-	u8				arch;
+	u8				s_ex_level;
 };
 
 /**