On Wed, Oct 26, 2022 at 08:58:23PM +0800, Baolu Lu wrote: > > + [_IOC_NR(_ioctl) - IOMMUFD_CMD_BASE] = { \ > > + .size = sizeof(_struct) + \ > > + BUILD_BUG_ON_ZERO(sizeof(union ucmd_buffer) < \ > > + sizeof(_struct)), \ > > + .min_size = offsetofend(_struct, _last), \ > > + .ioctl_num = _ioctl, \ > > + .execute = _fn, \ > > + } > > +static struct iommufd_ioctl_op iommufd_ioctl_ops[] = { > > How about making the ops "static const"? Yes both const's were missed > > +static void __exit iommufd_exit(void) > > +{ > > + misc_deregister(&iommu_misc_dev); > > +} > > + > > +module_init(iommufd_init); > > +module_exit(iommufd_exit); > > + > > +MODULE_DESCRIPTION("I/O Address Space Management for passthrough devices"); > > +MODULE_LICENSE("GPL"); > > Could above be "GPL v2"? It should be just "GPL", see Documentation/process/license-rules.rst: "GPL v2" Same as "GPL". It exists for historic > > --- /dev/null > > +++ b/include/uapi/linux/iommufd.h > > @@ -0,0 +1,55 @@ > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > > +/* Copyright (c) 2021-2022, NVIDIA CORPORATION & AFFILIATES. > > + */ > > +#ifndef _UAPI_IOMMUFD_H > > +#define _UAPI_IOMMUFD_H > > + > > +#include <linux/types.h> > > +#include <linux/ioctl.h> > > + > > +#define IOMMUFD_TYPE (';') > > + > > +/** > > + * DOC: General ioctl format > > + * > > + * The ioctl mechanims follows a general format to allow for extensibility. Each > ^^^^^^^^^ mechanism? How about "interface" ? Thanks, Jason