linux-kernel - Re: [PATCH v5 2/2] Drivers: hv: Introduce mshv

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <68143eb0-e6a7-4579-bedb-4c2ec5aaef6b@linux.microsoft.com>
Date: Thu, 17 Jul 2025 09:21:51 -0700
From: Nuno Das Neves <nunodasneves@...ux.microsoft.com>
To: Michael Kelley <mhklinux@...look.com>,
 Naman Jain <namjain@...ux.microsoft.com>,
 "K . Y . Srinivasan" <kys@...rosoft.com>,
 Haiyang Zhang <haiyangz@...rosoft.com>, Wei Liu <wei.liu@...nel.org>,
 Dexuan Cui <decui@...rosoft.com>
Cc: Roman Kisel <romank@...ux.microsoft.com>,
 Anirudh Rayabharam <anrayabh@...ux.microsoft.com>,
 Saurabh Sengar <ssengar@...ux.microsoft.com>,
 Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com>,
 ALOK TIWARI <alok.a.tiwari@...cle.com>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>
Subject: Re: [PATCH v5 2/2] Drivers: hv: Introduce mshv_vtl driver

On 7/9/2025 10:19 AM, Michael Kelley wrote:
> From: Naman Jain <namjain@...ux.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