Messages in this thread | | | From | Mike Leach <> | Date | Tue, 12 Jan 2021 11:23:03 +0000 | Subject | Re: [PATCH v1 1/7] coresight: etm-perf: Add support for PID tracing for kernel at EL2 |
| |
Hi Leo,
On Tue, 12 Jan 2021 at 08:58, Leo Yan <leo.yan@linaro.org> wrote: > > Hi Mike, > > On Mon, Jan 11, 2021 at 04:22:39PM +0000, Mike Leach wrote: > > [...] > > > > diff --git a/include/linux/coresight-pmu.h b/include/linux/coresight-pmu.h > > > index b0e35eec6499..927c6285ce5d 100644 > > > --- a/include/linux/coresight-pmu.h > > > +++ b/include/linux/coresight-pmu.h > > > @@ -11,16 +11,19 @@ > > > #define CORESIGHT_ETM_PMU_SEED 0x10 > > > > > > /* ETMv3.5/PTM's ETMCR config bit */ > > > -#define ETM_OPT_CYCACC 12 > > > -#define ETM_OPT_CTXTID 14 > > > -#define ETM_OPT_TS 28 > > > -#define ETM_OPT_RETSTK 29 > > > +#define ETM_OPT_CYCACC 12 > > > +#define ETM_OPT_CTXTID 14 > > > +#define ETM_OPT_CTXTID_IN_VMID 15 > > > > Minor issue here - ETMv3.x / PTM cannot trace CXTID in VMID so this > > may better be named ETM4_OPT_CTXTID_IN_VMID, rather than be grouped > > with the ETM3.5 options? > > I looked into this suggestion but found it's complex than I assumed. > This config bits are not only used for ETMv3.x / PTM, it's also used > as an configuration interface between user space in Perf and kernel > drivers. > > For example, in the userspace, perf tool sets bit ETM_OPT_TS to enable > timestamp [1], this is same for ETMv3 and ETMv4. In the kernel side, > the configuration is directly used ETMv3 (in coresight-etm3x-core.c), > but the configuration bits are converted for ETMv4 in the function > etm4_parse_event_config() [2]. > > So this is a historical issue, at the early period ETMv3 and ETMv4 can > be compatible with each other for configurations, but after evoluation, > some configs only belong to ETMv4 and cannot be applied on ETMv3 > anymore, but we still use ETMv3.5 config bits as the interface between > kernel and userspace. >
I was aware that etm3/ptm used these bits as both the options and the bit values for direct writing to the ETMCR register for ETMv3, and re-mapped to appropriate register values in ETMv4. In the past we have re-used etmv3.5 bit definitions ETM_xxx when appropriate, but where unique to ETM4 we have used a ETM4_xxx naming convention.
I am not suggesting re-factoring the options completely, just re-naming this single option to make it clear it is unique to ETM4+.
Looking at the etmv3 driver, at present it does not actually appear to support contextid tracing - and when it does, both bits 14 and 15 will be required to be used - as ETMCR defines these bits as ContextID size. Should this ever get fixed. then having an overlapping option bit - that appears to be valid for ETMv3 will be confusing.
Regards
Mike
> I'd like suggest we use a saperate patch set to refactor the > configuration bits, e.g. define the arbitrary configuration bits as > interface between user space and kernel, these bits are neutral for any > ETM version, in the kernel ETM drivers need to convert to its own > configuration formats. For this patch, I want to keep current > change as it is, is this okay for you? > > Thanks, > Leo > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/arch/arm/util/cs-etm.c?h=v5.11-rc3#n152 > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/hwtracing/coresight/coresight-etm4x-core.c?h=v5.11-rc3#n422
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
| |