[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <80853cdb-fd34-4a5e-99a0-1a71b8ce8226@linux.microsoft.com>
Date: Wed, 21 May 2025 11:33:29 +0530
From: Naman Jain <namjain@...ux.microsoft.com>
To: Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com>
Cc: "K . Y . Srinivasan" <kys@...rosoft.com>,
Haiyang Zhang <haiyangz@...rosoft.com>, Wei Liu <wei.liu@...nel.org>,
Dexuan Cui <decui@...rosoft.com>, Roman Kisel <romank@...ux.microsoft.com>,
Anirudh Rayabharam <anrayabh@...ux.microsoft.com>,
Saurabh Sengar <ssengar@...ux.microsoft.com>,
Nuno Das Neves <nunodasneves@...ux.microsoft.com>,
ALOK TIWARI <alok.a.tiwari@...cle.com>, linux-kernel@...r.kernel.org,
linux-hyperv@...r.kernel.org
Subject: Re: [PATCH v3 2/2] Drivers: hv: Introduce mshv_vtl driver
On 5/21/2025 12:25 AM, Stanislav Kinsburskii wrote:
> On Mon, May 19, 2025 at 10:26:42AM +0530, Naman Jain wrote:
>> 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>
>> Signed-off-by: Naman Jain <namjain@...ux.microsoft.com>
>> ---
>> drivers/hv/Kconfig | 20 +
>> drivers/hv/Makefile | 7 +-
>> drivers/hv/mshv_vtl.h | 52 +
>> drivers/hv/mshv_vtl_main.c | 1783 +++++++++++++++++++++++++++++++++++
>> include/hyperv/hvgdk_mini.h | 81 ++
>> include/hyperv/hvhdk.h | 1 +
>> include/uapi/linux/mshv.h | 82 ++
>> 7 files changed, 2025 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/hv/mshv_vtl.h
>> create mode 100644 drivers/hv/mshv_vtl_main.c
>>
>> diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
>> index eefa0b559b73..21cee5564d70 100644
>> --- a/drivers/hv/Kconfig
>> +++ b/drivers/hv/Kconfig
>> @@ -72,4 +72,24 @@ config MSHV_ROOT
>>
>> If unsure, say N.
>>
>> +config MSHV_VTL
>> + tristate "Microsoft Hyper-V VTL driver"
>> + depends on HYPERV && X86_64
>> + depends on TRANSPARENT_HUGEPAGE
>
> Why does it depend on TRANSPARENT_HUGEPAGE?
>
Thanks for reviewing. This config is required for below functions which
are used for mshv_vtl_low device.
vm_fault_t mshv_vtl_low_huge_fault ->
* vmf_insert_pfn_pmd
* vmf_insert_pfn_pud
> <snip>
>
>> diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
>> index 1be7f6a02304..cc11000e39f4 100644
>> --- a/include/hyperv/hvgdk_mini.h
>> +++ b/include/hyperv/hvgdk_mini.h
>> @@ -882,6 +882,23 @@ struct hv_get_vp_from_apic_id_in {
>> u32 apic_ids[];
>> } __packed;
>>
>> +union hv_register_vsm_partition_config {
>> + __u64 as_u64;
>
> Please, follow the file pattern: as_u64 -> as_uint64
>
>> + struct {
>> + __u64 enable_vtl_protection : 1;
>
> Ditto: __u64 -> u64
>
>> + __u64 default_vtl_protection_mask : 4;
>> + __u64 zero_memory_on_reset : 1;
>> + __u64 deny_lower_vtl_startup : 1;
>> + __u64 intercept_acceptance : 1;
>> + __u64 intercept_enable_vtl_protection : 1;
>> + __u64 intercept_vp_startup : 1;
>> + __u64 intercept_cpuid_unimplemented : 1;
>> + __u64 intercept_unrecoverable_exception : 1;
>> + __u64 intercept_page : 1;
>> + __u64 mbz : 51;
>> + };
>> +};
>> +
>
>> /*
>> diff --git a/include/hyperv/hvhdk.h b/include/hyperv/hvhdk.h
>> index b4067ada02cf..9b890126e8e8 100644
>> --- a/include/hyperv/hvhdk.h
>> +++ b/include/hyperv/hvhdk.h
>> @@ -479,6 +479,7 @@ struct hv_connection_info {
>> #define HV_EVENT_FLAGS_COUNT (256 * 8)
>> #define HV_EVENT_FLAGS_BYTE_COUNT (256)
>> #define HV_EVENT_FLAGS32_COUNT (256 / sizeof(u32))
>> +#define HV_EVENT_FLAGS_LONG_COUNT (HV_EVENT_FLAGS_BYTE_COUNT / sizeof(__u64))
>
> Ditto
>
>>
>> /* linux side we create long version of flags to use long bit ops on flags */
>> #define HV_EVENT_FLAGS_UL_COUNT (256 / sizeof(ulong))
>> diff --git a/include/uapi/linux/mshv.h b/include/uapi/linux/mshv.h
>> index 876bfe4e4227..a8c39b08b39a 100644
>> --- a/include/uapi/linux/mshv.h
>> +++ b/include/uapi/linux/mshv.h
>> @@ -288,4 +288,86 @@ struct mshv_get_set_vp_state {
>> * #define MSHV_ROOT_HVCALL _IOWR(MSHV_IOCTL, 0x07, struct mshv_root_hvcall)
>> */
>>
>> +/* Structure definitions, macros and IOCTLs for mshv_vtl */
>> +
>> +#define MSHV_CAP_CORE_API_STABLE 0x0
>> +#define MSHV_CAP_REGISTER_PAGE 0x1
>> +#define MSHV_CAP_VTL_RETURN_ACTION 0x2
>> +#define MSHV_CAP_DR6_SHARED 0x3
>> +#define MSHV_MAX_RUN_MSG_SIZE 256
>> +
>> +#define MSHV_VP_MAX_REGISTERS 128
>> +
>> +struct mshv_vp_registers {
>> + __u32 count; /* at most MSHV_VP_MAX_REGISTERS */
>
> Same here: __u{32,64} -> u{32,64}.
>
> Please, address everywhere.
>
I'll take care of all of these in my next patch.
> <snip>
>
>> +
>> +/* vtl device */
>> +#define MSHV_CREATE_VTL _IOR(MSHV_IOCTL, 0x1D, char)
>> +#define MSHV_VTL_ADD_VTL0_MEMORY _IOW(MSHV_IOCTL, 0x21, struct mshv_vtl_ram_disposition)
>> +#define MSHV_VTL_SET_POLL_FILE _IOW(MSHV_IOCTL, 0x25, struct mshv_vtl_set_poll_file)
>> +#define MSHV_VTL_RETURN_TO_LOWER_VTL _IO(MSHV_IOCTL, 0x27)
>> +#define MSHV_GET_VP_REGISTERS _IOWR(MSHV_IOCTL, 0x05, struct mshv_vp_registers)
>> +#define MSHV_SET_VP_REGISTERS _IOW(MSHV_IOCTL, 0x06, struct mshv_vp_registers)
>> +
>> +/* VMBus device IOCTLs */
>> +#define MSHV_SINT_SIGNAL_EVENT _IOW(MSHV_IOCTL, 0x22, struct mshv_vtl_signal_event)
>> +#define MSHV_SINT_POST_MESSAGE _IOW(MSHV_IOCTL, 0x23, struct mshv_vtl_sint_post_msg)
>> +#define MSHV_SINT_SET_EVENTFD _IOW(MSHV_IOCTL, 0x24, struct mshv_vtl_set_eventfd)
>> +#define MSHV_SINT_PAUSE_MESSAGE_STREAM _IOW(MSHV_IOCTL, 0x25, struct mshv_sint_mask)
>> +
>> +/* hv_hvcall device */
>> +#define MSHV_HVCALL_SETUP _IOW(MSHV_IOCTL, 0x1E, struct mshv_vtl_hvcall_setup)
>> +#define MSHV_HVCALL _IOWR(MSHV_IOCTL, 0x1F, struct mshv_vtl_hvcall)
>
> How many of these ioctls are actually used by the mshv root driver?
> Should those which are VTl-specific be named as such (like MSHV_VTL_SET_POLL_FILE)?
> Another option would be to keep all the names generic.
>
> Thanks,
> Stanislav
None of the IOCTLs in mshv_vtl section, introduced in this patch is used
by mshv_root driver. Since IOCTLs of mshv_root does not have MSHV_ROOT
prefix, I am OK with removing MSHV_VTL_* prefix from these IOCTL names.
You can let me know if you want me to prefix them with MSHV_VTL.
Thanks again for reviewing.
Regards,
Naman
>
>> #endif
>> --
>> 2.34.1
>>
Powered by blists - more mailing lists