Re: [PATCH] target/arm: allow DC CVA[D]P in user mode emulation

[Richard Henderson <richard.henderson@linaro.org>]

On 5/15/23 20:59, Zhuojia Shen wrote:
> DC CVAP and DC CVADP instructions can be executed in EL0 on Linux,
> either directly when SCTLR_EL1.UCI == 1 or emulated by the kernel (see
> user_cache_maint_handler() in arch/arm64/kernel/traps.c).  The Arm ARM
> documents the semantics of the two instructions that they behave as
> DC CVAC if the address pointed to by their register operand is not
> persistent memory.
> 
> This patch enables execution of the two instructions in user mode
> emulation as NOP while preserving their original emulation in full
> system virtualization.
> 
> Signed-off-by: Zhuojia Shen <chaosdefinition@hotmail.com>
> ---
>   target/arm/helper.c               | 26 +++++++++++++-----
>   tests/tcg/aarch64/Makefile.target | 11 ++++++++
>   tests/tcg/aarch64/dcpodp.c        | 45 +++++++++++++++++++++++++++++++
>   tests/tcg/aarch64/dcpop.c         | 45 +++++++++++++++++++++++++++++++
>   4 files changed, 120 insertions(+), 7 deletions(-)
>   create mode 100644 tests/tcg/aarch64/dcpodp.c
>   create mode 100644 tests/tcg/aarch64/dcpop.c
> 
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 0b7fd2e7e6..eeba5e7978 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -7432,23 +7432,37 @@ static void dccvap_writefn(CPUARMState *env, const ARMCPRegInfo *opaque,
>           }
>       }
>   }
> +#endif /*CONFIG_USER_ONLY*/
>   
>   static const ARMCPRegInfo dcpop_reg[] = {
>       { .name = "DC_CVAP", .state = ARM_CP_STATE_AA64,
>         .opc0 = 1, .opc1 = 3, .crn = 7, .crm = 12, .opc2 = 1,
> -      .access = PL0_W, .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END,
> +      .access = PL0_W,
>         .fgt = FGT_DCCVAP,
> -      .accessfn = aa64_cacheop_poc_access, .writefn = dccvap_writefn },
> +      .accessfn = aa64_cacheop_poc_access,
> +#ifdef CONFIG_USER_ONLY
> +      .type = ARM_CP_NOP,
> +#else
> +      .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END,
> +      .writefn = dccvap_writefn,
> +#endif
> +    },
>   };

Not quite correct, as CVAP to an unmapped address should SIGSEGV.  That'll be done by the 
probe_read within dccvap_writefn.

Need to make dccvap_writefn always present, ifdef out only the memory_region_from_host + 
memory_region_writeback from there.  Need to set SCTLR_EL1.UCI in arm_cpu_reset_hold in 
the CONFIG_USER_ONLY block.


r~

>   
>   static const ARMCPRegInfo dcpodp_reg[] = {
>       { .name = "DC_CVADP", .state = ARM_CP_STATE_AA64,
>         .opc0 = 1, .opc1 = 3, .crn = 7, .crm = 13, .opc2 = 1,
> -      .access = PL0_W, .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END,
> +      .access = PL0_W,
>         .fgt = FGT_DCCVADP,
> -      .accessfn = aa64_cacheop_poc_access, .writefn = dccvap_writefn },
> +      .accessfn = aa64_cacheop_poc_access,
> +#ifdef CONFIG_USER_ONLY
> +      .type = ARM_CP_NOP,
> +#else
> +      .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END,
> +      .writefn = dccvap_writefn,
> +#endif
> +    },
>   };
> -#endif /*CONFIG_USER_ONLY*/
>   
>   static CPAccessResult access_aa64_tid5(CPUARMState *env, const ARMCPRegInfo *ri,
>                                          bool isread)
> @@ -9092,7 +9106,6 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>       if (cpu_isar_feature(aa64_tlbios, cpu)) {
>           define_arm_cp_regs(cpu, tlbios_reginfo);
>       }
> -#ifndef CONFIG_USER_ONLY
>       /* Data Cache clean instructions up to PoP */
>       if (cpu_isar_feature(aa64_dcpop, cpu)) {
>           define_one_arm_cp_reg(cpu, dcpop_reg);
> @@ -9101,7 +9114,6 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>               define_one_arm_cp_reg(cpu, dcpodp_reg);
>           }
>       }
> -#endif /*CONFIG_USER_ONLY*/
>   
>       /*
>        * If full MTE is enabled, add all of the system registers.
> diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
> index 0315795487..3430fd3cd8 100644
> --- a/tests/tcg/aarch64/Makefile.target
> +++ b/tests/tcg/aarch64/Makefile.target
> @@ -21,12 +21,23 @@ config-cc.mak: Makefile
>   	$(quiet-@)( \
>   	    $(call cc-option,-march=armv8.1-a+sve,          CROSS_CC_HAS_SVE); \
>   	    $(call cc-option,-march=armv8.1-a+sve2,         CROSS_CC_HAS_SVE2); \
> +	    $(call cc-option,-march=armv8.2-a,              CROSS_CC_HAS_ARMV8_2); \
>   	    $(call cc-option,-march=armv8.3-a,              CROSS_CC_HAS_ARMV8_3); \
> +	    $(call cc-option,-march=armv8.5-a,              CROSS_CC_HAS_ARMV8_5); \
>   	    $(call cc-option,-mbranch-protection=standard,  CROSS_CC_HAS_ARMV8_BTI); \
>   	    $(call cc-option,-march=armv8.5-a+memtag,       CROSS_CC_HAS_ARMV8_MTE); \
>   	    $(call cc-option,-march=armv9-a+sme,            CROSS_CC_HAS_ARMV9_SME)) 3> config-cc.mak
>   -include config-cc.mak
>   
> +ifneq ($(CROSS_CC_HAS_ARMV8_2),)
> +AARCH64_TESTS += dcpop
> +dcpop: CFLAGS += -march=armv8.2-a
> +endif
> +ifneq ($(CROSS_CC_HAS_ARMV8_5),)
> +AARCH64_TESTS += dcpodp
> +dcpodp: CFLAGS += -march=armv8.5-a
> +endif
> +
>   # Pauth Tests
>   ifneq ($(CROSS_CC_HAS_ARMV8_3),)
>   AARCH64_TESTS += pauth-1 pauth-2 pauth-4 pauth-5
> diff --git a/tests/tcg/aarch64/dcpodp.c b/tests/tcg/aarch64/dcpodp.c
> new file mode 100644
> index 0000000000..dad61ce78c
> --- /dev/null
> +++ b/tests/tcg/aarch64/dcpodp.c
> @@ -0,0 +1,45 @@
> +/* Test execution of DC CVADP instruction */
> +
> +#include <asm/hwcap.h>
> +#include <sys/auxv.h>
> +
> +#include <signal.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +#ifndef HWCAP2_DCPODP
> +#define HWCAP2_DCPODP (1 << 0)
> +#endif
> +
> +static void sigill_handler(int sig)
> +{
> +    exit(EXIT_FAILURE);
> +}
> +
> +static int do_dc_cvadp(void)
> +{
> +    struct sigaction sa = {
> +        .sa_handler = sigill_handler,
> +    };
> +
> +    if (sigaction(SIGILL, &sa, NULL) < 0) {
> +        perror("sigaction");
> +        return EXIT_FAILURE;
> +    }
> +
> +    asm volatile("dc cvadp, %0\n\t" :: "r"(&sa));
> +
> +    return 0;
> +}
> +
> +int main(void)
> +{
> +    if (getauxval(AT_HWCAP) & HWCAP2_DCPODP) {
> +        return do_dc_cvadp();
> +    } else {
> +        printf("SKIP: no HWCAP2_DCPODP on this system\n");
> +        return 0;
> +    }
> +
> +    return 0;
> +}
> diff --git a/tests/tcg/aarch64/dcpop.c b/tests/tcg/aarch64/dcpop.c
> new file mode 100644
> index 0000000000..8b4ea7c91c
> --- /dev/null
> +++ b/tests/tcg/aarch64/dcpop.c
> @@ -0,0 +1,45 @@
> +/* Test execution of DC CVAP instruction */
> +
> +#include <asm/hwcap.h>
> +#include <sys/auxv.h>
> +
> +#include <signal.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +#ifndef HWCAP_DCPOP
> +#define HWCAP_DCPOP (1 << 16)
> +#endif
> +
> +static void sigill_handler(int sig)
> +{
> +    exit(EXIT_FAILURE);
> +}
> +
> +static int do_dc_cvap(void)
> +{
> +    struct sigaction sa = {
> +        .sa_handler = sigill_handler,
> +    };
> +
> +    if (sigaction(SIGILL, &sa, NULL) < 0) {
> +        perror("sigaction");
> +        return EXIT_FAILURE;
> +    }
> +
> +    asm volatile("dc cvap, %0\n\t" :: "r"(&sa));
> +
> +    return 0;
> +}
> +
> +int main(void)
> +{
> +    if (getauxval(AT_HWCAP) & HWCAP_DCPOP) {
> +        return do_dc_cvap();
> +    } else {
> +        printf("SKIP: no HWCAP_DCPOP on this system\n");
> +        return 0;
> +    }
> +
> +    return 0;
> +}