| Message ID | 1591603981-16879-4-git-send-email-pmorel@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: Testing the Channel Subsystem I/O | expand |
On 08/06/2020 10.12, Pierre Morel wrote: > If we use multiple source of interrupts, for example, using SCLP > console to print information while using I/O interrupts, we need > to have a re-entrant register saving interruption handling. > > Instead of saving at a static memory address, let's save the base > registers, the floating point registers and the floating point > control register on the stack in case of I/O interrupts > > Note that we keep the static register saving to recover from the > RESET tests. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > Acked-by: Janosch Frank <frankja@linux.ibm.com> > --- > s390x/cstart64.S | 41 +++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 39 insertions(+), 2 deletions(-) > > diff --git a/s390x/cstart64.S b/s390x/cstart64.S > index b50c42c..a9d8223 100644 > --- a/s390x/cstart64.S > +++ b/s390x/cstart64.S > @@ -119,6 +119,43 @@ memsetxc: > lmg %r0, %r15, GEN_LC_SW_INT_GRS > .endm > > +/* Save registers on the stack (r15), so we can have stacked interrupts. */ > + .macro SAVE_REGS_STACK > + /* Allocate a stack frame for 15 general registers */ > + slgfi %r15, 15 * 8 > + /* Store registers r0 to r14 on the stack */ > + stmg %r0, %r14, 0(%r15) > + /* Allocate a stack frame for 16 floating point registers */ > + /* The size of a FP register is the size of an double word */ > + slgfi %r15, 16 * 8 > + /* Save fp register on stack: offset to SP is multiple of reg number */ > + .irp i, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15 > + std \i, \i * 8(%r15) > + .endr > + /* Save fpc, but keep stack aligned on 64bits */ > + slgfi %r15, 8 > + efpc %r0 > + stg %r0, 0(%r15) > + .endm I wonder whether it would be sufficient to only save the registers here that are "volatile" according to the ELF ABI? ... that would save quite some space on the stack, I think... OTOH, the old code was also saving all registers, so maybe that's something for a separate patch later... Acked-by: Thomas Huth <thuth@redhat.com>
On 2020-06-08 11:05, Thomas Huth wrote: > On 08/06/2020 10.12, Pierre Morel wrote: >> If we use multiple source of interrupts, for example, using SCLP >> console to print information while using I/O interrupts, we need >> to have a re-entrant register saving interruption handling. >> >> Instead of saving at a static memory address, let's save the base >> registers, the floating point registers and the floating point >> control register on the stack in case of I/O interrupts >> >> Note that we keep the static register saving to recover from the >> RESET tests. >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> Acked-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> s390x/cstart64.S | 41 +++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 39 insertions(+), 2 deletions(-) >> >> diff --git a/s390x/cstart64.S b/s390x/cstart64.S >> index b50c42c..a9d8223 100644 >> --- a/s390x/cstart64.S >> +++ b/s390x/cstart64.S >> @@ -119,6 +119,43 @@ memsetxc: >> lmg %r0, %r15, GEN_LC_SW_INT_GRS >> .endm >> >> +/* Save registers on the stack (r15), so we can have stacked interrupts. */ >> + .macro SAVE_REGS_STACK >> + /* Allocate a stack frame for 15 general registers */ >> + slgfi %r15, 15 * 8 >> + /* Store registers r0 to r14 on the stack */ >> + stmg %r0, %r14, 0(%r15) >> + /* Allocate a stack frame for 16 floating point registers */ >> + /* The size of a FP register is the size of an double word */ >> + slgfi %r15, 16 * 8 >> + /* Save fp register on stack: offset to SP is multiple of reg number */ >> + .irp i, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15 >> + std \i, \i * 8(%r15) >> + .endr >> + /* Save fpc, but keep stack aligned on 64bits */ >> + slgfi %r15, 8 >> + efpc %r0 >> + stg %r0, 0(%r15) >> + .endm > > I wonder whether it would be sufficient to only save the registers here > that are "volatile" according to the ELF ABI? ... that would save quite > some space on the stack, I think... OTOH, the old code was also saving > all registers, so maybe that's something for a separate patch later... I don't think so for the general registers The "volatile" registers are lost during a C call, so it is the duty of the caller to save them before the call, if he wants, and this is possible for the programmer or the compiler to arrange that. For interruptions, we steal the CPU with all the registers from the program without warning, the program has no possibility to save them. So we must save all registers for him. For the FP registers, we surely can do something if we establish a usage convention on the floating point. A few tests need hardware floating point. If IRQ speed or stack size becomes an issue we can think about optimizing the floating point usage. > > Acked-by: Thomas Huth <thuth@redhat.com> > Thanks, Pierre
On 08/06/2020 16.24, Pierre Morel wrote: > > > On 2020-06-08 11:05, Thomas Huth wrote: >> On 08/06/2020 10.12, Pierre Morel wrote: >>> If we use multiple source of interrupts, for example, using SCLP >>> console to print information while using I/O interrupts, we need >>> to have a re-entrant register saving interruption handling. >>> >>> Instead of saving at a static memory address, let's save the base >>> registers, the floating point registers and the floating point >>> control register on the stack in case of I/O interrupts >>> >>> Note that we keep the static register saving to recover from the >>> RESET tests. >>> >>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >>> Acked-by: Janosch Frank <frankja@linux.ibm.com> >>> --- >>> s390x/cstart64.S | 41 +++++++++++++++++++++++++++++++++++++++-- >>> 1 file changed, 39 insertions(+), 2 deletions(-) >>> >>> diff --git a/s390x/cstart64.S b/s390x/cstart64.S >>> index b50c42c..a9d8223 100644 >>> --- a/s390x/cstart64.S >>> +++ b/s390x/cstart64.S >>> @@ -119,6 +119,43 @@ memsetxc: >>> lmg %r0, %r15, GEN_LC_SW_INT_GRS >>> .endm >>> +/* Save registers on the stack (r15), so we can have stacked >>> interrupts. */ >>> + .macro SAVE_REGS_STACK >>> + /* Allocate a stack frame for 15 general registers */ >>> + slgfi %r15, 15 * 8 >>> + /* Store registers r0 to r14 on the stack */ >>> + stmg %r0, %r14, 0(%r15) >>> + /* Allocate a stack frame for 16 floating point registers */ >>> + /* The size of a FP register is the size of an double word */ >>> + slgfi %r15, 16 * 8 >>> + /* Save fp register on stack: offset to SP is multiple of reg >>> number */ >>> + .irp i, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15 >>> + std \i, \i * 8(%r15) >>> + .endr >>> + /* Save fpc, but keep stack aligned on 64bits */ >>> + slgfi %r15, 8 >>> + efpc %r0 >>> + stg %r0, 0(%r15) >>> + .endm >> >> I wonder whether it would be sufficient to only save the registers here >> that are "volatile" according to the ELF ABI? ... that would save quite >> some space on the stack, I think... OTOH, the old code was also saving >> all registers, so maybe that's something for a separate patch later... > > I don't think so for the general registers > The "volatile" registers are lost during a C call, so it is the duty of > the caller to save them before the call, if he wants, and this is > possible for the programmer or the compiler to arrange that. > > For interruptions, we steal the CPU with all the registers from the > program without warning, the program has no possibility to save them. > So we must save all registers for him. We certainly have to save the registers that are marked as "volatile" in the ELF ABI, no discussion. But what about the others? If we do not touch them in the assembler code, and just jump to a C function, the C function will save them before changing them, and restore the old value before returning. So when the interrupt is done, the registers should contain their original values again, shouldn't they? > For the FP registers, we surely can do something if we establish a usage > convention on the floating point. > A few tests need hardware floating point. According to the ELF ABI, f0 - f7 are volatile, so they must be saved, but f8 - f15 should be saved by the called function instead, so I think we don't need to save them here? Anyway, as I said, that optimization could also be done in a future patch instead. Thomas
On 2020-06-08 17:29, Thomas Huth wrote: > On 08/06/2020 16.24, Pierre Morel wrote: >> >> >> On 2020-06-08 11:05, Thomas Huth wrote: >>> On 08/06/2020 10.12, Pierre Morel wrote: >>>> If we use multiple source of interrupts, for example, using SCLP >>>> console to print information while using I/O interrupts, we need >>>> to have a re-entrant register saving interruption handling. >>>> >>>> Instead of saving at a static memory address, let's save the base >>>> registers, the floating point registers and the floating point >>>> control register on the stack in case of I/O interrupts >>>> >>>> Note that we keep the static register saving to recover from the >>>> RESET tests. >>>> >>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >>>> Acked-by: Janosch Frank <frankja@linux.ibm.com> >>>> --- >>>> s390x/cstart64.S | 41 +++++++++++++++++++++++++++++++++++++++-- >>>> 1 file changed, 39 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/s390x/cstart64.S b/s390x/cstart64.S >>>> index b50c42c..a9d8223 100644 >>>> --- a/s390x/cstart64.S >>>> +++ b/s390x/cstart64.S >>>> @@ -119,6 +119,43 @@ memsetxc: >>>> lmg %r0, %r15, GEN_LC_SW_INT_GRS >>>> .endm >>>> +/* Save registers on the stack (r15), so we can have stacked >>>> interrupts. */ >>>> + .macro SAVE_REGS_STACK >>>> + /* Allocate a stack frame for 15 general registers */ >>>> + slgfi %r15, 15 * 8 >>>> + /* Store registers r0 to r14 on the stack */ >>>> + stmg %r0, %r14, 0(%r15) >>>> + /* Allocate a stack frame for 16 floating point registers */ >>>> + /* The size of a FP register is the size of an double word */ >>>> + slgfi %r15, 16 * 8 >>>> + /* Save fp register on stack: offset to SP is multiple of reg >>>> number */ >>>> + .irp i, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15 >>>> + std \i, \i * 8(%r15) >>>> + .endr >>>> + /* Save fpc, but keep stack aligned on 64bits */ >>>> + slgfi %r15, 8 >>>> + efpc %r0 >>>> + stg %r0, 0(%r15) >>>> + .endm >>> >>> I wonder whether it would be sufficient to only save the registers here >>> that are "volatile" according to the ELF ABI? ... that would save quite >>> some space on the stack, I think... OTOH, the old code was also saving >>> all registers, so maybe that's something for a separate patch later... >> >> I don't think so for the general registers >> The "volatile" registers are lost during a C call, so it is the duty of >> the caller to save them before the call, if he wants, and this is >> possible for the programmer or the compiler to arrange that. >> >> For interruptions, we steal the CPU with all the registers from the >> program without warning, the program has no possibility to save them. >> So we must save all registers for him. > > We certainly have to save the registers that are marked as "volatile" in > the ELF ABI, no discussion. But what about the others? If we do not > touch them in the assembler code, and just jump to a C function, the C > function will save them before changing them, and restore the old value > before returning. So when the interrupt is done, the registers should > contain their original values again, shouldn't they? Sorry, did not read correctly. Yes you are right, saving the "volatile" registers would be enough for the general and the FP registers. Regards, Pierre
diff --git a/s390x/cstart64.S b/s390x/cstart64.S index b50c42c..a9d8223 100644 --- a/s390x/cstart64.S +++ b/s390x/cstart64.S @@ -119,6 +119,43 @@ memsetxc: lmg %r0, %r15, GEN_LC_SW_INT_GRS .endm +/* Save registers on the stack (r15), so we can have stacked interrupts. */ + .macro SAVE_REGS_STACK + /* Allocate a stack frame for 15 general registers */ + slgfi %r15, 15 * 8 + /* Store registers r0 to r14 on the stack */ + stmg %r0, %r14, 0(%r15) + /* Allocate a stack frame for 16 floating point registers */ + /* The size of a FP register is the size of an double word */ + slgfi %r15, 16 * 8 + /* Save fp register on stack: offset to SP is multiple of reg number */ + .irp i, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15 + std \i, \i * 8(%r15) + .endr + /* Save fpc, but keep stack aligned on 64bits */ + slgfi %r15, 8 + efpc %r0 + stg %r0, 0(%r15) + .endm + +/* Restore the register in reverse order */ + .macro RESTORE_REGS_STACK + /* Restore fpc */ + lfpc 0(%r15) + algfi %r15, 8 + /* Restore fp register from stack: SP still where it was left */ + /* and offset to SP is a multiple of reg number */ + .irp i, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15 + ld \i, \i * 8(%r15) + .endr + /* Now that we're done, rewind the stack pointer by 16 double word */ + algfi %r15, 16 * 8 + /* Load the registers from stack */ + lmg %r0, %r14, 0(%r15) + /* Rewind the stack by 15 double word */ + algfi %r15, 15 * 8 + .endm + .section .text /* * load_reset calling convention: @@ -186,9 +223,9 @@ mcck_int: lpswe GEN_LC_MCCK_OLD_PSW io_int: - SAVE_REGS + SAVE_REGS_STACK brasl %r14, handle_io_int - RESTORE_REGS + RESTORE_REGS_STACK lpswe GEN_LC_IO_OLD_PSW svc_int: