[PATCH] powerpc: ipipe: Do full exit checks after __ipipe_call_mayday

[PATCH] powerpc: ipipe: Do full exit checks after __ipipe_call_mayday

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

The mayday event tends to push us into root mode. So we must check
__ipipe_root_p again afterwards.

This fixes the gdb synchronous stop test case in Xenomai.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

Ho, ho, this is an early X-mas gift.

 arch/powerpc/kernel/ipipe.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/ipipe.c b/arch/powerpc/kernel/ipipe.c
index 30c56d922a2c..fefb0542acd8 100644
--- a/arch/powerpc/kernel/ipipe.c
+++ b/arch/powerpc/kernel/ipipe.c
@@ -240,9 +240,7 @@ EXPORT_SYMBOL_GPL(ipipe_get_sysinfo);
 
 static int __ipipe_exit_irq(struct pt_regs *regs)
 {
-	int root = __ipipe_root_p;
-
-	if (root) {
+	if (__ipipe_root_p) {
 #ifdef CONFIG_PPC_970_NAP
 		struct thread_info *ti = current_thread_info();
 		/* Emulate the napping check when 100% sure we do run
@@ -258,7 +256,8 @@ static int __ipipe_exit_irq(struct pt_regs *regs)
 	if (user_mode(regs) && ipipe_test_thread_flag(TIP_MAYDAY))
 		__ipipe_call_mayday(regs);
 
-	if (root && !test_bit(IPIPE_STALL_FLAG, &__ipipe_root_status))
+	if (__ipipe_root_p &&
+	    !test_bit(IPIPE_STALL_FLAG, &__ipipe_root_status))
 		return 1;
 
 	return 0;
-- 
2.16.4


-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] powerpc: ipipe: Do full exit checks after __ipipe_call_mayday

[Jan Kiszka]

On 19.12.19 17:46, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> The mayday event tends to push us into root mode. So we must check
> __ipipe_root_p again afterwards.
> 
> This fixes the gdb synchronous stop test case in Xenomai.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
> Ho, ho, this is an early X-mas gift.
> 
>   arch/powerpc/kernel/ipipe.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/ipipe.c b/arch/powerpc/kernel/ipipe.c
> index 30c56d922a2c..fefb0542acd8 100644
> --- a/arch/powerpc/kernel/ipipe.c
> +++ b/arch/powerpc/kernel/ipipe.c
> @@ -240,9 +240,7 @@ EXPORT_SYMBOL_GPL(ipipe_get_sysinfo);
>   
>   static int __ipipe_exit_irq(struct pt_regs *regs)
>   {
> -	int root = __ipipe_root_p;
> -
> -	if (root) {
> +	if (__ipipe_root_p) {
>   #ifdef CONFIG_PPC_970_NAP
>   		struct thread_info *ti = current_thread_info();
>   		/* Emulate the napping check when 100% sure we do run
> @@ -258,7 +256,8 @@ static int __ipipe_exit_irq(struct pt_regs *regs)
>   	if (user_mode(regs) && ipipe_test_thread_flag(TIP_MAYDAY))
>   		__ipipe_call_mayday(regs);
>   
> -	if (root && !test_bit(IPIPE_STALL_FLAG, &__ipipe_root_status))
> +	if (__ipipe_root_p &&
> +	    !test_bit(IPIPE_STALL_FLAG, &__ipipe_root_status))
>   		return 1;
>   
>   	return 0;
> 

Looking at DoSyscall in entry_32.S, it seems we lack such a careful 
check there as well. But that's too much assembly for me ATM.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] powerpc: ipipe: Do full exit checks after __ipipe_call_mayday

[Steven Seeger]

On Thursday, December 19, 2019 11:53:36 AM EST Jan Kiszka wrote:
> > Ho, ho, this is an early X-mas gift.

Thanks for finding this. I'm a terrible PPC maintainer so if anyone else wants 
to volunteer to take my place.... ;)
 
> Looking at DoSyscall in entry_32.S, it seems we lack such a careful
> check there as well. But that's too much assembly for me ATM.

Thanks for the tip I should be able to look into this tomorrow for you. Can 
you point me to the relevant sections in arm that I should compare to? 

Any more details (such as desired check/operation) you can provide would be 
beneficial.

Steven

Re: [PATCH] powerpc: ipipe: Do full exit checks after __ipipe_call_mayday

[Jan Kiszka]

On 19.12.19 18:01, Steven Seeger wrote:
> On Thursday, December 19, 2019 11:53:36 AM EST Jan Kiszka wrote:
>>> Ho, ho, this is an early X-mas gift.
> 
> Thanks for finding this. I'm a terrible PPC maintainer so if anyone else wants
> to volunteer to take my place.... ;)
>   
>> Looking at DoSyscall in entry_32.S, it seems we lack such a careful
>> check there as well. But that's too much assembly for me ATM.
> 
> Thanks for the tip I should be able to look into this tomorrow for you. Can
> you point me to the relevant sections in arm that I should compare to?
> 
> Any more details (such as desired check/operation) you can provide would be
> beneficial.

Check ipipe-arm/ipipe/master:arch/arm/kernel/entry-common.S for the call 
to ipipe_call_mayday. I suspect that pattern transfers nicely.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] powerpc: ipipe: Do full exit checks after __ipipe_call_mayday

[Steven Seeger]

On Thursday, December 19, 2019 12:14:27 PM EST Jan Kiszka wrote:
> 
> Check ipipe-arm/ipipe/master:arch/arm/kernel/entry-common.S for the call
> to ipipe_call_mayday. I suspect that pattern transfers nicely.
> 
> Jan

Will do. Can you point me to a smokey test that will prove the implemention is 
fixed? Preferably if you have something that shows it's currently broken.

Steven

Re: [PATCH] powerpc: ipipe: Do full exit checks after __ipipe_call_mayday

[Jan Kiszka]

On 19.12.19 18:15, Steven Seeger wrote:
> On Thursday, December 19, 2019 12:14:27 PM EST Jan Kiszka wrote:
>>
>> Check ipipe-arm/ipipe/master:arch/arm/kernel/entry-common.S for the call
>> to ipipe_call_mayday. I suspect that pattern transfers nicely.
>>
>> Jan
> 
> Will do. Can you point me to a smokey test that will prove the implemention is
> fixed? Preferably if you have something that shows it's currently broken.

Hmm, I suppose we have none, otherwise this would have triggered 
earlier. The pattern exposing it may be: Thread A blocks on RT syscall, 
thread B injects Linux signal to thread A, then hands over control. 
Next, thread A should be migrated on return from the syscall, but the 
pending signal delivery should be delayed.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] powerpc: ipipe: Do full exit checks after __ipipe_call_mayday

[Steven Seeger]

Jan,

I took a look at entry-common.S and entry_32.S and I think we have the correct 
check. The flow is a little different, but it seems to work as far as I can 
tell.

This was originally Philippe's code. Maybe he can take a quick look. ;)

Steven

Re: [PATCH] powerpc: ipipe: Do full exit checks after __ipipe_call_mayday

[Steven Seeger]

On Thursday, December 19, 2019 11:53:36 AM EST Jan Kiszka wrote:
> > Ho, ho, this is an early X-mas gift.

Jan,

I got "gdb ok" when I removed the block I had in the entry_32.S I sent you 
around the recheck/do_user_signal path. I was pretty sure I didn't need the 
intret there.

Do you remember if you did that as well on your board?

This s the first time the test has passed for me without the extra trace 
prints in smokey's gdb.c. In fact I got the latest xenomai next branch and 
tested with that.

Some good news: your uclibc fix worked beautifully and I can now build the 
stock xenomai distro with my board without any of my other patches.

I'm going to look at the mayday/DoSyscall issue you suggested now. I'll be 
checking in a cleaned up entry_32.S as well. Will probably be Monday or so but 
expect 20 more emails from me until then as per my usual pattern. (You cc'd 
the list, so now the list must suffer the consequences of your actions.)

Steven

Re: [PATCH] powerpc: ipipe: Do full exit checks after __ipipe_call_mayday

[Jan Kiszka]

On 21.12.19 00:38, Steven Seeger wrote:
> On Thursday, December 19, 2019 11:53:36 AM EST Jan Kiszka wrote:
>>> Ho, ho, this is an early X-mas gift.
> 
> Jan,
> 
> I got "gdb ok" when I removed the block I had in the entry_32.S I sent you
> around the recheck/do_user_signal path. I was pretty sure I didn't need the
> intret there.
> 
> Do you remember if you did that as well on your board?

Nope, I don't. But one of the blocks was indeed redundant, if not worse.

> 
> This s the first time the test has passed for me without the extra trace
> prints in smokey's gdb.c. In fact I got the latest xenomai next branch and
> tested with that.
> 
> Some good news: your uclibc fix worked beautifully and I can now build the
> stock xenomai distro with my board without any of my other patches.
> 
> I'm going to look at the mayday/DoSyscall issue you suggested now. I'll be
> checking in a cleaned up entry_32.S as well. Will probably be Monday or so but
> expect 20 more emails from me until then as per my usual pattern. (You cc'd
> the list, so now the list must suffer the consequences of your actions.)

Perfect!

Jan

Re: [PATCH] powerpc: ipipe: Do full exit checks after __ipipe_call_mayday

[Steven Seeger]

Tried running with your patch, Jan. I made sure the files I sent you I had 
worked on were the same I had sent you. I wound up with a crash in 
do_user_signal during the smokey gdb test.

# LD_LIBRARY_PATH=/usr/xenomai/lib /usr/xenomai/bin/smokey --run=gdb
[   10.650031] Unable to handle kernel paging request for instruction fetch
[   10.656762] Faulting instruction address: 0xc0010b00
[   10.661736] Oops: Kernel access of bad area, sig: 11 [#1]
[   10.667124] BE PREEMPT Aitech SP0S100
[   10.670780] Modules linked in: unix
[   10.674268] CPU: 0 PID: 982 Comm: smokey Not tainted 4.19.55-aitech #96
[   10.680871] I-pipe domain: Linux
[   10.684089] NIP:  c0010b00 LR: c0010ad8 CTR: c00a8f1c
[   10.689131] REGS: ee8cbe90 TRAP: 0400   Not tainted  (4.19.55-aitech)
[   10.695559] MSR:  00009220 <EE,ME,DE,IR>  CR: 24000284  XER: 20000000
[   10.701998] 
[   10.701998] GPR00: 00000000 ee8cbf40 ef156bc0 00000000 c0007890 ef156bc0 
00000000 5d084753 
[   10.701998] GPR08: c055b068 00000001 c0599a60 00021000 22000282 1004870c 
00000000 00000000 
[   10.701998] GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 
00000000 10040000 
[   10.701998] GPR24: 00000000 b7ffc000 1001e480 1001e4b0 10041198 100425d0 
00000002 bffff9d0 
[   10.736886] NIP [c0010b00] do_user_signal+0x20/0x34
[   10.741755] LR [c0010ad8] recheck+0x48/0x50
[   10.745927] Call Trace:
[   10.748362] Instruction dump:
[   10.751322] 7d400124 48089e21 2c830000 4186ffa0 614a8000 7d400124 806100b0 
70600001 
[   10.759066] 41820010 bda10044 5463003c 906100b0 <38610010> 7d244b78 
4bff7add b9a10044 
[   10.766985] ---[ end trace d102d53b0f8d6db9 ]---
[   10.771591]