> -----Original Message-----
> From: Brian Cain <brian.c...@oss.qualcomm.com>
> Sent: Friday, February 28, 2025 11:28 PM
> To: qemu-devel@nongnu.org
> Cc: brian.c...@oss.qualcomm.com; richard.hender...@linaro.org;
> phi...@linaro.org; quic_mathb...@quicinc.com; a...@rev.ng; a...@rev.ng;
> quic_mlie...@quicinc.com; ltaylorsimp...@gmail.com;
> alex.ben...@linaro.org; quic_mbur...@quicinc.com;
> sidn...@quicinc.com; Brian Cain <bc...@quicinc.com>
> Subject: [PATCH 13/39] target/hexagon: Implement modify_syscfg()
>
> From: Brian Cain <bc...@quicinc.com>
>
> Signed-off-by: Brian Cain <brian.c...@oss.qualcomm.com>
> ---
> target/hexagon/op_helper.c | 51
> +++++++++++++++++++++++++++++++++++++-
> 1 file changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c
> index 03bed11f6e..42805d0f1d 100644
> --- a/target/hexagon/op_helper.c
> +++ b/target/hexagon/op_helper.c
> @@ -1522,7 +1522,56 @@ static bool
> handle_pmu_sreg_write(CPUHexagonState *env, uint32_t reg,
>
> static void modify_syscfg(CPUHexagonState *env, uint32_t val) {
> - g_assert_not_reached();
> + g_assert(bql_locked());
> +
> + uint32_t old;
> + uint32_t syscfg_read_only_mask = 0x80001c00;
> + uint32_t syscfg = arch_get_system_reg(env, HEX_SREG_SYSCFG);
> +
> + /* clear read-only bits if they are set in the new value. */
> + val &= ~syscfg_read_only_mask;
> + /* if read-only are currently set in syscfg keep them set. */
> + val |= (syscfg & syscfg_read_only_mask);
> +
> + uint32_t tmp = val;
> + old = arch_get_system_reg(env, HEX_SREG_SYSCFG);
This is the same as syscfg declared above
> + arch_set_system_reg(env, HEX_SREG_SYSCFG, tmp);
Why is tmp needed? Just use val here.
> +
> + /* Check for change in MMU enable */
> + target_ulong old_mmu_enable = GET_SYSCFG_FIELD(SYSCFG_MMUEN,
> old);
> + uint8_t old_en = GET_SYSCFG_FIELD(SYSCFG_PCYCLEEN, old);
> + uint8_t old_gie = GET_SYSCFG_FIELD(SYSCFG_GIE, old);
> + target_ulong new_mmu_enable =
> + GET_SYSCFG_FIELD(SYSCFG_MMUEN, val);
Move these declarations to the beginning of the function.
> + if (new_mmu_enable && !old_mmu_enable) {
> + hex_mmu_on(env);
> + } else if (!new_mmu_enable && old_mmu_enable) {
> + hex_mmu_off(env);
> + }
> +
> + /* Changing pcycle enable from 0 to 1 resets the counters */
> + uint8_t new_en = GET_SYSCFG_FIELD(SYSCFG_PCYCLEEN, val);
> + CPUState *cs;
Move the declarations to the beginning of the function
> + if (old_en == 0 && new_en == 1) {
You could put declaration of cs here if you prefer
> + CPU_FOREACH(cs) {
> + CPUHexagonState *_env = cpu_env(cs);
> + _env->t_cycle_count = 0;
I'm not a fan of _env as a variable name. Just do
cpu_env(cs)->t_cycle_count = 0
> + }
> + }
> +
> + /* See if global interrupts are turned on */
> + uint8_t new_gie = GET_SYSCFG_FIELD(SYSCFG_GIE, val);
Move the declaration to the beginning
> + if (!old_gie && new_gie) {
> + qemu_log_mask(CPU_LOG_INT, "%s: global interrupts enabled\n",
> __func__);
> + hex_interrupt_update(env);
> + }
> +
> + if (qemu_loglevel_mask(LOG_UNIMP)) {
> + int new_v2x = GET_SYSCFG_FIELD(SYSCFG_V2X, val);
> + if (!new_v2x) {
> + qemu_log("HVX: 64 byte vector length is unsupported\n");
> + }
> + }
> }
>
> static uint32_t hexagon_find_last_irq(CPUHexagonState *env, uint32_t vid)
> --
> 2.34.1
