Message ID | 20221028183617.121786-1-leandro.lupori@eldorado.org.br |
---|---|
State | New |
Headers | show |
Series | target/ppc: Fix regression in Radix MMU | expand |
On 28/10/2022 15:36, Leandro Lupori wrote: > Commit 47e83d9107 ended up unintentionally changing the control flow > of ppc_radix64_process_scoped_xlate(). When guest_visible is false, > it must not raise an exception, even if the radix configuration is > not valid. > > This regression prevented Linux boot in a nested environment with > L1 using TCG and emulating KVM (cap-nested-hv=on) and L2 using > KVM. L2 would hang on Linux's futex_init(), when it tested how a > futex_atomic_cmpxchg_inatomic() handled a fault, because L1 would > start a loop of trying to perform partition scoped translations > and raising exceptions. > > Fixes: 47e83d9107 ("target/ppc: Improve Radix xlate level validation") > Reported-by: Victor Colombo <victor.colombo@eldorado.org.br> > Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br> It now reaches the login screen on L2 Tested-by: Víctor Colombo <victor.colombo@eldorado.org.br>
On 10/28/22 15:36, Leandro Lupori wrote: > Commit 47e83d9107 ended up unintentionally changing the control flow > of ppc_radix64_process_scoped_xlate(). When guest_visible is false, > it must not raise an exception, even if the radix configuration is > not valid. > > This regression prevented Linux boot in a nested environment with > L1 using TCG and emulating KVM (cap-nested-hv=on) and L2 using > KVM. L2 would hang on Linux's futex_init(), when it tested how a > futex_atomic_cmpxchg_inatomic() handled a fault, because L1 would > start a loop of trying to perform partition scoped translations > and raising exceptions. > > Fixes: 47e83d9107 ("target/ppc: Improve Radix xlate level validation") > Reported-by: Victor Colombo <victor.colombo@eldorado.org.br> > Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br> > --- > target/ppc/mmu-radix64.c | 28 ++++++++++++++++++++-------- > 1 file changed, 20 insertions(+), 8 deletions(-) Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> I'll queue this up in the pending pull request. Thanks, Daniel > > diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c > index 00f2e9fa2e..171379db69 100644 > --- a/target/ppc/mmu-radix64.c > +++ b/target/ppc/mmu-radix64.c > @@ -238,6 +238,8 @@ static void ppc_radix64_set_rc(PowerPCCPU *cpu, MMUAccessType access_type, > > static bool ppc_radix64_is_valid_level(int level, int psize, uint64_t nls) > { > + bool ret; > + > /* > * Check if this is a valid level, according to POWER9 and POWER10 > * Processor User's Manuals, sections 4.10.4.1 and 5.10.6.1, respectively: > @@ -249,16 +251,24 @@ static bool ppc_radix64_is_valid_level(int level, int psize, uint64_t nls) > */ > switch (level) { > case 0: /* Root Page Dir */ > - return psize == 52 && nls == 13; > + ret = psize == 52 && nls == 13; > + break; > case 1: > case 2: > - return nls == 9; > + ret = nls == 9; > + break; > case 3: > - return nls == 9 || nls == 5; > + ret = nls == 9 || nls == 5; > + break; > default: > - qemu_log_mask(LOG_GUEST_ERROR, "invalid radix level: %d\n", level); > - return false; > + ret = false; > + } > + > + if (unlikely(!ret)) { > + qemu_log_mask(LOG_GUEST_ERROR, "invalid radix configuration: " > + "level %d size %d nls %ld\n", level, psize, nls); > } > + return ret; > } > > static int ppc_radix64_next_level(AddressSpace *as, vaddr eaddr, > @@ -519,11 +529,13 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu, > > if (!ppc_radix64_is_valid_level(level++, *g_page_size, nls)) { > fault_cause |= DSISR_R_BADCONFIG; > - return 1; > + ret = 1; > + } else { > + ret = ppc_radix64_next_level(cs->as, eaddr & R_EADDR_MASK, > + &h_raddr, &nls, g_page_size, > + &pte, &fault_cause); > } > > - ret = ppc_radix64_next_level(cs->as, eaddr & R_EADDR_MASK, &h_raddr, > - &nls, g_page_size, &pte, &fault_cause); > if (ret) { > /* No valid pte */ > if (guest_visible) {
diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c index 00f2e9fa2e..171379db69 100644 --- a/target/ppc/mmu-radix64.c +++ b/target/ppc/mmu-radix64.c @@ -238,6 +238,8 @@ static void ppc_radix64_set_rc(PowerPCCPU *cpu, MMUAccessType access_type, static bool ppc_radix64_is_valid_level(int level, int psize, uint64_t nls) { + bool ret; + /* * Check if this is a valid level, according to POWER9 and POWER10 * Processor User's Manuals, sections 4.10.4.1 and 5.10.6.1, respectively: @@ -249,16 +251,24 @@ static bool ppc_radix64_is_valid_level(int level, int psize, uint64_t nls) */ switch (level) { case 0: /* Root Page Dir */ - return psize == 52 && nls == 13; + ret = psize == 52 && nls == 13; + break; case 1: case 2: - return nls == 9; + ret = nls == 9; + break; case 3: - return nls == 9 || nls == 5; + ret = nls == 9 || nls == 5; + break; default: - qemu_log_mask(LOG_GUEST_ERROR, "invalid radix level: %d\n", level); - return false; + ret = false; + } + + if (unlikely(!ret)) { + qemu_log_mask(LOG_GUEST_ERROR, "invalid radix configuration: " + "level %d size %d nls %ld\n", level, psize, nls); } + return ret; } static int ppc_radix64_next_level(AddressSpace *as, vaddr eaddr, @@ -519,11 +529,13 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu, if (!ppc_radix64_is_valid_level(level++, *g_page_size, nls)) { fault_cause |= DSISR_R_BADCONFIG; - return 1; + ret = 1; + } else { + ret = ppc_radix64_next_level(cs->as, eaddr & R_EADDR_MASK, + &h_raddr, &nls, g_page_size, + &pte, &fault_cause); } - ret = ppc_radix64_next_level(cs->as, eaddr & R_EADDR_MASK, &h_raddr, - &nls, g_page_size, &pte, &fault_cause); if (ret) { /* No valid pte */ if (guest_visible) {
Commit 47e83d9107 ended up unintentionally changing the control flow of ppc_radix64_process_scoped_xlate(). When guest_visible is false, it must not raise an exception, even if the radix configuration is not valid. This regression prevented Linux boot in a nested environment with L1 using TCG and emulating KVM (cap-nested-hv=on) and L2 using KVM. L2 would hang on Linux's futex_init(), when it tested how a futex_atomic_cmpxchg_inatomic() handled a fault, because L1 would start a loop of trying to perform partition scoped translations and raising exceptions. Fixes: 47e83d9107 ("target/ppc: Improve Radix xlate level validation") Reported-by: Victor Colombo <victor.colombo@eldorado.org.br> Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br> --- target/ppc/mmu-radix64.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-)