[PATCH] target/ppc: Validate HTABMASK and reserved bits in SDR1 for 32-bit mode

[sesame_h]

From: Minhang Zhang <zhangminh...@kylinos.cn>

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 <zhangminh...@kylinos.cn>

Hi Chinmay,

Thanks a lot for your careful review and pointing out these issues.

You are absolutely right, I messed up the reserved-bits mask. I misread
the Power ISA bit numbering: the correct reserved-bits mask should be
0x0000FE00, not 0x007F0000.

I also agree with your suggestion to avoid hard-coded magic numbers, so
I constructed the mask using the existing SDR_32_HTABORG and
SDR_32_HTABMASK macros from mmu-hash32.h, following the same pattern as
the 64-bit implementation in ppc_store_sdr1().

Both issues are fixed in the v2 patch below.

Regards,
Minhang Zhang
---
 target/ppc/mmu_common.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
index 2499e61..31a221d 100644
--- a/target/ppc/mmu_common.c
+++ b/target/ppc/mmu_common.c
@@ -57,9 +57,25 @@ 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 sdr_mask = SDR_32_HTABORG | SDR_32_HTABMASK;
+        target_ulong htabmask = value & SDR_32_HTABMASK;
+
+        if (value & ~sdr_mask) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "Invalid bits 0x" TARGET_FMT_lx
+                          " set in SDR1\n", value & ~sdr_mask);
+            value &= sdr_mask;
+        }
+        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;
 }
 
-- 
2.43.0