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

[Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>]

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.

> > > +
> > > +/* 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.