* booke206_tlb_ways() returning 0
@ 2020-03-26 14:22 Peter Maydell
2020-03-26 16:44 ` Laurent Vivier
0 siblings, 1 reply; 2+ messages in thread
From: Peter Maydell @ 2020-03-26 14:22 UTC (permalink / raw)
To: QEMU Developers, qemu-ppc; +Cc: David Gibson
Hi; Coverity points out a possible issue in booke206_get_tlbm()
(this is CID 1421942):
static inline ppcmas_tlb_t *booke206_get_tlbm(CPUPPCState *env, const int tlbn,
target_ulong ea, int way)
{
int r;
uint32_t ways = booke206_tlb_ways(env, tlbn);
int ways_bits = ctz32(ways);
int tlb_bits = ctz32(booke206_tlb_size(env, tlbn));
int i;
way &= ways - 1;
ea >>= MAS2_EPN_SHIFT;
ea &= (1 << (tlb_bits - ways_bits)) - 1;
r = (ea << ways_bits) | way;
Here if booke206_tlb_ways() returns zero, then ways_bits()
will be 32 and the shift left 'ea << ways_bits' is undefined
behaviour.
My first assumption was that booke206_tlb_ways() is not supposed
to ever return 0 (it's looking at what I think are read-only
system registers, and it doesn't make much sense to have
a zero-way TLB). So I tried adding an assert:
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 88d94495554..aedb6bcb265 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2403,6 +2403,7 @@ static inline int booke206_tlb_ways(CPUPPCState
*env, int tlbn)
{
uint32_t tlbncfg = env->spr[SPR_BOOKE_TLB0CFG + tlbn];
int r = tlbncfg >> TLBnCFG_ASSOC_SHIFT;
+ assert(r > 0);
return r;
}
However, this isn't right, because it causes one of the check-acceptance
tests to fail, with this backtrace:
#3 0x00007ffff074d412 in __GI___assert_fail (assertion=0x5555560a0d7d
"r > 0", file=0x5555560a0d40
"/home/petmay01/linaro/qemu-from-laptop/qemu/target/ppc/cpu.h",
line=2406, function=0x5555560a1720 <__PRETTY_FUNCTION__.20811>
"booke206_tlb_ways") at assert.c:101
#4 0x0000555555a9939b in booke206_tlb_ways (env=0x555556e52a30,
tlbn=2) at /home/petmay01/linaro/qemu-from-laptop/qemu/target/ppc/cpu.h:2406
#5 0x0000555555a9b3ac in mmubooke206_get_physical_address
(env=0x555556e52a30, ctx=0x7fffd77008a0, address=9223380835095282947,
rw=0, access_type=0, mmu_idx=1) at
/home/petmay01/linaro/qemu-from-laptop/qemu/target/ppc/mmu_helper.c:1093
#6 0x0000555555a9c25d in get_physical_address_wtlb
(env=0x555556e52a30, ctx=0x7fffd77008a0, eaddr=9223380835095282947,
rw=0, access_type=0, mmu_idx=1) at
/home/petmay01/linaro/qemu-from-laptop/qemu/target/ppc/mmu_helper.c:1455
#7 0x0000555555a9c82b in cpu_ppc_handle_mmu_fault
(env=0x555556e52a30, address=9223380835095282947, rw=0, mmu_idx=1)
at /home/petmay01/linaro/qemu-from-laptop/qemu/target/ppc/mmu_helper.c:1597
#8 0x0000555555a9f975 in ppc_cpu_tlb_fill (cs=0x555556e49560,
addr=9223380835095282947, size=1, access_type=MMU_DATA_LOAD,
mmu_idx=1, probe=false, retaddr=140735610345781) at
/home/petmay01/linaro/qemu-from-laptop/qemu/target/ppc/mmu_helper.c:3053
#9 0x00005555558e1422 in tlb_fill (cpu=0x555556e49560,
addr=9223380835095282947, size=1, access_type=MMU_DATA_LOAD,
mmu_idx=1, retaddr=140735610345781) at
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/cputlb.c:1017
#10 0x00005555558e279b in load_helper (env=0x555556e52a30,
addr=9223380835095282947, oi=1, retaddr=140735610345781, op=MO_8,
code_read=false, full_load=0x5555558e2b3a <full_ldub_mmu>) at
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/cputlb.c:1534
#11 0x00005555558e2b80 in full_ldub_mmu (env=0x555556e52a30,
addr=9223380835095282947, oi=1, retaddr=140735610345781)
at /home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/cputlb.c:1624
#12 0x00005555558e2bb4 in helper_ret_ldub_mmu (env=0x555556e52a30,
addr=9223380835095282947, oi=1, retaddr=140735610345781)
at /home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/cputlb.c:1630
#13 0x00007fff900fd9c6 in code_gen_buffer ()
#14 0x00005555558f9915 in cpu_tb_exec (cpu=0x555556e49560,
itb=0x7fff900fd780 <code_gen_buffer+1038163>)
at /home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/cpu-exec.c:172
#15 0x00005555558fa732 in cpu_loop_exec_tb (cpu=0x555556e49560,
tb=0x7fff900fd780 <code_gen_buffer+1038163>, last_tb=0x7fffd7701078,
tb_exit=0x7fffd7701070) at
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/cpu-exec.c:619
#16 0x00005555558faa4c in cpu_exec (cpu=0x555556e49560) at
/home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/cpu-exec.c:732
#17 0x00005555558bcf29 in tcg_cpu_exec (cpu=0x555556e49560) at
/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1405
#18 0x00005555558bd77f in qemu_tcg_cpu_thread_fn (arg=0x555556e49560)
at /home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1713
#19 0x0000555555f2ff3f in qemu_thread_start (args=0x555556e8dd10) at
/home/petmay01/linaro/qemu-from-laptop/qemu/util/qemu-thread-posix.c:519
#20 0x00007ffff0b156db in start_thread (arg=0x7fffd7704700) at
pthread_create.c:463
#21 0x00007ffff083e88f in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:95
So under what circumstances can booke206_tlb_ways()
validly return 0? Should booke206_get_tlbm() cope with a
zero return, or can it assert() that it will never
call booke206_tlb_ways() in a way that will cause one?
thanks
-- PMM
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: booke206_tlb_ways() returning 0
2020-03-26 14:22 booke206_tlb_ways() returning 0 Peter Maydell
@ 2020-03-26 16:44 ` Laurent Vivier
0 siblings, 0 replies; 2+ messages in thread
From: Laurent Vivier @ 2020-03-26 16:44 UTC (permalink / raw)
To: Peter Maydell, QEMU Developers, qemu-ppc; +Cc: David Gibson
Le 26/03/2020 à 15:22, Peter Maydell a écrit :
> Hi; Coverity points out a possible issue in booke206_get_tlbm()
> (this is CID 1421942):
>
>
> static inline ppcmas_tlb_t *booke206_get_tlbm(CPUPPCState *env, const int tlbn,
> target_ulong ea, int way)
> {
> int r;
> uint32_t ways = booke206_tlb_ways(env, tlbn);
> int ways_bits = ctz32(ways);
> int tlb_bits = ctz32(booke206_tlb_size(env, tlbn));
> int i;
>
> way &= ways - 1;
> ea >>= MAS2_EPN_SHIFT;
> ea &= (1 << (tlb_bits - ways_bits)) - 1;
> r = (ea << ways_bits) | way;
>
> Here if booke206_tlb_ways() returns zero, then ways_bits()
> will be 32 and the shift left 'ea << ways_bits' is undefined
> behaviour.
>
> My first assumption was that booke206_tlb_ways() is not supposed
> to ever return 0 (it's looking at what I think are read-only
> system registers, and it doesn't make much sense to have
> a zero-way TLB). So I tried adding an assert:
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 88d94495554..aedb6bcb265 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -2403,6 +2403,7 @@ static inline int booke206_tlb_ways(CPUPPCState
> *env, int tlbn)
> {
> uint32_t tlbncfg = env->spr[SPR_BOOKE_TLB0CFG + tlbn];
> int r = tlbncfg >> TLBnCFG_ASSOC_SHIFT;
> + assert(r > 0);
> return r;
> }
>
> However, this isn't right, because it causes one of the check-acceptance
> tests to fail, with this backtrace:
>
> #3 0x00007ffff074d412 in __GI___assert_fail (assertion=0x5555560a0d7d
> "r > 0", file=0x5555560a0d40
> "/home/petmay01/linaro/qemu-from-laptop/qemu/target/ppc/cpu.h",
> line=2406, function=0x5555560a1720 <__PRETTY_FUNCTION__.20811>
> "booke206_tlb_ways") at assert.c:101
> #4 0x0000555555a9939b in booke206_tlb_ways (env=0x555556e52a30,
> tlbn=2) at /home/petmay01/linaro/qemu-from-laptop/qemu/target/ppc/cpu.h:2406
> #5 0x0000555555a9b3ac in mmubooke206_get_physical_address
> (env=0x555556e52a30, ctx=0x7fffd77008a0, address=9223380835095282947,
> rw=0, access_type=0, mmu_idx=1) at
> /home/petmay01/linaro/qemu-from-laptop/qemu/target/ppc/mmu_helper.c:1093
> #6 0x0000555555a9c25d in get_physical_address_wtlb
> (env=0x555556e52a30, ctx=0x7fffd77008a0, eaddr=9223380835095282947,
> rw=0, access_type=0, mmu_idx=1) at
> /home/petmay01/linaro/qemu-from-laptop/qemu/target/ppc/mmu_helper.c:1455
> #7 0x0000555555a9c82b in cpu_ppc_handle_mmu_fault
> (env=0x555556e52a30, address=9223380835095282947, rw=0, mmu_idx=1)
> at /home/petmay01/linaro/qemu-from-laptop/qemu/target/ppc/mmu_helper.c:1597
> #8 0x0000555555a9f975 in ppc_cpu_tlb_fill (cs=0x555556e49560,
> addr=9223380835095282947, size=1, access_type=MMU_DATA_LOAD,
> mmu_idx=1, probe=false, retaddr=140735610345781) at
> /home/petmay01/linaro/qemu-from-laptop/qemu/target/ppc/mmu_helper.c:3053
> #9 0x00005555558e1422 in tlb_fill (cpu=0x555556e49560,
> addr=9223380835095282947, size=1, access_type=MMU_DATA_LOAD,
> mmu_idx=1, retaddr=140735610345781) at
> /home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/cputlb.c:1017
> #10 0x00005555558e279b in load_helper (env=0x555556e52a30,
> addr=9223380835095282947, oi=1, retaddr=140735610345781, op=MO_8,
> code_read=false, full_load=0x5555558e2b3a <full_ldub_mmu>) at
> /home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/cputlb.c:1534
> #11 0x00005555558e2b80 in full_ldub_mmu (env=0x555556e52a30,
> addr=9223380835095282947, oi=1, retaddr=140735610345781)
> at /home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/cputlb.c:1624
> #12 0x00005555558e2bb4 in helper_ret_ldub_mmu (env=0x555556e52a30,
> addr=9223380835095282947, oi=1, retaddr=140735610345781)
> at /home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/cputlb.c:1630
> #13 0x00007fff900fd9c6 in code_gen_buffer ()
> #14 0x00005555558f9915 in cpu_tb_exec (cpu=0x555556e49560,
> itb=0x7fff900fd780 <code_gen_buffer+1038163>)
> at /home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/cpu-exec.c:172
> #15 0x00005555558fa732 in cpu_loop_exec_tb (cpu=0x555556e49560,
> tb=0x7fff900fd780 <code_gen_buffer+1038163>, last_tb=0x7fffd7701078,
> tb_exit=0x7fffd7701070) at
> /home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/cpu-exec.c:619
> #16 0x00005555558faa4c in cpu_exec (cpu=0x555556e49560) at
> /home/petmay01/linaro/qemu-from-laptop/qemu/accel/tcg/cpu-exec.c:732
> #17 0x00005555558bcf29 in tcg_cpu_exec (cpu=0x555556e49560) at
> /home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1405
> #18 0x00005555558bd77f in qemu_tcg_cpu_thread_fn (arg=0x555556e49560)
> at /home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1713
> #19 0x0000555555f2ff3f in qemu_thread_start (args=0x555556e8dd10) at
> /home/petmay01/linaro/qemu-from-laptop/qemu/util/qemu-thread-posix.c:519
> #20 0x00007ffff0b156db in start_thread (arg=0x7fffd7704700) at
> pthread_create.c:463
> #21 0x00007ffff083e88f in clone () at
> ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
>
> So under what circumstances can booke206_tlb_ways()
> validly return 0? Should booke206_get_tlbm() cope with a
> zero return, or can it assert() that it will never
> call booke206_tlb_ways() in a way that will cause one?
It seems the value of booke206_tlb_ways() is the associativity
(TLBnCFG_ASSOC_SHIFT) that seems to be generated by gen_tlbncfg() and
set in init_proc_e500().
And used values are: 2, 4 ,16, and 64, but only for tlbn 0 and 1.
According to PowerISA 2.06:
booke206_tlb_size() booke206_tlb_ways()
0 0 No TLB present
0 1 TLB geometry is completely
implementation-defined.
MAS0 ESEL is ignored
0 > 1 TLB geometry and number of
entries is implementation
defined,...
n > 0 n or 0 TLB is fully associative
In mmubooke206_get_physical_address(), helper_booke206_tlbsx(),
booke206_invalidate_ea_tlb() and helper_booke206_tlbilx3(),
booke206_get_tlbm() is not called if ways is 0.
In booke206_cur_tlb(), the function is only called with tlbn number
provided by SPR_BOOKE_MAS0, so we can guess we are using the ones with
the initialized values.
It is also called in mmubooke_create_initial_mapping() but only for tlbn 1.
But r can be 0 if ea and way are 0 (in
mmubooke206_get_physical_address() if ways > 0, way will start with
value 0 and ea >> ways_bit can be 0). I think it's why your assert() is
triggered.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-03-26 16:46 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-26 14:22 booke206_tlb_ways() returning 0 Peter Maydell
2020-03-26 16:44 ` Laurent Vivier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).