Re: [PATCH qemu v4 1/2] target/arm: Handle IC IVAU to improve compatibility with JITs

[Richard Henderson]

On 6/8/23 19:49, ~jhogberg wrote:
> From: John Högberg <john.hogberg@ericsson.com>
>
> Unlike architectures with precise self-modifying code semantics
> (e.g. x86) ARM processors do not maintain coherency for instruction
> execution and memory, requiring an instruction synchronization
> barrier on every core that will execute the new code, and on many
> models also the explicit use of cache management instructions.
>
> While this is required to make JITs work on actual hardware, QEMU
> has gotten away with not handling this since it does not emulate
> caches, and unconditionally invalidates code whenever the softmmu
> or the user-mode page protection logic detects that code has been
> modified.
>
> Unfortunately the latter does not work in the face of dual-mapped
> code (a common W^X workaround), where one page is executable and
> the other is writable: user-mode has no way to connect one with the
> other as that is only known to the kernel and the emulated
> application.
>
> This commit works around the issue by telling software that
> instruction cache invalidation is required by clearing the
> CPR_EL0.DIC flag (regardless of whether the emulated processor
> needs it), and then invalidating code in IC IVAU instructions.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1034
>
> Co-authored-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: John Högberg <john.hogberg@ericsson.com>
> ---
>   target/arm/cpu.c    | 13 +++++++++++++
>   target/arm/helper.c | 47 ++++++++++++++++++++++++++++++++++++++++++---
>   2 files changed, 57 insertions(+), 3 deletions(-)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 4d5bb57f07..b82fb46157 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1674,6 +1674,19 @@ static void arm_cpu_realizefn(DeviceState *dev, Error 
> **errp)
>           return;
>       }
>   
> +    /*
> +     * User mode relies on IC IVAU instructions to catch modification of
> +     * dual-mapped code.
> +     *
> +     * Clear CTR_EL0.DIC to ensure that software that honors these flags uses
> +     * IC IVAU even if the emulated processor does not normally require it.
> +     */
> +#ifdef CONFIG_USER_ONLY
> +    if (arm_feature(env, ARM_FEATURE_AARCH64)) {
> +        cpu->ctr = FIELD_DP64(cpu->ctr, CTR_EL0, DIC, 0);
> +    }
> +#endif

No need to check for aarch64 -- the bit is present at the same location with the same 
meaning in the v8 aarch32 view: "CTR".  (Prior to v8, this bit was part of the Format 
field, and had value 0 for v6 and v7, so still no need to check before writing this zero.)

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~