On 3/19/2025 4:12 PM, ltaylorsimpson@gmail.com wrote:
>
>> -----Original Message-----
>> From: Brian Cain <brian.cain@oss.qualcomm.com>
>> Sent: Friday, February 28, 2025 11:28 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 13/39] target/hexagon: Implement modify_syscfg()
>>
>> From: Brian Cain <bcain@quicinc.com>
>>
>> Signed-off-by: Brian Cain <brian.cain@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
Fixed in v2.