| Message ID | 20200525033123.13114-1-yuanjunqing66@163.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | MIPS: Fix IRQ tracing when call handle_fpe() | expand |
Hello! On 25.05.2020 6:31, YuanJunQing wrote: > Register "a1" is unsaved in this function, > when CONFIG_TRACE_IRQFLAGS is enabled, > the TRACE_IRQS_OFF macro will call trace_hardirqs_off(), > and this may change register "a1". > The variment of register "a1" may send SIGFPE signal Variment? > to task when call do_fpe(),and this may kill the task. Need space after comma. > Signed-off-by: YuanJunQing <yuanjunqing66@163.com> > --- > arch/mips/kernel/genex.S | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S > index 8236fb291e3f..956a76429773 100644 > --- a/arch/mips/kernel/genex.S > +++ b/arch/mips/kernel/genex.S > @@ -480,16 +480,18 @@ NESTED(nmi_handler, PT_SIZE, sp) > /* gas fails to assemble cfc1 for some archs (octeon).*/ \ > .set mips1 > SET_HARDFLOAT > - cfc1 a1, fcr31 > + cfc1 s0, fcr31 > .set pop > CLI > TRACE_IRQS_OFF > + move a1,s0 Need space after comma. > .endm > > .macro __build_clear_msa_fpe > - _cfcmsa a1, MSA_CSR > + _cfcmsa s0, MSA_CSR > CLI > TRACE_IRQS_OFF > + move a1,s0 Ditto. [...] MBR, Sergei
On Mon, May 25, 2020 at 11:31:23AM +0800, YuanJunQing wrote: > Register "a1" is unsaved in this function, > when CONFIG_TRACE_IRQFLAGS is enabled, > the TRACE_IRQS_OFF macro will call trace_hardirqs_off(), > and this may change register "a1". > The variment of register "a1" may send SIGFPE signal > to task when call do_fpe(),and this may kill the task. > > Signed-off-by: YuanJunQing <yuanjunqing66@163.com> > --- > arch/mips/kernel/genex.S | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S > index 8236fb291e3f..956a76429773 100644 > --- a/arch/mips/kernel/genex.S > +++ b/arch/mips/kernel/genex.S > @@ -480,16 +480,18 @@ NESTED(nmi_handler, PT_SIZE, sp) > /* gas fails to assemble cfc1 for some archs (octeon).*/ \ > .set mips1 > SET_HARDFLOAT > - cfc1 a1, fcr31 > + cfc1 s0, fcr31 > .set pop > CLI > TRACE_IRQS_OFF > + move a1,s0 > .endm do we realy need to read fcr31 that early ? Wouldn't it work to just move the cfc1 below TRACE_IRQS_OFF ? Thomas.
在 2020/5/25 下午4:42, Thomas Bogendoerfer 写道: > On Mon, May 25, 2020 at 11:31:23AM +0800, YuanJunQing wrote: >> Register "a1" is unsaved in this function, >> when CONFIG_TRACE_IRQFLAGS is enabled, >> the TRACE_IRQS_OFF macro will call trace_hardirqs_off(), >> and this may change register "a1". >> The variment of register "a1" may send SIGFPE signal >> to task when call do_fpe(),and this may kill the task. >> >> Signed-off-by: YuanJunQing <yuanjunqing66@163.com> >> --- >> arch/mips/kernel/genex.S | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S >> index 8236fb291e3f..956a76429773 100644 >> --- a/arch/mips/kernel/genex.S >> +++ b/arch/mips/kernel/genex.S >> @@ -480,16 +480,18 @@ NESTED(nmi_handler, PT_SIZE, sp) >> /* gas fails to assemble cfc1 for some archs (octeon).*/ \ >> .set mips1 >> SET_HARDFLOAT >> - cfc1 a1, fcr31 >> + cfc1 s0, fcr31 >> .set pop >> CLI >> TRACE_IRQS_OFF >> + move a1,s0 >> .endm > do we realy need to read fcr31 that early ? Wouldn't it work to > just move the cfc1 below TRACE_IRQS_OFF ? > > Thomas. yes, it can work when we just move the cfc1 below TRACE_IRQS_OFF, and the code is written as follows. CLI TRACE_IRQS_OFF .set mips1 SET_HARDFLOAT cfc1 a1, fcr31 .set pop .endm
On Tue, May 26, 2020 at 03:07:16PM +0800, yuanjunqing wrote: > > 在 2020/5/25 下午4:42, Thomas Bogendoerfer 写道: > > On Mon, May 25, 2020 at 11:31:23AM +0800, YuanJunQing wrote: > >> Register "a1" is unsaved in this function, > >> when CONFIG_TRACE_IRQFLAGS is enabled, > >> the TRACE_IRQS_OFF macro will call trace_hardirqs_off(), > >> and this may change register "a1". > >> The variment of register "a1" may send SIGFPE signal > >> to task when call do_fpe(),and this may kill the task. > >> > >> Signed-off-by: YuanJunQing <yuanjunqing66@163.com> > >> --- > >> arch/mips/kernel/genex.S | 6 ++++-- > >> 1 file changed, 4 insertions(+), 2 deletions(-) > >> > >> diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S > >> index 8236fb291e3f..956a76429773 100644 > >> --- a/arch/mips/kernel/genex.S > >> +++ b/arch/mips/kernel/genex.S > >> @@ -480,16 +480,18 @@ NESTED(nmi_handler, PT_SIZE, sp) > >> /* gas fails to assemble cfc1 for some archs (octeon).*/ \ > >> .set mips1 > >> SET_HARDFLOAT > >> - cfc1 a1, fcr31 > >> + cfc1 s0, fcr31 > >> .set pop > >> CLI > >> TRACE_IRQS_OFF > >> + move a1,s0 > >> .endm > > do we realy need to read fcr31 that early ? Wouldn't it work to > > just move the cfc1 below TRACE_IRQS_OFF ? > > > > yes, it can work when we just move the cfc1 below TRACE_IRQS_OFF, > and the code is written as follows. > > CLI > TRACE_IRQS_OFF > .set mips1 > SET_HARDFLOAT > cfc1 a1, fcr31 > .set pop > .endm good, could we do the same with _cfcmsa a1, MSA_CSR in the msa case ? Thomas.
On Tue, May 26, 2020 at 02:03:14PM +0800, Lichao Liu wrote: > From: YuanJunQing <yuanjunqing66@163.com> > > Register "a1" is unsaved in this function, > when CONFIG_TRACE_IRQFLAGS is enabled, > the TRACE_IRQS_OFF macro will call trace_hardirqs_off(), > and this may change register "a1". > The variment of register "a1" may send SIGFPE signal > to task when call do_fpe(),and this may kill the task. > > Signed-off-by: YuanJunQing <yuanjunqing66@163.com> if you send patches from other people, please add your Signed-off-by. Thomas.
yes, I will re-send email for this patch. 在 2020/5/26 下午9:04, Thomas Bogendoerfer 写道: > On Tue, May 26, 2020 at 03:07:16PM +0800, yuanjunqing wrote: >> 在 2020/5/25 下午4:42, Thomas Bogendoerfer 写道: >>> On Mon, May 25, 2020 at 11:31:23AM +0800, YuanJunQing wrote: >>>> Register "a1" is unsaved in this function, >>>> when CONFIG_TRACE_IRQFLAGS is enabled, >>>> the TRACE_IRQS_OFF macro will call trace_hardirqs_off(), >>>> and this may change register "a1". >>>> The variment of register "a1" may send SIGFPE signal >>>> to task when call do_fpe(),and this may kill the task. >>>> >>>> Signed-off-by: YuanJunQing <yuanjunqing66@163.com> >>>> --- >>>> arch/mips/kernel/genex.S | 6 ++++-- >>>> 1 file changed, 4 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S >>>> index 8236fb291e3f..956a76429773 100644 >>>> --- a/arch/mips/kernel/genex.S >>>> +++ b/arch/mips/kernel/genex.S >>>> @@ -480,16 +480,18 @@ NESTED(nmi_handler, PT_SIZE, sp) >>>> /* gas fails to assemble cfc1 for some archs (octeon).*/ \ >>>> .set mips1 >>>> SET_HARDFLOAT >>>> - cfc1 a1, fcr31 >>>> + cfc1 s0, fcr31 >>>> .set pop >>>> CLI >>>> TRACE_IRQS_OFF >>>> + move a1,s0 >>>> .endm >>> do we realy need to read fcr31 that early ? Wouldn't it work to >>> just move the cfc1 below TRACE_IRQS_OFF ? >>> >> yes, it can work when we just move the cfc1 below TRACE_IRQS_OFF, >> and the code is written as follows. >> >> CLI >> TRACE_IRQS_OFF >> .set mips1 >> SET_HARDFLOAT >> cfc1 a1, fcr31 >> .set pop >> .endm > good, could we do the same with _cfcmsa a1, MSA_CSR in the msa case ? > > Thomas. >
diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S index 8236fb291e3f..956a76429773 100644 --- a/arch/mips/kernel/genex.S +++ b/arch/mips/kernel/genex.S @@ -480,16 +480,18 @@ NESTED(nmi_handler, PT_SIZE, sp) /* gas fails to assemble cfc1 for some archs (octeon).*/ \ .set mips1 SET_HARDFLOAT - cfc1 a1, fcr31 + cfc1 s0, fcr31 .set pop CLI TRACE_IRQS_OFF + move a1,s0 .endm .macro __build_clear_msa_fpe - _cfcmsa a1, MSA_CSR + _cfcmsa s0, MSA_CSR CLI TRACE_IRQS_OFF + move a1,s0 .endm .macro __build_clear_ade
Register "a1" is unsaved in this function, when CONFIG_TRACE_IRQFLAGS is enabled, the TRACE_IRQS_OFF macro will call trace_hardirqs_off(), and this may change register "a1". The variment of register "a1" may send SIGFPE signal to task when call do_fpe(),and this may kill the task. Signed-off-by: YuanJunQing <yuanjunqing66@163.com> --- arch/mips/kernel/genex.S | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)