Re: [PATCH v2 2/2] target/riscv: Legalize MPP value in write_mstatus

[Richard Henderson]

On 4/6/23 00:25, Weiwei Li wrote:

+static target_ulong legalize_mpp(CPURISCVState *env, target_ulong old_mpp,
+                                 target_ulong val)
+{
+    target_ulong new_mpp = get_field(val, MSTATUS_MPP);
+    bool mpp_invalid = (new_mpp == PRV_S && !riscv_has_ext(env, RVS)) ||
+                       (new_mpp == PRV_U && !riscv_has_ext(env, RVU)) ||
+                       (new_mpp == PRV_H);
+
+    /* Remain field unchanged if new_mpp value is invalid */
+    return mpp_invalid ? set_field(val, MSTATUS_MPP, old_mpp) : val;
+}

Does anyone find PRV_H confusing, since that's not what it is?
I think it would be nice to remove it entirely.

This function might be better as

    bool valid = false;

    switch (new_mpp) {
    case PRV_M:
        valid = true;
        break;
    case PRV_S:
        valid = riscv_has_ext(env, RVS);
        break;
    case PRV_U:
        valid = riscv_has_ext(env, RVU);
        break;
    }
    if (!valid) {
        val = set_field(...);
    }
    return val;


r~