Re: what's parisc execve_wrapper doing in the end?

Re: what's parisc execve_wrapper doing in the end?

[James Bottomley]

[parisc list added just in case I get this wrong]
On Thu, 2012-10-04 at 05:51 +0100, Al Viro wrote:
> I'm really confused by that one:
>         /* If exec succeeded we need to load the args */
> 
>         ldo -1024(%r0),%r1
>         cmpb,>>= %r28,%r1,error_\execve
>         copy %r2,%r19
> 
> error_\execve:
>         bv %r0(%r19)
> 
> Just what is going on there?  That cmpb is non-nullifying, right? 

Right, it would have to have a ,n to nullify the following instruction,
so the copy is executed in both branches.

The compare and branch clearly has no effect, so it's a vestige of
something, I think.

>  So this
> assignment to %r2 happens whether condition is true or false.  And AFAICS
> IAOQ_Next is set to error_\execve in both cases.  So what the hell
> is that comparison for (and the comment about)?  Why not simply return
> via bv %r0(%r2) and be done with that?  It had been done that way since
> the initial parisc merge circa 2.4.0-test*...

Right, but no idea what it's a vestige of.

James

Re: what's parisc execve_wrapper doing in the end?

[James Bottomley]

[cc: parisc list]
On Thu, 2012-10-04 at 06:13 +0100, Al Viro wrote:
> On Thu, Oct 04, 2012 at 05:51:50AM +0100, Al Viro wrote:
> > I'm really confused by that one:
> >         /* If exec succeeded we need to load the args */
> > 
> >         ldo -1024(%r0),%r1
> >         cmpb,>>= %r28,%r1,error_\execve
> >         copy %r2,%r19
> > 
> > error_\execve:
> >         bv %r0(%r19)
> > 
> > Just what is going on there?  That cmpb is non-nullifying, right?  So this
> > assignment to %r2 happens whether condition is true or false.  And AFAICS
> > IAOQ_Next is set to error_\execve in both cases.  So what the hell
> > is that comparison for (and the comment about)?  Why not simply return
> > via bv %r0(%r2) and be done with that?  It had been done that way since
> > the initial parisc merge circa 2.4.0-test*...
> 
> FWIW, I suspect that hpux_execve_wrapper might have something to do with it -
> there we have
>         comb,>>= %r28,%r1,exec_error
>         copy %r2,%r19
>         ldo     -TASK_SZ_ALGN-64(%r30),%r1         ;! get task ptr
>         LDREG   TASK_PT_GR26(%r1),%r26
>         LDREG   TASK_PT_GR25(%r1),%r25
>         LDREG   TASK_PT_GR24(%r1),%r24
>         LDREG   TASK_PT_GR23(%r1),%r23
>         copy %r0,%r2    /* Flag to syscall_exit not to clear args */
> 
> exec_error:
> instead and that at least makes some sense (even though I don't see
> why syscall_exit would fail to restore them or would check %r2).  Is
> the execve_wrapper a rudiment of something similar?  It's beyond the
> history observable in the mainline, unfortunately... ;-/

It's plausible.  I just verified the theory that the branch is redundant
by successfully booting with this patch applied.

James

---

diff --git a/arch/parisc/kernel/entry.S b/arch/parisc/kernel/entry.S
index 18670a0..d1da93f 100644
--- a/arch/parisc/kernel/entry.S
+++ b/arch/parisc/kernel/entry.S
@@ -1893,15 +1893,7 @@ ENDPROC(sys_vfork_wrapper)
 
 	ldo -FRAME_SIZE(%r30),%r30
 	LDREG -RP_OFFSET(%r30),%r2
-
-	/* If exec succeeded we need to load the args */
-
-	ldo -1024(%r0),%r1
-	cmpb,>>= %r28,%r1,error_\execve
-	copy %r2,%r19

Re: what's parisc execve_wrapper doing in the end?

[Al Viro]

On Thu, Oct 04, 2012 at 11:02:16AM +0100, James Bottomley wrote:

> It's plausible.  I just verified the theory that the branch is redundant
> by successfully booting with this patch applied.

Somewhat related question: does CONFIG_HPUX work at all?  What we have there
is this:
        mfctl   %cr30,%r1
        xor     %r1,%r30,%r30                   /* ye olde xor trick */
        xor     %r1,%r30,%r1
        xor     %r1,%r30,%r30
        ldo     TASK_SZ_ALGN+FRAME_SIZE(%r30),%r30  /* set up kernel stack */
followed by saving registers into &((struct task_struct *)cr30)->thread.regs.
cr30 contains something very different, though - struct thread_info *.
Had been that way since 2002 or so.  And after we'd been finished with
syscall, we'll get to hpux_syscall_exit, tweak r22/r28 a bit and
sod off to syscall_exit.  Which does
        mfctl     %cr30, %r1
        LDREG     TI_TASK(%r1),%r1
and eventually restores the values of registers saved in
&(struct task_struct)r1->thread.regs.  Except that here the value of r1 is
((struct thread_info *)cr30)->task, not cr30 itself.  Which matches what we
have in current.h (and do_fork(), etc.), but not what we'd done when we
entered the syscall.  IOW, the values we restore will have nothing to do
with what we saved.

Unless I'm missing something really subtle, it looks like HPUX compat had
been very noticably broken since at least 2002.  Comments?

Re: what's parisc execve_wrapper doing in the end?

[James Bottomley]

On Thu, 2012-10-04 at 13:22 +0100, Al Viro wrote:
> On Thu, Oct 04, 2012 at 11:02:16AM +0100, James Bottomley wrote:
> 
> > It's plausible.  I just verified the theory that the branch is redundant
> > by successfully booting with this patch applied.
> 
> Somewhat related question: does CONFIG_HPUX work at all?

I don't believe so.  It's been bitrotting for some time.

>   What we have there
> is this:
>         mfctl   %cr30,%r1
>         xor     %r1,%r30,%r30                   /* ye olde xor trick */
>         xor     %r1,%r30,%r1
>         xor     %r1,%r30,%r30
>         ldo     TASK_SZ_ALGN+FRAME_SIZE(%r30),%r30  /* set up kernel stack */
> followed by saving registers into &((struct task_struct *)cr30)->thread.regs.
> cr30 contains something very different, though - struct thread_info *.
> Had been that way since 2002 or so.  And after we'd been finished with
> syscall, we'll get to hpux_syscall_exit, tweak r22/r28 a bit and
> sod off to syscall_exit.  Which does
>         mfctl     %cr30, %r1
>         LDREG     TI_TASK(%r1),%r1
> and eventually restores the values of registers saved in
> &(struct task_struct)r1->thread.regs.  Except that here the value of r1 is
> ((struct thread_info *)cr30)->task, not cr30 itself.  Which matches what we
> have in current.h (and do_fork(), etc.), but not what we'd done when we
> entered the syscall.  IOW, the values we restore will have nothing to do
> with what we saved.
> 
> Unless I'm missing something really subtle, it looks like HPUX compat had
> been very noticably broken since at least 2002.  Comments?

I think it probably has.  I don't believe there's anyone left with hpux
binaries actually checking it.

James

Re: what's parisc execve_wrapper doing in the end?

[Carlos O'Donell]

On Thu, Oct 4, 2012 at 8:57 AM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>> Unless I'm missing something really subtle, it looks like HPUX compat had
>> been very noticably broken since at least 2002.  Comments?
>
> I think it probably has.  I don't believe there's anyone left with hpux
> binaries actually checking it.

Was the HPUX support ever completed?

I had HPUX/Linux dual-boot on one of my systems and I was never able
to run an HPUX binary correctly.

Someone with the time and inclination is all that is missing to
cleanup the rotten HPUX bits.

There is no need to keep any of it around, that's what version control is for.

Cheers,
Carlos.

Re: what's parisc execve_wrapper doing in the end?

[Al Viro]

On Thu, Oct 04, 2012 at 09:30:16AM -0400, Carlos O'Donell wrote:
> On Thu, Oct 4, 2012 at 8:57 AM, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> >> Unless I'm missing something really subtle, it looks like HPUX compat had
> >> been very noticably broken since at least 2002.  Comments?
> >
> > I think it probably has.  I don't believe there's anyone left with hpux
> > binaries actually checking it.
> 
> Was the HPUX support ever completed?
> 
> I had HPUX/Linux dual-boot on one of my systems and I was never able
> to run an HPUX binary correctly.
> 
> Someone with the time and inclination is all that is missing to
> cleanup the rotten HPUX bits.
> 
> There is no need to keep any of it around, that's what version control is for.

FWIW, parisc seems to be the last architecture to keep such thing - sparc
and mips had dropped SunOS/Solaris/IRIX compat, iBCS* is long-dead (and hadn't
been in mainline kernel anyway).  Alpha might still be able to run some OSF
binaries, but there the situation is different - no alternative syscall
entry codepath, Linux just uses the same ABI and shares syscall numbers
where possible...

Re: what's parisc execve_wrapper doing in the end?

[John David Anglin]

On 4-Oct-12, at 10:07 AM, Al Viro wrote:

> On Thu, Oct 04, 2012 at 09:30:16AM -0400, Carlos O'Donell wrote:
>> On Thu, Oct 4, 2012 at 8:57 AM, James Bottomley
>> <James.Bottomley@hansenpartnership.com> wrote:
>>>> Unless I'm missing something really subtle, it looks like HPUX  
>>>> compat had
>>>> been very noticably broken since at least 2002.  Comments?
>>>
>>> I think it probably has.  I don't believe there's anyone left with  
>>> hpux
>>> binaries actually checking it.
>>
>> Was the HPUX support ever completed?
>>
>> I had HPUX/Linux dual-boot on one of my systems and I was never able
>> to run an HPUX binary correctly.
>>
>> Someone with the time and inclination is all that is missing to
>> cleanup the rotten HPUX bits.
>>
>> There is no need to keep any of it around, that's what version  
>> control is for.
>
> FWIW, parisc seems to be the last architecture to keep such thing -  
> sparc
> and mips had dropped SunOS/Solaris/IRIX compat, iBCS* is long-dead  
> (and hadn't
> been in mainline kernel anyway).  Alpha might still be able to run  
> some OSF
> binaries, but there the situation is different - no alternative  
> syscall
> entry codepath, Linux just uses the same ABI and shares syscall  
> numbers
> where possible...


I believe the HP-UX support should be removed.

I think it unlikely that anyone will complete the HP-UX  
implementation.  For example,
HP-UX has a 64-bit context for 32-bit applications when running a wide  
PA2.0
kernel.  Linux only has a 32-bit context.  As a result, it's not  
possible to exploit the 64-bit
registers and instructions in Linux.  We also don't have a 64-bit  
linux port, so 64-bit HP-UX
support is impossible.  There are quite a few other issues.

Dave
--
John David Anglin	dave.anglin@bell.net

Re: what's parisc execve_wrapper doing in the end?

[James Bottomley]

I tried out the code at

git://git.kernel.org/pub/scm/linux/kernel/git/viro/signal.git
experimental-kernel_thread

and it gives me this panic on boot.

James

---

PALO(F0) partition contains:
PALO is formatted EXT2/3
Command line for kernel: ' root=/dev/sda3 panic=5 console=ttyS1 palo_kernel=1/vmlinux-test'
Selected kernel: /vmlinux-test from partition 1
Selected ramdisk: /initrd.img-test from partition 1
ELF64 executable
Entry 00100000 first 00100000 n 4
Segment 0 load 00100000 size 5984256 mediaptr 0x1000
Segment 1 load 00741cf0 size 541128 mediaptr 0x5b6cf0
Segment 2 load 007c8000 size 190432 mediaptr 0x63b000
Segment 3 load 007f7000 size 140928 mediaptr 0x66a000
Loading ramdisk 2849281 bytes @ 3fd36000...
Branching to kernel entry point 0x00100000.  If this is the last
message you see, you may need to switch your console.  This is
a common symptom -- search the FAQ and mailing list at parisc-linux.org

[    0.000000] Initializing cgroup subsys cpuset
[    0.000000] Initializing cgroup subsys cpu
[    0.000000] Linux version 3.6.0+ (jejb@ion) (gcc version 4.2.4 (Debian 4.2.4-6)) #35 SMP Fri Oct 5 03:42:07 PDT 2012
[    0.000000] unwind_init: start = 0x405b7000, end = 0x405f15a0, entries = 14938
[    0.000000] WARNING: Out of order unwind entry! 00000000405b8590 and 00000000405b85a0
[    0.000000] WARNING: Out of order unwind entry! 00000000405b85a0 and 00000000405b85b0
[    0.000000] WARNING: Out of order unwind entry! 00000000405b9220 and 00000000405b9230
[    0.000000] WARNING: Out of order unwind entry! 00000000405b9230 and 00000000405b9240
[    0.000000] WARNING: Out of order unwind entry! 00000000405b9240 and 00000000405b9250
[    0.000000] WARNING: Out of order unwind entry! 00000000405b9250 and 00000000405b9260
[    0.000000] WARNING: Out of order unwind entry! 00000000405b9260 and 00000000405b9270
[    0.000000] WARNING: Out of order unwind entry! 00000000405b9270 and 00000000405b9280
[    0.000000] WARNING: Out of order unwind entry! 00000000405b9280 and 00000000405b9290
[    0.000000] WARNING: Out of order unwind entry! 00000000405b9290 and 00000000405b92a0
[    0.000000] WARNING: Out of order unwind entry! 00000000405b92c0 and 00000000405b92d0
[    0.000000] WARNING: Out of order unwind entry! 00000000405b92d0 and 00000000405b92e0
[    0.000000] WARNING: Out of order unwind entry! 00000000405b92e0 and 00000000405b92f0
[    0.000000] WARNING: Out of order unwind entry! 00000000405b92f0 and 00000000405b9300
[    0.000000] WARNING: Out of order unwind entry! 00000000405b9300 and 00000000405b9310
[    0.000000] WARNING: Out of order unwind entry! 00000000405b9310 and 00000000405b9320
[    0.000000] WARNING: Out of order unwind entry! 00000000405b9320 and 00000000405b9330
[    0.000000] WARNING: Out of order unwind entry! 00000000405b9330 and 00000000405b9340
[    0.000000] FP[0] enabled: Rev 1 Model 20
[    0.000000] The 64-bit Kernel has started...
[    0.000000] bootconsole [ttyB0] enabled
[    0.000000] Initialized PDC Console for debugging.
[    0.000000] Determining PDC firmware type: 64 bit PAT.
[    0.000000] model 00008870 00000491 00000000 00000002 3e0505e7352af710 100000f0 00000008 000000b2 000000b2
[    0.000000] vers  00000301
[    0.000000] CPUID vers 20 rev 4 (0x00000284)
[    0.000000] capabilities 0x35
[    0.000000] model 9000/800/rp3440  
[    0.000000] parisc_cache_init: Only equivalent aliasing supported!
[    0.000000] Memory Ranges:
[    0.000000]  0) Start 0x0000000000000000 End 0x000000003fffffff Size   1024 MB
[    0.000000]  1) Start 0x0000004040000000 End 0x000000407fdfffff Size   1022 MB
[    0.000000] Total Memory: 2046 MB
[    0.000000] initrd: 7fd36000-7ffeda01
[    0.000000] initrd: reserving 3fd36000-3ffeda01 (mem_max 7fe00000)
[    0.000000] PERCPU: Embedded 10 pages/cpu @0000000041942000 s9856 r8192 d22912 u40960
[    0.000000] SMP: bootstrap CPU ID is 0
[    0.000000] Built 2 zonelists in Zone order, mobility grouping on.  Total pages: 516615
[    0.000000] Kernel command line:  root=/dev/sda3 panic=5 console=ttyS1 palo_kernel=1/vmlinux-test
[    0.000000] PID hash table entries: 4096 (order: 3, 32768 bytes)
[    0.000000] Dentry cache hash table entries: 262144 (order: 9, 2097152 bytes)
[    0.000000] Inode-cache hash table entries: 131072 (order: 8, 1048576 bytes)
[    0.000000] Memory: 2048728k/2095104k available (4030k kernel code, 46376k reserved, 1765k data, 328k init)
[    0.000000] virtual kernel memory layout:
    0.000000]     vmalloc : 0x0000000000008000 - 0x000000003f000000   (1007 MB)
    0.000000]     memory  : 0x0000000040000000 - 0x00000040bfe00000   (264190 MB)
    0.000000]       .init : 0x00000000407c8000 - 0x000000004081a000   ( 328 kB)
    0.000000]       .data : 0x00000000404efb10 - 0x00000000406a8f10   (1765 kB)
    0.000000]       .text : 0x0000000040100000 - 0x00000000404efb10   (4030 kB)
[    0.000000] SLUB: Genslabs=11, HWalign=64, Order=0-3, MinObjects=0, CPUs=8, Nodes=8
[    0.000000] Hierarchical RCU implementation.
[    0.000000]  CONFIG_RCU_FANOUT set to non-default value of 32
[    0.000000] NR_IRQS:128
[    0.000000] Console: colour dummy device 160x64
[    0.000000] Calibrating delay loop... 1594.36 BogoMIPS (lpj=3188736)
[    0.048000] pid_max: default: 32768 minimum: 301
[    0.048000] Security Framework initialized
[    0.048000] SELinux:  Disabled at boot.
[    0.048000] Mount-cache hash table entries: 256
[    0.056000] Initializing cgroup subsys cpuacct
[    0.056000] Initializing cgroup subsys devices
[    0.056000] Initializing cgroup subsys freezer
[    0.064000] Initializing cgroup subsys net_cls
[    0.064000] Initializing cgroup subsys blkio
[    0.304000] Backtrace:
[    0.304000]  [<00000000401435b8>] copy_process+0x9d8/0x1698
[    0.304000]  [<00000000401151a0>] fork_idle+0x60/0xc8
[    0.304000]  [<00000000407d0c70>] idle_threads_init+0xc0/0x130
[    0.304000]  [<00000000407d2520>] smp_init+0x30/0x158
[    0.304000]  [<00000000407c87c0>] kernel_init+0x158/0x390
[    0.304000]  [<0000000040104020>] end_fault_vector+0x20/0x3c
[    0.304000]  [<000000004011bb00>] _raw_spin_trylock+0x0/0x18
[    0.304000] 
[    0.304000] 
[    0.304000] Kernel Fault: Code=26 regs=000000007f84c850 (Addr=0000000000000018)
[    0.304000] 
[    0.304000]      YZrvWESTHLNXBCVMcbcbcbcbOGFRQPDI
[    0.304000] PSW: 00001000000001001111111100001111 Not tainted
[    0.304000] r00-03  000000ff0804ff0f 00000000407c4a90 0000000040129270 000000007f85d410
[    0.304000] r04-07  00000000407a0290 0000000000000000 000000007f84c348 000000007f8a0000
[    0.304000] r08-11  0000000000000000 0000000000000100 000000004069f088 0000000000000000
[    0.304000] r12-15  0000000000000000 0000000000000000 0000000000000000 000000007f84c348
[    0.304000] r16-19  0000000000000000 0000000000000000 0000000000000000 0000000000000000
[    0.304000] r20-23  0000000000000006 000000007f85da30 0000000000000000 000000007f85d410
[    0.304000] r24-27  0000000000000000 0000000000000000 000000007f85d780 00000000407a0290
[    0.304000] r28-31  0000000040104000 000000007f84c820 000000007f84c850 000000007f8a0100
[    0.304000] sr00-03  0000000000000000 0000000000000000 0000000000000000 0000000000000000
[    0.304000] sr04-07  0000000000000000 0000000000000000 0000000000000000 0000000000000000
[    0.304000] 
[    0.304000] IASQ: 0000000000000000 0000000000000000 IAOQ: 0000000040129288 000000004012928c
[    0.304000]  IIR: 50bf0030    ISR: 0000000000000000  IOR: 0000000000000018
[    0.304000]  CPU:        0   CR30: 000000007f84c000 CR31: fffffff0f0e098e0
[    0.304000]  ORIG_R28: 0000000000000000
[    0.304000]  IAOQ[0]: copy_thread+0xf8/0x110
[    0.304000]  IAOQ[1]: copy_thread+0xfc/0x110
[    0.304000]  RP(r2): copy_thread+0xe0/0x110
[    0.304000] Backtrace:
[    0.304000]  [<00000000401435b8>] copy_process+0x9d8/0x1698
[    0.304000]  [<00000000401151a0>] fork_idle+0x60/0xc8
[    0.304000]  [<00000000407d0c70>] idle_threads_init+0xc0/0x130
[    0.304000]  [<00000000407d2520>] smp_init+0x30/0x158
[    0.304000]  [<00000000407c87c0>] kernel_init+0x158/0x390
[    0.304000]  [<0000000040104020>] end_fault_vector+0x20/0x3c
[    0.304000]  [<000000004011bb00>] _raw_spin_trylock+0x0/0x18
[    0.304000] 
[    0.304000] Kernel panic - not syncing: Kernel Fault
[    0.304000] Rebooting in 5 seconds..

Re: what's parisc execve_wrapper doing in the end?

[James Bottomley]

On Fri, 2012-10-05 at 12:07 +0100, James Bottomley wrote:
> I tried out the code at
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/viro/signal.git
> experimental-kernel_thread
> 
> and it gives me this panic on boot.

OK, found the fix:  the idle thread is a kernel thread, but it doesn't
come through kernel_thread().  The fix is to check for it (fortunately
it has the signal usp == 0).

I'm now getting as freeing the init memory, which then hangs, so I
suspect some type of execve failure trying to start the initrd... I'm
debugging.

James

---

diff --git a/arch/parisc/kernel/process.c b/arch/parisc/kernel/process.c
index a67f122..44e8534 100644
--- a/arch/parisc/kernel/process.c
+++ b/arch/parisc/kernel/process.c
@@ -254,7 +254,7 @@ copy_thread(unsigned long clone_flags, unsigned long usp,
 	extern void * const hpux_child_return;
 #endif
 
-	if (unlikely(p->flags & PF_KTHREAD)) {
+	if (unlikely((p->flags & PF_KTHREAD) && usp != 0)) {
 		memset(cregs, 0, sizeof(struct pt_regs));
 		/* kernel thread */
 		cregs->ksp = (unsigned long)stack + THREAD_SZ_ALGN;

Re: what's parisc execve_wrapper doing in the end?

[James Bottomley]

On Fri, 2012-10-05 at 14:44 +0100, James Bottomley wrote:
> On Fri, 2012-10-05 at 12:07 +0100, James Bottomley wrote:
> > I tried out the code at
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/viro/signal.git
> > experimental-kernel_thread
> > 
> > and it gives me this panic on boot.
> 
> OK, found the fix:  the idle thread is a kernel thread, but it doesn't
> come through kernel_thread().  The fix is to check for it (fortunately
> it has the signal usp == 0).
> 
> I'm now getting as freeing the init memory, which then hangs, so I
> suspect some type of execve failure trying to start the initrd... I'm
> debugging.

And the fix for this one is below: ret_from_kernel_execve has pt_regs in
%r26 ... it needs to be in %r16 for intr_return, so we just copy them
over.

With this, I can bring up the kernel all the way to a login prompt with
no apparent ill effects.

James

---

diff --git a/arch/parisc/kernel/entry.S b/arch/parisc/kernel/entry.S
index 272c7a0..dd137fc 100644
--- a/arch/parisc/kernel/entry.S
+++ b/arch/parisc/kernel/entry.S
@@ -744,7 +744,8 @@ ENDPROC(ret_from_kernel_thread)
 ENTRY(ret_from_kernel_execve)
 	mfctl   %cr30, %r1
 	ldo	THREAD_SZ_ALGN(%r1), %r30
-	b,n	intr_return    /* forward */
+	b	intr_return    /* forward */
+	copy	%r26,%r16      /* pt_regs into r16 */
 ENDPROC(ret_from_kernel_execve)
 
 

Re: what's parisc execve_wrapper doing in the end?

[Al Viro]

On Fri, Oct 05, 2012 at 02:44:24PM +0100, James Bottomley wrote:
> On Fri, 2012-10-05 at 12:07 +0100, James Bottomley wrote:
> > I tried out the code at
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/viro/signal.git
> > experimental-kernel_thread
> > 
> > and it gives me this panic on boot.
> 
> OK, found the fix:  the idle thread is a kernel thread, but it doesn't
> come through kernel_thread().  The fix is to check for it (fortunately
> it has the signal usp == 0).

Um...  I see, but I really wonder if that's the right fix.  FWIW, sparc
will have the same problem...  Hell knows.  OTOH, it's a nice way to
get of implicit interplay between copy_thread() and idle_regs() - note
that SMP architectures doing default idle_regs() need to be damn careful
about what they do in their "is that kernel thread" logics; all-zeros
pt_regs might give varying results on user_mode(regs) tests, etc.
Might be better to go for
	if (p->flags & PF_KTHREAD) {
		if (!usp) {
			we are starting an idle thread
		} else {
			we are setting things up for kernel_thread()
		}
	} else {
		we are forking
	}
kind of logics, looking at regs only in the last case.  And to hell with
(separate and overridable) idle_regs() once everything goes that way...
 
> I'm now getting as freeing the init memory, which then hangs, so I
> suspect some type of execve failure trying to start the initrd... I'm
> debugging.

What are you using for toolchain, BTW?  With gcc 4.3 / binutils 2.20
(cross-builds) I'm getting panics from mainline kernel on parisc32 all
way back to 2.6.28...

Re: what's parisc execve_wrapper doing in the end?

[James Bottomley]

On Fri, 2012-10-05 at 15:48 +0100, Al Viro wrote:
> On Fri, Oct 05, 2012 at 02:44:24PM +0100, James Bottomley wrote:
> > On Fri, 2012-10-05 at 12:07 +0100, James Bottomley wrote:
> > > I tried out the code at
> > > 
> > > git://git.kernel.org/pub/scm/linux/kernel/git/viro/signal.git
> > > experimental-kernel_thread
> > > 
> > > and it gives me this panic on boot.
> > 
> > OK, found the fix:  the idle thread is a kernel thread, but it doesn't
> > come through kernel_thread().  The fix is to check for it (fortunately
> > it has the signal usp == 0).
> 
> Um...  I see, but I really wonder if that's the right fix.  FWIW, sparc
> will have the same problem...  Hell knows.  OTOH, it's a nice way to
> get of implicit interplay between copy_thread() and idle_regs() - note
> that SMP architectures doing default idle_regs() need to be damn careful
> about what they do in their "is that kernel thread" logics; all-zeros
> pt_regs might give varying results on user_mode(regs) tests, etc.
> Might be better to go for
> 	if (p->flags & PF_KTHREAD) {
> 		if (!usp) {
> 			we are starting an idle thread
> 		} else {
> 			we are setting things up for kernel_thread()
> 		}
> 	} else {
> 		we are forking
> 	}
> kind of logics, looking at regs only in the last case.  And to hell with
> (separate and overridable) idle_regs() once everything goes that way...

But there's not a lot of point.  forking an idle thread actually doesn't
care about any of the register execution setup because it never really
uses it to execute.  That's why it was safe for us to use the user
thread setup ... I suppose the interior of the kernel thread case could
be conditioned on if (usp).


> > I'm now getting as freeing the init memory, which then hangs, so I
> > suspect some type of execve failure trying to start the initrd... I'm
> > debugging.
> 
> What are you using for toolchain, BTW?  With gcc 4.3 / binutils 2.20
> (cross-builds) I'm getting panics from mainline kernel on parisc32 all
> way back to 2.6.28...

My toolchain is

jejb@ion> gcc -v
Using built-in specs.
Target: hppa-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian 4.4.5-8'
--with-bugurl=file:///usr/share/doc/gcc-4.4/README.Bugs
--enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr
--program-suffix=-4.4 --enable-shared --enable-multiarch
--enable-linker-build-id --with-system-zlib --libexecdir=/usr/lib
--without-included-gettext --enable-threads=posix
--with-gxx-include-dir=/usr/include/c++/4.4 --libdir=/usr/lib
--enable-nls --enable-clocale=gnu --enable-libstdcxx-debug
--disable-libssp --enable-objc-gc --disable-libstdcxx-pch
--enable-checking=release --build=hppa-linux-gnu --host=hppa-linux-gnu
--target=hppa-linux-gnu
Thread model: posix
gcc version 4.4.5 (Debian 4.4.5-8) 


It's the one that was in debian testing before parisc got dumped

James

Re: what's parisc execve_wrapper doing in the end?

[Al Viro]

On Fri, Oct 05, 2012 at 03:55:36PM +0100, James Bottomley wrote:

> But there's not a lot of point.  forking an idle thread actually doesn't
> care about any of the register execution setup because it never really
> uses it to execute.  That's why it was safe for us to use the user
> thread setup ... I suppose the interior of the kernel thread case could
> be conditioned on if (usp).

FWIW, I'm digging through the fork_idle() interplay with copy_thread()
right now and the more I'm looking the less I like having called
copy_thread() at all on that path.  Note that copy_process() can trivially
tell it from the normal (do_fork()) caller, just by checking if pid
is non-NULL.  And it has to check that anyway when it decides to generate
the pid for child...

Hell knows; I wonder how much of copy_thread() work is needed in that case...

Re: what's parisc execve_wrapper doing in the end?

[John David Anglin]

On 5-Oct-12, at 10:48 AM, Al Viro wrote:

> What are you using for toolchain, BTW?  With gcc 4.3 / binutils 2.20
> (cross-builds) I'm getting panics from mainline kernel on parisc32 all
> way back to 2.6.28...


Current debian unstable tools are available in shirka.esiee.fr:/home/ 
dave/archives
and shirka.esiee.fr:/home/dave/archives-new.  It's pretty straight  
forward to update
binutils, gcc, libc, etc.

gcc 4.4, 4.5, 4.6 and 4.7 are available.  In particular, 4.6 and 4.7  
have significant
bug fixes.

James seems to have an account on shirka.

I don't have any place to distribute the debs that are on shirka, or  
the time to remove
obsolete builds.  Any volunteers?

Dave
--
John David Anglin	dave.anglin@bell.net

Re: what's parisc execve_wrapper doing in the end?

[Al Viro]

On Fri, Oct 05, 2012 at 03:55:36PM +0100, James Bottomley wrote:
> On Fri, 2012-10-05 at 15:48 +0100, Al Viro wrote:
> > On Fri, Oct 05, 2012 at 02:44:24PM +0100, James Bottomley wrote:
> > > On Fri, 2012-10-05 at 12:07 +0100, James Bottomley wrote:
> > > > I tried out the code at
> > > > 
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/viro/signal.git
> > > > experimental-kernel_thread
> > > > 
> > > > and it gives me this panic on boot.
> > > 
> > > OK, found the fix:  the idle thread is a kernel thread, but it doesn't
> > > come through kernel_thread().  The fix is to check for it (fortunately
> > > it has the signal usp == 0).
> > 
> > Um...  I see, but I really wonder if that's the right fix.  FWIW, sparc
> > will have the same problem...  Hell knows.  OTOH, it's a nice way to
> > get of implicit interplay between copy_thread() and idle_regs() - note
> > that SMP architectures doing default idle_regs() need to be damn careful
> > about what they do in their "is that kernel thread" logics; all-zeros
> > pt_regs might give varying results on user_mode(regs) tests, etc.
> > Might be better to go for
> > 	if (p->flags & PF_KTHREAD) {
> > 		if (!usp) {
> > 			we are starting an idle thread
> > 		} else {
> > 			we are setting things up for kernel_thread()
> > 		}
> > 	} else {
> > 		we are forking
> > 	}
> > kind of logics, looking at regs only in the last case.  And to hell with
> > (separate and overridable) idle_regs() once everything goes that way...
> 
> But there's not a lot of point.  forking an idle thread actually doesn't
> care about any of the register execution setup because it never really
> uses it to execute.  That's why it was safe for us to use the user
> thread setup ... I suppose the interior of the kernel thread case could
> be conditioned on if (usp).

BTW, speaking of parisc copy_thread()...  Why the hell do we bother
with *cregs = *pregs in userland case?  It's a part of task_struct,
after all, and we have copied that wholesale in arch_dup_task_struct().

Another thing: why do we bother with
        STREG   %r30,PT_GR21(%r1)
in fork wrapper?  We bloody well know what the offset will be, after all -
right in the beginning of that sucker we'd done
        LDREG   TI_TASK-THREAD_SZ_ALGN-FRAME_SIZE(%r30), %r1
so we rely on %r30 having been (unsigned long)current_thread_info() + 
THREAD_SZ_ALGN + FRAME_SIZE.  Then we add FRAME_SIZE again.  IOW, the
offset is a known constant.  Hell, in child_return you rely on its
value...   While we are at it, I'm not sure you need to go through
wrapper_exit on the way out in parent - saving cr27 can be done via
e.g. r28 instead of r3, at which point you can simply branch to
sys_clone() with no work left for wrapper_exit.  *Child* obviously
needs to restore these registers, so let it do that in child_return,
but why bother in parent?  After all, we are talking about the callee-saved
registers, so sys_clone() is going to revert whatever changes it makes
to them...

BTW, TIF_SYSCALL_TRACE and singlestepping are turned off in child, so I don't
see any need for child_return to know where the parent had come from - it
won't have anything to do in tracesys_exit anyway.

I've folded your fixes and pushed the result; I've added (again, completely
untested) optimizations along the lines of the above on top of those, as
a separate commit.  Comments?

Re: what's parisc execve_wrapper doing in the end?

[Al Viro]

On Fri, Oct 05, 2012 at 06:54:02PM -0400, John David Anglin wrote:
> On 5-Oct-12, at 10:48 AM, Al Viro wrote:
> 
> >What are you using for toolchain, BTW?  With gcc 4.3 / binutils 2.20
> >(cross-builds) I'm getting panics from mainline kernel on parisc32 all
> >way back to 2.6.28...
> 
> 
> Current debian unstable tools are available in
> shirka.esiee.fr:/home/dave/archives
> and shirka.esiee.fr:/home/dave/archives-new.  It's pretty straight
> forward to update
> binutils, gcc, libc, etc.
> 
> gcc 4.4, 4.5, 4.6 and 4.7 are available.  In particular, 4.6 and 4.7
> have significant
> bug fixes.
> 
> James seems to have an account on shirka.
> 
> I don't have any place to distribute the debs that are on shirka, or
> the time to remove
> obsolete builds.  Any volunteers?

Hmm...  debian-ports would be a natural place, but TBH I'd very much prefer
to deal with cross-builds - doing kernel builds on a 100MHz 7100LC with
96M of RAM is really not fun...

Re: what's parisc execve_wrapper doing in the end?

[John David Anglin]

On 5-Oct-12, at 7:32 PM, Al Viro wrote:

> On Fri, Oct 05, 2012 at 06:54:02PM -0400, John David Anglin wrote:
>> On 5-Oct-12, at 10:48 AM, Al Viro wrote:
>>
>>> What are you using for toolchain, BTW?  With gcc 4.3 / binutils 2.20
>>> (cross-builds) I'm getting panics from mainline kernel on parisc32  
>>> all
>>> way back to 2.6.28...
>>
>>
>> Current debian unstable tools are available in
>> shirka.esiee.fr:/home/dave/archives
>> and shirka.esiee.fr:/home/dave/archives-new.  It's pretty straight
>> forward to update
>> binutils, gcc, libc, etc.
>>
>> gcc 4.4, 4.5, 4.6 and 4.7 are available.  In particular, 4.6 and 4.7
>> have significant
>> bug fixes.
>>
>> James seems to have an account on shirka.
>>
>> I don't have any place to distribute the debs that are on shirka, or
>> the time to remove
>> obsolete builds.  Any volunteers?
>
> Hmm...  debian-ports would be a natural place, but TBH I'd very much  
> prefer
> to deal with cross-builds - doing kernel builds on a 100MHz 7100LC  
> with
> 96M of RAM is really not fun...
>

Right...

I had setup a buildd system system on lafayette.esiee.fr but  
unfortunately
a processor module died.  I also never received a response from debian- 
ports
(several emails) about restarting buildd for parisc.

I've continued to build packages on the rp3440 that I have at home.   
It doesn't have
much trouble keeping up with unstable.  At the same time, I use it for  
gcc testing
and development.

I would say you should be able to build cross tools using the binutils  
2.22 branch
and gcc 4.6 or 4.7.  They should work better than gcc 4.3 with  
binutils 2.20.

The reason I say that is I have built many debian packages with the  
current tools
and I have been slowly chipping away at the toolchain bugs that I have  
found.

Dave
--
John David Anglin	dave.anglin@bell.net

Re: what's parisc execve_wrapper doing in the end?

[James Bottomley]

On Sat, 2012-10-06 at 00:04 +0100, Al Viro wrote:
> On Fri, Oct 05, 2012 at 03:55:36PM +0100, James Bottomley wrote:
> > On Fri, 2012-10-05 at 15:48 +0100, Al Viro wrote:
> > > On Fri, Oct 05, 2012 at 02:44:24PM +0100, James Bottomley wrote:
> > > > On Fri, 2012-10-05 at 12:07 +0100, James Bottomley wrote:
> > > > > I tried out the code at
> > > > > 
> > > > > git://git.kernel.org/pub/scm/linux/kernel/git/viro/signal.git
> > > > > experimental-kernel_thread
> > > > > 
> > > > > and it gives me this panic on boot.
> > > > 
> > > > OK, found the fix:  the idle thread is a kernel thread, but it doesn't
> > > > come through kernel_thread().  The fix is to check for it (fortunately
> > > > it has the signal usp == 0).
> > > 
> > > Um...  I see, but I really wonder if that's the right fix.  FWIW, sparc
> > > will have the same problem...  Hell knows.  OTOH, it's a nice way to
> > > get of implicit interplay between copy_thread() and idle_regs() - note
> > > that SMP architectures doing default idle_regs() need to be damn careful
> > > about what they do in their "is that kernel thread" logics; all-zeros
> > > pt_regs might give varying results on user_mode(regs) tests, etc.
> > > Might be better to go for
> > > 	if (p->flags & PF_KTHREAD) {
> > > 		if (!usp) {
> > > 			we are starting an idle thread
> > > 		} else {
> > > 			we are setting things up for kernel_thread()
> > > 		}
> > > 	} else {
> > > 		we are forking
> > > 	}
> > > kind of logics, looking at regs only in the last case.  And to hell with
> > > (separate and overridable) idle_regs() once everything goes that way...
> > 
> > But there's not a lot of point.  forking an idle thread actually doesn't
> > care about any of the register execution setup because it never really
> > uses it to execute.  That's why it was safe for us to use the user
> > thread setup ... I suppose the interior of the kernel thread case could
> > be conditioned on if (usp).
> 
> BTW, speaking of parisc copy_thread()...  Why the hell do we bother
> with *cregs = *pregs in userland case?  It's a part of task_struct,
> after all, and we have copied that wholesale in arch_dup_task_struct().
> 
> Another thing: why do we bother with
>         STREG   %r30,PT_GR21(%r1)
> in fork wrapper?  We bloody well know what the offset will be, after all -
> right in the beginning of that sucker we'd done
>         LDREG   TI_TASK-THREAD_SZ_ALGN-FRAME_SIZE(%r30), %r1
> so we rely on %r30 having been (unsigned long)current_thread_info() + 
> THREAD_SZ_ALGN + FRAME_SIZE.  Then we add FRAME_SIZE again.  IOW, the
> offset is a known constant.  Hell, in child_return you rely on its
> value...   While we are at it, I'm not sure you need to go through
> wrapper_exit on the way out in parent - saving cr27 can be done via
> e.g. r28 instead of r3, at which point you can simply branch to
> sys_clone() with no work left for wrapper_exit.  *Child* obviously
> needs to restore these registers, so let it do that in child_return,
> but why bother in parent?  After all, we are talking about the callee-saved
> registers, so sys_clone() is going to revert whatever changes it makes
> to them...

It sounds plausible.  I'm checking your branch out now, modulo a couple
of compile failures:

arch/parisc/kernel/entry.S: Assembler messages:
arch/parisc/kernel/entry.S:1754: Error: Invalid operands 

arch/parisc/kernel/process.c: In function 'copy_thread':
arch/parisc/kernel/process.c:256: error: 'FRAME_SIZE' undeclared (first use in this function)
arch/parisc/kernel/process.c:256: error: (Each undeclared identifier is reported only once
arch/parisc/kernel/process.c:256: error: for each function it appears in.)
arch/parisc/kernel/process.c:258: error: expected ')' before '{' token
arch/parisc/kernel/process.c:297: error: expected expression before '}' token
arch/parisc/kernel/process.c:252: warning: unused variable 'child_return'
arch/parisc/kernel/process.c:251: warning: unused variable 'ret_from_kernel_thread'

Fix up below.

> BTW, TIF_SYSCALL_TRACE and singlestepping are turned off in child, so I don't
> see any need for child_return to know where the parent had come from - it
> won't have anything to do in tracesys_exit anyway.
> 
> I've folded your fixes and pushed the result; I've added (again, completely
> untested) optimizations along the lines of the above on top of those, as
> a separate commit.  Comments?

Even with the patch applied, it's hanging on boot around the first
kthread spawns.  I'm investigating.

James

---

diff --git a/arch/parisc/kernel/entry.S b/arch/parisc/kernel/entry.S
index f253f36..47fb6dd 100644
--- a/arch/parisc/kernel/entry.S
+++ b/arch/parisc/kernel/entry.S
@@ -1751,7 +1751,7 @@ ENTRY(sys_vfork_wrapper)
 	mfctl	%cr27, %r28
 	STREG	%r28, PT_CR27(%r1)
 
-	b	sys_vfork,%r2
+	b	sys_vfork
 	copy	%r1,%r26
 ENDPROC(sys_vfork_wrapper)
 
diff --git a/arch/parisc/kernel/process.c b/arch/parisc/kernel/process.c
index 47eba15..61113c3 100644
--- a/arch/parisc/kernel/process.c
+++ b/arch/parisc/kernel/process.c
@@ -52,6 +52,7 @@
 
 #include <asm/io.h>
 #include <asm/asm-offsets.h>
+#include <asm/assembly.h>
 #include <asm/pdc.h>
 #include <asm/pdc_chassis.h>
 #include <asm/pgalloc.h>
@@ -255,7 +256,7 @@ copy_thread(unsigned long clone_flags, unsigned long usp,
 #endif
 	cregs->ksp = (unsigned long)stack + THREAD_SZ_ALGN + FRAME_SIZE;
 
-	if (unlikely((p->flags & PF_KTHREAD)) {
+	if (unlikely(p->flags & PF_KTHREAD)) {
 		memset(cregs, 0, sizeof(struct pt_regs));
 		if (!usp) /* idle thread */
 			return 0;

Re: what's parisc execve_wrapper doing in the end?

[James Bottomley]

On Mon, 2012-10-08 at 12:28 +0100, James Bottomley wrote:
> Even with the patch applied, it's hanging on boot around the first
> kthread spawns.  I'm investigating.

Actually an obvious fix: you can't set up ksp and then zero the
registers.

I note that the stack frame you're creating is too big since
THREAD_SZ_ALGN includes space for a stack frame by design so adding
another one is redundant.  However, this confusion seems to permeate the
syscall code as well, so perhaps fixing it up later is better.

With this patch applied, I can now boot up successfully to a login
prompt.

James

---

diff --git a/arch/parisc/kernel/process.c b/arch/parisc/kernel/process.c
index 61113c3..38db36f 100644
--- a/arch/parisc/kernel/process.c
+++ b/arch/parisc/kernel/process.c
@@ -254,8 +254,6 @@ copy_thread(unsigned long clone_flags, unsigned long usp,
 #ifdef CONFIG_HPUX
 	extern void * const hpux_child_return;
 #endif
-	cregs->ksp = (unsigned long)stack + THREAD_SZ_ALGN + FRAME_SIZE;

Re: what's parisc execve_wrapper doing in the end?

[Al Viro]

On Tue, Oct 09, 2012 at 10:55:21AM +0100, James Bottomley wrote:
> On Mon, 2012-10-08 at 12:28 +0100, James Bottomley wrote:
> > Even with the patch applied, it's hanging on boot around the first
> > kthread spawns.  I'm investigating.
> 
> Actually an obvious fix: you can't set up ksp and then zero the
> registers.
> 
> I note that the stack frame you're creating is too big since
> THREAD_SZ_ALGN includes space for a stack frame by design so adding
> another one is redundant.  However, this confusion seems to permeate the
> syscall code as well, so perhaps fixing it up later is better.
> 
> With this patch applied, I can now boot up successfully to a login
> prompt.

Thanks.  Folded and pushed with the rebase - it's in experimental-kernel_thread
pushed right now.  Should propagate to git.kernel.org shortly...