Re: [EXTERNAL] [PATCH] target/arm/whpx: fix whpx-arm reset hang

> On 18. Aug 2026, at 23:05, Doug Cook (WINDOWS) <[email protected]> wrote:
> 
>>> +    { WHvArm64RegisterCntvCvalEl0, WHPX_SYSREG(14, 3, 3, 3, 2) },
>>> +    { WHvArm64RegisterSpEl1, WHPX_SYSREG(4, 1, 3, 4, 0) },
>>> };
>> 
>> Ow, carried this mess in formats from HVF, don’t think we wanna keep this
>> really.
>> 
>> (switching to the ENCODE_AA64_CP_REG format across the board would be
>> better)
> 
> I'm happy to do that.
> 
>>> +
>>> +    if (level == WHPX_LEVEL_RESET_STATE) {
>>> +        /*
>>> +         * WHPX keeps the "vCPU is in a low-power wait" state (entered when
>> the
>>> +         * guest executes WFI/WFE) in WHvRegisterInternalActivityState, and
>> that
>>> +         * state is not affected by writing the architectural registers above.
>>> +         * If the guest happened to be idle when the reset was requested, the
>>> +         * vCPU would stay suspended forever.
>>> +         *
>>> +         * StartupSuspend is the bit WHP uses to hold a vCPU that has not been
>>> +         * started yet, and it is how secondary vCPUs are parked until PSCI
>>> +         * CPU_ON releases them.
>>> +         */
>>> +        WHV_REGISTER_NAME name = WHvRegisterInternalActivityState;
>>> +        bool powered_off = arm_cpu->power_state == PSCI_OFF;
>>> +        WHV_REGISTER_VALUE ia;
>>> +        HRESULT hr;
>>> +
>>> +        clean_whv_register_value(&ia);
>>> +        hr = whp_dispatch.WHvGetVirtualProcessorRegisters(
>>> +            whpx_global.partition, cpu->cpu_index, &name, 1, &ia);
>>> +        if (SUCCEEDED(hr) &&
>>> +            (ia.InternalActivity.IdleSuspend ||
>>> +             ia.InternalActivity.HaltSuspend ||
>>> +             ia.InternalActivity.StartupSuspend != powered_off)) {
>>> +            ia.InternalActivity.IdleSuspend = 0;
>>> +            ia.InternalActivity.HaltSuspend = 0;
>>> +            ia.InternalActivity.StartupSuspend = powered_off;
>>> +            whpx_set_reg(cpu, WHvRegisterInternalActivityState, ia);
>>> +        }
>>> +    }
>>> }
>> 
>> Looks good :) Although perhaps this is better as a separate commit from the
>> change above.
> 
> Happy to do that too.
> 
>> But not this bit, which should be dropped.
>> 
>> The CPU model gets synced through this bit and registers that error out
>> through ARM_CP_NO_RAW should be behind a #ifdef SYNC_NO_RAW_REGS
>> block.
>> 
>> The former characteristic is what allows for example -cpu cortex-a72 to be
>> used on WHPX instead of only the native machine, while masking off the
>> newer feature from feature regs.
>> 
>>> +        if (ri && !(ri->type & (ARM_CP_NO_RAW | ARM_CP_CONST))) {
>>>            whpx_sreg_match[i].cp_idx = sregs_cnt;
>>>            arm_cpu->cpreg_indexes[sregs_cnt++] = cpreg_to_kvm_id(key);
>>>        } else {
> 
> I'll need a bit of advice in this case. Without this change, we start getting
> assertion failures after fixing ENCODE_AA64_CP_REG parameter ordering, due to
> register read-back not matching after write. Do you have a recommendation on
> how to approach this?
> 

For HVF, the way sync is handled when read back that wasn’t the same* was to put the affected
registers outside of the sysreg sync array and do it manually for those.

Given that we don’t have full save/restore support yet (the GIC state sync is missing for now)
it’s ok to comment the affected sysregs from now and return to this later

* notably, on some HVs, the timer registers can be ones of the oddballs around that front, as they can keep counting when the specific vCPU is not running