| Message ID | 20230316051025.1424093-2-kconsul@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Improving calls to kvmppc_hv_entry | expand |
On Thu Mar 16, 2023 at 3:10 PM AEST, Kautuk Consul wrote: > kvmppc_hv_entry isn't called from anywhere other than > book3s_hv_rmhandlers.S itself. Removing .global scope for > this function and annotating it with SYM_INNER_LABEL. > > Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com> > --- > arch/powerpc/kvm/book3s_hv_rmhandlers.S | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > index acf80915f406..b81ba4ee0521 100644 > --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S > +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > @@ -502,8 +502,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) > * * > *****************************************************************************/ > > -.global kvmppc_hv_entry I think this is okay. > -kvmppc_hv_entry: > +SYM_INNER_LABEL(kvmppc_hv_entry, SYM_L_LOCAL) The documentation for SYM_INNER_LABEL says it for labels inside a SYM function block, is that a problem? This is a function but doesn't have C calling convention, so asm annotation docs say that it should use SYM_CODE_START_LOCAL? BTW. why don't our _GLOBAL() macros use these SYM annotations? I haven't really looked into them. Thanks, Nick
On 2023-03-27 19:19:37, Nicholas Piggin wrote: > On Thu Mar 16, 2023 at 3:10 PM AEST, Kautuk Consul wrote: > > kvmppc_hv_entry isn't called from anywhere other than > > book3s_hv_rmhandlers.S itself. Removing .global scope for > > this function and annotating it with SYM_INNER_LABEL. > > > > Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com> > > --- > > arch/powerpc/kvm/book3s_hv_rmhandlers.S | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > index acf80915f406..b81ba4ee0521 100644 > > --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > @@ -502,8 +502,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) > > * * > > *****************************************************************************/ > > > > -.global kvmppc_hv_entry > > I think this is okay. > > > -kvmppc_hv_entry: > > +SYM_INNER_LABEL(kvmppc_hv_entry, SYM_L_LOCAL) > > The documentation for SYM_INNER_LABEL says it for labels inside a SYM > function block, is that a problem? This is a function but doesn't have > C calling convention, so asm annotation docs say that it should use > SYM_CODE_START_LOCAL? That is correct. Will create a v4 patch for this and send it. > > BTW. why don't our _GLOBAL() macros use these SYM annotations? I haven't > really looked into them. Not sure. Was mostly just concentrating on the kvmppc_hv_entry code. > > Thanks, > Nick
On 2023-03-27 14:58:03, Kautuk Consul wrote: > On 2023-03-27 19:19:37, Nicholas Piggin wrote: > > On Thu Mar 16, 2023 at 3:10 PM AEST, Kautuk Consul wrote: > > > kvmppc_hv_entry isn't called from anywhere other than > > > book3s_hv_rmhandlers.S itself. Removing .global scope for > > > this function and annotating it with SYM_INNER_LABEL. > > > > > > Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com> > > > --- > > > arch/powerpc/kvm/book3s_hv_rmhandlers.S | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > > index acf80915f406..b81ba4ee0521 100644 > > > --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > > +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > > @@ -502,8 +502,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) > > > * * > > > *****************************************************************************/ > > > > > > -.global kvmppc_hv_entry > > > > I think this is okay. > > > > > -kvmppc_hv_entry: > > > +SYM_INNER_LABEL(kvmppc_hv_entry, SYM_L_LOCAL) > > > > The documentation for SYM_INNER_LABEL says it for labels inside a SYM > > function block, is that a problem? This is a function but doesn't have > > C calling convention, so asm annotation docs say that it should use > > SYM_CODE_START_LOCAL? > That is correct. Will create a v4 patch for this and send it. But using SYM_CODE_START_LOCAL again causes a warning in the build (which we were trying to avoid): arch/powerpc/kvm/book3s_hv_rmhandlers.o: warning: objtool: .text+0x48: unannotated intra-function call > > > > BTW. why don't our _GLOBAL() macros use these SYM annotations? I haven't > > really looked into them. > Not sure. Was mostly just concentrating on the kvmppc_hv_entry code. > > > > Thanks, > > Nick
On 2023-03-27 15:04:38, Kautuk Consul wrote: > On 2023-03-27 14:58:03, Kautuk Consul wrote: > > On 2023-03-27 19:19:37, Nicholas Piggin wrote: > > > On Thu Mar 16, 2023 at 3:10 PM AEST, Kautuk Consul wrote: > > > > kvmppc_hv_entry isn't called from anywhere other than > > > > book3s_hv_rmhandlers.S itself. Removing .global scope for > > > > this function and annotating it with SYM_INNER_LABEL. > > > > > > > > Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com> > > > > --- > > > > arch/powerpc/kvm/book3s_hv_rmhandlers.S | 3 +-- > > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > > > diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > > > index acf80915f406..b81ba4ee0521 100644 > > > > --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > > > +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > > > @@ -502,8 +502,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) > > > > * * > > > > *****************************************************************************/ > > > > > > > > -.global kvmppc_hv_entry > > > > > > I think this is okay. > > > > > > > -kvmppc_hv_entry: > > > > +SYM_INNER_LABEL(kvmppc_hv_entry, SYM_L_LOCAL) > > > > > > The documentation for SYM_INNER_LABEL says it for labels inside a SYM > > > function block, is that a problem? This is a function but doesn't have > > > C calling convention, so asm annotation docs say that it should use > > > SYM_CODE_START_LOCAL? > > That is correct. Will create a v4 patch for this and send it. > But using SYM_CODE_START_LOCAL again causes a warning in the build > (which we were trying to avoid): > arch/powerpc/kvm/book3s_hv_rmhandlers.o: warning: objtool: .text+0x48: unannotated intra-function call For that matter, even SYM_INNER_LABEL shows that warning. When I tested it on my setup it seemed to work fine. I'll investigate and try to come up with a solution. > > > > > > BTW. why don't our _GLOBAL() macros use these SYM annotations? I haven't > > > really looked into them. > > Not sure. Was mostly just concentrating on the kvmppc_hv_entry code. > > > > > > Thanks, > > > Nick
On Mon Mar 27, 2023 at 7:34 PM AEST, Kautuk Consul wrote: > On 2023-03-27 14:58:03, Kautuk Consul wrote: > > On 2023-03-27 19:19:37, Nicholas Piggin wrote: > > > On Thu Mar 16, 2023 at 3:10 PM AEST, Kautuk Consul wrote: > > > > kvmppc_hv_entry isn't called from anywhere other than > > > > book3s_hv_rmhandlers.S itself. Removing .global scope for > > > > this function and annotating it with SYM_INNER_LABEL. > > > > > > > > Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com> > > > > --- > > > > arch/powerpc/kvm/book3s_hv_rmhandlers.S | 3 +-- > > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > > > diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > > > index acf80915f406..b81ba4ee0521 100644 > > > > --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > > > +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > > > @@ -502,8 +502,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) > > > > * * > > > > *****************************************************************************/ > > > > > > > > -.global kvmppc_hv_entry > > > > > > I think this is okay. > > > > > > > -kvmppc_hv_entry: > > > > +SYM_INNER_LABEL(kvmppc_hv_entry, SYM_L_LOCAL) > > > > > > The documentation for SYM_INNER_LABEL says it for labels inside a SYM > > > function block, is that a problem? This is a function but doesn't have > > > C calling convention, so asm annotation docs say that it should use > > > SYM_CODE_START_LOCAL? > > That is correct. Will create a v4 patch for this and send it. > But using SYM_CODE_START_LOCAL again causes a warning in the build > (which we were trying to avoid): > arch/powerpc/kvm/book3s_hv_rmhandlers.o: warning: objtool: .text+0x48: unannotated intra-function call Are you using SYM_FUNC_END as well? Looks like you need that to annotate the type properly. It should be the same as SYM_INNER_LABEL in the end AFAIKS. > > > > > > BTW. why don't our _GLOBAL() macros use these SYM annotations? I haven't > > > really looked into them. > > Not sure. Was mostly just concentrating on the kvmppc_hv_entry code. Looks like it's because we have a .type @function annotation in those already. Not sure if we should end up converting all that over to use the SYM annotations or if it's okay to leave it as is. Thanks, Nick
On 2023-03-27 19:51:34, Nicholas Piggin wrote: > On Mon Mar 27, 2023 at 7:34 PM AEST, Kautuk Consul wrote: > > On 2023-03-27 14:58:03, Kautuk Consul wrote: > > > On 2023-03-27 19:19:37, Nicholas Piggin wrote: > > > > On Thu Mar 16, 2023 at 3:10 PM AEST, Kautuk Consul wrote: > > > > > kvmppc_hv_entry isn't called from anywhere other than > > > > > book3s_hv_rmhandlers.S itself. Removing .global scope for > > > > > this function and annotating it with SYM_INNER_LABEL. > > > > > > > > > > Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com> > > > > > --- > > > > > arch/powerpc/kvm/book3s_hv_rmhandlers.S | 3 +-- > > > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > > > > > diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > > > > index acf80915f406..b81ba4ee0521 100644 > > > > > --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > > > > +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > > > > @@ -502,8 +502,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) > > > > > * * > > > > > *****************************************************************************/ > > > > > > > > > > -.global kvmppc_hv_entry > > > > > > > > I think this is okay. > > > > > > > > > -kvmppc_hv_entry: > > > > > +SYM_INNER_LABEL(kvmppc_hv_entry, SYM_L_LOCAL) > > > > > > > > The documentation for SYM_INNER_LABEL says it for labels inside a SYM > > > > function block, is that a problem? This is a function but doesn't have > > > > C calling convention, so asm annotation docs say that it should use > > > > SYM_CODE_START_LOCAL? > > > That is correct. Will create a v4 patch for this and send it. > > But using SYM_CODE_START_LOCAL again causes a warning in the build > > (which we were trying to avoid): > > arch/powerpc/kvm/book3s_hv_rmhandlers.o: warning: objtool: .text+0x48: unannotated intra-function call > > Are you using SYM_FUNC_END as well? Looks like you need that to > annotate the type properly. It should be the same as SYM_INNER_LABEL > in the end AFAIKS. What about SYM_CODE_START_LOCAL and SYM_CODE_END ? This seems to work fine for me without any build warnings from objtool. > > > > > > > > > BTW. why don't our _GLOBAL() macros use these SYM annotations? I haven't > > > > really looked into them. > > > Not sure. Was mostly just concentrating on the kvmppc_hv_entry code. > > Looks like it's because we have a .type @function annotation in those > already. Not sure if we should end up converting all that over to use > the SYM annotations or if it's okay to leave it as is. > > Thanks, > Nick
On 2023-03-27 15:25:24, Kautuk Consul wrote: > On 2023-03-27 19:51:34, Nicholas Piggin wrote: > > On Mon Mar 27, 2023 at 7:34 PM AEST, Kautuk Consul wrote: > > > On 2023-03-27 14:58:03, Kautuk Consul wrote: > > > > On 2023-03-27 19:19:37, Nicholas Piggin wrote: > > > > > On Thu Mar 16, 2023 at 3:10 PM AEST, Kautuk Consul wrote: > > > > > > kvmppc_hv_entry isn't called from anywhere other than > > > > > > book3s_hv_rmhandlers.S itself. Removing .global scope for > > > > > > this function and annotating it with SYM_INNER_LABEL. > > > > > > > > > > > > Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com> > > > > > > --- > > > > > > arch/powerpc/kvm/book3s_hv_rmhandlers.S | 3 +-- > > > > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > > > > > > > diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > > > > > index acf80915f406..b81ba4ee0521 100644 > > > > > > --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > > > > > +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > > > > > @@ -502,8 +502,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) > > > > > > * * > > > > > > *****************************************************************************/ > > > > > > > > > > > > -.global kvmppc_hv_entry > > > > > > > > > > I think this is okay. > > > > > > > > > > > -kvmppc_hv_entry: > > > > > > +SYM_INNER_LABEL(kvmppc_hv_entry, SYM_L_LOCAL) > > > > > > > > > > The documentation for SYM_INNER_LABEL says it for labels inside a SYM > > > > > function block, is that a problem? This is a function but doesn't have > > > > > C calling convention, so asm annotation docs say that it should use > > > > > SYM_CODE_START_LOCAL? > > > > That is correct. Will create a v4 patch for this and send it. > > > But using SYM_CODE_START_LOCAL again causes a warning in the build > > > (which we were trying to avoid): > > > arch/powerpc/kvm/book3s_hv_rmhandlers.o: warning: objtool: .text+0x48: unannotated intra-function call > > > > Are you using SYM_FUNC_END as well? Looks like you need that to > > annotate the type properly. It should be the same as SYM_INNER_LABEL > > in the end AFAIKS. > > What about SYM_CODE_START_LOCAL and SYM_CODE_END ? > This seems to work fine for me without any build warnings from objtool. Sent a v4 with this fix. Thanks. > > > > > > > > > > > > BTW. why don't our _GLOBAL() macros use these SYM annotations? I haven't > > > > > really looked into them. > > > > Not sure. Was mostly just concentrating on the kvmppc_hv_entry code. > > > > Looks like it's because we have a .type @function annotation in those > > already. Not sure if we should end up converting all that over to use > > the SYM annotations or if it's okay to leave it as is. > > > > Thanks, > > Nick
On Mon Mar 27, 2023 at 7:55 PM AEST, Kautuk Consul wrote: > On 2023-03-27 19:51:34, Nicholas Piggin wrote: > > On Mon Mar 27, 2023 at 7:34 PM AEST, Kautuk Consul wrote: > > > On 2023-03-27 14:58:03, Kautuk Consul wrote: > > > > On 2023-03-27 19:19:37, Nicholas Piggin wrote: > > > > > On Thu Mar 16, 2023 at 3:10 PM AEST, Kautuk Consul wrote: > > > > > > kvmppc_hv_entry isn't called from anywhere other than > > > > > > book3s_hv_rmhandlers.S itself. Removing .global scope for > > > > > > this function and annotating it with SYM_INNER_LABEL. > > > > > > > > > > > > Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com> > > > > > > --- > > > > > > arch/powerpc/kvm/book3s_hv_rmhandlers.S | 3 +-- > > > > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > > > > > > > diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > > > > > index acf80915f406..b81ba4ee0521 100644 > > > > > > --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > > > > > +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > > > > > @@ -502,8 +502,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) > > > > > > * * > > > > > > *****************************************************************************/ > > > > > > > > > > > > -.global kvmppc_hv_entry > > > > > > > > > > I think this is okay. > > > > > > > > > > > -kvmppc_hv_entry: > > > > > > +SYM_INNER_LABEL(kvmppc_hv_entry, SYM_L_LOCAL) > > > > > > > > > > The documentation for SYM_INNER_LABEL says it for labels inside a SYM > > > > > function block, is that a problem? This is a function but doesn't have > > > > > C calling convention, so asm annotation docs say that it should use > > > > > SYM_CODE_START_LOCAL? > > > > That is correct. Will create a v4 patch for this and send it. > > > But using SYM_CODE_START_LOCAL again causes a warning in the build > > > (which we were trying to avoid): > > > arch/powerpc/kvm/book3s_hv_rmhandlers.o: warning: objtool: .text+0x48: unannotated intra-function call > > > > Are you using SYM_FUNC_END as well? Looks like you need that to > > annotate the type properly. It should be the same as SYM_INNER_LABEL > > in the end AFAIKS. > > What about SYM_CODE_START_LOCAL and SYM_CODE_END ? > This seems to work fine for me without any build warnings from objtool. That's what I meant. So you were just missing SYM_CODE_END? Thanks, Nick
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S index acf80915f406..b81ba4ee0521 100644 --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S @@ -502,8 +502,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) * * *****************************************************************************/ -.global kvmppc_hv_entry -kvmppc_hv_entry: +SYM_INNER_LABEL(kvmppc_hv_entry, SYM_L_LOCAL) /* Required state: *
kvmppc_hv_entry isn't called from anywhere other than book3s_hv_rmhandlers.S itself. Removing .global scope for this function and annotating it with SYM_INNER_LABEL. Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com> --- arch/powerpc/kvm/book3s_hv_rmhandlers.S | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)