lkml.org 
[lkml]   [2021]   [Jul]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [PATCH v2 02/10] coresight: etm4x: Use Trace Filtering controls dynamically
    From
    Date

    On 7/23/21 6:16 PM, Suzuki K Poulose wrote:
    > The Trace Filtering support (FEAT_TRF) ensures that the ETM
    > can be prohibited from generating any trace for a given EL.
    > This is much stricter knob, than the TRCVICTLR exception level

    Could you please explain 'stricter' ? Are you suggesting that TRCVICTLR
    based exception filtering some times might not implement the filtering
    even if configured ?

    > masks. At the moment, we do a onetime enable trace at user and
    > kernel and leave it untouched for the kernel life time.
    >
    > This patch makes the switch dynamic, by honoring the filters
    > set by the user and enforcing them in the TRFCR controls.

    TRFCR actually helps in making the exception level filtering dynamic
    which was not possible earlier with TRCVICTLR.

    > We also rename the cpu_enable_tracing() appropriately to
    > cpu_detect_trace_filtering() and the drvdata member
    > trfc => trfcr to indicate the "value" of the TRFCR_EL1.

    Makes sense.

    >
    > Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
    > Cc: Al Grant <al.grant@arm.com>
    > Cc: Mike Leach <mike.leach@linaro.org>
    > Cc: Leo Yan <leo.yan@linaro.org>
    > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
    > ---
    > .../coresight/coresight-etm4x-core.c | 61 ++++++++++++++-----
    > drivers/hwtracing/coresight/coresight-etm4x.h | 5 +-
    > .../coresight/coresight-self-hosted-trace.h | 7 +++
    > 3 files changed, 55 insertions(+), 18 deletions(-)
    >
    > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
    > index 3e548dac9b05..adba84b29455 100644
    > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
    > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
    > @@ -237,6 +237,43 @@ struct etm4_enable_arg {
    > int rc;
    > };
    >
    > +/*
    > + * etm4x_prohibit_trace - Prohibit the CPU from tracing at all ELs.
    > + * When the CPU supports FEAT_TRF, we could move the ETM to a trace
    > + * prohibited state by filtering the Exception levels via TRFCR_EL1.
    > + */
    > +static void etm4x_prohibit_trace(struct etmv4_drvdata *drvdata)
    > +{
    > + if (drvdata->trfcr)
    > + cpu_prohibit_trace();

    Should it be as etm4x_allow_trace() instead, where drvdata->trfcr
    indicates the presence of FEAT_TRF - just to be clear ?

    /* If the CPU doesn't support FEAT_TRF, nothing to do */
    if (!drvdata->trfcr)
    return;

    cpu_prohibit_trace();

    > +}
    > +
    > +/*
    > + * etm4x_allow_trace - Allow CPU tracing in the respective ELs,
    > + * as configured by the drvdata->config.mode for the current
    > + * session. Even though we have TRCVICTLR bits to filter the
    > + * trace in the ELs, it doesn't prevent the ETM from generating
    > + * a packet (e.g, TraceInfo) that might contain the addresses from
    > + * the excluded levels. Thus we use the additional controls provided
    > + * via the Trace Filtering controls (FEAT_TRF) to make sure no trace
    > + * is generated for the excluded ELs.
    > + */
    > +static void etm4x_allow_trace(struct etmv4_drvdata *drvdata)
    > +{
    > + u64 trfcr = drvdata->trfcr;
    > +
    > + /* If the CPU doesn't support FEAT_TRF, nothing to do */
    > + if (!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;
    > +
    > + write_trfcr(trfcr);
    > +}
    > +
    > #ifdef CONFIG_ETM4X_IMPDEF_FEATURE
    >
    > #define HISI_HIP08_AMBA_ID 0x000b6d01
    > @@ -441,6 +478,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
    > if (etm4x_is_ete(drvdata))
    > etm4x_relaxed_write32(csa, TRCRSR_TA, TRCRSR);
    >
    > + etm4x_allow_trace(drvdata);
    > /* Enable the trace unit */
    > etm4x_relaxed_write32(csa, 1, TRCPRGCTLR);
    >
    > @@ -719,7 +757,6 @@ static int etm4_enable(struct coresight_device *csdev,
    > static void etm4_disable_hw(void *info)
    > {
    > u32 control;
    > - u64 trfcr;
    > struct etmv4_drvdata *drvdata = info;
    > struct etmv4_config *config = &drvdata->config;
    > struct coresight_device *csdev = drvdata->csdev;
    > @@ -746,12 +783,7 @@ static void etm4_disable_hw(void *info)
    > * If the CPU supports v8.4 Trace filter Control,
    > * set the ETM to trace prohibited region.
    > */
    > - if (drvdata->trfc) {
    > - trfcr = read_sysreg_s(SYS_TRFCR_EL1);
    > - write_sysreg_s(trfcr & ~(TRFCR_ELx_ExTRE | TRFCR_ELx_E0TRE),
    > - SYS_TRFCR_EL1);
    > - isb();
    > - }
    > + etm4x_prohibit_trace(drvdata);
    > /*
    > * Make sure everything completes before disabling, as recommended
    > * by section 7.3.77 ("TRCVICTLR, ViewInst Main Control Register,
    > @@ -767,9 +799,6 @@ static void etm4_disable_hw(void *info)
    > if (coresight_timeout(csa, TRCSTATR, TRCSTATR_PMSTABLE_BIT, 1))
    > dev_err(etm_dev,
    > "timeout while waiting for PM stable Trace Status\n");
    > - if (drvdata->trfc)
    > - write_sysreg_s(trfcr, SYS_TRFCR_EL1);
    > -
    > /* read the status of the single shot comparators */
    > for (i = 0; i < drvdata->nr_ss_cmp; i++) {
    > config->ss_status[i] =
    > @@ -964,15 +993,15 @@ static bool etm4_init_csdev_access(struct etmv4_drvdata *drvdata,
    > return false;
    > }
    >
    > -static void cpu_enable_tracing(struct etmv4_drvdata *drvdata)
    > +static void cpu_detect_trace_filtering(struct etmv4_drvdata *drvdata)
    > {
    > u64 dfr0 = read_sysreg(id_aa64dfr0_el1);
    > u64 trfcr;
    >
    > + drvdata->trfcr = 0;
    > if (!cpuid_feature_extract_unsigned_field(dfr0, ID_AA64DFR0_TRACE_FILT_SHIFT))
    > return;
    >
    > - drvdata->trfc = true;
    > /*
    > * If the CPU supports v8.4 SelfHosted Tracing, enable
    > * tracing at the kernel EL and EL0, forcing to use the
    > @@ -986,7 +1015,7 @@ static void cpu_enable_tracing(struct etmv4_drvdata *drvdata)
    > if (is_kernel_in_hyp_mode())
    > trfcr |= TRFCR_EL2_CX;
    >
    > - write_trfcr(trfcr);
    > + drvdata->trfcr = trfcr;
    > }
    >
    > static void etm4_init_arch_data(void *info)
    > @@ -1177,7 +1206,7 @@ static void etm4_init_arch_data(void *info)
    > /* NUMCNTR, bits[30:28] number of counters available for tracing */
    > drvdata->nr_cntr = BMVAL(etmidr5, 28, 30);
    > etm4_cs_lock(drvdata, csa);
    > - cpu_enable_tracing(drvdata);
    > + cpu_detect_trace_filtering(drvdata);
    > }
    >
    > static inline u32 etm4_get_victlr_access_type(struct etmv4_config *config)
    > @@ -1673,7 +1702,7 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata)
    > int ret = 0;
    >
    > /* Save the TRFCR irrespective of whether the ETM is ON */
    > - if (drvdata->trfc)
    > + if (drvdata->trfcr)
    > drvdata->save_trfcr = read_trfcr();
    > /*
    > * Save and restore the ETM Trace registers only if
    > @@ -1782,7 +1811,7 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
    >
    > static void etm4_cpu_restore(struct etmv4_drvdata *drvdata)
    > {
    > - if (drvdata->trfc)
    > + if (drvdata->trfcr)
    > write_trfcr(drvdata->save_trfcr);
    > if (drvdata->state_needs_restore)
    > __etm4_cpu_restore(drvdata);
    > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
    > index 82cba16b73a6..724819592c2e 100644
    > --- a/drivers/hwtracing/coresight/coresight-etm4x.h
    > +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
    > @@ -919,7 +919,8 @@ struct etmv4_save_state {
    > * @nooverflow: Indicate if overflow prevention is supported.
    > * @atbtrig: If the implementation can support ATB triggers
    > * @lpoverride: If the implementation can support low-power state over.
    > - * @trfc: If the implementation supports Arm v8.4 trace filter controls.
    > + * @trfcr: If the CPU supportfs FEAT_TRF, value of the TRFCR_ELx with

    Typo here. ^^^^^^ s/supportfs/supports

    > + * trace allowed at user and kernel ELs. Otherwise, 0.

    The sentence here does not make sense. Is not the exception level ELx and EL0
    can be filtered out independently ? Should this be something like ...

    "If the CPU supports FEAT_TRF, value of the TRFCR_ELx - indicating whether
    trace is allowed at user [and/or] kernel ELs. Otherwise, 0."

    > * @config: structure holding configuration parameters.
    > * @save_trfcr: Saved TRFCR_EL1 register during a CPU PM event.
    > * @save_state: State to be preserved across power loss
    > @@ -972,7 +973,7 @@ struct etmv4_drvdata {
    > bool nooverflow;
    > bool atbtrig;
    > bool lpoverride;
    > - bool trfc;
    > + u64 trfcr;
    > struct etmv4_config config;
    > u64 save_trfcr;
    > struct etmv4_save_state *save_state;
    > diff --git a/drivers/hwtracing/coresight/coresight-self-hosted-trace.h b/drivers/hwtracing/coresight/coresight-self-hosted-trace.h
    > index 53b35a28075e..586d26e0cba3 100644
    > --- a/drivers/hwtracing/coresight/coresight-self-hosted-trace.h
    > +++ b/drivers/hwtracing/coresight/coresight-self-hosted-trace.h
    > @@ -22,4 +22,11 @@ static inline void write_trfcr(u64 val)
    > isb();
    > }
    >
    > +static inline void cpu_prohibit_trace(void)
    > +{
    > + u64 trfcr = read_trfcr();
    > +
    > + /* Prohibit tracing at EL0 & the kernel EL */
    > + write_trfcr(trfcr & ~(TRFCR_ELx_ExTRE | TRFCR_ELx_E0TRE));
    > +}
    > #endif /* __CORESIGHT_SELF_HOSTED_TRACE_H */
    >

    \
     
     \ /
      Last update: 2021-07-30 05:48    [W:4.101 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site