On 12/19/2024 12:00 PM, Roman Kisel wrote: > > > On 12/19/2024 11:13 AM, Easwar Hariharan wrote: >> On 12/19/2024 10:40 AM, Nuno Das Neves wrote: >>> On 12/18/2024 12:54 PM, Roman Kisel wrote: >>>> Commit bc905fa8b633 ("hyperv: Switch from hyperv-tlfs.h to hyperv/ >>>> hvhdk.h") >>>> changed the type of the output pointer to `struct hv_register_assoc` >>>> from >>>> `struct hv_get_vp_registers_output`. That leads to an incorrect >>>> computation, >>>> and leaves the system broken. >>>> >>> My bad! But, lets not use `struct hv_get_registers_output`. Instead, use >>> `struct hv_register_value`, since that is the more complete >>> definition of a >>> register value. The output of the get_vp_registers hypercall is just >>> an array >>> of these values. >>> >>> Ideally we remove `struct hv_get_vp_registers_output` at some point, >>> since >>> it serves the same role as `struct hv_register_value` but in a more >>> limited >>> capacity. >>> >>> Thanks >>> Nuno >>> >> >> I had much the same conversation with Roman off-list yesterday. > Appreciate your help, Easwar! > >> >> The choice is between using hv_get_registers_output which is clearly the >> output of the GetVpRegisters hypercall by name, albeit limited as you >> said, and hv_register_value which is the more complete definition and >> what the hypervisor actually returns, but does not currently include the >> arm64 definitions in our copy of hvgdk_mini.h. hv_get_registers_output >> and hv_register_value overlap in layout for Roman's purposes. >> >> FWIW, I'm in favor of adding the arm64 definitions to hv_register_value >> and using it for this get_vtl() patch. > I could do that, will wait to see if no objections. > >> >> This could be accompanied with migration of hv_get_vpreg128 in arm64/ >> and removal of struct hv_get_registers_output, or that could be deferred >> to a later patch. > Having an equivalent of `hv_get_vpreg128` would be awesome on x64! > I'd bet something like that should exist in dom0/mshv already if you'd > like to have a dedicated function. So perhaps we could let mshv take > the lead with that? > > If the desire is also to use the fast hypercall technique as > `hv_get_vpreg128` does, then we'd need fast hypercalls on x64 that > can use XMM registers (aka the fast extended hypercalls) that aren't > implemented in the hyperv drivers from my read of the code (although > it seems KVM knows how to do that when it emulates Hyper-V, > arch/x86/kvm/hyperv.c, struct kvm_hv_hcall *hc). > > These considerations make me lean towards deferring hv_get_vpreg128-like > implementation to a later time. > To clarify, I didn't mean to include implementing extended fast hypercalls with the XMM registers, or an implementation of hv_get_vpreg128() for x64 in my suggestion. Just to fix up the existing hv_get_vpreg128 in arch/arm64 to use hv_register_value (or its Linux-specific helper as Nuno suggested). Thanks, Easwar