On 12/19/2024 11:32 AM, Easwar Hariharan wrote:
On 12/19/2024 11:23 AM, Nuno Das Neves 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.
The choice is between using hv_get_registers_output which is clearly the
output of the GetVpRegisters hypercall by name, albeit limited as you
If it's desirable to have a more 'friendly' naming here, then I'd be okay with:
```
/* NOTE: Linux helper struct - NOT from Hyper-V code */
struct hv_output_get_vp_registers {
DECLARE_FLEX_ARRAY(struct hv_register_value, values);
}
```
Note also the name is prefixed with "hv_output_" to match other hypercall outputs.
I like the idea of improving our code readability and consistency in
interface naming independently of the hypervisor. That comment should
allow for clarity when new definitions are imported.
Thank you, Easwar and Nuno, I'll use that in v2!
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.
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.
I'd be happy to submit a followup patch to update the arm64 code to use
hv_register_value, or a new struct as outlined above.
It is a pretty small change though, it might be easier to just include it in
this series.
Thank you! I'll leave it you and Roman to decide how you go about that.
- Easwar
--
Thank you,
Roman
