| 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
| ||
|
Message-ID: <f2f33bee-8aef-f2e6-89a3-4e417dc6071d@intel.com>
Date: Fri, 4 Sep 2020 09:12:47 +0800
From: "Liu, Shuo A" <shuo.a.liu@...el.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
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 9/3/2020 20:53, Greg Kroah-Hartman wrote:
> 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?
This was left when i removed an api version ioctl from the v1 patch set.
Let me move it to a later patch.
>> 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.
OK.
>
>> +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.
OK.
>
>> +
>> +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;
>
> :)
OK. Thanks.
>
>> +}
>> +
>> +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.
Sure. I will move to later one.
Thanks
shuo
Powered by blists - more mailing lists