On 8/11/26 18:47, Chinmay Rath wrote:
Hi Minhang,
Thanks for fixing this. Took me a while to get hold of the docs for 32
bit mmu, hence the late response.
On 8/7/26 14:47, [email protected] wrote:
From: Minhang Zhang <[email protected]>
ppc_store_sdr1() had validation for 64-bit SDR1 values but lacked
corresponding checks for the 32-bit case. According to the Power ISA,
in 32-bit mode SDR1 bits 16-22 are reserved (must be zero) and
HTABMASK (bits 23-31) must consist of a consecutive string of
1-bits starting from the LSB, i.e., be of the form 2^n-1.
Add checks to reject invalid HTABMASK values and log a guest error
for non-zero reserved bits, following the same pattern used by the
existing 64-bit validation.
Signed-off-by: Minhang Zhang <[email protected]>
---
target/ppc/mmu_common.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
index 2499e61..59e8324 100644
--- a/target/ppc/mmu_common.c
+++ b/target/ppc/mmu_common.c
@@ -57,9 +57,23 @@ void ppc_store_sdr1(CPUPPCState *env, target_ulong
value)
" stored in SDR1", htabsize);
return;
}
- }
+ } else
#endif /* defined(TARGET_PPC64) */
- /* FIXME: Should check for valid HTABMASK values in 32-bit case */
+ {
+ target_ulong htabmask = value & SDR_32_HTABMASK;
+ if (value & 0x007F0000UL) {
From what I see in the programming manual, shouldn't the bits of this
flag be the other way around : 0x0000FE00 ?
"bits of this mask", is what I meant. Not "flag", my bad.
Nevertheless, it would be better to form the mask by ORing the
SDR_32_HTABORG and SDR_32_HTABMASK fields from mmu-hash32.h, instead
of hardcoding it; the way it is already done for 64 bits in the same
function.
Regards,
Chinmay
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "Invalid SDR1: reserved bits 0x"
TARGET_FMT_lx
+ " set\n", value & 0x007F0000UL);
+ value &= ~0x007F0000UL;
+ }
+ if ((htabmask & (htabmask + 1)) != 0) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "Invalid HTABMASK 0x" TARGET_FMT_lx
+ " in SDR1 (must be of form 2^n-1)\n",
htabmask);
+ return;
+ }
+ }
env->spr[SPR_SDR1] = value;
}
