RE: [RFC PATCH 04/18] virt/mshv: request version ioctl

[Michael Kelley]

From: Nuno Das Neves <nunodasneves@xxxxxxxxxxxxxxxxxxx> Sent: Friday, November 20, 2020 4:30 PM
> 
> Reserve ioctl number in userpsace-api/ioctl/ioctl-number.rst
> Introduce MSHV_REQUEST_VERSION ioctl.
> Introduce documentation for /dev/mshv in Documentation/virt/mshv
> 
> Signed-off-by: Nuno Das Neves <nunodasneves@xxxxxxxxxxxxxxxxxxx>
> ---
>  .../userspace-api/ioctl/ioctl-number.rst      |  2 +
>  Documentation/virt/mshv/api.rst               | 62 +++++++++++++++++++
>  include/linux/mshv.h                          | 11 ++++
>  include/uapi/linux/mshv.h                     | 19 ++++++
>  virt/mshv/mshv_main.c                         | 49 +++++++++++++++
>  5 files changed, 143 insertions(+)
>  create mode 100644 Documentation/virt/mshv/api.rst
>  create mode 100644 include/linux/mshv.h
>  create mode 100644 include/uapi/linux/mshv.h
> 
> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst
> b/Documentation/userspace-api/ioctl/ioctl-number.rst
> index 55a2d9b2ce33..13a4d3ecafca 100644
> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
> @@ -343,6 +343,8 @@ Code  Seq#    Include File                                           Comments
>  0xB5  00-0F  uapi/linux/rpmsg.h                                      <mailto:linux-
> remoteproc@xxxxxxxxxxxxxxx>
>  0xB6  all    linux/fpga-dfl.h
>  0xB7  all    uapi/linux/remoteproc_cdev.h                            <mailto:linux-
> remoteproc@xxxxxxxxxxxxxxx>
> +0xB8  all    uapi/linux/mshv.h                                       Microsoft Hypervisor root partition APIs
> +                                                                     <mailto:linux-hyperv@xxxxxxxxxxxxxxx>
>  0xC0  00-0F  linux/usb/iowarrior.h
>  0xCA  00-0F  uapi/misc/cxl.h
>  0xCA  10-2F  uapi/misc/ocxl.h
> diff --git a/Documentation/virt/mshv/api.rst b/Documentation/virt/mshv/api.rst
> new file mode 100644
> index 000000000000..82e32de48d03
> --- /dev/null
> +++ b/Documentation/virt/mshv/api.rst
> @@ -0,0 +1,62 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=====================================================
> +Microsoft Hypervisor Root Partition API Documentation
> +=====================================================
> +
> +1. Overview
> +===========
> +
> +This document describes APIs for creating and managing guest virtual machines
> +when running Linux as the root partition on the Microsoft Hypervisor.
> +
> +This API is not yet stable.
> +
> +2. Glossary/Terms
> +=================
> +
> +hv
> +--
> +Short for Hyper-V. This name is used in the kernel to describe interfaces to
> +the Microsoft Hypervisor.
> +
> +mshv
> +----
> +Short for Microsoft Hypervisor. This is the name of the userland API module
> +described in this document.
> +
> +Partition
> +---------
> +A virtual machine running on the Microsoft Hypervisor.
> +
> +Root Partition
> +--------------
> +The partition that is created and assumes control when the machine boots. The
> +root partition can use mshv APIs to create guest partitions.
> +
> +3. API description
> +==================
> +
> +The module is named mshv and can be configured with CONFIG_HYPERV_ROOT_API.
> +
> +Mshv is file descriptor-based, following a similar pattern to KVM.
> +
> +To get a handle to the mshv driver, use open("/dev/mshv").
> +
> +3.1 MSHV_REQUEST_VERSION
> +------------------------
> +:Type: /dev/mshv ioctl
> +:Parameters: pointer to a u32
> +:Returns: 0 on success
> +
> +Before issuing any other ioctls, a MSHV_REQUEST_VERSION ioctl must be called to
> +establish the interface version with the kernel module.
> +
> +The caller should pass the MSHV_VERSION as an argument.
> +
> +The kernel module will check which interface versions it supports and return 0
> +if one of them matches.
> +
> +This /dev/mshv file descriptor will remain 'locked' to that version as long as
> +it is open - this ioctl can only be called once per open.

To clarify the wording:

The caller should pass the requested version as an argument.  If the requested
version is one that the kernel module supports, the ioctl will return 0.  If the
requested version is not supported by the kernel module, the caller may try
the ioctl repeatedly to find a version that the caller supports and that the kernel
module supports.   Once a match is found, the /dev/mshv file descriptor is
'locked' to that version as long as it is open; i.e., the ioctl can succeed
only once per open.

> +
> diff --git a/include/linux/mshv.h b/include/linux/mshv.h
> new file mode 100644
> index 000000000000..a0982fe2c0b8
> --- /dev/null
> +++ b/include/linux/mshv.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef _LINUX_MSHV_H
> +#define _LINUX_MSHV_H
> +
> +/*
> + * Microsoft Hypervisor root partition driver for /dev/mshv
> + */
> +
> +#include <uapi/linux/mshv.h>
> +
> +#endif
> diff --git a/include/uapi/linux/mshv.h b/include/uapi/linux/mshv.h
> new file mode 100644
> index 000000000000..dd30fc2f0a80
> --- /dev/null
> +++ b/include/uapi/linux/mshv.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +#ifndef _UAPI_LINUX_MSHV_H
> +#define _UAPI_LINUX_MSHV_H
> +
> +/*
> + * Userspace interface for /dev/mshv
> + * Microsoft Hypervisor root partition APIs
> + */
> +
> +#include <linux/types.h>
> +
> +#define MSHV_VERSION	0x0
> +
> +#define MSHV_IOCTL 0xB8
> +
> +/* mshv device */
> +#define MSHV_REQUEST_VERSION	_IOW(MSHV_IOCTL, 0x00, __u32)
> +
> +#endif
> diff --git a/virt/mshv/mshv_main.c b/virt/mshv/mshv_main.c
> index ecb9089761fe..62f631f85301 100644
> --- a/virt/mshv/mshv_main.c
> +++ b/virt/mshv/mshv_main.c
> @@ -11,25 +11,74 @@
>  #include <linux/module.h>
>  #include <linux/fs.h>
>  #include <linux/miscdevice.h>
> +#include <linux/slab.h>
> +#include <linux/mshv.h>
> 
>  MODULE_AUTHOR("Microsoft");
>  MODULE_LICENSE("GPL");
> 
> +#define MSHV_INVALID_VERSION	0xFFFFFFFF
> +#define MSHV_CURRENT_VERSION	MSHV_VERSION
> +
> +static u32 supported_versions[] = {
> +	MSHV_CURRENT_VERSION,
> +};

I'm not sure that the concept of "CURRENT_VERSION" makes sense
as a fixed constant.  We have an array of supported versions, any of
which are valid and supported by the kernel module.   The array
should list individual versions.   The current version is 0, which 
might be labelled as MSHV_VERSION_PRERELEASE, or something
similar.  Then later we might have MSHV_VERSION_RELEASE_1,
HSMV_VERSION_RELEASE_2, as needed.  Or maybe the versions
are tied to releases of the Microsoft Hypervisor.

> +
> +static long
> +mshv_ioctl_request_version(u32 *version, void __user *user_arg)
> +{
> +	u32 arg;
> +	int i;
> +
> +	if (copy_from_user(&arg, user_arg, sizeof(arg)))
> +		return -EFAULT;
> +
> +	for (i = 0; i < ARRAY_SIZE(supported_versions); ++i) {
> +		if (supported_versions[i] == arg) {
> +			*version = supported_versions[i];
> +			return 0;
> +		}
> +	}
> +	return -ENOTSUPP;
> +}
> +
>  static long
>  mshv_dev_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
>  {
> +	u32 *version = (u32 *)filp->private_data;
> +
> +	if (ioctl == MSHV_REQUEST_VERSION) {
> +		/* Version can only be set once */
> +		if (*version != MSHV_INVALID_VERSION)
> +			return -EBADFD;
> +
> +		return mshv_ioctl_request_version(version, (void __user *)arg);
> +	}
> +
> +	/* Version must be set before other ioctls can be called */
> +	if (*version == MSHV_INVALID_VERSION)
> +		return -EBADFD;
> +
> +	/* TODO other ioctls */
> +
>  	return -ENOTTY;
>  }
> 
>  static int
>  mshv_dev_open(struct inode *inode, struct file *filp)
>  {
> +	filp->private_data = kmalloc(sizeof(u32), GFP_KERNEL);
> +	if (!filp->private_data)
> +		return -ENOMEM;
> +	*(u32 *)filp->private_data = MSHV_INVALID_VERSION;
> +
>  	return 0;
>  }
> 
>  static int
>  mshv_dev_release(struct inode *inode, struct file *filp)
>  {
> +	kfree(filp->private_data);
>  	return 0;
>  }
> 
> --
> 2.25.1