[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] mips: Correct the handling of writes to CP0.Sta
From: |
Leon Alrae |
Subject: |
Re: [Qemu-devel] [PATCH] mips: Correct the handling of writes to CP0.Status for MIPSr6 |
Date: |
Mon, 17 Nov 2014 13:10:33 +0000 |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 |
On 10/11/2014 13:45, Maciej W. Rozycki wrote:
> Correct these issues with the handling of CP0.Status for MIPSr6:
>
> * only ignore the bit pattern of 0b11 on writes to CP0.Status.KSU, that
> is for processors that do implement Supervisor Mode, let the bit
> pattern be written to CP0.Status.UM:R0 freely (of course the value
> written to read-only CP0.Status.R0 will be discarded anyway); this is
> in accordance to the relevant architecture specification[1],
>
> * check the newly written pattern rather than the current contents of
> CP0.Status for the KSU bits being 0b11,
>
> * use meaningful macro names to refer to CP0.Status bits rather than
> magic numbers.
>
> References:
>
> [1] "MIPS Architecture For Programmers, Volume III: MIPS64 / microMIPS64
> Privileged Resource Architecture", MIPS Technologies, Inc., Document
> Number: MD00091, Revision 6.00, March 31, 2014, Table 9.45 "Status
> Register Field Descriptions", pp. 210-211.
>
> Signed-off-by: Maciej W. Rozycki <address@hidden>
> ---
> Leon,
>
> Noticed in porting the next change I'm going to post. NB I have no
> reasonable way to do run-time checks of an r6 configuration, so please
> double check this works for you even though I believe it is obviously
> correct; I did check CP0.Status.KSU rejects the pattern of 0b11 via GDB
> on MIPS64R6-generic with the aid of the next change.
>
> I suggest making it a policy not to accept new code using magic
> numbers. Then the existing places can be gradually identified and
> corrected.
>
> Please apply.
>
> Thanks,
>
> Maciej
>
> qemu-mips-r6-status-r0.diff
> Index: qemu-git-trunk/target-mips/op_helper.c
> ===================================================================
> --- qemu-git-trunk.orig/target-mips/op_helper.c 2014-11-09
> 23:44:45.467759913 +0000
> +++ qemu-git-trunk/target-mips/op_helper.c 2014-11-09 23:45:27.977531070
> +0000
> @@ -1423,10 +1423,12 @@ void helper_mtc0_status(CPUMIPSState *en
> uint32_t mask = env->CP0_Status_rw_bitmask;
>
> if (env->insn_flags & ISA_MIPS32R6) {
> - if (extract32(env->CP0_Status, CP0St_KSU, 2) == 0x3) {
> + bool has_supervisor = extract32(mask, CP0St_KSU, 2) == 0x3;
> +
> + if (has_supervisor && extract32(arg1, CP0St_KSU, 2) == 0x3) {
> mask &= ~(3 << CP0St_KSU);
> }
> - mask &= ~(0x00180000 & arg1);
> + mask &= ~(((1 << CP0St_SR) | (1 << CP0St_NMI)) & arg1);
> }
>
> val = arg1 & mask;
>
Thanks for fixing and cleaning this up.
Reviewed-by: Leon Alrae <address@hidden>