linux-kernel - RE: [PATCH v6 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] [day] [month] [year] [list]
Message-ID:
 <SN6PR02MB4157495A60189FB3D9A7C5CAD458A@SN6PR02MB4157.namprd02.prod.outlook.com>
Date: Sat, 26 Jul 2025 23:50:40 +0000
From: Michael Kelley <mhklinux@...look.com>
To: 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>, Nuno Das Neves
	<nunodasneves@...ux.microsoft.com>, ALOK TIWARI <alok.a.tiwari@...cle.com>,
	Markus Elfring <Markus.Elfring@....de>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-hyperv@...r.kernel.org"
	<linux-hyperv@...r.kernel.org>
Subject: RE: [PATCH v6 2/2] Drivers: hv: Introduce mshv_vtl driver

From: Naman Jain <namjain@...ux.microsoft.com> Sent: Thursday, July 24, 2025 1:26 AM
> 
> Provide an interface for Virtual Machine Monitor like OpenVMM and its
> use as OpenHCL paravisor to control VTL0 (Virtual trust Level).
> Expose devices and support IOCTLs for features like VTL creation,
> VTL0 memory management, context switch, making hypercalls,
> mapping VTL0 address space to VTL2 userspace, getting new VMBus
> messages and channel events in VTL2 etc.
> 
> Co-developed-by: Roman Kisel <romank@...ux.microsoft.com>
> Signed-off-by: Roman Kisel <romank@...ux.microsoft.com>
> Co-developed-by: Saurabh Sengar <ssengar@...ux.microsoft.com>
> Signed-off-by: Saurabh Sengar <ssengar@...ux.microsoft.com>
> Reviewed-by: Roman Kisel <romank@...ux.microsoft.com>
> Reviewed-by: Alok Tiwari <alok.a.tiwari@...cle.com>
> Message-ID: <20250512140432.2387503-3-namjain@...ux.microsoft.com>
> Reviewed-by: Saurabh Sengar <ssengar@...ux.microsoft.com>
> Reviewed-by: Nuno Das Neves <nunodasneves@...ux.microsoft.com>
> Signed-off-by: Naman Jain <namjain@...ux.microsoft.com>
> ---
>  drivers/hv/Kconfig          |   22 +
>  drivers/hv/Makefile         |    7 +-
>  drivers/hv/mshv_vtl.h       |   52 ++
>  drivers/hv/mshv_vtl_main.c  | 1508 +++++++++++++++++++++++++++++++++++
>  include/hyperv/hvgdk_mini.h |  106 +++
>  include/uapi/linux/mshv.h   |   80 ++
>  6 files changed, 1774 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/hv/mshv_vtl.h
>  create mode 100644 drivers/hv/mshv_vtl_main.c
>

[snip]

> +
> +static int mshv_vtl_set_reg(struct hv_register_assoc *regs)
> +{
> +	u64 reg64;
> +	enum hv_register_name gpr_name;
> +	int i;
> +
> +	gpr_name = regs->name;
> +	reg64 = regs->value.reg64;
> +
> +	/* Search for the register in the table */
> +	for (i = 0; i < ARRAY_SIZE(reg_table); i++) {
> +		if (reg_table[i].reg_name == gpr_name) {
> +			if (reg_table[i].debug_reg_num != -1) {
> +				/* Handle debug registers */
> +				if (gpr_name == HV_X64_REGISTER_DR6 &&
> +				    !mshv_vsm_capabilities.dr6_shared)
> +					goto hypercall;
> +				native_set_debugreg(reg_table[i].debug_reg_num, reg64);
> +			} else {
> +				/* Handle MSRs */
> +				wrmsrl(reg_table[i].msr_addr, reg64);
> +			}
> +			return 0;
> +		}
> +	}
> +
> +hypercall:
> +	return 1;
> +}
> +
> +static int mshv_vtl_get_reg(struct hv_register_assoc *regs)
> +{
> +	u64 *reg64;
> +	enum hv_register_name gpr_name;
> +	int i;
> +
> +	gpr_name = regs->name;
> +	reg64 = (u64 *)&regs->value.reg64;
> +
> +	/* Search for the register in the table */
> +	for (i = 0; i < ARRAY_SIZE(reg_table); i++) {
> +		if (reg_table[i].reg_name == gpr_name) {
> +			if (reg_table[i].debug_reg_num != -1) {
> +				/* Handle debug registers */
> +				if (gpr_name == HV_X64_REGISTER_DR6 &&
> +				    !mshv_vsm_capabilities.dr6_shared)
> +					goto hypercall;
> +				*reg64 = native_get_debugreg(reg_table[i].debug_reg_num);
> +			} else {
> +				/* Handle MSRs */
> +				rdmsrl(reg_table[i].msr_addr, *reg64);
> +			}
> +			return 0;
> +		}
> +	}
> +
> +hypercall:
> +	return 1;
> +}
> +

One more comment on this patch. What do you think about
combining mshv_vtl_set_reg() and mshv_vtl_get_reg() into a single
function? The two functions have a lot code duplication that could be
avoided. Here's my untested version (not even compile tested):

+static int mshv_vtl_get_set_reg(struct hv_register_assoc *regs, bool set)
+{
+	u64 *reg64;
+	enum hv_register_name gpr_name;
+	int i;
+
+	gpr_name = regs->name;
+	reg64 = &regs->value.reg64;
+
+	/* Search for the register in the table */
+	for (i = 0; i < ARRAY_SIZE(reg_table); i++) {
+		if (reg_table[i].reg_name != gpr_name)
+			continue;
+		if (reg_table[i].debug_reg_num != -1) {
+			/* Handle debug registers */
+			if (gpr_name == HV_X64_REGISTER_DR6 &&
+			    !mshv_vsm_capabilities.dr6_shared)
+				goto hypercall;
+			if (set)
+				native_set_debugreg(reg_table[i].debug_reg_num, *reg64);
+			else
+				*reg64 = native_get_debugreg(reg_table[i].debug_reg_num);
+		} else {
+			/* Handle MSRs */
+			if (set)
+				wrmsrl(reg_table[i].msr_addr, *reg64);
+			else
+				rdmsrl(reg_table[i].msr_addr, *reg64);
+		}
+		return 0;
+	}
+
+hypercall:
+	return 1;
+}
+

Two call sites would need to be updated to pass "true" and "false",
respectively, for the "set" parameter.

I changed the gpr_name matching to do "continue" on a mismatch
just to avoid a level of indentation. It's functionally the same as your
code.

Michael