On 29.10.2016 05:00, John David Anglin wrote: > On 2016-10-28, at 4:04 PM, Helge Deller wrote: > >> We have one critical section in the syscall entry path in which we switch from >> the userspace stack to kernel stack. In the event of an external interrupt, the >> interrupt code distinguishes between those two states by analyzing the value of >> sr7. If sr7 is zero, it uses the kernel stack. Therefore it's important, that >> the value of sr7 is in sync with the currently enabled stack. >> >> This patch now disables interrupts while executing the critical section. This >> prevents the interrupt handler to possibly see an inconsitent state which in >> the worst case can lead to crashes. >> >> Interestingly, in the syscall exit path interrupts were already disabled in the >> critical section which switches back to the userspace stack. >> >> Cc: <stable@xxxxxxxxxxxxxxx> >> Signed-off-by: Helge Deller <deller@xxxxxx> >> >> diff --git a/arch/parisc/kernel/syscall.S b/arch/parisc/kernel/syscall.S >> index d03422e..f13836b 100644 >> --- a/arch/parisc/kernel/syscall.S >> +++ b/arch/parisc/kernel/syscall.S >> @@ -106,8 +106,6 @@ linux_gateway_entry: >> mtsp %r0,%sr4 /* get kernel space into sr4 */ >> mtsp %r0,%sr5 /* get kernel space into sr5 */ >> mtsp %r0,%sr6 /* get kernel space into sr6 */ >> - mfsp %sr7,%r1 /* save user sr7 */ >> - mtsp %r1,%sr3 /* and store it in sr3 */ >> >> #ifdef CONFIG_64BIT >> /* for now we can *always* set the W bit on entry to the syscall >> @@ -134,20 +132,26 @@ linux_gateway_entry: >> 1: >> #endif >> mfctl %cr30,%r1 >> - xor %r1,%r30,%r30 /* ye olde xor trick */ >> - xor %r1,%r30,%r1 >> - xor %r1,%r30,%r30 >> - >> - ldo THREAD_SZ_ALGN+FRAME_SIZE(%r30),%r30 /* set up kernel stack */ >> + ldo THREAD_SZ_ALGN+FRAME_SIZE(%r1),%r1 /* set up kernel stack */ >> >> /* N.B.: It is critical that we don't set sr7 to 0 until r30 >> * contains a valid kernel stack pointer. It is also >> * critical that we don't start using the kernel stack >> - * until after sr7 has been set to 0. >> + * until after sr7 has been set to 0. To ensure this we >> + * use a rsm/ssm pair to make this operation atomic. >> + * At syscall entry %sr2 points to kernel space, otherwise >> + * syscalls wouldn't work. >> */ >> + rsm PSW_SM_I, %r0 /* turn interrupts off */ >> + STREGM %r30,FRAME_SIZE(%sr2, %r1) /* save usp on kernel stack */ >> + copy %r1, %r30 /* switch to kernel stack */ >> + >> + mfsp %sr7,%r1 /* get user sr7 */ >> + mtsp %r1,%sr3 /* store user sr7 in sr3 */ >> >> mtsp %r0,%sr7 /* get kernel space into sr7 */ >> - STREGM %r1,FRAME_SIZE(%r30) /* save r1 (usp) here for now */ >> + ssm PSW_SM_I, %r0 /* turn interrupts on */ >> + >> mfctl %cr30,%r1 /* get task ptr in %r1 */ >> LDREG TI_TASK(%r1),%r1 > > > The above doesn't assemble. The STREGM instruction doesn't allow explicit space register > selection with long displacements. Argh. The assembler didn't even complained, but after looking at the generated assembly you are right. Thanks for noticing! > The attached patch does assemble and v4.7.10 boots successfully with change. > I'm thinking we might now be able to remove the restriction on scheduling on the gateway page. Yes, probably. Helge -- To unsubscribe from this list: send the line "unsubscribe linux-parisc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
