RE: [PATCH 14/38] target/hexagon: Add new macro definitions for sysemu

[<ltaylorsimpson@gmail.com>]

> -----Original Message-----
> From: Brian Cain <brian.cain@oss.qualcomm.com>
> Sent: Friday, February 28, 2025 11:26 PM
> To: qemu-devel@nongnu.org
> Cc: brian.cain@oss.qualcomm.com; richard.henderson@linaro.org;
> philmd@linaro.org; quic_mathbern@quicinc.com; ale@rev.ng; anjo@rev.ng;
> quic_mliebel@quicinc.com; ltaylorsimpson@gmail.com;
> alex.bennee@linaro.org; quic_mburton@quicinc.com;
> sidneym@quicinc.com; Brian Cain <bcain@quicinc.com>
> Subject: [PATCH 14/38] target/hexagon: Add new macro definitions for
> sysemu
> 
> From: Brian Cain <bcain@quicinc.com>
> 
> Also: add nop TCG overrides for break,unpause,fetchbo,dczeroa

dczeroa is modelled by QEMU.  It writes zero's to the cache line.

> 
> break: this hardware breakpoint instruction is used with the in-silicon
> debugger feature, this is not modeled.
> 
> unpause: this instruction is used to resume hardware threads that are stalled
> by pause instructions.  pause is modeled as a nop, or in RR mode as an
> EXCP_YIELD.  This instruction is safe to ignore.
> 
> Since cache/prefetch functions are not modeled, dczero and fetchbo are
> safe to ignore.

dczero is modelled.

> 
> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
> ---
>  target/hexagon/gen_tcg.h    |   9 ++
>  target/hexagon/macros.h     |  28 ++++-
>  target/hexagon/sys_macros.h | 238
> ++++++++++++++++++++++++++++++++++++
>  target/hexagon/op_helper.c  |   1 +
>  4 files changed, 272 insertions(+), 4 deletions(-)  create mode 100644
> target/hexagon/sys_macros.h
> 
> diff --git a/target/hexagon/gen_tcg.h b/target/hexagon/gen_tcg.h index
> 8a3b801287..71f8a0e2d0 100644
> --- a/target/hexagon/gen_tcg.h
> +++ b/target/hexagon/gen_tcg.h
> @@ -488,6 +488,7 @@
> 
>  /* dczeroa clears the 32 byte cache line at the address given */  #define
> fGEN_TCG_Y2_dczeroa(SHORTCODE) SHORTCODE
> +#define fGEN_TCG_Y2_dczeroa_nt(SHORTCODE) SHORTCODE

Is there a Y2_dczeroa_nt instruction?  If not, remove this.

>          ctx->dczero_addr = tcg_temp_new(); \
>          tcg_gen_mov_tl(ctx->dczero_addr, (REG)); \
>      } while (0)
> +#else
> +#define fDCZEROA(REG) ((void) REG)

This isn't needed because all the instances of fDCZEROA are inside QEMU_GENERATE.


>  #endif
> 
> diff --git a/target/hexagon/sys_macros.h b/target/hexagon/sys_macros.h
> new file mode 100644 index 0000000000..3c4c3c7aa5
> --- /dev/null
> +++ b/target/hexagon/sys_macros.h
> +#define READ_SREG(NUM) arch_get_system_reg(env, NUM)
> +#define READ_SGP0()    arch_get_system_reg(env, HEX_SREG_SGP0)
> +#define READ_SGP1()    arch_get_system_reg(env, HEX_SREG_SGP1)
> +#define READ_SGP10()   ((uint64_t)arch_get_system_reg(env,
> HEX_SREG_SGP0) | \
> +    ((uint64_t)arch_get_system_reg(env, HEX_SREG_SGP1) << 32))
> +
> +#define WRITE_SREG(NUM, VAL)      log_sreg_write(env, NUM, VAL, slot)
> +#define WRITE_SGP0(VAL)           log_sreg_write(env, HEX_SREG_SGP0,
> VAL, slot)
> +#define WRITE_SGP1(VAL)           log_sreg_write(env, HEX_SREG_SGP1,
> VAL, slot)
> +#define WRITE_SGP10(VAL) \
> +    do { \
> +        log_sreg_write(env, HEX_SREG_SGP0, (VAL) & 0xFFFFFFFF, slot); \
> +        log_sreg_write(env, HEX_SREG_SGP1, (VAL) >> 32, slot); \
> +    } while (0)
> +

READ_SREG and WRITE_SREG look like a hangover for the original generator scripts which have been rewritten.  Are they needed?


> +#ifdef QEMU_GENERATE
> +#define GET_SSR_FIELD(RES, FIELD) \
> +    GET_FIELD(RES, FIELD, hex_t_sreg[HEX_SREG_SSR]) #else
> +
> +#define GET_SSR_FIELD(FIELD, REGIN) \
> +    (uint32_t)GET_FIELD(FIELD, REGIN)
> +#define GET_SYSCFG_FIELD(FIELD, REGIN) \
> +    (uint32_t)GET_FIELD(FIELD, REGIN)
> +#define SET_SYSTEM_FIELD(ENV, REG, FIELD, VAL) \
> +    do { \
> +        uint32_t regval = arch_get_system_reg(ENV, REG); \
> +        fINSERT_BITS(regval, reg_field_info[FIELD].width, \
> +                     reg_field_info[FIELD].offset, (VAL)); \
> +        arch_set_system_reg(ENV, REG, regval); \
> +    } while (0)
> +#define SET_SSR_FIELD(ENV, FIELD, VAL) \
> +    SET_SYSTEM_FIELD(ENV, HEX_SREG_SSR, FIELD, VAL) #define
> +SET_SYSCFG_FIELD(ENV, FIELD, VAL) \
> +    SET_SYSTEM_FIELD(ENV, HEX_SREG_SYSCFG, FIELD, VAL)
> +
> +#define CCR_FIELD_SET(ENV, FIELD) \
> +    (!!GET_FIELD(FIELD, arch_get_system_reg(ENV, HEX_SREG_CCR)))
> +
> +/*
> + * Direct-to-guest is not implemented yet, continuing would cause
> +unexpected
> + * behavior, so we abort.
> + */
> +#define ASSERT_DIRECT_TO_GUEST_UNSET(ENV, EXCP) \
> +    do { \
> +        switch (EXCP) { \
> +        case HEX_EVENT_TRAP0: \
> +            g_assert(!CCR_FIELD_SET(ENV, CCR_GTE)); \
> +            break; \
> +        case HEX_EVENT_IMPRECISE: \
> +        case HEX_EVENT_PRECISE: \
> +        case HEX_EVENT_FPTRAP: \
> +            g_assert(!CCR_FIELD_SET(ENV, CCR_GEE)); \
> +            break; \
> +        default: \
> +            if ((EXCP) >= HEX_EVENT_INT0) { \
> +                g_assert(!CCR_FIELD_SET(ENV, CCR_GIE)); \
> +            } \
> +            break; \
> +        } \
> +    } while (0)
> +#endif
> +
> +#define fREAD_ELR() (READ_SREG(HEX_SREG_ELR))
> +
> +#define fLOAD_PHYS(NUM, SIZE, SIGN, SRC1, SRC2, DST) { \
> +  const uintptr_t rs = ((unsigned long)(unsigned)(SRC1)) & 0x7ff; \
> +  const uintptr_t rt = ((unsigned long)(unsigned)(SRC2)) << 11; \
> +  const uintptr_t addr = rs + rt;         \
> +  cpu_physical_memory_read(addr, &DST, sizeof(uint32_t)); \ }
> +
> +#define fPOW2_HELP_ROUNDUP(VAL) \
> +    ((VAL) | \
> +     ((VAL) >> 1) | \
> +     ((VAL) >> 2) | \
> +     ((VAL) >> 4) | \
> +     ((VAL) >> 8) | \
> +     ((VAL) >> 16))
> +#define fPOW2_ROUNDUP(VAL) (fPOW2_HELP_ROUNDUP((VAL) - 1) + 1)
> +
> +#define fFRAMECHECK(ADDR, EA)  g_assert_not_reached();

Add a FIXME or comment on why this is not implemented.  Or is it implemented in a subsequent patch in this series?

> +#define fSET_TLB_LOCK()       g_assert_not_reached()
> +#define fCLEAR_TLB_LOCK()     g_assert_not_reached()
> +
> +#define fSET_K0_LOCK()        g_assert_not_reached()
> +#define fCLEAR_K0_LOCK()      g_assert_not_reached()

Add a comment that these are implemented in a later patch in the series.