Re: Re: [PATCH] powerpc/64: Set LR to a non-NULL value in task pt_regs on scv entry
On Fri, Feb 02, 2024 at 01:02:39PM +1100, Michael Ellerman wrote: > Segher Boessenkool writes: > > Hi! > > > > On Thu, Jan 25, 2024 at 05:12:28PM +0530, Naveen N Rao wrote: > >> diff --git a/arch/powerpc/kernel/interrupt_64.S > >> b/arch/powerpc/kernel/interrupt_64.S > >> index bd863702d812..5cf3758a19d3 100644 > >> --- a/arch/powerpc/kernel/interrupt_64.S > >> +++ b/arch/powerpc/kernel/interrupt_64.S > >> @@ -53,6 +53,7 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_\name) > >>ld r1,PACAKSAVE(r13) > >>std r10,0(r1) > >>std r11,_NIP(r1) > >> + std r11,_LINK(r1) > > > > Please add a comment here then, saying what the store is for? > > Yeah a comment would be good. > > Also the r11 value comes from LR, so it's not that we're storing the NIP > value into the LR slot, rather the value we store in NIP is from LR, see: > > EXC_VIRT_BEGIN(system_call_vectored, 0x3000, 0x1000) > /* SCV 0 */ > mr r9,r13 > GET_PACA(r13) > mflrr11 > ... > b system_call_vectored_common > > That's slightly pedantic, but I think it answers the question of why > it's OK to use the same value for NIP & LR, or why we don't have to do > mflr in system_call_vectored_common to get the actual LR value. Thanks for clarifying that. I should have done a better job describing that in the commit log. I'll update that, add a comment here and send a v2. - Naveen
Re: [PATCH] powerpc/64: Set LR to a non-NULL value in task pt_regs on scv entry
Segher Boessenkool writes: > Hi! > > On Thu, Jan 25, 2024 at 05:12:28PM +0530, Naveen N Rao wrote: >> diff --git a/arch/powerpc/kernel/interrupt_64.S >> b/arch/powerpc/kernel/interrupt_64.S >> index bd863702d812..5cf3758a19d3 100644 >> --- a/arch/powerpc/kernel/interrupt_64.S >> +++ b/arch/powerpc/kernel/interrupt_64.S >> @@ -53,6 +53,7 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_\name) >> ld r1,PACAKSAVE(r13) >> std r10,0(r1) >> std r11,_NIP(r1) >> +std r11,_LINK(r1) > > Please add a comment here then, saying what the store is for? Yeah a comment would be good. Also the r11 value comes from LR, so it's not that we're storing the NIP value into the LR slot, rather the value we store in NIP is from LR, see: EXC_VIRT_BEGIN(system_call_vectored, 0x3000, 0x1000) /* SCV 0 */ mr r9,r13 GET_PACA(r13) mflrr11 ... b system_call_vectored_common That's slightly pedantic, but I think it answers the question of why it's OK to use the same value for NIP & LR, or why we don't have to do mflr in system_call_vectored_common to get the actual LR value. cheers
Re: [PATCH] powerpc/64: Set LR to a non-NULL value in task pt_regs on scv entry
Hi! On Thu, Jan 25, 2024 at 05:12:28PM +0530, Naveen N Rao wrote: > diff --git a/arch/powerpc/kernel/interrupt_64.S > b/arch/powerpc/kernel/interrupt_64.S > index bd863702d812..5cf3758a19d3 100644 > --- a/arch/powerpc/kernel/interrupt_64.S > +++ b/arch/powerpc/kernel/interrupt_64.S > @@ -53,6 +53,7 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_\name) > ld r1,PACAKSAVE(r13) > std r10,0(r1) > std r11,_NIP(r1) > + std r11,_LINK(r1) Please add a comment here then, saying what the store is for? Segher
