Re: [PATCH 5/7] hw/hyperv/syndbg: common compilation unit

["Alex Bennée" <alex.bennee@linaro.org>]

"Maciej S. Szmigiero" <mail@maciej.szmigiero.name> writes:

> On 6.03.2025 23:56, Pierrick Bouvier wrote:
>> On 3/6/25 09:58, Philippe Mathieu-Daudé wrote:
>>> On 6/3/25 17:23, Pierrick Bouvier wrote:
>>>> On 3/6/25 08:19, Richard Henderson wrote:
>>>>> On 3/5/25 22:41, Pierrick Bouvier wrote:
>>>>>> Replace TARGET_PAGE.* by runtime calls
>>>>>>
>>>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>>>> ---
>>>>>>     hw/hyperv/syndbg.c    | 7 ++++---
>>>>>>     hw/hyperv/meson.build | 2 +-
>>>>>>     2 files changed, 5 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/hw/hyperv/syndbg.c b/hw/hyperv/syndbg.c
>>>>>> index d3e39170772..f9382202ed3 100644
>>>>>> --- a/hw/hyperv/syndbg.c
>>>>>> +++ b/hw/hyperv/syndbg.c
>>>>>> @@ -14,7 +14,7 @@
>>>>>>     #include "migration/vmstate.h"
>>>>>>     #include "hw/qdev-properties.h"
>>>>>>     #include "hw/loader.h"
>>>>>> -#include "cpu.h"
>>>>>> +#include "exec/target_page.h"
>>>>>>     #include "hw/hyperv/hyperv.h"
>>>>>>     #include "hw/hyperv/vmbus-bridge.h"
>>>>>>     #include "hw/hyperv/hyperv-proto.h"
>>>>>> @@ -188,7 +188,8 @@ static uint16_t handle_recv_msg(HvSynDbg *syndbg,
>>>>>> uint64_t outgpa,
>>>>>>                                     uint64_t timeout, uint32_t
>>>>>> *retrieved_count)
>>>>>>     {
>>>>>>         uint16_t ret;
>>>>>> -    uint8_t data_buf[TARGET_PAGE_SIZE - UDP_PKT_HEADER_SIZE];
>>>>>> +    const size_t buf_size = qemu_target_page_size() -
>>>>>> UDP_PKT_HEADER_SIZE;
>>>>>> +    uint8_t *data_buf = g_alloca(buf_size);
>>>>>>         hwaddr out_len;
>>>>>>         void *out_data;
>>>>>>         ssize_t recv_byte_count;
>>>>>
>>>>> We've purged the code base of VLAs, and those are preferable to alloca.
>>>>> Just use g_malloc and g_autofree.
>>>>>
>>>>
>>>> I hesitated, due to potential performance considerations for people
>>>> reviewing the patch. I'll switch to heap based storage.
>>>
>>> OTOH hyperv is x86-only, so we could do:
>>>
>>> #define BUFSZ (4 * KiB)
>>>
>>> handle_recv_msg()
>>> {
>>>     uint8_t data_buf[BUFSZ - UDP_PKT_HEADER_SIZE];
>>>     ...
>>>
>>> hv_syndbg_class_init()
>>> {
>>>     assert(BUFSZ > qemu_target_page_size());
>>>     ...
>>>
>>> and call it a day.
>> Could be possible for now yes.
>> Any opinion from concerned maintainers?
>
> I think essentially hardcoding 4k pages in hyperv is okay
> (with an appropriate checking/enforcement asserts() of course),
> since even if this gets ported to ARM64 at some point
> it is going to need *a lot* of changes anyway.

There was a talk at last years KVM Forum about porting WHPX for Windows
on Arm:

  https://kvm-forum.qemu.org/2024/Qemu_support_for_Windows_on_Arm_GgKlLjf.pdf

but am I right in thinking all the hyperv code in QEMU is about
providing guest facing enlightenments for Windows guests under KVM? I
guess no one is working on that at the moment.


>
> Thanks,
> Maciej

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro