[RFC] tty/sysrq: Add alternative SysRq key

[RFC] tty/sysrq: Add alternative SysRq key

[Andrzej Pietrasiewicz]

There exist machines which don't have SysRq key at all, e.g. chromebooks.

This patch allows configuring an alternative key to act as SysRq. Devices
which declare KEY_SYSRQ in their 'keybit' bitmap continue using KEY_SYSRQ,
but other devices use the alternative SysRq key instead, by default F10.
Which key is actually used can be modified with sysrq's module parameter.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
I'd like to resurrect an old thread regarding supporting alternative SysRq
key for machines which don't have a physical SysRq key at all.

The old thread:

https://www.spinics.net/lists/linux-input/msg67982.html

I'm resending this patch, rebased onto v5.15.

Any (new) thoughts about it?


 drivers/tty/sysrq.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index c911196ac893..6dd288e53ce9 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -634,6 +634,7 @@ EXPORT_SYMBOL(handle_sysrq);
 
 #ifdef CONFIG_INPUT
 static int sysrq_reset_downtime_ms;
+static unsigned short alternative_sysrq_key = KEY_F10;
 
 /* Simple translation table for the SysRq keys */
 static const unsigned char sysrq_xlate[KEY_CNT] =
@@ -653,6 +654,7 @@ struct sysrq_state {
 	unsigned int alt_use;
 	unsigned int shift;
 	unsigned int shift_use;
+	unsigned short sys
 	bool active;
 	bool need_reinject;
 	bool reinjecting;
@@ -802,10 +804,10 @@ static void sysrq_reinject_alt_sysrq(struct work_struct *work)
 
 		/* Simulate press and release of Alt + SysRq */
 		input_inject_event(handle, EV_KEY, alt_code, 1);
-		input_inject_event(handle, EV_KEY, KEY_SYSRQ, 1);
+		input_inject_event(handle, EV_KEY, sysrq->sysrq_key, 1);
 		input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
 
-		input_inject_event(handle, EV_KEY, KEY_SYSRQ, 0);
+		input_inject_event(handle, EV_KEY, sysrq->sysrq_key, 0);
 		input_inject_event(handle, EV_KEY, alt_code, 0);
 		input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
 
@@ -845,6 +847,7 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
 			sysrq->shift = code;
 		break;
 
+key_sysrq:
 	case KEY_SYSRQ:
 		if (value == 1 && sysrq->alt != KEY_RESERVED) {
 			sysrq->active = true;
@@ -867,11 +870,15 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
 		 * triggering print screen function.
 		 */
 		if (sysrq->active)
-			clear_bit(KEY_SYSRQ, sysrq->handle.dev->key);
+			clear_bit(sysrq->sysrq_key, sysrq->handle.dev->key);
 
 		break;
 
 	default:
+		/* handle non-default sysrq key */
+		if (code == sysrq->sysrq_key)
+			goto key_sysrq;
+
 		if (sysrq->active && value && value != 2) {
 			unsigned char c = sysrq_xlate[code];
 
@@ -970,6 +977,14 @@ static int sysrq_connect(struct input_handler *handler,
 	sysrq->handle.private = sysrq;
 	timer_setup(&sysrq->keyreset_timer, sysrq_do_reset, 0);
 
+	if (test_bit(KEY_SYSRQ, dev->keybit)) {
+		sysrq->sysrq_key = KEY_SYSRQ;
+		pr_info("%s: using default sysrq key [%x]\n", dev->name, KEY_SYSRQ);
+	} else {
+		sysrq->sysrq_key = alternative_sysrq_key;
+		pr_info("%s: Using alternative sysrq key: [%x]\n", dev->name, sysrq->sysrq_key);
+	}
+
 	error = input_register_handle(&sysrq->handle);
 	if (error) {
 		pr_err("Failed to register input sysrq handler, error %d\n",
@@ -1078,6 +1093,13 @@ module_param_array_named(reset_seq, sysrq_reset_seq, sysrq_reset_seq,
 
 module_param_named(sysrq_downtime_ms, sysrq_reset_downtime_ms, int, 0644);
 
+module_param(alternative_sysrq_key, ushort, 0644);
+MODULE_PARM_DESC(alternative_sysrq_key,
+	"Alternative SysRq key for input devices that don't have SysRq key. F10 by default.\n"
+	"Example\n"
+	"Using F9 as SysRq:\n"
+	"sysrq.alternative_sysrq_key=0x43\n");
+
 #else
 
 static inline void sysrq_register_handler(void)
-- 
2.17.1

Re: [RFC] tty/sysrq: Add alternative SysRq key

[Randy Dunlap]

On 11/3/21 8:54 AM, Andrzej Pietrasiewicz wrote:
> There exist machines which don't have SysRq key at all, e.g. chromebooks.
> 
> This patch allows configuring an alternative key to act as SysRq. Devices
> which declare KEY_SYSRQ in their 'keybit' bitmap continue using KEY_SYSRQ,
> but other devices use the alternative SysRq key instead, by default F10.
> Which key is actually used can be modified with sysrq's module parameter.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
> I'd like to resurrect an old thread regarding supporting alternative SysRq
> key for machines which don't have a physical SysRq key at all.
> 
> The old thread:
> 
> https://www.spinics.net/lists/linux-input/msg67982.html
> 
> I'm resending this patch, rebased onto v5.15.
> 
> Any (new) thoughts about it?
> 

Hi,
Did you test it with this patch?


> 
>   drivers/tty/sysrq.c | 28 +++++++++++++++++++++++++---
>   1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index c911196ac893..6dd288e53ce9 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -634,6 +634,7 @@ EXPORT_SYMBOL(handle_sysrq);
>   
>   #ifdef CONFIG_INPUT
>   static int sysrq_reset_downtime_ms;
> +static unsigned short alternative_sysrq_key = KEY_F10;
>   
>   /* Simple translation table for the SysRq keys */
>   static const unsigned char sysrq_xlate[KEY_CNT] =
> @@ -653,6 +654,7 @@ struct sysrq_state {
>   	unsigned int alt_use;
>   	unsigned int shift;
>   	unsigned int shift_use;
> +	unsigned short sys

That line appears to need an ending ';'.
Or maybe that line was truncated since 'sys' isn't used anywhere in this patch.

>   	bool active;
>   	bool need_reinject;
>   	bool reinjecting;
> @@ -802,10 +804,10 @@ static void sysrq_reinject_alt_sysrq(struct work_struct *work)
>   
>   		/* Simulate press and release of Alt + SysRq */
>   		input_inject_event(handle, EV_KEY, alt_code, 1);
> -		input_inject_event(handle, EV_KEY, KEY_SYSRQ, 1);
> +		input_inject_event(handle, EV_KEY, sysrq->sysrq_key, 1);
>   		input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
>   
> -		input_inject_event(handle, EV_KEY, KEY_SYSRQ, 0);
> +		input_inject_event(handle, EV_KEY, sysrq->sysrq_key, 0);
>   		input_inject_event(handle, EV_KEY, alt_code, 0);
>   		input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
>   
> @@ -845,6 +847,7 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
>   			sysrq->shift = code;
>   		break;
>   
> +key_sysrq:
>   	case KEY_SYSRQ:
>   		if (value == 1 && sysrq->alt != KEY_RESERVED) {
>   			sysrq->active = true;
> @@ -867,11 +870,15 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
>   		 * triggering print screen function.
>   		 */
>   		if (sysrq->active)
> -			clear_bit(KEY_SYSRQ, sysrq->handle.dev->key);
> +			clear_bit(sysrq->sysrq_key, sysrq->handle.dev->key);
>   
>   		break;
>   
>   	default:
> +		/* handle non-default sysrq key */
> +		if (code == sysrq->sysrq_key)
> +			goto key_sysrq;
> +
>   		if (sysrq->active && value && value != 2) {
>   			unsigned char c = sysrq_xlate[code];
>   
> @@ -970,6 +977,14 @@ static int sysrq_connect(struct input_handler *handler,
>   	sysrq->handle.private = sysrq;
>   	timer_setup(&sysrq->keyreset_timer, sysrq_do_reset, 0);
>   
> +	if (test_bit(KEY_SYSRQ, dev->keybit)) {
> +		sysrq->sysrq_key = KEY_SYSRQ;
> +		pr_info("%s: using default sysrq key [%x]\n", dev->name, KEY_SYSRQ);
> +	} else {
> +		sysrq->sysrq_key = alternative_sysrq_key;
> +		pr_info("%s: Using alternative sysrq key: [%x]\n", dev->name, sysrq->sysrq_key);
> +	}
> +
>   	error = input_register_handle(&sysrq->handle);
>   	if (error) {
>   		pr_err("Failed to register input sysrq handler, error %d\n",
> @@ -1078,6 +1093,13 @@ module_param_array_named(reset_seq, sysrq_reset_seq, sysrq_reset_seq,
>   
>   module_param_named(sysrq_downtime_ms, sysrq_reset_downtime_ms, int, 0644);
>   
> +module_param(alternative_sysrq_key, ushort, 0644);
> +MODULE_PARM_DESC(alternative_sysrq_key,
> +	"Alternative SysRq key for input devices that don't have SysRq key. F10 by default.\n"
> +	"Example\n"
> +	"Using F9 as SysRq:\n"
> +	"sysrq.alternative_sysrq_key=0x43\n");
> +
>   #else
>   
>   static inline void sysrq_register_handler(void)
> 


-- 
~Randy

Re: [RFC] tty/sysrq: Add alternative SysRq key

[Andrzej Pietrasiewicz]

Hi Randy,

W dniu 03.11.2021 o 17:19, Randy Dunlap pisze:
> On 11/3/21 8:54 AM, Andrzej Pietrasiewicz wrote:
>> There exist machines which don't have SysRq key at all, e.g. chromebooks.
>>
>> This patch allows configuring an alternative key to act as SysRq. Devices
>> which declare KEY_SYSRQ in their 'keybit' bitmap continue using KEY_SYSRQ,
>> but other devices use the alternative SysRq key instead, by default F10.
>> Which key is actually used can be modified with sysrq's module parameter.
>>
>> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
>> ---
>> I'd like to resurrect an old thread regarding supporting alternative SysRq
>> key for machines which don't have a physical SysRq key at all.
>>
>> The old thread:
>>
>> https://www.spinics.net/lists/linux-input/msg67982.html
>>
>> I'm resending this patch, rebased onto v5.15.
>>
>> Any (new) thoughts about it?
>>
> 
> Hi,
> Did you test it with this patch?


Thanks... I must have sent an incorrect version (with incomplete
conflict resolution).

Andrzej

> 
> 
>>
>>   drivers/tty/sysrq.c | 28 +++++++++++++++++++++++++---
>>   1 file changed, 25 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
>> index c911196ac893..6dd288e53ce9 100644
>> --- a/drivers/tty/sysrq.c
>> +++ b/drivers/tty/sysrq.c
>> @@ -634,6 +634,7 @@ EXPORT_SYMBOL(handle_sysrq);
>>   #ifdef CONFIG_INPUT
>>   static int sysrq_reset_downtime_ms;
>> +static unsigned short alternative_sysrq_key = KEY_F10;
>>   /* Simple translation table for the SysRq keys */
>>   static const unsigned char sysrq_xlate[KEY_CNT] =
>> @@ -653,6 +654,7 @@ struct sysrq_state {
>>       unsigned int alt_use;
>>       unsigned int shift;
>>       unsigned int shift_use;
>> +    unsigned short sys
> 
> That line appears to need an ending ';'.
> Or maybe that line was truncated since 'sys' isn't used anywhere in this patch.
> 
>>       bool active;
>>       bool need_reinject;
>>       bool reinjecting;
>> @@ -802,10 +804,10 @@ static void sysrq_reinject_alt_sysrq(struct work_struct 
>> *work)
>>           /* Simulate press and release of Alt + SysRq */
>>           input_inject_event(handle, EV_KEY, alt_code, 1);
>> -        input_inject_event(handle, EV_KEY, KEY_SYSRQ, 1);
>> +        input_inject_event(handle, EV_KEY, sysrq->sysrq_key, 1);
>>           input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
>> -        input_inject_event(handle, EV_KEY, KEY_SYSRQ, 0);
>> +        input_inject_event(handle, EV_KEY, sysrq->sysrq_key, 0);
>>           input_inject_event(handle, EV_KEY, alt_code, 0);
>>           input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
>> @@ -845,6 +847,7 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
>>               sysrq->shift = code;
>>           break;
>> +key_sysrq:
>>       case KEY_SYSRQ:
>>           if (value == 1 && sysrq->alt != KEY_RESERVED) {
>>               sysrq->active = true;
>> @@ -867,11 +870,15 @@ static bool sysrq_handle_keypress(struct sysrq_state 
>> *sysrq,
>>            * triggering print screen function.
>>            */
>>           if (sysrq->active)
>> -            clear_bit(KEY_SYSRQ, sysrq->handle.dev->key);
>> +            clear_bit(sysrq->sysrq_key, sysrq->handle.dev->key);
>>           break;
>>       default:
>> +        /* handle non-default sysrq key */
>> +        if (code == sysrq->sysrq_key)
>> +            goto key_sysrq;
>> +
>>           if (sysrq->active && value && value != 2) {
>>               unsigned char c = sysrq_xlate[code];
>> @@ -970,6 +977,14 @@ static int sysrq_connect(struct input_handler *handler,
>>       sysrq->handle.private = sysrq;
>>       timer_setup(&sysrq->keyreset_timer, sysrq_do_reset, 0);
>> +    if (test_bit(KEY_SYSRQ, dev->keybit)) {
>> +        sysrq->sysrq_key = KEY_SYSRQ;
>> +        pr_info("%s: using default sysrq key [%x]\n", dev->name, KEY_SYSRQ);
>> +    } else {
>> +        sysrq->sysrq_key = alternative_sysrq_key;
>> +        pr_info("%s: Using alternative sysrq key: [%x]\n", dev->name, 
>> sysrq->sysrq_key);
>> +    }
>> +
>>       error = input_register_handle(&sysrq->handle);
>>       if (error) {
>>           pr_err("Failed to register input sysrq handler, error %d\n",
>> @@ -1078,6 +1093,13 @@ module_param_array_named(reset_seq, sysrq_reset_seq, 
>> sysrq_reset_seq,
>>   module_param_named(sysrq_downtime_ms, sysrq_reset_downtime_ms, int, 0644);
>> +module_param(alternative_sysrq_key, ushort, 0644);
>> +MODULE_PARM_DESC(alternative_sysrq_key,
>> +    "Alternative SysRq key for input devices that don't have SysRq key. F10 
>> by default.\n"
>> +    "Example\n"
>> +    "Using F9 as SysRq:\n"
>> +    "sysrq.alternative_sysrq_key=0x43\n");
>> +
>>   #else
>>   static inline void sysrq_register_handler(void)
>>
> 
> 

[RFC] tty/sysrq: Add alternative SysRq key

[Andrzej Pietrasiewicz]

There exist machines which don't have SysRq key at all, e.g. chromebooks.

This patch allows configuring an alternative key to act as SysRq. Devices
which declare KEY_SYSRQ in their 'keybit' bitmap continue using KEY_SYSRQ,
but other devices use the alternative SysRq key instead, by default F10.
Which key is actually used can be modified with sysrq's module parameter.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---


A corrected version with completed conflict resolution.



 drivers/tty/sysrq.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index c911196ac893..fb59745b23c9 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -634,6 +634,7 @@ EXPORT_SYMBOL(handle_sysrq);
 
 #ifdef CONFIG_INPUT
 static int sysrq_reset_downtime_ms;
+static unsigned short alternative_sysrq_key = KEY_F10;
 
 /* Simple translation table for the SysRq keys */
 static const unsigned char sysrq_xlate[KEY_CNT] =
@@ -653,6 +654,7 @@ struct sysrq_state {
 	unsigned int alt_use;
 	unsigned int shift;
 	unsigned int shift_use;
+	unsigned short sysrq_key;
 	bool active;
 	bool need_reinject;
 	bool reinjecting;
@@ -802,10 +804,10 @@ static void sysrq_reinject_alt_sysrq(struct work_struct *work)
 
 		/* Simulate press and release of Alt + SysRq */
 		input_inject_event(handle, EV_KEY, alt_code, 1);
-		input_inject_event(handle, EV_KEY, KEY_SYSRQ, 1);
+		input_inject_event(handle, EV_KEY, sysrq->sysrq_key, 1);
 		input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
 
-		input_inject_event(handle, EV_KEY, KEY_SYSRQ, 0);
+		input_inject_event(handle, EV_KEY, sysrq->sysrq_key, 0);
 		input_inject_event(handle, EV_KEY, alt_code, 0);
 		input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
 
@@ -845,6 +847,7 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
 			sysrq->shift = code;
 		break;
 
+key_sysrq:
 	case KEY_SYSRQ:
 		if (value == 1 && sysrq->alt != KEY_RESERVED) {
 			sysrq->active = true;
@@ -867,11 +870,15 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
 		 * triggering print screen function.
 		 */
 		if (sysrq->active)
-			clear_bit(KEY_SYSRQ, sysrq->handle.dev->key);
+			clear_bit(sysrq->sysrq_key, sysrq->handle.dev->key);
 
 		break;
 
 	default:
+		/* handle non-default sysrq key */
+		if (code == sysrq->sysrq_key)
+			goto key_sysrq;
+
 		if (sysrq->active && value && value != 2) {
 			unsigned char c = sysrq_xlate[code];
 
@@ -970,6 +977,14 @@ static int sysrq_connect(struct input_handler *handler,
 	sysrq->handle.private = sysrq;
 	timer_setup(&sysrq->keyreset_timer, sysrq_do_reset, 0);
 
+	if (test_bit(KEY_SYSRQ, dev->keybit)) {
+		sysrq->sysrq_key = KEY_SYSRQ;
+		pr_info("%s: using default sysrq key [%x]\n", dev->name, KEY_SYSRQ);
+	} else {
+		sysrq->sysrq_key = alternative_sysrq_key;
+		pr_info("%s: Using alternative sysrq key: [%x]\n", dev->name, sysrq->sysrq_key);
+	}
+
 	error = input_register_handle(&sysrq->handle);
 	if (error) {
 		pr_err("Failed to register input sysrq handler, error %d\n",
@@ -1078,6 +1093,13 @@ module_param_array_named(reset_seq, sysrq_reset_seq, sysrq_reset_seq,
 
 module_param_named(sysrq_downtime_ms, sysrq_reset_downtime_ms, int, 0644);
 
+module_param(alternative_sysrq_key, ushort, 0644);
+MODULE_PARM_DESC(alternative_sysrq_key,
+	"Alternative SysRq key for input devices that don't have SysRq key. F10 by default.\n"
+	"Example\n"
+	"Using F9 as SysRq:\n"
+	"sysrq.alternative_sysrq_key=0x43\n");
+
 #else
 
 static inline void sysrq_register_handler(void)
-- 
2.17.1

Re: [RFC] tty/sysrq: Add alternative SysRq key

[Greg Kroah-Hartman]

On Thu, Nov 04, 2021 at 10:34:29AM +0100, Andrzej Pietrasiewicz wrote:
> There exist machines which don't have SysRq key at all, e.g. chromebooks.
> 
> This patch allows configuring an alternative key to act as SysRq. Devices
> which declare KEY_SYSRQ in their 'keybit' bitmap continue using KEY_SYSRQ,
> but other devices use the alternative SysRq key instead, by default F10.
> Which key is actually used can be modified with sysrq's module parameter.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
> 
> 
> A corrected version with completed conflict resolution.
> 
> 
> 
>  drivers/tty/sysrq.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index c911196ac893..fb59745b23c9 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -634,6 +634,7 @@ EXPORT_SYMBOL(handle_sysrq);
>  
>  #ifdef CONFIG_INPUT
>  static int sysrq_reset_downtime_ms;
> +static unsigned short alternative_sysrq_key = KEY_F10;
>  
>  /* Simple translation table for the SysRq keys */
>  static const unsigned char sysrq_xlate[KEY_CNT] =
> @@ -653,6 +654,7 @@ struct sysrq_state {
>  	unsigned int alt_use;
>  	unsigned int shift;
>  	unsigned int shift_use;
> +	unsigned short sysrq_key;
>  	bool active;
>  	bool need_reinject;
>  	bool reinjecting;
> @@ -802,10 +804,10 @@ static void sysrq_reinject_alt_sysrq(struct work_struct *work)
>  
>  		/* Simulate press and release of Alt + SysRq */
>  		input_inject_event(handle, EV_KEY, alt_code, 1);
> -		input_inject_event(handle, EV_KEY, KEY_SYSRQ, 1);
> +		input_inject_event(handle, EV_KEY, sysrq->sysrq_key, 1);
>  		input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
>  
> -		input_inject_event(handle, EV_KEY, KEY_SYSRQ, 0);
> +		input_inject_event(handle, EV_KEY, sysrq->sysrq_key, 0);
>  		input_inject_event(handle, EV_KEY, alt_code, 0);
>  		input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
>  
> @@ -845,6 +847,7 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
>  			sysrq->shift = code;
>  		break;
>  
> +key_sysrq:
>  	case KEY_SYSRQ:
>  		if (value == 1 && sysrq->alt != KEY_RESERVED) {
>  			sysrq->active = true;
> @@ -867,11 +870,15 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
>  		 * triggering print screen function.
>  		 */
>  		if (sysrq->active)
> -			clear_bit(KEY_SYSRQ, sysrq->handle.dev->key);
> +			clear_bit(sysrq->sysrq_key, sysrq->handle.dev->key);
>  
>  		break;
>  
>  	default:
> +		/* handle non-default sysrq key */
> +		if (code == sysrq->sysrq_key)
> +			goto key_sysrq;
> +
>  		if (sysrq->active && value && value != 2) {
>  			unsigned char c = sysrq_xlate[code];
>  
> @@ -970,6 +977,14 @@ static int sysrq_connect(struct input_handler *handler,
>  	sysrq->handle.private = sysrq;
>  	timer_setup(&sysrq->keyreset_timer, sysrq_do_reset, 0);
>  
> +	if (test_bit(KEY_SYSRQ, dev->keybit)) {
> +		sysrq->sysrq_key = KEY_SYSRQ;
> +		pr_info("%s: using default sysrq key [%x]\n", dev->name, KEY_SYSRQ);
> +	} else {
> +		sysrq->sysrq_key = alternative_sysrq_key;
> +		pr_info("%s: Using alternative sysrq key: [%x]\n", dev->name, sysrq->sysrq_key);
> +	}
> +
>  	error = input_register_handle(&sysrq->handle);
>  	if (error) {
>  		pr_err("Failed to register input sysrq handler, error %d\n",
> @@ -1078,6 +1093,13 @@ module_param_array_named(reset_seq, sysrq_reset_seq, sysrq_reset_seq,
>  
>  module_param_named(sysrq_downtime_ms, sysrq_reset_downtime_ms, int, 0644);
>  
> +module_param(alternative_sysrq_key, ushort, 0644);
> +MODULE_PARM_DESC(alternative_sysrq_key,
> +	"Alternative SysRq key for input devices that don't have SysRq key. F10 by default.\n"
> +	"Example\n"
> +	"Using F9 as SysRq:\n"
> +	"sysrq.alternative_sysrq_key=0x43\n");
> +
>  #else
>  
>  static inline void sysrq_register_handler(void)
> -- 
> 2.17.1
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/SubmittingPatches for what needs to be done
  here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

Re: [RFC] tty/sysrq: Add alternative SysRq key

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 602 bytes --]

Hi!

> There exist machines which don't have SysRq key at all, e.g. chromebooks.
> 
> This patch allows configuring an alternative key to act as SysRq. Devices
> which declare KEY_SYSRQ in their 'keybit' bitmap continue using KEY_SYSRQ,
> but other devices use the alternative SysRq key instead, by default F10.
> Which key is actually used can be modified with sysrq's module
> parameter.

Is F10 sensible default? Would it make sense to use something like
alt-shift-esc so that this can be enabled by default?

Best regards,
								Pavel
-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [RFC] tty/sysrq: Add alternative SysRq key

[Andrzej Pietrasiewicz]

Hi Pavel,

W dniu 04.11.2021 o 13:01, Pavel Machek pisze:
> Hi!
> 
>> There exist machines which don't have SysRq key at all, e.g. chromebooks.
>>
>> This patch allows configuring an alternative key to act as SysRq. Devices
>> which declare KEY_SYSRQ in their 'keybit' bitmap continue using KEY_SYSRQ,
>> but other devices use the alternative SysRq key instead, by default F10.
>> Which key is actually used can be modified with sysrq's module
>> parameter.
> 
> Is F10 sensible default? Would it make sense to use something like
> alt-shift-esc so that this can be enabled by default?

Why "alt-shift-esc" could be enabled by default? Do you mean to enable it for
all systems regardless of whether they declare or don't declare KEY_SYSRQ
in their 'keybit' bitmap?

Andrzej

Re: [RFC] tty/sysrq: Add alternative SysRq key

[Maciej W. Rozycki]

On Thu, 4 Nov 2021, Andrzej Pietrasiewicz wrote:

> > Is F10 sensible default? Would it make sense to use something like
> > alt-shift-esc so that this can be enabled by default?
> 
> Why "alt-shift-esc" could be enabled by default? Do you mean to enable it for
> all systems regardless of whether they declare or don't declare KEY_SYSRQ
> in their 'keybit' bitmap?

 FWIW from my perspective it'll work better as a replacement rather than 
additional key combination.

 The reason for this is with their more recent laptops Lenovo in their 
infinite wisdom have placed the <PrintScreen> key (which in a traditional 
PS/2-keyboard manner produces <SysRq> when combined with <Alt>) in their 
keyboards between the right <Alt> and <Ctrl> keys.  With thumbs not being 
as accurate as other fingers (and the overall misdesign of the keyboard 
and touchpad interface) you can imagine how often I have inadvertently hit 
<SysRq> combined with a letter key, wreaking havoc to my system (and of 
course I want to keep the key enabled for times when I do need it).

 Also Documentation/admin-guide/sysrq.rst mentions that you can set an 
alternative keycode sequence for KEY_SYSRQ with `setkeycodes <sequence> 
99' already, but I find this pretty limiting as this only works for single 
keypresses rather than combinations of keys, because <sequence> actually 
refers to a single scancode (possibly 0xe0-prefixed).

  Maciej

Re: [RFC] tty/sysrq: Add alternative SysRq key

[Maciej W. Rozycki]

On Thu, 4 Nov 2021, Maciej W. Rozycki wrote:

>  The reason for this is with their more recent laptops Lenovo in their 
> infinite wisdom have placed the <PrintScreen> key (which in a traditional 
> PS/2-keyboard manner produces <SysRq> when combined with <Alt>) in their 
> keyboards between the right <Alt> and <Ctrl> keys.  With thumbs not being 
> as accurate as other fingers (and the overall misdesign of the keyboard 
> and touchpad interface) you can imagine how often I have inadvertently hit 
> <SysRq> combined with a letter key, wreaking havoc to my system (and of 
> course I want to keep the key enabled for times when I do need it).

 On second thoughts this can be disabled with `setkeycodes 54 0' once we 
do have an alternative combination available.

  Maciej

Re: [RFC] tty/sysrq: Add alternative SysRq key

[Andrzej Pietrasiewicz]

Hi Maciej,

W dniu 04.11.2021 o 14:13, Maciej W. Rozycki pisze:
> On Thu, 4 Nov 2021, Maciej W. Rozycki wrote:
> 
>>   The reason for this is with their more recent laptops Lenovo in their
>> infinite wisdom have placed the <PrintScreen> key (which in a traditional
>> PS/2-keyboard manner produces <SysRq> when combined with <Alt>) in their
>> keyboards between the right <Alt> and <Ctrl> keys.  With thumbs not being
>> as accurate as other fingers (and the overall misdesign of the keyboard
>> and touchpad interface) you can imagine how often I have inadvertently hit
>> <SysRq> combined with a letter key, wreaking havoc to my system (and of
>> course I want to keep the key enabled for times when I do need it).
> 
>   On second thoughts this can be disabled with `setkeycodes 54 0' once we
> do have an alternative combination available.
> 

Doesn't `setkeycodes` affect only one keyboard? What if there are more
keyboards connected to a machine?

 From drivers/tty/vt/keyboard.c:

/*
  * Translation of scancodes to keycodes. We set them on only the first
  * keyboard in the list that accepts the scancode and keycode.
  * Explanation for not choosing the first attached keyboard anymore:
  *  USB keyboards for example have two event devices: one for all "normal"
  *  keys and one for extra function keys (like "volume up", "make coffee",
  *  etc.). So this means that scancodes for the extra function keys won't
  *  be valid for the first event device, but will be for the second.
  */

Regards,

Andrzej

Re: [RFC] tty/sysrq: Add alternative SysRq key

[Andrzej Pietrasiewicz]

Hi,

W dniu 04.11.2021 o 15:17, Andrzej Pietrasiewicz pisze:
> Hi Maciej,
> 
> W dniu 04.11.2021 o 14:13, Maciej W. Rozycki pisze:
>> On Thu, 4 Nov 2021, Maciej W. Rozycki wrote:
>>
>>>   The reason for this is with their more recent laptops Lenovo in their
>>> infinite wisdom have placed the <PrintScreen> key (which in a traditional
>>> PS/2-keyboard manner produces <SysRq> when combined with <Alt>) in their
>>> keyboards between the right <Alt> and <Ctrl> keys.  With thumbs not being
>>> as accurate as other fingers (and the overall misdesign of the keyboard
>>> and touchpad interface) you can imagine how often I have inadvertently hit
>>> <SysRq> combined with a letter key, wreaking havoc to my system (and of
>>> course I want to keep the key enabled for times when I do need it).
>>
>>   On second thoughts this can be disabled with `setkeycodes 54 0' once we
>> do have an alternative combination available.
>>
> 
> Doesn't `setkeycodes` affect only one keyboard? What if there are more
> keyboards connected to a machine?
> 
>  From drivers/tty/vt/keyboard.c:
> 
> /*
>   * Translation of scancodes to keycodes. We set them on only the first
>   * keyboard in the list that accepts the scancode and keycode.
>   * Explanation for not choosing the first attached keyboard anymore:
>   *  USB keyboards for example have two event devices: one for all "normal"
>   *  keys and one for extra function keys (like "volume up", "make coffee",
>   *  etc.). So this means that scancodes for the extra function keys won't
>   *  be valid for the first event device, but will be for the second.
>   */
> 

My second thoughts: if we run `setkeycodes` to map, say, F10 as SysRq,
don't we lose F10?

Andrzej

Re: [RFC] tty/sysrq: Add alternative SysRq key

[Greg Kroah-Hartman]

On Fri, Nov 05, 2021 at 02:01:23PM +0100, Andrzej Pietrasiewicz wrote:
> Hi,
> 
> W dniu 04.11.2021 o 15:17, Andrzej Pietrasiewicz pisze:
> > Hi Maciej,
> > 
> > W dniu 04.11.2021 o 14:13, Maciej W. Rozycki pisze:
> > > On Thu, 4 Nov 2021, Maciej W. Rozycki wrote:
> > > 
> > > >   The reason for this is with their more recent laptops Lenovo in their
> > > > infinite wisdom have placed the <PrintScreen> key (which in a traditional
> > > > PS/2-keyboard manner produces <SysRq> when combined with <Alt>) in their
> > > > keyboards between the right <Alt> and <Ctrl> keys.  With thumbs not being
> > > > as accurate as other fingers (and the overall misdesign of the keyboard
> > > > and touchpad interface) you can imagine how often I have inadvertently hit
> > > > <SysRq> combined with a letter key, wreaking havoc to my system (and of
> > > > course I want to keep the key enabled for times when I do need it).
> > > 
> > >   On second thoughts this can be disabled with `setkeycodes 54 0' once we
> > > do have an alternative combination available.
> > > 
> > 
> > Doesn't `setkeycodes` affect only one keyboard? What if there are more
> > keyboards connected to a machine?
> > 
> >  From drivers/tty/vt/keyboard.c:
> > 
> > /*
> >   * Translation of scancodes to keycodes. We set them on only the first
> >   * keyboard in the list that accepts the scancode and keycode.
> >   * Explanation for not choosing the first attached keyboard anymore:
> >   *  USB keyboards for example have two event devices: one for all "normal"
> >   *  keys and one for extra function keys (like "volume up", "make coffee",
> >   *  etc.). So this means that scancodes for the extra function keys won't
> >   *  be valid for the first event device, but will be for the second.
> >   */
> > 
> 
> My second thoughts: if we run `setkeycodes` to map, say, F10 as SysRq,
> don't we lose F10?

The fact that this patch adds a "new" sysrq key no matter what is a
non-starter, please think through the consequences of such a change...

So no, as-is, this change is not acceptable at all, and I would be
amazed if anyone would ship such a thing.

thanks,

greg k-h

Re: [RFC] tty/sysrq: Add alternative SysRq key

[Andrzej Pietrasiewicz]

Hi Greg,

W dniu 05.11.2021 o 14:27, Greg Kroah-Hartman pisze:
> On Fri, Nov 05, 2021 at 02:01:23PM +0100, Andrzej Pietrasiewicz wrote:
>> Hi,
>>
>> W dniu 04.11.2021 o 15:17, Andrzej Pietrasiewicz pisze:
>>> Hi Maciej,
>>>
>>> W dniu 04.11.2021 o 14:13, Maciej W. Rozycki pisze:
>>>> On Thu, 4 Nov 2021, Maciej W. Rozycki wrote:
>>>>
>>>>>    The reason for this is with their more recent laptops Lenovo in their
>>>>> infinite wisdom have placed the <PrintScreen> key (which in a traditional
>>>>> PS/2-keyboard manner produces <SysRq> when combined with <Alt>) in their
>>>>> keyboards between the right <Alt> and <Ctrl> keys.  With thumbs not being
>>>>> as accurate as other fingers (and the overall misdesign of the keyboard
>>>>> and touchpad interface) you can imagine how often I have inadvertently hit
>>>>> <SysRq> combined with a letter key, wreaking havoc to my system (and of
>>>>> course I want to keep the key enabled for times when I do need it).
>>>>
>>>>    On second thoughts this can be disabled with `setkeycodes 54 0' once we
>>>> do have an alternative combination available.
>>>>
>>>
>>> Doesn't `setkeycodes` affect only one keyboard? What if there are more
>>> keyboards connected to a machine?
>>>
>>>   From drivers/tty/vt/keyboard.c:
>>>
>>> /*
>>>    * Translation of scancodes to keycodes. We set them on only the first
>>>    * keyboard in the list that accepts the scancode and keycode.
>>>    * Explanation for not choosing the first attached keyboard anymore:
>>>    *  USB keyboards for example have two event devices: one for all "normal"
>>>    *  keys and one for extra function keys (like "volume up", "make coffee",
>>>    *  etc.). So this means that scancodes for the extra function keys won't
>>>    *  be valid for the first event device, but will be for the second.
>>>    */
>>>
>>
>> My second thoughts: if we run `setkeycodes` to map, say, F10 as SysRq,
>> don't we lose F10?
> 
> The fact that this patch adds a "new" sysrq key no matter what is a
> non-starter, please think through the consequences of such a change...
> 

I wouldn't say this RFC adds a "new" sysrq no matter what. It does so only
when the input device (keyboard) does _not_ have SysRq key at all. So I would
say that this patch adds a replacement SysRq key if the SysRq key proper is
_physically_ absent. Which seems not such a bad thing to me. The problem I'm
trying to solve is exactly this: what to use as SysRq if there's no SysRq?

Andrzej

Re: [RFC] tty/sysrq: Add alternative SysRq key

[Andrzej Pietrasiewicz]

Jiri, Greg,


W dniu 05.11.2021 o 15:06, Andrzej Pietrasiewicz pisze:
> Hi Greg,
> 
> W dniu 05.11.2021 o 14:27, Greg Kroah-Hartman pisze:
>> On Fri, Nov 05, 2021 at 02:01:23PM +0100, Andrzej Pietrasiewicz wrote:
>>> Hi,
>>>
>>> W dniu 04.11.2021 o 15:17, Andrzej Pietrasiewicz pisze:
>>>> Hi Maciej,
>>>>
>>>> W dniu 04.11.2021 o 14:13, Maciej W. Rozycki pisze:
>>>>> On Thu, 4 Nov 2021, Maciej W. Rozycki wrote:
>>>>>
>>>>>>    The reason for this is with their more recent laptops Lenovo in their
>>>>>> infinite wisdom have placed the <PrintScreen> key (which in a traditional
>>>>>> PS/2-keyboard manner produces <SysRq> when combined with <Alt>) in their
>>>>>> keyboards between the right <Alt> and <Ctrl> keys.  With thumbs not being
>>>>>> as accurate as other fingers (and the overall misdesign of the keyboard
>>>>>> and touchpad interface) you can imagine how often I have inadvertently hit
>>>>>> <SysRq> combined with a letter key, wreaking havoc to my system (and of
>>>>>> course I want to keep the key enabled for times when I do need it).
>>>>>
>>>>>    On second thoughts this can be disabled with `setkeycodes 54 0' once we
>>>>> do have an alternative combination available.
>>>>>
>>>>
>>>> Doesn't `setkeycodes` affect only one keyboard? What if there are more
>>>> keyboards connected to a machine?
>>>>
>>>>   From drivers/tty/vt/keyboard.c:
>>>>
>>>> /*
>>>>    * Translation of scancodes to keycodes. We set them on only the first
>>>>    * keyboard in the list that accepts the scancode and keycode.
>>>>    * Explanation for not choosing the first attached keyboard anymore:
>>>>    *  USB keyboards for example have two event devices: one for all "normal"
>>>>    *  keys and one for extra function keys (like "volume up", "make coffee",
>>>>    *  etc.). So this means that scancodes for the extra function keys won't
>>>>    *  be valid for the first event device, but will be for the second.
>>>>    */
>>>>
>>>
>>> My second thoughts: if we run `setkeycodes` to map, say, F10 as SysRq,
>>> don't we lose F10?
>>
>> The fact that this patch adds a "new" sysrq key no matter what is a
>> non-starter, please think through the consequences of such a change...
>>
> 
> I wouldn't say this RFC adds a "new" sysrq no matter what. It does so only
> when the input device (keyboard) does _not_ have SysRq key at all. So I would
> say that this patch adds a replacement SysRq key if the SysRq key proper is
> _physically_ absent. Which seems not such a bad thing to me. The problem I'm
> trying to solve is exactly this: what to use as SysRq if there's no SysRq?
> 

What approach is acceptable then? Any criteria other than "go guess"?
Is "connect an external keyboard" the _only_ choice Linux wants to offer
to its users in case of devices such as e.g. Chromebooks?

Regards,

Andrzej

Re: [RFC] tty/sysrq: Add alternative SysRq key

[Greg Kroah-Hartman]

On Mon, Nov 15, 2021 at 03:48:59PM +0100, Andrzej Pietrasiewicz wrote:
> Jiri, Greg,
> 
> 
> W dniu 05.11.2021 o 15:06, Andrzej Pietrasiewicz pisze:
> > Hi Greg,
> > 
> > W dniu 05.11.2021 o 14:27, Greg Kroah-Hartman pisze:
> > > On Fri, Nov 05, 2021 at 02:01:23PM +0100, Andrzej Pietrasiewicz wrote:
> > > > Hi,
> > > > 
> > > > W dniu 04.11.2021 o 15:17, Andrzej Pietrasiewicz pisze:
> > > > > Hi Maciej,
> > > > > 
> > > > > W dniu 04.11.2021 o 14:13, Maciej W. Rozycki pisze:
> > > > > > On Thu, 4 Nov 2021, Maciej W. Rozycki wrote:
> > > > > > 
> > > > > > >    The reason for this is with their more recent laptops Lenovo in their
> > > > > > > infinite wisdom have placed the <PrintScreen> key (which in a traditional
> > > > > > > PS/2-keyboard manner produces <SysRq> when combined with <Alt>) in their
> > > > > > > keyboards between the right <Alt> and <Ctrl> keys.  With thumbs not being
> > > > > > > as accurate as other fingers (and the overall misdesign of the keyboard
> > > > > > > and touchpad interface) you can imagine how often I have inadvertently hit
> > > > > > > <SysRq> combined with a letter key, wreaking havoc to my system (and of
> > > > > > > course I want to keep the key enabled for times when I do need it).
> > > > > > 
> > > > > >    On second thoughts this can be disabled with `setkeycodes 54 0' once we
> > > > > > do have an alternative combination available.
> > > > > > 
> > > > > 
> > > > > Doesn't `setkeycodes` affect only one keyboard? What if there are more
> > > > > keyboards connected to a machine?
> > > > > 
> > > > >   From drivers/tty/vt/keyboard.c:
> > > > > 
> > > > > /*
> > > > >    * Translation of scancodes to keycodes. We set them on only the first
> > > > >    * keyboard in the list that accepts the scancode and keycode.
> > > > >    * Explanation for not choosing the first attached keyboard anymore:
> > > > >    *  USB keyboards for example have two event devices: one for all "normal"
> > > > >    *  keys and one for extra function keys (like "volume up", "make coffee",
> > > > >    *  etc.). So this means that scancodes for the extra function keys won't
> > > > >    *  be valid for the first event device, but will be for the second.
> > > > >    */
> > > > > 
> > > > 
> > > > My second thoughts: if we run `setkeycodes` to map, say, F10 as SysRq,
> > > > don't we lose F10?
> > > 
> > > The fact that this patch adds a "new" sysrq key no matter what is a
> > > non-starter, please think through the consequences of such a change...
> > > 
> > 
> > I wouldn't say this RFC adds a "new" sysrq no matter what. It does so only
> > when the input device (keyboard) does _not_ have SysRq key at all. So I would
> > say that this patch adds a replacement SysRq key if the SysRq key proper is
> > _physically_ absent. Which seems not such a bad thing to me. The problem I'm
> > trying to solve is exactly this: what to use as SysRq if there's no SysRq?
> > 
> 
> What approach is acceptable then? Any criteria other than "go guess"?
> Is "connect an external keyboard" the _only_ choice Linux wants to offer
> to its users in case of devices such as e.g. Chromebooks?

I don't see how this patch only kicks in if a keyboard does not have a
sysrq key present given that you are allowing a different key to be used
as it also.

Anyway, it's an RFC, which can't be applied to the tree so I'll wait to
review it "for real" when you feel comfortable enough to submit it for
inclusion.

thanks,

greg k-h