| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 3 Sep 2020 14:53:59 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: shuo.a.liu@...el.com
Cc: linux-kernel@...r.kernel.org, x86@...nel.org,
"H . Peter Anvin" <hpa@...or.com>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Sean Christopherson <sean.j.christopherson@...el.com>,
Yu Wang <yu1.wang@...el.com>,
Reinette Chatre <reinette.chatre@...el.com>,
Dave Hansen <dave.hansen@...el.com>,
Zhi Wang <zhi.a.wang@...el.com>,
Zhenyu Wang <zhenyuw@...ux.intel.com>
Subject: Re: [PATCH v2 05/17] virt: acrn: Introduce ACRN HSM basic driver
On Thu, Sep 03, 2020 at 08:41:49PM +0800, shuo.a.liu@...el.com wrote:
> From: Shuo Liu <shuo.a.liu@...el.com>
>
> ACRN Hypervisor Service Module (HSM) is a kernel module in Service VM
> which communicates with ACRN userspace through ioctls and talks to ACRN
> Hypervisor through hypercalls.
>
> Add a basic HSM driver which allows Service VM userspace to communicate
> with ACRN. The following patches will add more ioctls, guest VM memory
> mapping caching, I/O request processing, ioeventfd and irqfd into this
> module. HSM exports a char device interface (/dev/acrn_hsm) to userspace.
>
> Signed-off-by: Shuo Liu <shuo.a.liu@...el.com>
> Reviewed-by: Reinette Chatre <reinette.chatre@...el.com>
> Cc: Dave Hansen <dave.hansen@...el.com>
> Cc: Zhi Wang <zhi.a.wang@...el.com>
> Cc: Zhenyu Wang <zhenyuw@...ux.intel.com>
> Cc: Yu Wang <yu1.wang@...el.com>
> Cc: Reinette Chatre <reinette.chatre@...el.com>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> ---
> .../userspace-api/ioctl/ioctl-number.rst | 1 +
> MAINTAINERS | 2 +
> drivers/virt/Kconfig | 2 +
> drivers/virt/Makefile | 1 +
> drivers/virt/acrn/Kconfig | 14 +++
> drivers/virt/acrn/Makefile | 3 +
> drivers/virt/acrn/acrn_drv.h | 19 ++++
> drivers/virt/acrn/hsm.c | 98 +++++++++++++++++++
> include/uapi/linux/acrn.h | 17 ++++
> 9 files changed, 157 insertions(+)
> create mode 100644 drivers/virt/acrn/Kconfig
> create mode 100644 drivers/virt/acrn/Makefile
> create mode 100644 drivers/virt/acrn/acrn_drv.h
> create mode 100644 drivers/virt/acrn/hsm.c
> create mode 100644 include/uapi/linux/acrn.h
>
> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
> index 2a198838fca9..ac60efedb104 100644
> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
> @@ -319,6 +319,7 @@ Code Seq# Include File Comments
> 0xA0 all linux/sdp/sdp.h Industrial Device Project
> <mailto:kenji@...gate.com>
> 0xA1 0 linux/vtpm_proxy.h TPM Emulator Proxy Driver
> +0xA2 all uapi/linux/acrn.h ACRN hypervisor
You don't have any ioctls in this patch, so why add this documentation
here?
> 0xA3 80-8F Port ACL in development:
> <mailto:tlewis@...dspring.com>
> 0xA3 90-9F linux/dtlk.h
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e0fea5e464b4..d4c1ef303c2d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -442,6 +442,8 @@ L: acrn-dev@...ts.projectacrn.org
> S: Supported
> W: https://projectacrn.org
> F: Documentation/virt/acrn/
> +F: drivers/virt/acrn/
> +F: include/uapi/linux/acrn.h
This uapi file is not used in this patch, please add it in a later
patch.
> +static long acrn_dev_ioctl(struct file *filp, unsigned int cmd,
> + unsigned long ioctl_param)
> +{
> + return 0;
> +}
As your ioctl does nothing, no need to include it here, add it in a
later patch.
> +
> +static int acrn_dev_release(struct inode *inode, struct file *filp)
> +{
> + struct acrn_vm *vm = filp->private_data;
> +
> + kfree(vm);
> + return 0;
> +}
> +
> +static const struct file_operations acrn_fops = {
> + .owner = THIS_MODULE,
> + .open = acrn_dev_open,
> + .release = acrn_dev_release,
> + .unlocked_ioctl = acrn_dev_ioctl,
> +};
> +
> +static struct miscdevice acrn_dev = {
> + .minor = MISC_DYNAMIC_MINOR,
> + .name = "acrn_hsm",
> + .fops = &acrn_fops,
> +};
> +
> +static int __init hsm_init(void)
> +{
> + int ret;
> +
> + if (x86_hyper_type != X86_HYPER_ACRN)
> + return -ENODEV;
> +
> + if (!acrn_is_privileged_vm())
> + return -EPERM;
> +
> + ret = misc_register(&acrn_dev);
> + if (ret) {
> + pr_err("Create misc dev failed!\n");
> + return ret;
> + }
> +
> + return 0;
Tiny tiny nit, these lines can be rewritten as:
if (ret)
pr_err("Create misc dev failed!\n");
return ret;
:)
> +}
> +
> +static void __exit hsm_exit(void)
> +{
> + misc_deregister(&acrn_dev);
> +}
> +module_init(hsm_init);
> +module_exit(hsm_exit);
> +
> +MODULE_AUTHOR("Intel Corporation");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("ACRN Hypervisor Service Module (HSM)");
> diff --git a/include/uapi/linux/acrn.h b/include/uapi/linux/acrn.h
> new file mode 100644
> index 000000000000..4ae34f86e2be
> --- /dev/null
> +++ b/include/uapi/linux/acrn.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Userspace interface for /dev/acrn_hsm - ACRN Hypervisor Service Module
> + *
> + * This file can be used by applications that need to communicate with the HSM
> + * via the ioctl interface.
> + */
> +
> +#ifndef _UAPI_ACRN_H
> +#define _UAPI_ACRN_H
> +
> +#include <linux/types.h>
> +
> +/* The ioctl type, documented in ioctl-number.rst */
> +#define ACRN_IOCTL_TYPE 0xA2
This isn't used in this patch, so save it for a later one please.
thanks,
greg k-h
Powered by blists - more mailing lists