Re: [PATCH 7/7] [v4] drivers/virt: introduce Freescale hypervisor management driver

On Wed, 8 Jun 2011 17:45:54 -0500 Timur Tabi wrote:

Add the drivers/virt directory, which houses drivers that support
virtualization environments, and add the Freescale hypervisor management
driver.

It can't go in linux/virt or linux/virt/fsl instead?  why drivers/ ?

or maybe linux/virt should be drivers/virt ?

The Freescale hypervisor management driver provides several services to
drivers and applications related to the Freescale hypervisor:

1. An ioctl interface for querying and managing partitions

2. A file interface to reading incoming doorbells

3. An interrupt handler for shutting down the partition upon receiving the
   shutdown doorbell from a manager partition

4. An interface for receiving callbacks when a managed partition shuts down.

Signed-off-by: Timur Tabi <redacted>
---
 drivers/Kconfig                |    2 +
 drivers/Makefile               |    3 +
 drivers/virt/Kconfig           |   22 +
 drivers/virt/Makefile          |    5 +
 drivers/virt/fsl_hypervisor.c  |  961 ++++++++++++++++++++++++++++++++++++++++
 include/linux/Kbuild           |    1 +
 include/linux/fsl_hypervisor.h |  214 +++++++++
 7 files changed, 1208 insertions(+), 0 deletions(-)
 create mode 100644 drivers/virt/Kconfig
 create mode 100644 drivers/virt/Makefile
 create mode 100644 drivers/virt/fsl_hypervisor.c
 create mode 100644 include/linux/fsl_hypervisor.h

Why any later version?  I thought that <powers> decided that Linux
is GPL v2.

+ *
+ * This software is provided by Freescale Semiconductor "as is" and any
+ * express or implied warranties, including, but not limited to, the implied
+ * warranties of merchantability and fitness for a particular purpose are
+ * disclaimed. In no event shall Freescale Semiconductor be liable for any
+ * direct, indirect, incidental, special, exemplary, or consequential damages
+ * (including, but not limited to, procurement of substitute goods or services;
+ * loss of use, data, or profits; or business interruption) however caused and
+ * on any theory of liability, whether in contract, strict liability, or tort
+ * (including negligence or otherwise) arising in any way out of the use of this
+ * software, even if advised of the possibility of such damage.
+ *
+ * This file is used by the Freescale hypervisor management driver.  It can
+ * also be included by applications that need to communicate with the driver
+ * via the ioctl interface.
+ */
+
+#ifndef FSL_HYPERVISOR_H
+#define FSL_HYPERVISOR_H
+
+#include <linux/types.h>
+
+/**
+ * struct fsl_hv_ioctl_restart: restart a partition

This syntax should be (for all structs here):

 * struct fsl_hv_ioctl_restart - restart a partition

but the struct fields/members do use ':' instead of '-'.

Darn, I checked Documentation/kernel-doc-nano-HOWTO.txt and it says
that ':' is optional but '-' is needed, so you could use

 * struct fsl_hv_ioctl_restart: - restart a partition

+ * @ret: return error code from the hypervisor
+ * @partition: the ID of the partition to restart, or -1 for the
+ *             calling partition
+ *
+ * Used by FSL_HV_IOCTL_PARTITION_RESTART
+ */
+struct fsl_hv_ioctl_restart {
+	__u32 ret;
+	__u32 partition;
+};
+
+/**
+ * struct fsl_hv_ioctl_status: get a partition's status
+ * @ret: return error code from the hypervisor
+ * @partition: the ID of the partition to query, or -1 for the
+ *             calling partition
+ * @status: The returned status of the partition
+ *
+ * Used by FSL_HV_IOCTL_PARTITION_GET_STATUS
+ *
+ * Values of 'status':
+ *    0 = Stopped
+ *    1 = Running
+ *    2 = Starting
+ *    3 = Stopping
+ */
+struct fsl_hv_ioctl_status {
+	__u32 ret;
+	__u32 partition;
+	__u32 status;
+};
+
+/**
+ * struct fsl_hv_ioctl_start: start a partition
+ * @ret: return error code from the hypervisor
+ * @partition: the ID of the partition to control
+ * @entry_point: The offset within the guest IMA to start execution
+ * @load: If non-zero, reload the partition's images before starting
+ *
+ * Used by FSL_HV_IOCTL_PARTITION_START
+ */
+struct fsl_hv_ioctl_start {
+	__u32 ret;
+	__u32 partition;
+	__u32 entry_point;
+	__u32 load;
+};
+
+/**
+ * struct fsl_hv_ioctl_stop: stop a partition
+ * @ret: return error code from the hypervisor
+ * @partition: the ID of the partition to stop, or -1 for the calling
+ *             partition
+ *
+ * Used by FSL_HV_IOCTL_PARTITION_STOP
+ */
+struct fsl_hv_ioctl_stop {
+	__u32 ret;
+	__u32 partition;
+};
+
+/**
+ * struct fsl_hv_ioctl_memcpy: copy memory between partitions
+ * @ret: return error code from the hypervisor
+ * @source: the partition ID of the source partition, or -1 for this
+ *          partition
+ * @target: the partition ID of the target partition, or -1 for this
+ *          partition
+ * @local_addr: user-space virtual address of a buffer in the local
+ *              partition
+ * @remote_addr: guest physical address of a buffer in the
+ *           remote partition
+ * @count: the number of bytes to copy.  Both the local and remote
+ *         buffers must be at least 'count' bytes long
+ *
+ * Used by FSL_HV_IOCTL_MEMCPY
+ *
+ * The 'local' partition is the partition that calls this ioctl.  The
+ * 'remote' partition is a different partition.  The data is copied from
+ * the 'source' paritition' to the 'target' partition.
+ *
+ * The buffer in the remote partition must be guest physically
+ * contiguous.
+ *
+ * This ioctl does not support copying memory between two remote
+ * partitions or within the same partition, so either 'source' or
+ * 'target' (but not both) must be -1.  In other words, either
+ *
+ *      source == local and target == remote
+ * or
+ *      source == remote and target == local
+ */
+struct fsl_hv_ioctl_memcpy {
+	__u32 ret;
+	__u32 source;
+	__u32 target;
+	__u64 local_vaddr;
+	__u64 remote_paddr;
+	__u64 count;
+};
+
+/**
+ * struct fsl_hv_ioctl_doorbell: ring a doorbell
+ * @ret: return error code from the hypervisor
+ * @doorbell: the handle of the doorbell to ring doorbell
+ *
+ * Used by FSL_HV_IOCTL_DOORBELL
+ */
+struct fsl_hv_ioctl_doorbell {
+	__u32 ret;
+	__u32 doorbell;
+};
+
+/**
+ * struct fsl_hv_ioctl_prop: get/set a device tree property
+ * @ret: return error code from the hypervisor
+ * @handle: handle of partition whose tree to access
+ * @path: virtual address of path name of node to access
+ * @propname: virtual address of name of property to access
+ * @propval: virtual address of property data buffer
+ * @proplen: Size of property data buffer
+ *
+ * Used by FSL_HV_IOCTL_DOORBELL
+ */
+struct fsl_hv_ioctl_prop {
+	__u32 ret;
+	__u32 handle;
+	__u64 path;
+	__u64 propname;
+	__u64 propval;
+	__u32 proplen;
+};
+
+/*
+ * ioctl commands.
+ */
+enum {
+	FSL_HV_IOCTL_PARTITION_RESTART = 1, /* Boot another partition */
+	FSL_HV_IOCTL_PARTITION_GET_STATUS = 2, /* Boot another partition */
+	FSL_HV_IOCTL_PARTITION_START = 3, /* Boot another partition */
+	FSL_HV_IOCTL_PARTITION_STOP = 4, /* Stop this or another partition */
+	FSL_HV_IOCTL_MEMCPY = 5, /* Copy data from one partition to another */
+	FSL_HV_IOCTL_DOORBELL = 6, /* Ring a doorbell */
+
+	/* Get a property from another guest's device tree */
+	FSL_HV_IOCTL_GETPROP = 7,
+
+	/* Set a property in another guest's device tree */
+	FSL_HV_IOCTL_SETPROP = 8,
+};
+
+#ifdef __KERNEL__
+
+/**
+ * fsl_hv_event_register -- register a callback for failover events

Documentation/kernel-doc-nano-HOWTO.txt says to use one '-' here, not 2.

+ *
+ * This function is called by device drivers to register their callback
+ * functions for fail-over events.
+ */
+int fsl_hv_failover_register(struct notifier_block *nb);
+
+/**
+ * fsl_hv_event_unregister -- unregister a callback for failover events

ditto.
And function arg descriptions are missing for both of these functions.

+ */
+int fsl_hv_failover_unregister(struct notifier_block *nb);
+
+#endif
+
+#endif
-- 

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***