| Message ID | 20230220052355.109033-2-kconsul@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Improving calls to kvmppc_hv_entry | expand |
Hi Kautuk, On 20/02/23 10:53, Kautuk Consul wrote: > kvmppc_hv_entry isn't called from anywhere other than > book3s_hv_rmhandlers.S itself. Removing .global scope for > this function. > > Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com> > --- > arch/powerpc/kvm/book3s_hv_rmhandlers.S | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > index acf80915f406..7e063fde7adc 100644 > --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S > +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > @@ -502,7 +502,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) > * * > *****************************************************************************/ > > -.global kvmppc_hv_entry > kvmppc_hv_entry: > > /* Required state: I see the following objtool warning with this patch applied. arch/powerpc/kvm/book3s_hv_rmhandlers.o: warning: objtool: .text+0x48: unannotated intra-function call Annotating kvmppc_hv_entry symbol with SYM_FUNC_START_LOCAL and SYM_FUNC_END macros should help fix this warning. Thanks, Sathvika
Hi Sathvika, (Sorry didn't include list in earlier email.) On Mon, Feb 20, 2023 at 12:35:09PM +0530, Sathvika Vasireddy wrote: > Hi Kautuk, > > On 20/02/23 10:53, Kautuk Consul wrote: > > kvmppc_hv_entry isn't called from anywhere other than > > book3s_hv_rmhandlers.S itself. Removing .global scope for > > this function. > > > > Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com> > > --- > > arch/powerpc/kvm/book3s_hv_rmhandlers.S | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > index acf80915f406..7e063fde7adc 100644 > > --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > @@ -502,7 +502,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) > > * * > > *****************************************************************************/ > > -.global kvmppc_hv_entry > > kvmppc_hv_entry: > > /* Required state: > I see the following objtool warning with this patch applied. > arch/powerpc/kvm/book3s_hv_rmhandlers.o: warning: objtool: .text+0x48: > unannotated intra-function call > > Annotating kvmppc_hv_entry symbol with SYM_FUNC_START_LOCAL and SYM_FUNC_END > macros should help fix this warning. Not sure where to put the SYM_FUNC_END annotation. Will the following do: <snip> ld r0, VCPU_GPR(R0)(r4) ld r2, VCPU_GPR(R2)(r4) ld r3, VCPU_GPR(R3)(r4) ld r4, VCPU_GPR(R4)(r4) HRFI_TO_GUEST b . SYM_FUNC_END(kvmppc_hv_entry) secondary_too_late: li r12, 0 ? Thanks. > > Thanks, > Sathvika > >
On 20/02/23 12:58, Kautuk Consul wrote: > Hi Sathvika, > > (Sorry didn't include list in earlier email.) > > On Mon, Feb 20, 2023 at 12:35:09PM +0530, Sathvika Vasireddy wrote: >> Hi Kautuk, >> >> On 20/02/23 10:53, Kautuk Consul wrote: >>> kvmppc_hv_entry isn't called from anywhere other than >>> book3s_hv_rmhandlers.S itself. Removing .global scope for >>> this function. >>> >>> Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com> >>> --- >>> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 1 - >>> 1 file changed, 1 deletion(-) >>> >>> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S >>> index acf80915f406..7e063fde7adc 100644 >>> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S >>> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S >>> @@ -502,7 +502,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) >>> * * >>> *****************************************************************************/ >>> -.global kvmppc_hv_entry >>> kvmppc_hv_entry: >>> /* Required state: >> I see the following objtool warning with this patch applied. >> arch/powerpc/kvm/book3s_hv_rmhandlers.o: warning: objtool: .text+0x48: >> unannotated intra-function call >> >> Annotating kvmppc_hv_entry symbol with SYM_FUNC_START_LOCAL and SYM_FUNC_END >> macros should help fix this warning. > Not sure where to put the SYM_FUNC_END annotation. > Will the following do: > <snip> > ld r0, VCPU_GPR(R0)(r4) > ld r2, VCPU_GPR(R2)(r4) > ld r3, VCPU_GPR(R3)(r4) > ld r4, VCPU_GPR(R4)(r4) > HRFI_TO_GUEST > b . > > SYM_FUNC_END(kvmppc_hv_entry) > > secondary_too_late: > li r12, 0 > > ? > > Thanks. Placing SYM_FUNC_END(kvmppc_hv_entry) before kvmppc_got_guest() should do: @@ -502,12 +500,10 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) * * *****************************************************************************/ -.global kvmppc_hv_entry -kvmppc_hv_entry: +SYM_FUNC_START_LOCAL(kvmppc_hv_entry) /* Required state: * - * R4 = vcpu pointer (or NULL) * MSR = ~IR|DR * R13 = PACA * R1 = host R1 @@ -525,6 +521,8 @@ kvmppc_hv_entry: li r6, KVM_GUEST_MODE_HOST_HV stb r6, HSTATE_IN_GUEST(r13) + ld r4, HSTATE_KVM_VCPU(r13) + #ifdef CONFIG_KVM_BOOK3S_HV_P8_TIMING /* Store initial timestamp */ cmpdi r4, 0 @@ -619,6 +617,8 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) /* Do we have a guest vcpu to run? */ 10: cmpdi r4, 0 beq kvmppc_primary_no_guest +SYM_FUNC_END(kvmppc_hv_entry) + kvmppc_got_guest: /* Increment yield count if they have a VPA */ ld r3, VCPU_VPA(r4) Thanks, Sathvika
On Mon, Feb 20, 2023 at 01:31:40PM +0530, Sathvika Vasireddy wrote: > Placing SYM_FUNC_END(kvmppc_hv_entry) before kvmppc_got_guest() should do: > > @@ -502,12 +500,10 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) > * * > *****************************************************************************/ > > -.global kvmppc_hv_entry > -kvmppc_hv_entry: > +SYM_FUNC_START_LOCAL(kvmppc_hv_entry) > > /* Required state: > * > - * R4 = vcpu pointer (or NULL) > * MSR = ~IR|DR > * R13 = PACA > * R1 = host R1 > @@ -525,6 +521,8 @@ kvmppc_hv_entry: > li r6, KVM_GUEST_MODE_HOST_HV > stb r6, HSTATE_IN_GUEST(r13) > > + ld r4, HSTATE_KVM_VCPU(r13) > + > #ifdef CONFIG_KVM_BOOK3S_HV_P8_TIMING > /* Store initial timestamp */ > cmpdi r4, 0 > @@ -619,6 +617,8 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) > /* Do we have a guest vcpu to run? */ > 10: cmpdi r4, 0 > beq kvmppc_primary_no_guest > +SYM_FUNC_END(kvmppc_hv_entry) > + > kvmppc_got_guest: > /* Increment yield count if they have a VPA */ > ld r3, VCPU_VPA(r4) > Thanks! Will send out a v2 after I get some response for PATCH 2/2 with comments. > > Thanks, > Sathvika
On Mon, Feb 20, 2023 at 01:41:38PM +0530, Kautuk Consul wrote: > On Mon, Feb 20, 2023 at 01:31:40PM +0530, Sathvika Vasireddy wrote: > > Placing SYM_FUNC_END(kvmppc_hv_entry) before kvmppc_got_guest() should do: > > > > @@ -502,12 +500,10 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) > > * * > > *****************************************************************************/ > > > > -.global kvmppc_hv_entry > > -kvmppc_hv_entry: > > +SYM_FUNC_START_LOCAL(kvmppc_hv_entry) > > > > /* Required state: > > * > > - * R4 = vcpu pointer (or NULL) > > * MSR = ~IR|DR > > * R13 = PACA > > * R1 = host R1 > > @@ -525,6 +521,8 @@ kvmppc_hv_entry: > > li r6, KVM_GUEST_MODE_HOST_HV > > stb r6, HSTATE_IN_GUEST(r13) > > > > + ld r4, HSTATE_KVM_VCPU(r13) > > + > > #ifdef CONFIG_KVM_BOOK3S_HV_P8_TIMING > > /* Store initial timestamp */ > > cmpdi r4, 0 > > @@ -619,6 +617,8 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) > > /* Do we have a guest vcpu to run? */ > > 10: cmpdi r4, 0 > > beq kvmppc_primary_no_guest > > +SYM_FUNC_END(kvmppc_hv_entry) Just one question though. Went through the code again and I think that this place shouldn't be proper to insert a SYM_FUNC_END because we haven't entered the guest at this point and the name of the function is kvmppc_hv_entry which I think implies that this SYM_FUNC_END should be at some place after the HRFI_TO_GUEST. What do you think ? > > + > > kvmppc_got_guest: > > /* Increment yield count if they have a VPA */ > > ld r3, VCPU_VPA(r4) > > > > Thanks! Will send out a v2 after I get some response for > PATCH 2/2 with comments. > > > > Thanks, > > Sathvika
Hi Sathvika, > > Just one question though. Went through the code again and I think > that this place shouldn't be proper to insert a SYM_FUNC_END > because we haven't entered the guest at this point and the name > of the function is kvmppc_hv_entry which I think implies that > this SYM_FUNC_END should be at some place after the HRFI_TO_GUEST. > > What do you think ? Any updates on this ? Is there any other way to avoid this warning ?
On 23/02/23 10:39, Kautuk Consul wrote: > Hi Sathvika, >> Just one question though. Went through the code again and I think >> that this place shouldn't be proper to insert a SYM_FUNC_END >> because we haven't entered the guest at this point and the name >> of the function is kvmppc_hv_entry which I think implies that >> this SYM_FUNC_END should be at some place after the HRFI_TO_GUEST. >> >> What do you think ? > Any updates on this ? Is there any other way to avoid this warning ? Hmm, to mark the end of the kvmppc_hv_entry function, I think SYM_FUNC_END(kvmppc_hv_entry) should be placed before the next symbol, which is kvmppc_got_guest() in this case. However, if you think it needs to be put at a different place, then it does not make sense to have any other symbols before that. You may want to consider checking if other macros like SYM_INNER_LABEL() can be used. - Sathvika
On 2023-02-24 16:45:45, Sathvika Vasireddy wrote: > On 23/02/23 10:39, Kautuk Consul wrote: > > > Hi Sathvika, > > > Just one question though. Went through the code again and I think > > > that this place shouldn't be proper to insert a SYM_FUNC_END > > > because we haven't entered the guest at this point and the name > > > of the function is kvmppc_hv_entry which I think implies that > > > this SYM_FUNC_END should be at some place after the HRFI_TO_GUEST. > > > > > > What do you think ? > > Any updates on this ? Is there any other way to avoid this warning ? > Hmm, to mark the end of the kvmppc_hv_entry function, I think > SYM_FUNC_END(kvmppc_hv_entry) should be placed before the next symbol, which > is kvmppc_got_guest() in this case. > > However, if you think it needs to be put at a different place, then it does > not make sense to have any other symbols before that. You may want to > consider checking if other macros like SYM_INNER_LABEL() can be used. SYM_INNER_LABEL works fine for me. I will post a v2 with this change. Thanks! :-) > > - Sathvika >
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S index acf80915f406..7e063fde7adc 100644 --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S @@ -502,7 +502,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) * * *****************************************************************************/ -.global kvmppc_hv_entry kvmppc_hv_entry: /* Required state:
kvmppc_hv_entry isn't called from anywhere other than book3s_hv_rmhandlers.S itself. Removing .global scope for this function. Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com> --- arch/powerpc/kvm/book3s_hv_rmhandlers.S | 1 - 1 file changed, 1 deletion(-)