clocksource: arm_arch_timer: disable the evtstrm via the cmdline
Subsystems (2)
RFC PATCH 1/1 clocksource: arm_arch_timer: disable the evtstrm via the cmdline
Disabling the eventstream can be useful for debugging and development purposes and is currently controlled via a Kconfig option (CONFIG_ARM_ARCH_TIMER_EVTSTREAM). Whilst this does the trick, it's often desirable to toggle the feature on the command line, so this patch adds a "noevtstrm" command line option to do just that.
---
Sending as an RFC because we might want to remove the Kconfig option altogether if we have this.
drivers/clocksource/arm_arch_timer.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
drivers/clocksource/arm_arch_timer.c +10 -1
@@ -79,6 +79,15 @@ static enum ppi_nr arch_timer_uses_ppi = VIRT_PPI;static bool arch_timer_c3stop;static bool arch_timer_mem_use_virtual; +static bool evtstrm_disable; + +static int __init early_evtstrm_disable (char *buf) +{ + evtstrm_disable = true; +return 0 ; +} +early_param ("noevtstrm" , early_evtstrm_disable); +/* * Architected system timer support. */
@@ -372,7 +381,7 @@ static int arch_timer_setup(struct clock_event_device *clk)enable_percpu_irq (arch_timer_ppi[PHYS_NONSECURE_PPI ],0 );arch_counter_set_user_access (); -if (IS_ENABLED (CONFIG_ARM_ARCH_TIMER_EVTSTREAM )) +if (IS_ENABLED (CONFIG_ARM_ARCH_TIMER_EVTSTREAM ) && !evtstrm_disable)arch_timer_configure_evtstream ();return 0 ;
Signature · 1 line
2.1.4
Mark Rutland
maintainerRFC PATCH 1/1 clocksource: arm_arch_timer: disable the evtstrm via the cmdline
Quoted 5 lines · Will Deacon
Disabling the eventstream can be useful for debugging and development purposes and is currently controlled via a
Kconfigoption (CONFIG_ARM_ARCH_TIMER_EVTSTREAM). Whilst this does the trick, it's often desirable to toggle the feature on the command line, so this patch adds a "noevtstrm" command line option to do just that.
I think anything that allows for better debugging on a production kernels is great, so FWIW:
We might want to drop something in Documentation/kernel-parameters.txt
Mark.
Quoted 48 lines
Signed-off-by Will Deacon <[email protected]>---
Sending as an RFC because we might want to remove the
Kconfigoption altogether if we have this.drivers/clocksource/arm_arch_timer.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)drivers/clocksource/arm_arch_timer.c +10
-1@@ -79,6 +79,15 @@ static enum ppi_nr arch_timer_uses_ppi = VIRT_PPI;static bool arch_timer_c3stop;static bool arch_timer_mem_use_virtual; +static bool evtstrm_disable; + +static int __init early_evtstrm_disable (char *buf) +{ + evtstrm_disable = true; +return 0 ; +} +early_param ("noevtstrm" , early_evtstrm_disable); +/* * Architected system timer support. */ @@ -372,7 +381,7 @@ static int arch_timer_setup(struct clock_event_device *clk)enable_percpu_irq (arch_timer_ppi[PHYS_NONSECURE_PPI ],0 );arch_counter_set_user_access (); -if (IS_ENABLED (CONFIG_ARM_ARCH_TIMER_EVTSTREAM )) +if (IS_ENABLED (CONFIG_ARM_ARCH_TIMER_EVTSTREAM ) && !evtstrm_disable)arch_timer_configure_evtstream ();return 0 ;Signature · 7 lines
2.1.4 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
RFC PATCH 1/1 clocksource: arm_arch_timer: disable the evtstrm via the cmdline
Quoted 9 lines · Mark Rutland
Quoted 5 lines · Will Deacon
Disabling the eventstream can be useful for debugging and development purposes and is currently controlled via a
Kconfigoption (CONFIG_ARM_ARCH_TIMER_EVTSTREAM). Whilst this does the trick, it's often desirable to toggle the feature on the command line, so this patch adds a "noevtstrm" command line option to do just that.I think anything that allows for better debugging on a production kernels is great, so FWIW:
Agree, I run linux in arm esl and the linux can't boot up without ARM_ARCH_TIMER_EVTSTREAM recently.
The esl guys is looking into the issue, and it is useful to debug issue with one Image(dynamic switching in cmdline).
BRs,
Kefeng
Quoted 63 lines
Acked-by Mark Rutland <[email protected]>We might want to drop something in
Documentation/kernel-parameters.txtMark.
Quoted 48 lines
Signed-off-by Will Deacon <[email protected]>---
Sending as an RFC because we might want to remove the
Kconfigoption altogether if we have this.drivers/clocksource/arm_arch_timer.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)drivers/clocksource/arm_arch_timer.c +10
-1@@ -79,6 +79,15 @@ static enum ppi_nr arch_timer_uses_ppi = VIRT_PPI;static bool arch_timer_c3stop;static bool arch_timer_mem_use_virtual; +static bool evtstrm_disable; + +static int __init early_evtstrm_disable (char *buf) +{ + evtstrm_disable = true; +return 0 ; +} +early_param ("noevtstrm" , early_evtstrm_disable); +/* * Architected system timer support. */ @@ -372,7 +381,7 @@ static int arch_timer_setup(struct clock_event_device *clk)enable_percpu_irq (arch_timer_ppi[PHYS_NONSECURE_PPI ],0 );arch_counter_set_user_access (); -if (IS_ENABLED (CONFIG_ARM_ARCH_TIMER_EVTSTREAM )) +if (IS_ENABLED (CONFIG_ARM_ARCH_TIMER_EVTSTREAM ) && !evtstrm_disable)arch_timer_configure_evtstream ();return 0 ;Signature · 7 lines
2.1.4 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel.
RFC PATCH 1/1 clocksource: arm_arch_timer: disable the evtstrm via the cmdline
In reply to Will DeaconOn 06/17/2016 03:43 PM, Will Deacon wrote:
[ Cc'ed tglx ]
Quoted 2 lines
Disabling the eventstream can be useful for debugging and development purposes
If it is for debugging and development, why upstream this change ?
Quoted 44 lines
and is currently controlled via a
Kconfigoption (CONFIG_ARM_ARCH_TIMER_EVTSTREAM). Whilst this does the trick, it's often desirable to toggle the feature on the command line, so this patch adds a "noevtstrm" command line option to do just that.Signed-off-by Will Deacon <[email protected]>---
Sending as an RFC because we might want to remove the
Kconfigoption altogether if we have this.drivers/clocksource/arm_arch_timer.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)drivers/clocksource/arm_arch_timer.c +10
-1@@ -79,6 +79,15 @@ static enum ppi_nr arch_timer_uses_ppi = VIRT_PPI;static bool arch_timer_c3stop;static bool arch_timer_mem_use_virtual; +static bool evtstrm_disable; + +static int __init early_evtstrm_disable (char *buf) +{ + evtstrm_disable = true; +return 0 ; +} +early_param ("noevtstrm" , early_evtstrm_disable); +/* * Architected system timer support. */ @@ -372,7 +381,7 @@ static int arch_timer_setup(struct clock_event_device *clk)enable_percpu_irq (arch_timer_ppi[PHYS_NONSECURE_PPI ],0 );arch_counter_set_user_access (); -if (IS_ENABLED (CONFIG_ARM_ARCH_TIMER_EVTSTREAM )) +if (IS_ENABLED (CONFIG_ARM_ARCH_TIMER_EVTSTREAM ) && !evtstrm_disable)arch_timer_configure_evtstream ();return 0 ;
Signature · 5 lines
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
RFC PATCH 1/1 clocksource: arm_arch_timer: disable the evtstrm via the cmdline
Quoted 8 lines · Daniel Lezcano
On 06/17/2016 03:43 PM, Will Deacon wrote:
[ Cc'ed tglx ]
Quoted 2 lines
Disabling the eventstream can be useful for debugging and development purposes
If it is for debugging and development, why upstream this change ?
Mainly because it's desirable to be able to debug systems remotely, on machines that you don't have direct access to and where recompiling the kernel isn't necessarily an option. There are plenty of "no*" kernel parameters already that fall into a similar category.
Will
RFC PATCH 1/1 clocksource: arm_arch_timer: disable the evtstrm via the cmdline
Quoted 8 lines · On Mon, 20 Jun 2016 09:21:57 +0100 Will Deacon <[email protected]> wrote:
Quoted 7 lines · Daniel Lezcano
On 06/17/2016 03:43 PM, Will Deacon wrote:
[ Cc'ed tglx ]
Quoted 2 lines
Disabling the eventstream can be useful for debugging and development purposes
Quoted 6 lines
Quoted 1 line
If it is for debugging and development, why upstream this change ?
Mainly because it's desirable to be able to debug systems remotely, on machines that you don't have direct access to and where recompiling the kernel isn't necessarily an option. There are plenty of "no*" kernel parameters already that fall into a similar category.
I would add that being able to toggle this without affecting the memory layout or any of the code paths is extremely valuable. The event stream can have an effect on the spinlock behaviour (through the WFE instruction), so having a way to exercise it without changing anything else is pretty cool. Also, being able to debug a production kernel at zero cost is rare enough to make me jump on the opportunity.
So for this patch, and Mark's suggestion of adding some documentation to it:
M.
Signature · 1 line
Jazz is not dead. It just smells funny.
RFC PATCH 1/1 clocksource: arm_arch_timer: disable the evtstrm via the cmdline
In reply to Will DeaconQuoted 8 lines · Will Deacon
Quoted 7 lines · Daniel Lezcano
On 06/17/2016 03:43 PM, Will Deacon wrote:
[ Cc'ed tglx ]
Quoted 2 lines
Disabling the eventstream can be useful for debugging and development purposes
Quoted 6 lines
Quoted 1 line
If it is for debugging and development, why upstream this change ?
Mainly because it's desirable to be able to debug systems remotely, on machines that you don't have direct access to and where recompiling the kernel isn't necessarily an option. There are plenty of "no*" kernel parameters already that fall into a similar category.
Hi Will,
if the kernel is in development and debug, why this option can't be part of debugging code ?
If recompiling isn't an option, how this can be for "debugging and development" ?
I'm not a big fan of the all the specific driver options for the kernel parameters. If there is a real need to disable some parts of a driver, it would be much more interesting to write a framework for that and then use it from arm_arch_timer, thus giving the other drivers the opportunity to provide the same feature.
-- Daniel
Signature · 5 lines
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
Mark Rutland
maintainerRFC PATCH 1/1 clocksource: arm_arch_timer: disable the evtstrm via the cmdline
Quoted 16 lines · Daniel Lezcano
Quoted 8 lines · Will Deacon
Quoted 7 lines · Daniel Lezcano
On 06/17/2016 03:43 PM, Will Deacon wrote:
[ Cc'ed tglx ]
Quoted 2 lines
Disabling the eventstream can be useful for debugging and development purposes
Quoted 6 lines
Quoted 1 line
If it is for debugging and development, why upstream this change ?
Mainly because it's desirable to be able to debug systems remotely, on machines that you don't have direct access to and where recompiling the kernel isn't necessarily an option. There are plenty of "no*" kernel parameters already that fall into a similar category.
Quoted 7 lines
Hi Will,
if the kernel is in development and debug, why this option can't be part of debugging code ?
If recompiling isn't an option, how this can be for "debugging and development" ?
We already have the config option for the cases you wish to set this at build time, and people use it.
There are situations where you do not know at build time that you want this, e.g. debugging in the field, for which recompilation may change the code in other ways (e.g. layout of data and functions).
For example, if we get a bug report that a production kernel wedges in spinlock code after running for 10 hours, being able to say "pass noevstrm" is much better than trying to get the end-user to recompile their kernel, which may or may not be possible, and may (for other reasons), mask the issue we wish to debug.
The code size impact is negligible, and there is no runtime performance impact if this option is not used.
Quoted 5 lines
I'm not a big fan of the all the specific driver options for the kernel parameters. If there is a real need to disable some parts of a driver, it would be much more interesting to write a framework for that and then use it from
arm_arch_timer, thus giving the other drivers the opportunity to provide the same feature.
I'm not aware of other devices which have an event stream option.
Additionally, the architected timer is at least slightly special in that it is architected, and this HW features was designed with architectural implications in mind (e.g. the behaviour of locking primitives). So while this happens to live in the driver code, it's in effect an architecture option (note that we have HWCAP_EVTSTREAM).
Thanks,
Mark.
RFC PATCH 1/1 clocksource: arm_arch_timer: disable the evtstrm via the cmdline
In reply to Daniel LezcanoQuoted 16 lines · Daniel Lezcano
Quoted 8 lines · Will Deacon
Quoted 7 lines · Daniel Lezcano
On 06/17/2016 03:43 PM, Will Deacon wrote:
[ Cc'ed tglx ]
Quoted 2 lines
Disabling the eventstream can be useful for debugging and development purposes
Quoted 6 lines
Quoted 1 line
If it is for debugging and development, why upstream this change ?
Mainly because it's desirable to be able to debug systems remotely, on machines that you don't have direct access to and where recompiling the kernel isn't necessarily an option. There are plenty of "no*" kernel parameters already that fall into a similar category.
Quoted 5 lines
if the kernel is in development and debug, why this option can't be part of debugging code ?
If recompiling isn't an option, how this can be for "debugging and development" ?
Sorry, I wasn't very clear. The sort of use-case I'm envisaging is where somebody is running a distro kernel on non-public hardware and sends me an email about spinlock performance. Being able to disable the event stream easily is a powerful tool in the small box of remote debugging tools and would be useful in pinning down things like missing events.
So when I say "development and debug" I'm really thinking about *remote* debug via a user, and then potentially the subsequent development of a patch.
Quoted 5 lines
I'm not a big fan of the all the specific driver options for the kernel parameters. If there is a real need to disable some parts of a driver, it would be much more interesting to write a framework for that and then use it from
arm_arch_timer, thus giving the other drivers the opportunity to provide the same feature.
Such a framework sounds horribly ill-specified and far beyond the scope of the simple change I'm proposing here. Are you planning to submit patches for this?
Will
Mark Rutland
maintainerRFC PATCH 1/1 clocksource: arm_arch_timer: disable the evtstrm via the cmdline
Quoted 29 lines · Will Deacon
Quoted 16 lines · Daniel Lezcano
Quoted 8 lines · Will Deacon
Quoted 7 lines · Daniel Lezcano
On 06/17/2016 03:43 PM, Will Deacon wrote:
[ Cc'ed tglx ]
Quoted 2 lines
Disabling the eventstream can be useful for debugging and development purposes
Quoted 6 lines
Quoted 1 line
If it is for debugging and development, why upstream this change ?
Mainly because it's desirable to be able to debug systems remotely, on machines that you don't have direct access to and where recompiling the kernel isn't necessarily an option. There are plenty of "no*" kernel parameters already that fall into a similar category.
Quoted 5 lines
if the kernel is in development and debug, why this option can't be part of debugging code ?
If recompiling isn't an option, how this can be for "debugging and development" ?
Sorry, I wasn't very clear. The sort of use-case I'm envisaging is where somebody is running a distro kernel on non-public hardware and sends me an email about spinlock performance. Being able to disable the event stream easily is a powerful tool in the small box of remote debugging tools and would be useful in pinning down things like missing events.
Quoted 3 lines
So when I say "development and debug" I'm really thinking about *remote* debug via a user, and then potentially the subsequent development of a patch.
We should probably place this rationale in the commit message. e.g. (fake emphasis):
Disabling the eventstream can be useful for *remotely* debugging *a deployed production system*.
Mark.
RFC PATCH 1/1 clocksource: arm_arch_timer: disable the evtstrm via the cmdline
In reply to Daniel LezcanoQuoted 11 lines · Daniel Lezcano
Quoted 10 lines · Will Deacon
Quoted 8 lines · Daniel Lezcano
On 06/17/2016 03:43 PM, Will Deacon wrote:
[ Cc'ed tglx ]
Quoted 2 lines
Disabling the eventstream can be useful for debugging and development purposes
If it is for debugging and development, why upstream this change ?
Quoted 7 lines
Quoted 4 lines
Mainly because it's desirable to be able to debug systems remotely, on machines that you don't have direct access to and where recompiling the kernel isn't necessarily an option. There are plenty of "no*" kernel parameters already that fall into a similar category.
if the kernel is in development and debug, why this option can't be part of debugging code ?
Because we may actually be debugging the hardware rather than the software. With the event stream enabled, WFE is woken up periodically. This can be a handy feature for user locking primitives or a simple workaround for hardware bugs (and we've seen them before). But the side effect is that it may be potentially hiding hardware issues.
For hardware testing/validation, you'd want to sometimes disable this feature to check whether your event generation (usually as a result of exclusive monitor clearing) is working as expected. It's much easier to do with a command line option.
Quoted 5 lines
I'm not a big fan of the all the specific driver options for the kernel parameters. If there is a real need to disable some parts of a driver, it would be much more interesting to write a framework for that and then use it from
arm_arch_timer, thus giving the other drivers the opportunity to provide the same feature.
Well, how many non-ARM timer drivers have an exclusive monitor event stream feature to make sense for a framework?
Signature · 1 line
Catalin
RFC PATCH 1/1 clocksource: arm_arch_timer: disable the evtstrm via the cmdline
Quoted 31 lines · Catalin Marinas
Quoted 11 lines · Daniel Lezcano
Quoted 10 lines · Will Deacon
Quoted 8 lines · Daniel Lezcano
On 06/17/2016 03:43 PM, Will Deacon wrote:
[ Cc'ed tglx ]
Quoted 2 lines
Disabling the eventstream can be useful for debugging and development purposes
If it is for debugging and development, why upstream this change ?
Quoted 7 lines
Quoted 4 lines
Mainly because it's desirable to be able to debug systems remotely, on machines that you don't have direct access to and where recompiling the kernel isn't necessarily an option. There are plenty of "no*" kernel parameters already that fall into a similar category.
if the kernel is in development and debug, why this option can't be part of debugging code ?
Because we may actually be debugging the hardware rather than the software. With the event stream enabled, WFE is woken up periodically. This can be a handy feature for user locking primitives or a simple workaround for hardware bugs (and we've seen them before). But the side effect is that it may be potentially hiding hardware issues.
For hardware testing/validation, you'd want to sometimes disable this feature to check whether your event generation (usually as a result of exclusive monitor clearing) is working as expected. It's much easier to do with a command line option.
Quoted 8 lines
Quoted 5 lines
I'm not a big fan of the all the specific driver options for the kernel parameters. If there is a real need to disable some parts of a driver, it would be much more interesting to write a framework for that and then use it from
arm_arch_timer, thus giving the other drivers the opportunity to provide the same feature.Well, how many non-ARM timer drivers have an exclusive monitor event stream feature to make sense for a framework?
Ok.
What about an option like:
clocksource.arch_arm.evtstream = 0/1
Signature · 5 lines
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog