On 7/9/2025 10:19 AM, Michael Kelley wrote:
> From: Naman Jain <namj...@linux.microsoft.com> Sent: Wednesday, June 11, 2025
> 12:27 AM
>> +
>> +union mshv_synic_overlay_page_msr {
>> + u64 as_uint64;
>> + struct {
>> + u64 enabled: 1;
>> + u64 reserved: 11;
>> + u64 pfn: 52;
>> + };
>
> Since this appear to be a Hyper-V synthetic MSR, add __packed?
>
>> +};
>> +
>> +union hv_register_vsm_capabilities {
>> + u64 as_uint64;
>> + struct {
>> + u64 dr6_shared: 1;
>> + u64 mbec_vtl_mask: 16;
>> + u64 deny_lower_vtl_startup: 1;
>> + u64 supervisor_shadow_stack: 1;
>> + u64 hardware_hvpt_available: 1;
>> + u64 software_hvpt_available: 1;
>> + u64 hardware_hvpt_range_bits: 6;
>> + u64 intercept_page_available: 1;
>> + u64 return_action_available: 1;
>> + u64 reserved: 35;
>> + } __packed;
>> +};
>> +
>> +union hv_register_vsm_page_offsets {
>> + struct {
>> + u64 vtl_call_offset : 12;
>> + u64 vtl_return_offset : 12;
>> + u64 reserved_mbz : 40;
>> + };
>> + u64 as_uint64;
>> +} __packed;
>
> We've usually put the __packed on the struct definition. Consistency .... :-)
>
> Don't these three register definitions belong somewhere in the
> hvhdk or hvgdk include files?
>
I agree, hv_register_vsm_capabilities and hv_register_vsm_page_offsets
can be moved to the appropriate include/hyperv/ header/s.
Regarding mshv_synic_overlay_page_msr, it is a generic structure that
appears to be used for several overlay page MSRs (SIMP, SIEF, etc).
But, the type doesn't appear in the hv*dk headers explicitly; it's just
used internally by the hypervisor.
I think it should be renamed with a hv_ prefix to indicate it's part of
the hypervisor ABI, and a brief comment with the provenance:
/* SYNIC_OVERLAY_PAGE_MSR - internal, identical to hv_synic_simp */
union hv_synic_overlay_page_msr {
/* <snip> */
};
And I'm fine with it staying in this file since it's only used here right
now, and doesn't really come from the one of the hyperv headers.
Nuno
