| 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
| ||
|
Message-ID: <6a6e50da-43e8-4fb1-a010-13f43b062adc@linux.microsoft.com>
Date: Tue, 29 Jul 2025 10:39:46 +0530
From: Naman Jain <namjain@...ux.microsoft.com>
To: Michael Kelley <mhklinux@...look.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
On 7/27/2025 5:20 AM, Michael Kelley wrote:
> 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 *)®s->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 = ®s->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
Acked, looks good. Thanks for sharing the improvements. Sending v7 now.
Regards,
Naman
Powered by blists - more mailing lists