Re: [PATCH v3 2/2] Drivers: hv: Introduce mshv_vtl driver

[Naman Jain <namjain@linux.microsoft.com>]

On 5/22/2025 11:40 PM, Stanislav Kinsburskii wrote:
> On Wed, May 21, 2025 at 11:33:29AM +0530, Naman Jain wrote:
>>
>>
>> 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@linux.microsoft.com>
>>>> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
>>>> Co-developed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
>>>> Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
>>>> Reviewed-by: Roman Kisel <romank@linux.microsoft.com>
>>>> Reviewed-by: Alok Tiwari <alok.a.tiwari@oracle.com>
>>>> Message-ID: <20250512140432.2387503-3-namjain@linux.microsoft.com>
>>>> Signed-off-by: Naman Jain <namjain@linux.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?
>>>
>>
> 
> Let me rephrase: can this driver work without transparent huge pages?
> If yes, then it shouldn't depend on the option and rather select it.
> If not, then it should be either fixed to be able to or have an
> expalanation why this dependecy was introduced.

No, it won't work. Reason being - we are adding support to map VTL0 
address space to a user-mode process in VTL2. VTL2 for OpenHCL makes use 
of Huge pages to improve performance on VMs having large memory 
requirements. Thus, we need TRANSPARENT_HUGEPAGE.

I will add this as a comment in Kconfig.

> 
>>>> +
>>>> +/* 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
>>
> 
> As these ioctls share the same "namespace" (MSHV_IOCTL), removal os the
> VTL suffix looks a better optio to me.
> 
> Thanks,
> Stanislav.


Will make the changes in next patch. Thanks.

Regards,
Naman