|
From: | liweiwei |
Subject: | Re: [PATCH v2 2/2] target/riscv: Legalize MPP value in write_mstatus |
Date: | Fri, 7 Apr 2023 09:24:58 +0800 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.9.0 |
On 2023/4/7 03:33, Richard Henderson wrote:
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.
I agree. Maybe PRV_RESERVED is more readable in some cases.
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;
OK. This is more readable. I'll update this. Regards, Weiwei Li
r~
[Prev in Thread] | Current Thread | [Next in Thread] |