On Thu, 2 Feb 2023 at 21:13, Aaron Lindsay <aa...@os.amperecomputing.com> wrote:
>
> Signed-off-by: Aaron Lindsay <aa...@os.amperecomputing.com>
> ---
> target/arm/pauth_helper.c | 26 ++++++++++++++++++++++++++
> target/arm/syndrome.h | 6 ++++++
> 2 files changed, 32 insertions(+)
>
> diff --git a/target/arm/pauth_helper.c b/target/arm/pauth_helper.c
> index 66dc90a289..3a2772de0e 100644
> --- a/target/arm/pauth_helper.c
> +++ b/target/arm/pauth_helper.c
> @@ -385,6 +385,21 @@ static uint64_t pauth_original_ptr(uint64_t ptr,
> ARMVAParameters param)
> return deposit64(ptr, bot_pac_bit, top_pac_bit - bot_pac_bit, extfield);
> }
>
> +static G_NORETURN
> +void pauth_fail_exception(CPUARMState *env, int error_code)
> +{
> + int target_el = arm_current_el(env);
> + if (target_el == 0) {
> + uint64_t hcr = arm_hcr_el2_eff(env);
> + if (arm_is_el2_enabled(env) && (hcr & HCR_TGE))
> + target_el = 2;
> + else
> + target_el = 1;
> + }
> +
> + raise_exception_ra(env, EXCP_UDEF, syn_pacfail(error_code), target_el,
> GETPC());
This won't work -- you must call GETPC() from the top-level helper
function directly called from JITted code, so that it can get the
PC of the callsite in the JITted code. Otherwise you'll get a PC
somewhere inside QEMU's C code, which won't do the right thing.
This is why pauth_check_trap() takes an 'ra' argument (for
'return address') and all the top level helper functions call
GETPC() to get the value to pass.
> +}
> +
> static uint64_t pauth_auth(CPUARMState *env, uint64_t ptr, uint64_t modifier,
> ARMPACKey *key, bool data, int keynumber,
> bool is_combined)
> @@ -403,6 +418,17 @@ static uint64_t pauth_auth(CPUARMState *env, uint64_t
> ptr, uint64_t modifier,
> uint64_t xor_mask = MAKE_64BIT_MASK(bot_bit, top_bit - bot_bit + 1) &
> ~MAKE_64BIT_MASK(55, 1);
> result = ((ptr ^ pac) & xor_mask) | (ptr & ~xor_mask);
> + if (cpu_isar_feature(aa64_fpac_combine, env_archcpu(env)) ||
> + (cpu_isar_feature(aa64_fpac, env_archcpu(env)) &&
> + !is_combined)) {
> + int fpac_top = param.tbi ? 55 : 64;
> + uint64_t fpac_mask = MAKE_64BIT_MASK(bot_bit, fpac_top -
> bot_bit);
> + test = (result ^ sextract64(result, 55, 1)) & fpac_mask;
> + if (unlikely(test)) {
> + int error_code = ((data ? 1 : 0) << 1) | (keynumber);
> + pauth_fail_exception(env, error_code);
> + }
> + }
> } else {
> test = (pac ^ ptr) & ~MAKE_64BIT_MASK(55, 1);
> if (unlikely(extract64(test, bot_bit, top_bit - bot_bit))) {
> diff --git a/target/arm/syndrome.h b/target/arm/syndrome.h
> index 73df5e3793..885a85735c 100644
> --- a/target/arm/syndrome.h
> +++ b/target/arm/syndrome.h
> @@ -48,6 +48,7 @@ enum arm_exception_class {
> EC_AA64_SMC = 0x17,
> EC_SYSTEMREGISTERTRAP = 0x18,
> EC_SVEACCESSTRAP = 0x19,
> + EC_PACFAIL = 0x1c,
> EC_SMETRAP = 0x1d,
> EC_INSNABORT = 0x20,
> EC_INSNABORT_SAME_EL = 0x21,
> @@ -221,6 +222,11 @@ static inline uint32_t syn_smetrap(SMEExceptionType
> etype, bool is_16bit)
> | (is_16bit ? 0 : ARM_EL_IL) | etype;
> }
>
> +static inline uint32_t syn_pacfail(int error_code)
> +{
> + return (EC_PACFAIL << ARM_EL_EC_SHIFT) | error_code;
You need ARM_EL_IL here too, I think.
I would suggest that you make the syn_pacfail() function take
two arguments (bool data and int keynumber), and put them in to
bits 0 and 1 in this function. That avoids the need to
construct an error code at the callsite.
> +}
> +
> static inline uint32_t syn_pactrap(void)
> {
> return EC_PACTRAP << ARM_EL_EC_SHIFT;
> --
> 2.25.1
thanks
-- PMM
