RE: [PATCH v11 8/8] PPC: Don't use hardcoded opcode for ePAPR hcall invocation
> -Original Message- > From: Wood Scott-B07421 > Sent: Monday, July 02, 2012 12:25 PM > To: Alexander Graf > Cc: Yoder Stuart-B08248; kvm-...@vger.kernel.org; kvm@vger.kernel.org; Tabi > Timur-B04825 > Subject: Re: [PATCH v11 8/8] PPC: Don't use hardcoded opcode for ePAPR hcall > invocation > > On 07/02/2012 12:17 PM, Alexander Graf wrote: > > > > On 02.07.2012, at 19:16, Scott Wood wrote: > > > >> On 07/02/2012 12:13 PM, Alexander Graf wrote: > >>> > >>> On 02.07.2012, at 19:10, Scott Wood wrote: > >>> > >>>> On 07/02/2012 07:30 AM, Alexander Graf wrote: > >>>>> > >>>>> On 22.06.2012, at 22:06, Stuart Yoder wrote: > >>>>> > >>>>>> From: Liu Yu-B13201 > >>>>>> > >>>>>> Signed-off-by: Liu Yu > >>>>>> Signed-off-by: Stuart Yoder > >>>>>> --- > >>>>>> -v11: no changes > >>>>>> > >>>>>> arch/powerpc/include/asm/epapr_hcalls.h | 22 +- > >>>>>> arch/powerpc/include/asm/fsl_hcalls.h | 36 > >>>>>> +++--- > >>>>>> 2 files changed, 29 insertions(+), 29 deletions(-) > >>>>>> > >>>>>> diff --git a/arch/powerpc/include/asm/epapr_hcalls.h > >>>>>> b/arch/powerpc/include/asm/epapr_hcalls.h > >>>>>> index 833ce2c..b8d9445 100644 > >>>>>> --- a/arch/powerpc/include/asm/epapr_hcalls.h > >>>>>> +++ b/arch/powerpc/include/asm/epapr_hcalls.h > >>>>>> @@ -195,7 +195,7 @@ static inline unsigned int > >>>>>> ev_int_set_config(unsigned int interrupt, > >>>>>>r5 = priority; > >>>>>>r6 = destination; > >>>>>> > >>>>>> - __asm__ __volatile__ ("sc 1" > >>>>>> + asm volatile("blepapr_hypercall_start" > >>>>>>: "+r" (r11), "+r" (r3), "+r" (r4), "+r" (r5), "+r" (r6) > >>>>>>: : EV_HCALL_CLOBBERS4 > >>>>> > >>>>> Hrm. ePAPR hypercalls are allowed to clobber lr, right? But our > >>>>> hypercall entry code depends on lr > staying alive: > >>>> > >>>> ePAPR 1.1 says LR is nonvolatile. > >>> > >>> Why is it in the clobber list then? > >> > >> Because the inline assembly code is clobbering it -- not the hv-provided > >> hcall instructions. > > > > Only after the change, not before it. > > Hmm. The comment says, "XER, CTR, and LR are currently listed as > clobbers because it's uncertain whether they will be clobbered." Maybe > it dates back to when the ABI was still being discussed? Timur, do you > recall? > > In any case, LR is nonvolatile in the spec and in the implementations I > know about (KVM and Topaz). Based on this thread I am going to leave this patch as is. Stuart
Re: [PATCH v11 8/8] PPC: Don't use hardcoded opcode for ePAPR hcall invocation
Scott Wood wrote: >> > So are you saying that it was wrong before, but it's correct now? > Not really *wrong* before, but unnecessary. In that case, my code was really just ahead of its time. :-) -- Timur Tabi Linux kernel developer at Freescale -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v11 8/8] PPC: Don't use hardcoded opcode for ePAPR hcall invocation
On 07/02/2012 12:53 PM, Timur Tabi wrote: > Scott Wood wrote: I'm still a little confused. Which inline assembly code is clobbering LR? Are you talking about the "BL" instruction, which wasn't there before? > >> Yes, I didn't realize that LR had been in the clobber list before that. > > So are you saying that it was wrong before, but it's correct now? Not really *wrong* before, but unnecessary. -Scott -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v11 8/8] PPC: Don't use hardcoded opcode for ePAPR hcall invocation
Scott Wood wrote: >> > I'm still a little confused. Which inline assembly code is clobbering LR? >> > Are you talking about the "BL" instruction, which wasn't there before? > Yes, I didn't realize that LR had been in the clobber list before that. So are you saying that it was wrong before, but it's correct now? -- Timur Tabi Linux kernel developer at Freescale -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v11 8/8] PPC: Don't use hardcoded opcode for ePAPR hcall invocation
On 07/02/2012 12:34 PM, Timur Tabi wrote: > Scott Wood wrote: > >> Hmm. The comment says, "XER, CTR, and LR are currently listed as >> clobbers because it's uncertain whether they will be clobbered." Maybe >> it dates back to when the ABI was still being discussed? Timur, do you >> recall? > > Nope, sorry. I'm sure we discussed this and looked at the code. My guess > is that we weren't certain what the compiler was going to do at the time. I'm not sure how the compiler is involved here -- we're just telling it what our asm code (and the asm code in the hypervisor) will do. > I'm still a little confused. Which inline assembly code is clobbering LR? > Are you talking about the "BL" instruction, which wasn't there before? Yes, I didn't realize that LR had been in the clobber list before that. -Scott -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v11 8/8] PPC: Don't use hardcoded opcode for ePAPR hcall invocation
Scott Wood wrote: > Hmm. The comment says, "XER, CTR, and LR are currently listed as > clobbers because it's uncertain whether they will be clobbered." Maybe > it dates back to when the ABI was still being discussed? Timur, do you > recall? Nope, sorry. I'm sure we discussed this and looked at the code. My guess is that we weren't certain what the compiler was going to do at the time. I'm still a little confused. Which inline assembly code is clobbering LR? Are you talking about the "BL" instruction, which wasn't there before? -- Timur Tabi Linux kernel developer at Freescale -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v11 8/8] PPC: Don't use hardcoded opcode for ePAPR hcall invocation
On 07/02/2012 07:30 AM, Alexander Graf wrote: > > On 22.06.2012, at 22:06, Stuart Yoder wrote: > >> From: Liu Yu-B13201 >> >> Signed-off-by: Liu Yu >> Signed-off-by: Stuart Yoder >> --- >> -v11: no changes >> >> arch/powerpc/include/asm/epapr_hcalls.h | 22 +- >> arch/powerpc/include/asm/fsl_hcalls.h | 36 +++--- >> 2 files changed, 29 insertions(+), 29 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/epapr_hcalls.h >> b/arch/powerpc/include/asm/epapr_hcalls.h >> index 833ce2c..b8d9445 100644 >> --- a/arch/powerpc/include/asm/epapr_hcalls.h >> +++ b/arch/powerpc/include/asm/epapr_hcalls.h >> @@ -195,7 +195,7 @@ static inline unsigned int ev_int_set_config(unsigned >> int interrupt, >> r5 = priority; >> r6 = destination; >> >> -__asm__ __volatile__ ("sc 1" >> +asm volatile("blepapr_hypercall_start" >> : "+r" (r11), "+r" (r3), "+r" (r4), "+r" (r5), "+r" (r6) >> : : EV_HCALL_CLOBBERS4 > > Hrm. ePAPR hypercalls are allowed to clobber lr, right? But our hypercall > entry code depends on lr staying alive: ePAPR 1.1 says LR is nonvolatile. -Scott -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v11 8/8] PPC: Don't use hardcoded opcode for ePAPR hcall invocation
On 07/02/2012 12:17 PM, Alexander Graf wrote: > > On 02.07.2012, at 19:16, Scott Wood wrote: > >> On 07/02/2012 12:13 PM, Alexander Graf wrote: >>> >>> On 02.07.2012, at 19:10, Scott Wood wrote: >>> On 07/02/2012 07:30 AM, Alexander Graf wrote: > > On 22.06.2012, at 22:06, Stuart Yoder wrote: > >> From: Liu Yu-B13201 >> >> Signed-off-by: Liu Yu >> Signed-off-by: Stuart Yoder >> --- >> -v11: no changes >> >> arch/powerpc/include/asm/epapr_hcalls.h | 22 +- >> arch/powerpc/include/asm/fsl_hcalls.h | 36 >> +++--- >> 2 files changed, 29 insertions(+), 29 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/epapr_hcalls.h >> b/arch/powerpc/include/asm/epapr_hcalls.h >> index 833ce2c..b8d9445 100644 >> --- a/arch/powerpc/include/asm/epapr_hcalls.h >> +++ b/arch/powerpc/include/asm/epapr_hcalls.h >> @@ -195,7 +195,7 @@ static inline unsigned int >> ev_int_set_config(unsigned int interrupt, >> r5 = priority; >> r6 = destination; >> >> -__asm__ __volatile__ ("sc 1" >> +asm volatile("blepapr_hypercall_start" >> : "+r" (r11), "+r" (r3), "+r" (r4), "+r" (r5), "+r" (r6) >> : : EV_HCALL_CLOBBERS4 > > Hrm. ePAPR hypercalls are allowed to clobber lr, right? But our hypercall > entry code depends on lr staying alive: ePAPR 1.1 says LR is nonvolatile. >>> >>> Why is it in the clobber list then? >> >> Because the inline assembly code is clobbering it -- not the hv-provided >> hcall instructions. > > Only after the change, not before it. Hmm. The comment says, "XER, CTR, and LR are currently listed as clobbers because it's uncertain whether they will be clobbered." Maybe it dates back to when the ABI was still being discussed? Timur, do you recall? In any case, LR is nonvolatile in the spec and in the implementations I know about (KVM and Topaz). -Scott -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v11 8/8] PPC: Don't use hardcoded opcode for ePAPR hcall invocation
On 02.07.2012, at 19:16, Scott Wood wrote: > On 07/02/2012 12:13 PM, Alexander Graf wrote: >> >> On 02.07.2012, at 19:10, Scott Wood wrote: >> >>> On 07/02/2012 07:30 AM, Alexander Graf wrote: On 22.06.2012, at 22:06, Stuart Yoder wrote: > From: Liu Yu-B13201 > > Signed-off-by: Liu Yu > Signed-off-by: Stuart Yoder > --- > -v11: no changes > > arch/powerpc/include/asm/epapr_hcalls.h | 22 +- > arch/powerpc/include/asm/fsl_hcalls.h | 36 > +++--- > 2 files changed, 29 insertions(+), 29 deletions(-) > > diff --git a/arch/powerpc/include/asm/epapr_hcalls.h > b/arch/powerpc/include/asm/epapr_hcalls.h > index 833ce2c..b8d9445 100644 > --- a/arch/powerpc/include/asm/epapr_hcalls.h > +++ b/arch/powerpc/include/asm/epapr_hcalls.h > @@ -195,7 +195,7 @@ static inline unsigned int ev_int_set_config(unsigned > int interrupt, > r5 = priority; > r6 = destination; > > - __asm__ __volatile__ ("sc 1" > + asm volatile("blepapr_hypercall_start" > : "+r" (r11), "+r" (r3), "+r" (r4), "+r" (r5), "+r" (r6) > : : EV_HCALL_CLOBBERS4 Hrm. ePAPR hypercalls are allowed to clobber lr, right? But our hypercall entry code depends on lr staying alive: >>> >>> ePAPR 1.1 says LR is nonvolatile. >> >> Why is it in the clobber list then? > > Because the inline assembly code is clobbering it -- not the hv-provided > hcall instructions. Only after the change, not before it. Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v11 8/8] PPC: Don't use hardcoded opcode for ePAPR hcall invocation
On 07/02/2012 12:13 PM, Alexander Graf wrote: > > On 02.07.2012, at 19:10, Scott Wood wrote: > >> On 07/02/2012 07:30 AM, Alexander Graf wrote: >>> >>> On 22.06.2012, at 22:06, Stuart Yoder wrote: >>> From: Liu Yu-B13201 Signed-off-by: Liu Yu Signed-off-by: Stuart Yoder --- -v11: no changes arch/powerpc/include/asm/epapr_hcalls.h | 22 +- arch/powerpc/include/asm/fsl_hcalls.h | 36 +++--- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/arch/powerpc/include/asm/epapr_hcalls.h b/arch/powerpc/include/asm/epapr_hcalls.h index 833ce2c..b8d9445 100644 --- a/arch/powerpc/include/asm/epapr_hcalls.h +++ b/arch/powerpc/include/asm/epapr_hcalls.h @@ -195,7 +195,7 @@ static inline unsigned int ev_int_set_config(unsigned int interrupt, r5 = priority; r6 = destination; - __asm__ __volatile__ ("sc 1" + asm volatile("blepapr_hypercall_start" : "+r" (r11), "+r" (r3), "+r" (r4), "+r" (r5), "+r" (r6) : : EV_HCALL_CLOBBERS4 >>> >>> Hrm. ePAPR hypercalls are allowed to clobber lr, right? But our hypercall >>> entry code depends on lr staying alive: >> >> ePAPR 1.1 says LR is nonvolatile. > > Why is it in the clobber list then? Because the inline assembly code is clobbering it -- not the hv-provided hcall instructions. -Scott -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v11 8/8] PPC: Don't use hardcoded opcode for ePAPR hcall invocation
On 02.07.2012, at 19:10, Scott Wood wrote: > On 07/02/2012 07:30 AM, Alexander Graf wrote: >> >> On 22.06.2012, at 22:06, Stuart Yoder wrote: >> >>> From: Liu Yu-B13201 >>> >>> Signed-off-by: Liu Yu >>> Signed-off-by: Stuart Yoder >>> --- >>> -v11: no changes >>> >>> arch/powerpc/include/asm/epapr_hcalls.h | 22 +- >>> arch/powerpc/include/asm/fsl_hcalls.h | 36 >>> +++--- >>> 2 files changed, 29 insertions(+), 29 deletions(-) >>> >>> diff --git a/arch/powerpc/include/asm/epapr_hcalls.h >>> b/arch/powerpc/include/asm/epapr_hcalls.h >>> index 833ce2c..b8d9445 100644 >>> --- a/arch/powerpc/include/asm/epapr_hcalls.h >>> +++ b/arch/powerpc/include/asm/epapr_hcalls.h >>> @@ -195,7 +195,7 @@ static inline unsigned int ev_int_set_config(unsigned >>> int interrupt, >>> r5 = priority; >>> r6 = destination; >>> >>> - __asm__ __volatile__ ("sc 1" >>> + asm volatile("blepapr_hypercall_start" >>> : "+r" (r11), "+r" (r3), "+r" (r4), "+r" (r5), "+r" (r6) >>> : : EV_HCALL_CLOBBERS4 >> >> Hrm. ePAPR hypercalls are allowed to clobber lr, right? But our hypercall >> entry code depends on lr staying alive: > > ePAPR 1.1 says LR is nonvolatile. Why is it in the clobber list then? Not complaining - we need to have it in there for the bl to work out - but still surprised. arch/powerpc/include/asm/epapr_hcalls.h:#define EV_HCALL_CLOBBERS "r0", "r12", "xer", "ctr", "lr", "cc", "memory" Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v11 8/8] PPC: Don't use hardcoded opcode for ePAPR hcall invocation
On 22.06.2012, at 22:06, Stuart Yoder wrote: > From: Liu Yu-B13201 > > Signed-off-by: Liu Yu > Signed-off-by: Stuart Yoder > --- > -v11: no changes > > arch/powerpc/include/asm/epapr_hcalls.h | 22 +- > arch/powerpc/include/asm/fsl_hcalls.h | 36 +++--- > 2 files changed, 29 insertions(+), 29 deletions(-) > > diff --git a/arch/powerpc/include/asm/epapr_hcalls.h > b/arch/powerpc/include/asm/epapr_hcalls.h > index 833ce2c..b8d9445 100644 > --- a/arch/powerpc/include/asm/epapr_hcalls.h > +++ b/arch/powerpc/include/asm/epapr_hcalls.h > @@ -195,7 +195,7 @@ static inline unsigned int ev_int_set_config(unsigned int > interrupt, > r5 = priority; > r6 = destination; > > - __asm__ __volatile__ ("sc 1" > + asm volatile("blepapr_hypercall_start" > : "+r" (r11), "+r" (r3), "+r" (r4), "+r" (r5), "+r" (r6) > : : EV_HCALL_CLOBBERS4 Hrm. ePAPR hypercalls are allowed to clobber lr, right? But our hypercall entry code depends on lr staying alive: .global epapr_hypercall_start epapr_hypercall_start: li r3, -1 nop nop nop blr So I suppose for this to work, we'd have to store lr off to the stack before doing the hypercall in epapr_hypercall_start. Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v11 8/8] PPC: Don't use hardcoded opcode for ePAPR hcall invocation
From: Liu Yu-B13201 Signed-off-by: Liu Yu Signed-off-by: Stuart Yoder --- -v11: no changes arch/powerpc/include/asm/epapr_hcalls.h | 22 +- arch/powerpc/include/asm/fsl_hcalls.h | 36 +++--- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/arch/powerpc/include/asm/epapr_hcalls.h b/arch/powerpc/include/asm/epapr_hcalls.h index 833ce2c..b8d9445 100644 --- a/arch/powerpc/include/asm/epapr_hcalls.h +++ b/arch/powerpc/include/asm/epapr_hcalls.h @@ -195,7 +195,7 @@ static inline unsigned int ev_int_set_config(unsigned int interrupt, r5 = priority; r6 = destination; - __asm__ __volatile__ ("sc 1" + asm volatile("blepapr_hypercall_start" : "+r" (r11), "+r" (r3), "+r" (r4), "+r" (r5), "+r" (r6) : : EV_HCALL_CLOBBERS4 ); @@ -224,7 +224,7 @@ static inline unsigned int ev_int_get_config(unsigned int interrupt, r11 = EV_HCALL_TOKEN(EV_INT_GET_CONFIG); r3 = interrupt; - __asm__ __volatile__ ("sc 1" + asm volatile("blepapr_hypercall_start" : "+r" (r11), "+r" (r3), "=r" (r4), "=r" (r5), "=r" (r6) : : EV_HCALL_CLOBBERS4 ); @@ -254,7 +254,7 @@ static inline unsigned int ev_int_set_mask(unsigned int interrupt, r3 = interrupt; r4 = mask; - __asm__ __volatile__ ("sc 1" + asm volatile("blepapr_hypercall_start" : "+r" (r11), "+r" (r3), "+r" (r4) : : EV_HCALL_CLOBBERS2 ); @@ -279,7 +279,7 @@ static inline unsigned int ev_int_get_mask(unsigned int interrupt, r11 = EV_HCALL_TOKEN(EV_INT_GET_MASK); r3 = interrupt; - __asm__ __volatile__ ("sc 1" + asm volatile("blepapr_hypercall_start" : "+r" (r11), "+r" (r3), "=r" (r4) : : EV_HCALL_CLOBBERS2 ); @@ -307,7 +307,7 @@ static inline unsigned int ev_int_eoi(unsigned int interrupt) r11 = EV_HCALL_TOKEN(EV_INT_EOI); r3 = interrupt; - __asm__ __volatile__ ("sc 1" + asm volatile("blepapr_hypercall_start" : "+r" (r11), "+r" (r3) : : EV_HCALL_CLOBBERS1 ); @@ -346,7 +346,7 @@ static inline unsigned int ev_byte_channel_send(unsigned int handle, r7 = be32_to_cpu(p[2]); r8 = be32_to_cpu(p[3]); - __asm__ __volatile__ ("sc 1" + asm volatile("blepapr_hypercall_start" : "+r" (r11), "+r" (r3), "+r" (r4), "+r" (r5), "+r" (r6), "+r" (r7), "+r" (r8) : : EV_HCALL_CLOBBERS6 @@ -385,7 +385,7 @@ static inline unsigned int ev_byte_channel_receive(unsigned int handle, r3 = handle; r4 = *count; - __asm__ __volatile__ ("sc 1" + asm volatile("blepapr_hypercall_start" : "+r" (r11), "+r" (r3), "+r" (r4), "=r" (r5), "=r" (r6), "=r" (r7), "=r" (r8) : : EV_HCALL_CLOBBERS6 @@ -423,7 +423,7 @@ static inline unsigned int ev_byte_channel_poll(unsigned int handle, r11 = EV_HCALL_TOKEN(EV_BYTE_CHANNEL_POLL); r3 = handle; - __asm__ __volatile__ ("sc 1" + asm volatile("blepapr_hypercall_start" : "+r" (r11), "+r" (r3), "=r" (r4), "=r" (r5) : : EV_HCALL_CLOBBERS3 ); @@ -456,7 +456,7 @@ static inline unsigned int ev_int_iack(unsigned int handle, r11 = EV_HCALL_TOKEN(EV_INT_IACK); r3 = handle; - __asm__ __volatile__ ("sc 1" + asm volatile("blepapr_hypercall_start" : "+r" (r11), "+r" (r3), "=r" (r4) : : EV_HCALL_CLOBBERS2 ); @@ -480,7 +480,7 @@ static inline unsigned int ev_doorbell_send(unsigned int handle) r11 = EV_HCALL_TOKEN(EV_DOORBELL_SEND); r3 = handle; - __asm__ __volatile__ ("sc 1" + asm volatile("blepapr_hypercall_start" : "+r" (r11), "+r" (r3) : : EV_HCALL_CLOBBERS1 ); @@ -500,7 +500,7 @@ static inline unsigned int ev_idle(void) r11 = EV_HCALL_TOKEN(EV_IDLE); - __asm__ __volatile__ ("sc 1" + asm volatile("blepapr_hypercall_start" : "+r" (r11), "=r" (r3) : : EV_HCALL_CLOBBERS1 ); diff --git a/arch/powerpc/include/asm/fsl_hcalls.h b/arch/powerpc/include/asm/fsl_hcalls.h index 922d9b5..3abb583 100644 --- a/arch/powerpc/include/asm/fsl_hcalls.h +++ b/arch/powerpc/include/asm/fsl_hcalls.h @@ -96,7 +96,7 @@ static inline unsigned int fh_send_nmi(unsigned int vcpu_mask) r11 = FH_HCALL_TOKEN(FH_SEND_NMI); r3 = vcpu_mask; - __asm__ __volatile__ ("sc 1" + asm volatile("blepapr_hypercall_start" : "+r" (r11), "+r" (r3) : : EV_HCALL_CLOBBERS1 ); @@ -151,7 +151,7 @@ static inline unsigned int fh_partition_get_dtprop(int han