Re: [PATCH] target/ppc: Fix regression in Radix MMU
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
Signed-off-by: Leandro Lupori
---
target/ppc/mmu-radix64.c | 28
1 file changed, 20 insertions(+), 8 deletions(-)
Reviewed-by: Daniel Henrique Barboza
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) {
Re: [PATCH] target/ppc: Fix regression in Radix MMU
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
Signed-off-by: Leandro Lupori
It now reaches the login screen on L2
Tested-by: Víctor Colombo
--
Víctor Cora Colombo
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer
[PATCH] target/ppc: Fix regression in Radix MMU
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
Signed-off-by: Leandro Lupori
---
target/ppc/mmu-radix64.c | 28
1 file changed, 20 insertions(+), 8 deletions(-)
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) {
--
2.25.1
