* [PATCH 0/1] arm64: defconfig: Add Coresight as module @ 2022-09-21 14:05 James Clark 2022-09-21 14:05 ` [PATCH 1/1] " James Clark 2022-09-21 15:08 ` [PATCH 0/1] " Catalin Marinas 0 siblings, 2 replies; 10+ messages in thread From: James Clark @ 2022-09-21 14:05 UTC (permalink / raw) To: coresight, catalin.marinas, linux-arm-kernel Cc: suzuki.poulose, mathieu.poirier, mike.leach, leo.yan, broonie, linux-kernel, James Clark, Will Deacon As suggested by Catalin here's the change to add Coresight to defconfig. Unfortunately I don't think we should add CONFIG_CORESIGHT_SOURCE_ETM4X which builds a few files until [1] is merged because of the overhead of CONFIG_PID_IN_CONTEXTIDR. [1]: https://lore.kernel.org/lkml/20211021134530.206216-1-leo.yan@linaro.org/T/ James Clark (1): arm64: defconfig: Add Coresight as module arch/arm64/configs/defconfig | 9 +++++++++ 1 file changed, 9 insertions(+) -- 2.28.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/1] arm64: defconfig: Add Coresight as module 2022-09-21 14:05 [PATCH 0/1] arm64: defconfig: Add Coresight as module James Clark @ 2022-09-21 14:05 ` James Clark 2022-09-21 15:26 ` Mark Brown 2022-09-21 15:08 ` [PATCH 0/1] " Catalin Marinas 1 sibling, 1 reply; 10+ messages in thread From: James Clark @ 2022-09-21 14:05 UTC (permalink / raw) To: coresight, catalin.marinas, linux-arm-kernel Cc: suzuki.poulose, mathieu.poirier, mike.leach, leo.yan, broonie, linux-kernel, James Clark, Will Deacon Add Coresight to defconfig so that build errors are caught. CONFIG_CORESIGHT_SOURCE_ETM4X is excluded because it depends on CONFIG_PID_IN_CONTEXTIDR which has a performance cost. Signed-off-by: James Clark <james.clark@arm.com> --- arch/arm64/configs/defconfig | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index ef3467092ded..c183914ab999 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -1340,4 +1340,13 @@ CONFIG_DEBUG_FS=y # CONFIG_SCHED_DEBUG is not set # CONFIG_DEBUG_PREEMPT is not set # CONFIG_FTRACE is not set +CONFIG_CORESIGHT=m +CONFIG_CORESIGHT_LINK_AND_SINK_TMC=m +CONFIG_CORESIGHT_CATU=m +CONFIG_CORESIGHT_SINK_TPIU=m +CONFIG_CORESIGHT_SINK_ETBV10=m +CONFIG_CORESIGHT_STM=m +CONFIG_CORESIGHT_CPU_DEBUG=m +CONFIG_CORESIGHT_CTI=m +CONFIG_CORESIGHT_CTI_INTEGRATION_REGS=y CONFIG_MEMTEST=y -- 2.28.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] arm64: defconfig: Add Coresight as module 2022-09-21 14:05 ` [PATCH 1/1] " James Clark @ 2022-09-21 15:26 ` Mark Brown 2022-09-21 16:46 ` Mathieu Poirier 0 siblings, 1 reply; 10+ messages in thread From: Mark Brown @ 2022-09-21 15:26 UTC (permalink / raw) To: James Clark Cc: coresight, catalin.marinas, linux-arm-kernel, suzuki.poulose, mathieu.poirier, mike.leach, leo.yan, linux-kernel, Will Deacon [-- Attachment #1.1: Type: text/plain, Size: 269 bytes --] On Wed, Sep 21, 2022 at 03:05:35PM +0100, James Clark wrote: > +CONFIG_CORESIGHT_CTI=m > +CONFIG_CORESIGHT_CTI_INTEGRATION_REGS=y Do we want this turned on by default? According to the description it's a bit dangerous and it's exposed via sysfs rather than debugfs. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] arm64: defconfig: Add Coresight as module 2022-09-21 15:26 ` Mark Brown @ 2022-09-21 16:46 ` Mathieu Poirier 2022-09-22 9:04 ` James Clark 0 siblings, 1 reply; 10+ messages in thread From: Mathieu Poirier @ 2022-09-21 16:46 UTC (permalink / raw) To: Mark Brown Cc: James Clark, coresight, catalin.marinas, linux-arm-kernel, suzuki.poulose, mike.leach, leo.yan, linux-kernel, Will Deacon On Wed, Sep 21, 2022 at 04:26:59PM +0100, Mark Brown wrote: > On Wed, Sep 21, 2022 at 03:05:35PM +0100, James Clark wrote: > > > +CONFIG_CORESIGHT_CTI=m > > +CONFIG_CORESIGHT_CTI_INTEGRATION_REGS=y > I agree - integration registers should not be enabled by default. > Do we want this turned on by default? According to the > description it's a bit dangerous and it's exposed via sysfs > rather than debugfs. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] arm64: defconfig: Add Coresight as module 2022-09-21 16:46 ` Mathieu Poirier @ 2022-09-22 9:04 ` James Clark 2022-09-22 9:26 ` Suzuki K Poulose 0 siblings, 1 reply; 10+ messages in thread From: James Clark @ 2022-09-22 9:04 UTC (permalink / raw) To: Mathieu Poirier, Mark Brown Cc: coresight, catalin.marinas, linux-arm-kernel, suzuki.poulose, mike.leach, leo.yan, linux-kernel, Will Deacon On 21/09/2022 17:46, Mathieu Poirier wrote: > On Wed, Sep 21, 2022 at 04:26:59PM +0100, Mark Brown wrote: >> On Wed, Sep 21, 2022 at 03:05:35PM +0100, James Clark wrote: >> >>> +CONFIG_CORESIGHT_CTI=m >>> +CONFIG_CORESIGHT_CTI_INTEGRATION_REGS=y >> > > I agree - integration registers should not be enabled by default. > >> Do we want this turned on by default? According to the >> description it's a bit dangerous and it's exposed via sysfs >> rather than debugfs. > > Should I disable just CONFIG_CORESIGHT_CTI_INTEGRATION_REGS or CONFIG_CORESIGHT_CTI as well? There are other writable registers exposed via sysfs outside of these two options, so I wanted to check if it's just the integration registers that are the issue. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] arm64: defconfig: Add Coresight as module 2022-09-22 9:04 ` James Clark @ 2022-09-22 9:26 ` Suzuki K Poulose 0 siblings, 0 replies; 10+ messages in thread From: Suzuki K Poulose @ 2022-09-22 9:26 UTC (permalink / raw) To: James Clark, Mathieu Poirier, Mark Brown Cc: coresight, catalin.marinas, linux-arm-kernel, mike.leach, leo.yan, linux-kernel, Will Deacon On 22/09/2022 10:04, James Clark wrote: > > > On 21/09/2022 17:46, Mathieu Poirier wrote: >> On Wed, Sep 21, 2022 at 04:26:59PM +0100, Mark Brown wrote: >>> On Wed, Sep 21, 2022 at 03:05:35PM +0100, James Clark wrote: >>> >>>> +CONFIG_CORESIGHT_CTI=m >>>> +CONFIG_CORESIGHT_CTI_INTEGRATION_REGS=y >>> >> >> I agree - integration registers should not be enabled by default. >> >>> Do we want this turned on by default? According to the >>> description it's a bit dangerous and it's exposed via sysfs >>> rather than debugfs. >> >> > > Should I disable just CONFIG_CORESIGHT_CTI_INTEGRATION_REGS or > CONFIG_CORESIGHT_CTI as well? There are other writable registers exposed > via sysfs outside of these two options, so I wanted to check if it's > just the integration registers that are the issue. It is good/fine to keep CORESIGHT_CTI. But you may remove the INTEGRATION_REGS. They are there for "verification" of the CTI integration on the SoC. We added them only for the platform bring up purposes. Suzuki _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/1] arm64: defconfig: Add Coresight as module 2022-09-21 14:05 [PATCH 0/1] arm64: defconfig: Add Coresight as module James Clark 2022-09-21 14:05 ` [PATCH 1/1] " James Clark @ 2022-09-21 15:08 ` Catalin Marinas 2022-09-22 9:34 ` James Clark 1 sibling, 1 reply; 10+ messages in thread From: Catalin Marinas @ 2022-09-21 15:08 UTC (permalink / raw) To: James Clark Cc: coresight, linux-arm-kernel, suzuki.poulose, mathieu.poirier, mike.leach, leo.yan, broonie, linux-kernel, Will Deacon, Mark Rutland On Wed, Sep 21, 2022 at 03:05:34PM +0100, James Clark wrote: > As suggested by Catalin here's the change to add Coresight to defconfig. > > Unfortunately I don't think we should add CONFIG_CORESIGHT_SOURCE_ETM4X > which builds a few files until [1] is merged because of the overhead > of CONFIG_PID_IN_CONTEXTIDR. > > [1]: https://lore.kernel.org/lkml/20211021134530.206216-1-leo.yan@linaro.org/T/ I thought the overhead wasn't the problem, it's mostly negligible. We can probably save a few more cycles on the __switch_to() path by replacing several isb()s in those functions with a single one just before cpu_switch_to(). IIRC the issue is that unless a process runs in the root pid namespace, the actual pid written to contextidr is meaningless. Now that you reminded me of that thread, I see three options (sorry, not entirely related to the defconfig updates): 1. Remove CONFIG_PID_IN_CONTEXTIDR and corresponding code completely, find other events to correlate the task with the trace. 2. Always on CONFIG_PID_IN_CONTEXTIDR (we might as well remove the Kconfig entry). This would write the root pid namespace value (task_pid_nr()). 3. Similar to (2) but instead write task_pid_nr_ns(). An alternative here is to write -1 if the task is not in the root pid namespace. Strong preference for (1). -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/1] arm64: defconfig: Add Coresight as module 2022-09-21 15:08 ` [PATCH 0/1] " Catalin Marinas @ 2022-09-22 9:34 ` James Clark 2022-09-22 10:52 ` Catalin Marinas 0 siblings, 1 reply; 10+ messages in thread From: James Clark @ 2022-09-22 9:34 UTC (permalink / raw) To: Catalin Marinas Cc: coresight, linux-arm-kernel, suzuki.poulose, mathieu.poirier, mike.leach, leo.yan, broonie, linux-kernel, Will Deacon, Mark Rutland On 21/09/2022 16:08, Catalin Marinas wrote: > On Wed, Sep 21, 2022 at 03:05:34PM +0100, James Clark wrote: >> As suggested by Catalin here's the change to add Coresight to defconfig. >> >> Unfortunately I don't think we should add CONFIG_CORESIGHT_SOURCE_ETM4X >> which builds a few files until [1] is merged because of the overhead >> of CONFIG_PID_IN_CONTEXTIDR. >> >> [1]: https://lore.kernel.org/lkml/20211021134530.206216-1-leo.yan@linaro.org/T/ > > I thought the overhead wasn't the problem, it's mostly negligible. We > can probably save a few more cycles on the __switch_to() path by > replacing several isb()s in those functions with a single one just > before cpu_switch_to(). > > IIRC the issue is that unless a process runs in the root pid namespace, > the actual pid written to contextidr is meaningless. This is true, and Leo has recently disabled it in that scenario in aab473867fed. > > Now that you reminded me of that thread, I see three options (sorry, not > entirely related to the defconfig updates): > > 1. Remove CONFIG_PID_IN_CONTEXTIDR and corresponding code completely, > find other events to correlate the task with the trace. Unfortunately when tracing per core we would need kernel timestamps in the trace to correlate to the switch records. At the moment Coresight is using a different clock source so it's not possible and we're still using the context ID to correlate samples. With FEAT_TRF in v8.4 it will be possible to do this and we've started working towards that here: 0f00b223ea22. But we'd still have to support older hardware too, so CONFIG_PID_IN_CONTEXTIDR can't be removed completely. For SPE it's not required because we already have the right timestamps in the samples and we've added support for no context IDs in the decoder here: 27d113cfe892 > > 2. Always on CONFIG_PID_IN_CONTEXTIDR (we might as well remove the > Kconfig entry). This would write the root pid namespace value > (task_pid_nr()). If we're not worried about the overhead after all, this would be the easiest solution. And then SPE or Coresight already decide whether they want to use the value or not, so no further changes are needed. From Leo's patch there is a table that shows a 1% overhead with it enabled permanently, and I've heard a figure like that mentioned before. So I could also resurrect that patch to use static keys? Although it's a bit more complicated, that would be my preference. And then we can have that mode always on. > > 3. Similar to (2) but instead write task_pid_nr_ns(). An alternative > here is to write -1 if the task is not in the root pid namespace. > > Strong preference for (1). > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/1] arm64: defconfig: Add Coresight as module 2022-09-22 9:34 ` James Clark @ 2022-09-22 10:52 ` Catalin Marinas 2022-09-22 13:06 ` James Clark 0 siblings, 1 reply; 10+ messages in thread From: Catalin Marinas @ 2022-09-22 10:52 UTC (permalink / raw) To: James Clark Cc: coresight, linux-arm-kernel, suzuki.poulose, mathieu.poirier, mike.leach, leo.yan, broonie, linux-kernel, Will Deacon, Mark Rutland On Thu, Sep 22, 2022 at 10:34:45AM +0100, James Clark wrote: > On 21/09/2022 16:08, Catalin Marinas wrote: > > 2. Always on CONFIG_PID_IN_CONTEXTIDR (we might as well remove the > > Kconfig entry). This would write the root pid namespace value > > (task_pid_nr()). > > If we're not worried about the overhead after all, this would be the > easiest solution. And then SPE or Coresight already decide whether they > want to use the value or not, so no further changes are needed. > > From Leo's patch there is a table that shows a 1% overhead with it > enabled permanently, and I've heard a figure like that mentioned before. > So I could also resurrect that patch to use static keys? Although it's a > bit more complicated, that would be my preference. And then we can have > that mode always on. I don't think we should bother with static keys, just always enable it but try to reduce/group the ISBs from all the functions called on the __switch_to() path. We may actually get a speed-up. -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/1] arm64: defconfig: Add Coresight as module 2022-09-22 10:52 ` Catalin Marinas @ 2022-09-22 13:06 ` James Clark 0 siblings, 0 replies; 10+ messages in thread From: James Clark @ 2022-09-22 13:06 UTC (permalink / raw) To: Catalin Marinas Cc: coresight, linux-arm-kernel, suzuki.poulose, mathieu.poirier, mike.leach, leo.yan, broonie, linux-kernel, Will Deacon, Mark Rutland On 22/09/2022 11:52, Catalin Marinas wrote: > On Thu, Sep 22, 2022 at 10:34:45AM +0100, James Clark wrote: >> On 21/09/2022 16:08, Catalin Marinas wrote: >>> 2. Always on CONFIG_PID_IN_CONTEXTIDR (we might as well remove the >>> Kconfig entry). This would write the root pid namespace value >>> (task_pid_nr()). >> >> If we're not worried about the overhead after all, this would be the >> easiest solution. And then SPE or Coresight already decide whether they >> want to use the value or not, so no further changes are needed. >> >> From Leo's patch there is a table that shows a 1% overhead with it >> enabled permanently, and I've heard a figure like that mentioned before. >> So I could also resurrect that patch to use static keys? Although it's a >> bit more complicated, that would be my preference. And then we can have >> that mode always on. > > I don't think we should bother with static keys, just always enable it > but try to reduce/group the ISBs from all the functions called on the > __switch_to() path. We may actually get a speed-up. > Ok thanks I will take a look at this _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-09-22 13:08 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-09-21 14:05 [PATCH 0/1] arm64: defconfig: Add Coresight as module James Clark 2022-09-21 14:05 ` [PATCH 1/1] " James Clark 2022-09-21 15:26 ` Mark Brown 2022-09-21 16:46 ` Mathieu Poirier 2022-09-22 9:04 ` James Clark 2022-09-22 9:26 ` Suzuki K Poulose 2022-09-21 15:08 ` [PATCH 0/1] " Catalin Marinas 2022-09-22 9:34 ` James Clark 2022-09-22 10:52 ` Catalin Marinas 2022-09-22 13:06 ` James Clark
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).