Hi Sathvika,
First bad commit (maybe != root cause):
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 5dc4c995db9eb45f6373a956eb1f69460e69e6d4
commit: c984aef8c8326035570ff6e01d0ff9e79a5dfa76 objtool/powerpc: Add --mcount specific implementation
date: 8 weeks ago
config: powerpc-randconfig-c042-20230116
compiler: powerpc-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c984aef8c8326035570ff6e01d0ff9e79a5dfa76
git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git fetch --no-tags linus master
git checkout c984aef8c8326035570ff6e01d0ff9e79a5dfa76
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash arch/powerpc/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> arch/powerpc/kernel/head_85xx.o: warning: objtool: .head.text+0x1a6c: unannotated intra-function call
--
arch/powerpc/kvm/booke.c:626:6: warning: no previous prototype for 'kvmppc_watchdog_func' [-Wmissing-prototypes]
626 | void kvmppc_watchdog_func(struct timer_list *t)
| ^~~~~~~~~~~~~~~~~~~~
arch/powerpc/kvm/booke.c:1009:5: warning: no previous prototype for 'kvmppc_handle_exit' [-Wmissing-prototypes]
1009 | int kvmppc_handle_exit(struct kvm_vcpu *vcpu, unsigned int exit_nr)
| ^~~~~~~~~~~~~~~~~~
arch/powerpc/kvm/booke.c:1947:6: warning: no previous prototype for 'kvm_guest_protect_msr' [-Wmissing-prototypes]
1947 | void kvm_guest_protect_msr(struct kvm_vcpu *vcpu, ulong prot_bitmap, bool set)
| ^~~~~~~~~~~~~~~~~~~~~
>> arch/powerpc/kvm/booke.o: warning: objtool: kvmppc_fill_pt_regs+0x30: unannotated intra-function call
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for HOTPLUG_CPU
Depends on [n]: SMP [=y] && (PPC_PSERIES [=n] || PPC_PMAC [=n] || PPC_POWERNV [=n] || FSL_SOC_BOOKE [=n])
Selected by [y]:
- PM_SLEEP_SMP [=y] && SMP [=y] && (ARCH_SUSPEND_POSSIBLE [=y] || ARCH_HIBERNATION_POSSIBLE [=y]) && PM_SLEEP [=y]
objdump-func vmlinux.o .head.text:
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
Hi all,
On 16/01/23 17:25, kernel test robot wrote:
> Hi Sathvika,
>
> First bad commit (maybe != root cause):
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 5dc4c995db9eb45f6373a956eb1f69460e69e6d4
> commit: c984aef8c8326035570ff6e01d0ff9e79a5dfa76 objtool/powerpc: Add --mcount specific implementation
> date: 8 weeks ago
> config: powerpc-randconfig-c042-20230116
> compiler: powerpc-linux-gcc (GCC) 12.1.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c984aef8c8326035570ff6e01d0ff9e79a5dfa76
> git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> git fetch --no-tags linus master
> git checkout c984aef8c8326035570ff6e01d0ff9e79a5dfa76
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc olddefconfig
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash arch/powerpc/
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <[email protected]>
>
> All warnings (new ones prefixed by >>):
>
>>> arch/powerpc/kernel/head_85xx.o: warning: objtool: .head.text+0x1a6c: unannotated intra-function call
After annotating symbols with SYM_FUNC_START_LOCAL and SYM_FUNC_END
macros, I don't see this warning anymore.
Below diff works for me:
diff --git a/arch/powerpc/kernel/head_85xx.S
b/arch/powerpc/kernel/head_85xx.S
index d438ca74e96c..fdbee1093e2b 100644
--- a/arch/powerpc/kernel/head_85xx.S
+++ b/arch/powerpc/kernel/head_85xx.S
@@ -864,7 +864,7 @@ _GLOBAL(load_up_spe)
* SPE unavailable trap from kernel - print a message, but let
* the task use SPE in the kernel until it returns to user mode.
*/
-KernelSPE:
+SYM_FUNC_START_LOCAL(KernelSPE)
lwz r3,_MSR(r1)
oris r3,r3,MSR_SPE@h
stw r3,_MSR(r1) /* enable use of SPE after return */
@@ -881,6 +881,7 @@ KernelSPE:
#endif
.align 4,0
+SYM_FUNC_END(KernelSPE)
#endif /* CONFIG_SPE */
/*
>>> arch/powerpc/kvm/booke.o: warning: objtool: kvmppc_fill_pt_regs+0x30: unannotated intra-function call
As an attempt to fix it, I tried expanding ANNOTATE_INTRA_FUNCTION_CALL
macro to indicate that the branch target is valid. It then threw another
warning (arch/powerpc/kvm/booke.o: warning: objtool:
kvmppc_fill_pt_regs+0x38: intra_function_call not a direct call). The
below diff just removes the warnings for me, but I'm not very sure if
this is the best way to fix the objtool warnings seen with this
particular file. Please let me know if there are any better ways to fix it.
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 0dce93ccaadf..b6a413824b98 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -917,7 +917,9 @@ static void kvmppc_fill_pt_regs(struct pt_regs *regs)
asm("mr %0, 1" : "=r"(r1));
asm("mflr %0" : "=r"(lr));
asm("mfmsr %0" : "=r"(msr));
+ asm(".pushsection .discard.intra_function_calls; .long 999f;
.popsection; 999:");
asm("bl 1f; 1: mflr %0" : "=r"(ip));
+ asm("nop");
memset(regs, 0, sizeof(*regs));
regs->gpr[1] = r1;
Thanks,
Sathvika
Sathvika Vasireddy wrote:
>
>>>> arch/powerpc/kvm/booke.o: warning: objtool: kvmppc_fill_pt_regs+0x30: unannotated intra-function call
>
> As an attempt to fix it, I tried expanding ANNOTATE_INTRA_FUNCTION_CALL
> macro to indicate that the branch target is valid. It then threw another
> warning (arch/powerpc/kvm/booke.o: warning: objtool:
> kvmppc_fill_pt_regs+0x38: intra_function_call not a direct call). The
> below diff just removes the warnings for me, but I'm not very sure if
> this is the best way to fix the objtool warnings seen with this
> particular file. Please let me know if there are any better ways to fix it.
>
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index 0dce93ccaadf..b6a413824b98 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -917,7 +917,9 @@ static void kvmppc_fill_pt_regs(struct pt_regs *regs)
> asm("mr %0, 1" : "=r"(r1));
> asm("mflr %0" : "=r"(lr));
> asm("mfmsr %0" : "=r"(msr));
> + asm(".pushsection .discard.intra_function_calls; .long 999f;
> .popsection; 999:");
> asm("bl 1f; 1: mflr %0" : "=r"(ip));
I don't think you can assume that there won't be anything in between two
asm statements. Does it work if you combine both the above asm
statements into a single one?
Even if that works, I don't think it is good to expand the macro here.
That asm statement looks to be trying to grab the current nip. I don't
know enough about that code, and someone who knows more about KVM may be
able to help, but it looks like we should be able to simply set 'ip' to
the address of kvmppc_fill_pt_regs()?
- Naveen
"Naveen N. Rao" <[email protected]> writes:
> Sathvika Vasireddy wrote:
>>
>>>>> arch/powerpc/kvm/booke.o: warning: objtool: kvmppc_fill_pt_regs+0x30: unannotated intra-function call
>>
>> As an attempt to fix it, I tried expanding ANNOTATE_INTRA_FUNCTION_CALL
>> macro to indicate that the branch target is valid. It then threw another
>> warning (arch/powerpc/kvm/booke.o: warning: objtool:
>> kvmppc_fill_pt_regs+0x38: intra_function_call not a direct call). The
>> below diff just removes the warnings for me, but I'm not very sure if
>> this is the best way to fix the objtool warnings seen with this
>> particular file. Please let me know if there are any better ways to fix it.
>>
>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
>> index 0dce93ccaadf..b6a413824b98 100644
>> --- a/arch/powerpc/kvm/booke.c
>> +++ b/arch/powerpc/kvm/booke.c
>> @@ -917,7 +917,9 @@ static void kvmppc_fill_pt_regs(struct pt_regs *regs)
>> asm("mr %0, 1" : "=r"(r1));
>> asm("mflr %0" : "=r"(lr));
>> asm("mfmsr %0" : "=r"(msr));
>> + asm(".pushsection .discard.intra_function_calls; .long 999f;
>> .popsection; 999:");
>> asm("bl 1f; 1: mflr %0" : "=r"(ip));
>
> I don't think you can assume that there won't be anything in between two
> asm statements.
Yeah, compiler could interleave something theoretically.
> Even if that works, I don't think it is good to expand the macro here.
> That asm statement looks to be trying to grab the current nip. I don't
> know enough about that code, and someone who knows more about KVM may be
> able to help, but it looks like we should be able to simply set 'ip' to
> the address of kvmppc_fill_pt_regs()?
There is _THIS_IP_ which should be sufficient.
cheers
Hi!
On Wed, Jan 25, 2023 at 12:57:35PM +0530, Naveen N. Rao wrote:
> Sathvika Vasireddy wrote:
> >--- a/arch/powerpc/kvm/booke.c
> >+++ b/arch/powerpc/kvm/booke.c
> >@@ -917,7 +917,9 @@ static void kvmppc_fill_pt_regs(struct pt_regs *regs)
> > ??????? asm("mr %0, 1" : "=r"(r1));
> > ??????? asm("mflr %0" : "=r"(lr));
> > ??????? asm("mfmsr %0" : "=r"(msr));
> >+?????? asm(".pushsection .discard.intra_function_calls; .long 999f;
> >.popsection; 999:");
> > ??????? asm("bl 1f; 1: mflr %0" : "=r"(ip));
>
> I don't think you can assume that there won't be anything in between two
> asm statements.
It would be a false assumption. There is nothing that stops the
compiler from moving, duplicating, or even removing these statements
(removing only if no outputs from the asm are required of course).
> Does it work if you combine both the above asm
> statements into a single one?
>
> Even if that works, I don't think it is good to expand the macro here.
> That asm statement looks to be trying to grab the current nip. I don't
> know enough about that code, and someone who knows more about KVM may be
> able to help, but it looks like we should be able to simply set 'ip' to
> the address of kvmppc_fill_pt_regs()?
Such things are much better as actual assembler code (like, a .s file).
You have to be certain the compiler doesn't transform this in unexpected
ways, like, copy and move it to all callers for example. You need the
mfmsr to remain somewhat in place for example.
A big reason to not want inline asm for things like this is you need so
very many operands in a single asm that way; it becomes very hard to
write, esp. if you want it to be correct code as well. That is a good
hint there are better way to do this ;-)
Segher