> On 3. Aug 2025, at 02:41, Richard Henderson <richard.hender...@linaro.org>
> wrote:
>
> On 8/2/25 18:17, Mohamed Mediouni wrote:
>> winhvemulation is x86_64 only.
>> Exception exit bitmaps are also x86_64 only
>> Others are just variable definitions not used on arm64.
>> Signed-off-by: Mohamed Mediouni <moha...@unpredictable.fr>
>> ---
>> accel/whpx/whpx-common.c | 20 ++++++++++++++++++--
>> include/system/whpx-all.h | 2 ++
>> include/system/whpx-common.h | 2 ++
>> include/system/whpx-internal.h | 7 ++++++-
>> 4 files changed, 28 insertions(+), 3 deletions(-)
>> diff --git a/accel/whpx/whpx-common.c b/accel/whpx/whpx-common.c
>> index b5e5fda696..eeefaea329 100644
>> --- a/accel/whpx/whpx-common.c
>> +++ b/accel/whpx/whpx-common.c
>> @@ -41,7 +41,9 @@
>> bool whpx_allowed;
>> static bool whp_dispatch_initialized;
>> static HMODULE hWinHvPlatform;
>> +#ifdef __x86_64__
>> static HMODULE hWinHvEmulation;
>> +#endif
>> struct whpx_state whpx_global;
>> struct WHPDispatch whp_dispatch;
>> @@ -106,11 +108,16 @@ int whpx_first_vcpu_starting(CPUState *cpu)
>> * whpx_translate_cpu_breakpoints(). WHPX breakpoints must
>> * now be recomputed.
>> */
>> +#ifdef __x86_64__
>> whpx_translate_cpu_breakpoints(&whpx->breakpoints, cpu, i);
>> +#endif
>> }
>> +#ifdef __x86_64__
>> /* Actually insert the breakpoints into the memory. */
>> whpx_apply_breakpoints(whpx->breakpoints.breakpoints, cpu, true);
>> +#endif
>> }
>
> These are still static in target/i386/whpx/whpx-all.c, so the split in the
> previous patch didn't work. You probably want arm to *have* these functions,
> even if they're empty.
>
>> +#ifdef __x86_64__
>> HRESULT hr;
>> uint64_t exception_mask;
>> if (whpx->step_pending ||
>> @@ -132,6 +139,7 @@ int whpx_first_vcpu_starting(CPUState *cpu)
>> "hr=%08lx.", hr);
>> return 1;
>> }
>> +#endif
>
> I'm not quite sure what this is doing, but it probably needs to be split out
> to a new function which can be specialized between architectures.
Hello,
Arm WHPX has no specific functionality for debugging today, but we could
support kernel mode breakpoints via replacing them by an unhandled hvc #2,
stubbing might be the right route to go...
>
>> @@ -238,8 +246,10 @@ void whpx_destroy_vcpu(CPUState *cpu)
>> struct whpx_state *whpx = &whpx_global;
>> whp_dispatch.WHvDeleteVirtualProcessor(whpx->partition,
>> cpu->cpu_index);
>> +#ifdef __x86_64__
>> AccelCPUState *vcpu = cpu->accel;
>> whp_dispatch.WHvEmulatorDestroyEmulator(vcpu->emulator);
>> +#endif
>
> Does the os function exist on arm? The answer to that determines whether we
> must split this out to an architecture function or if we can do
>
> if (vcpu->emulator) {
> destroy
> }
The winhvemulation header has x86-isms and isn’t parsable on arm unfortunately.
The whole winhvemulation library is x86_64 specific.
That said, I don’t think we need winhvemulation on x86 in the first place and
it could be a good idea to get rid of the dependency there too.
>> @@ -414,8 +424,12 @@ static bool load_whp_dispatch_fns(HMODULE *handle,
>> LIST_WINHVPLATFORM_FUNCTIONS(WHP_LOAD_FIELD)
>> break;
>> case WINHV_EMULATION_FNS_DEFAULT:
>> +#ifdef __x86_64__
>> WHP_LOAD_LIB(WINHV_EMULATION_DLL, hLib)
>> LIST_WINHVEMULATION_FUNCTIONS(WHP_LOAD_FIELD)
>> +#else
>> + abort();
>> +#endif
>
> Use g_assert_not_reached not abort.
>
> That said, it might be cleaner to split this into two functions, rather than
> select pieces of the function via an enumerator. At which point I could see
> the entire set of emulation code staying in target/i386/.
There’s still significant shared code that might be worth keeping I think… will
test a bit on x86 and see what makes sense to do…
Thank you,
>
> r~
>
