From: Naman Jain <namj...@linux.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 <rom...@linux.microsoft.com>
> Signed-off-by: Roman Kisel <rom...@linux.microsoft.com>
> Co-developed-by: Saurabh Sengar <ssen...@linux.microsoft.com>
> Signed-off-by: Saurabh Sengar <ssen...@linux.microsoft.com>
> Reviewed-by: Roman Kisel <rom...@linux.microsoft.com>
> Reviewed-by: Alok Tiwari <alok.a.tiw...@oracle.com>
> Message-ID: <20250512140432.2387503-3-namj...@linux.microsoft.com>
> Reviewed-by: Saurabh Sengar <ssen...@linux.microsoft.com>
> Reviewed-by: Nuno Das Neves <nunodasne...@linux.microsoft.com>
> Signed-off-by: Naman Jain <namj...@linux.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
