答复: [PATCH v1 01/14] vfio: Create vfio_fs_type with inode per device
> -邮件原件- > 发件人: Alex Williamson [mailto:alex.william...@redhat.com] > 发送时间: 2021年4月9日 22:24 > 收件人: Zengtao (B) > 抄送: coh...@redhat.com; k...@vger.kernel.org; > linux-kernel@vger.kernel.org; j...@nvidia.com; pet...@redhat.com > 主题: Re: [PATCH v1 01/14] vfio: Create vfio_fs_type with inode per device > > On Fri, 9 Apr 2021 04:54:23 + > "Zengtao (B)" wrote: > > > > -邮件原件- > > > 发件人: Alex Williamson [mailto:alex.william...@redhat.com] > > > 发送时间: 2021年3月9日 5:47 > > > 收件人: alex.william...@redhat.com > > > 抄送: coh...@redhat.com; k...@vger.kernel.org; > > > linux-kernel@vger.kernel.org; j...@nvidia.com; pet...@redhat.com > > > 主题: [PATCH v1 01/14] vfio: Create vfio_fs_type with inode per device > > > > > > By linking all the device fds we provide to userspace to an address > > > space through a new pseudo fs, we can use tools like > > > unmap_mapping_range() to zap all vmas associated with a device. > > > > > > Suggested-by: Jason Gunthorpe > > > Signed-off-by: Alex Williamson > > > --- > > > drivers/vfio/vfio.c | 54 > > > +++ > > > 1 file changed, 54 insertions(+) > > > > > > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index > > > 38779e6fd80c..abdf8d52a911 100644 > > > --- a/drivers/vfio/vfio.c > > > +++ b/drivers/vfio/vfio.c > > > @@ -32,11 +32,18 @@ > > > #include > > > #include > > > #include > > > +#include > > > +#include > > Minor: keep the headers in alphabetical order. > > They started out that way, but various tree-wide changes ignoring that, and > likely oversights on my part as well, has left us with numerous breaks in that > rule already. > > > > > > > #define DRIVER_VERSION "0.3" > > > #define DRIVER_AUTHOR"Alex Williamson > " > > > #define DRIVER_DESC "VFIO - User Level meta-driver" > > > > > > +#define VFIO_MAGIC 0x5646494f /* "VFIO" */ > > Move to include/uapi/linux/magic.h ? > > Hmm, yeah, I suppose it probably should go there. Thanks. > > FWIW, I'm still working on a next version of this series, currently struggling > how to handle an arbitrary number of vmas per user DMA mapping. Thanks, > Will do some testing on our platform, and want to make sure the issue I reported is included : https://patchwork.kernel.org/project/kvm/patch/1615201890-887-1-git-send-email-prime.z...@hisilicon.com/ Thanks zengtao > Alex
答复: [PATCH v1 01/14] vfio: Create vfio_fs_type with inode per device
> -邮件原件- > 发件人: Jason Gunthorpe [mailto:j...@nvidia.com] > 发送时间: 2021年4月10日 1:32 > 收件人: Alex Williamson > 抄送: Zengtao (B) ; coh...@redhat.com; > k...@vger.kernel.org; linux-kernel@vger.kernel.org; pet...@redhat.com > 主题: Re: [PATCH v1 01/14] vfio: Create vfio_fs_type with inode per device > > On Fri, Apr 09, 2021 at 08:24:00AM -0600, Alex Williamson wrote: > > > > > #define DRIVER_VERSION "0.3" > > > > #define DRIVER_AUTHOR "Alex Williamson > " > > > > #define DRIVER_DESC"VFIO - User Level meta-driver" > > > > > > > > +#define VFIO_MAGIC 0x5646494f /* "VFIO" */ > > > Move to include/uapi/linux/magic.h ? > > > > Hmm, yeah, I suppose it probably should go there. Thanks. > > From my review we haven't been doing that unless it is actually uapi relavent > for some reason (this is not) > Yes, it's not uapi relavent, and I still think it's better to put magic in a single header, and currently not all micros in magic.h are for uapi, some work should be done for that, but no ideas now, :) > In particular with CH having a patch series to eliminate all of these cases > and > drop the constants.. > Interested, could you share the links for that? Thanks Zengtao > Jason
Re: [PATCH v1 01/14] vfio: Create vfio_fs_type with inode per device
On Fri, Apr 09, 2021 at 08:24:00AM -0600, Alex Williamson wrote: > > > #define DRIVER_VERSION "0.3" > > > #define DRIVER_AUTHOR"Alex Williamson " > > > #define DRIVER_DESC "VFIO - User Level meta-driver" > > > > > > +#define VFIO_MAGIC 0x5646494f /* "VFIO" */ > > Move to include/uapi/linux/magic.h ? > > Hmm, yeah, I suppose it probably should go there. Thanks. >From my review we haven't been doing that unless it is actually uapi relavent for some reason (this is not) In particular with CH having a patch series to eliminate all of these cases and drop the constants.. Jason
Re: [PATCH v1 01/14] vfio: Create vfio_fs_type with inode per device
On Fri, 9 Apr 2021 04:54:23 + "Zengtao (B)" wrote: > > -邮件原件- > > 发件人: Alex Williamson [mailto:alex.william...@redhat.com] > > 发送时间: 2021年3月9日 5:47 > > 收件人: alex.william...@redhat.com > > 抄送: coh...@redhat.com; k...@vger.kernel.org; > > linux-kernel@vger.kernel.org; j...@nvidia.com; pet...@redhat.com > > 主题: [PATCH v1 01/14] vfio: Create vfio_fs_type with inode per device > > > > By linking all the device fds we provide to userspace to an address space > > through a new pseudo fs, we can use tools like > > unmap_mapping_range() to zap all vmas associated with a device. > > > > Suggested-by: Jason Gunthorpe > > Signed-off-by: Alex Williamson > > --- > > drivers/vfio/vfio.c | 54 > > +++ > > 1 file changed, 54 insertions(+) > > > > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index > > 38779e6fd80c..abdf8d52a911 100644 > > --- a/drivers/vfio/vfio.c > > +++ b/drivers/vfio/vfio.c > > @@ -32,11 +32,18 @@ > > #include > > #include > > #include > > +#include > > +#include > Minor: keep the headers in alphabetical order. They started out that way, but various tree-wide changes ignoring that, and likely oversights on my part as well, has left us with numerous breaks in that rule already. > > > > #define DRIVER_VERSION "0.3" > > #define DRIVER_AUTHOR "Alex Williamson " > > #define DRIVER_DESC"VFIO - User Level meta-driver" > > > > +#define VFIO_MAGIC 0x5646494f /* "VFIO" */ > Move to include/uapi/linux/magic.h ? Hmm, yeah, I suppose it probably should go there. Thanks. FWIW, I'm still working on a next version of this series, currently struggling how to handle an arbitrary number of vmas per user DMA mapping. Thanks, Alex
答复: [PATCH v1 01/14] vfio: Create vfio_fs_type with inode per device
> -邮件原件-
> 发件人: Alex Williamson [mailto:alex.william...@redhat.com]
> 发送时间: 2021年3月9日 5:47
> 收件人: alex.william...@redhat.com
> 抄送: coh...@redhat.com; k...@vger.kernel.org;
> linux-kernel@vger.kernel.org; j...@nvidia.com; pet...@redhat.com
> 主题: [PATCH v1 01/14] vfio: Create vfio_fs_type with inode per device
>
> By linking all the device fds we provide to userspace to an address space
> through a new pseudo fs, we can use tools like
> unmap_mapping_range() to zap all vmas associated with a device.
>
> Suggested-by: Jason Gunthorpe
> Signed-off-by: Alex Williamson
> ---
> drivers/vfio/vfio.c | 54
> +++
> 1 file changed, 54 insertions(+)
>
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index
> 38779e6fd80c..abdf8d52a911 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -32,11 +32,18 @@
> #include
> #include
> #include
> +#include
> +#include
Minor: keep the headers in alphabetical order.
>
> #define DRIVER_VERSION "0.3"
> #define DRIVER_AUTHOR"Alex Williamson "
> #define DRIVER_DESC "VFIO - User Level meta-driver"
>
> +#define VFIO_MAGIC 0x5646494f /* "VFIO" */
Move to include/uapi/linux/magic.h ?
> +
> +static int vfio_fs_cnt;
> +static struct vfsmount *vfio_fs_mnt;
> +
> static struct vfio {
> struct class*class;
> struct list_headiommu_drivers_list;
> @@ -97,6 +104,7 @@ struct vfio_device {
> struct vfio_group *group;
> struct list_headgroup_next;
> void*device_data;
> + struct inode*inode;
> };
>
> #ifdef CONFIG_VFIO_NOIOMMU
> @@ -529,6 +537,34 @@ static struct vfio_group
> *vfio_group_get_from_dev(struct device *dev)
> return group;
> }
>
> +static int vfio_fs_init_fs_context(struct fs_context *fc) {
> + return init_pseudo(fc, VFIO_MAGIC) ? 0 : -ENOMEM; }
> +
> +static struct file_system_type vfio_fs_type = {
> + .name = "vfio",
> + .owner = THIS_MODULE,
> + .init_fs_context = vfio_fs_init_fs_context,
> + .kill_sb = kill_anon_super,
> +};
> +
> +static struct inode *vfio_fs_inode_new(void) {
> + struct inode *inode;
> + int ret;
> +
> + ret = simple_pin_fs(_fs_type, _fs_mnt, _fs_cnt);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + inode = alloc_anon_inode(vfio_fs_mnt->mnt_sb);
> + if (IS_ERR(inode))
> + simple_release_fs(_fs_mnt, _fs_cnt);
> +
> + return inode;
> +}
> +
> /**
> * Device objects - create, release, get, put, search
> */
> @@ -539,11 +575,19 @@ struct vfio_device
> *vfio_group_create_device(struct vfio_group *group,
>void *device_data)
> {
> struct vfio_device *device;
> + struct inode *inode;
>
> device = kzalloc(sizeof(*device), GFP_KERNEL);
> if (!device)
> return ERR_PTR(-ENOMEM);
>
> + inode = vfio_fs_inode_new();
> + if (IS_ERR(inode)) {
> + kfree(device);
> + return ERR_CAST(inode);
> + }
> + device->inode = inode;
> +
> kref_init(>kref);
> device->dev = dev;
> device->group = group;
> @@ -574,6 +618,9 @@ static void vfio_device_release(struct kref *kref)
>
> dev_set_drvdata(device->dev, NULL);
>
> + iput(device->inode);
> + simple_release_fs(_fs_mnt, _fs_cnt);
> +
> kfree(device);
>
> /* vfio_del_group_dev may be waiting for this device */ @@ -1488,6
> +1535,13 @@ static int vfio_group_get_device_fd(struct vfio_group *group,
> char *buf)
>*/
> filep->f_mode |= (FMODE_LSEEK | FMODE_PREAD | FMODE_PWRITE);
>
> + /*
> + * Use the pseudo fs inode on the device to link all mmaps
> + * to the same address space, allowing us to unmap all vmas
> + * associated to this device using unmap_mapping_range().
> + */
> + filep->f_mapping = device->inode->i_mapping;
> +
> atomic_inc(>container_users);
>
> fd_install(ret, filep);
Re: [PATCH v1 01/14] vfio: Create vfio_fs_type with inode per device
On Mon, Mar 08, 2021 at 02:47:28PM -0700, Alex Williamson wrote:
> By linking all the device fds we provide to userspace to an
> address space through a new pseudo fs, we can use tools like
> unmap_mapping_range() to zap all vmas associated with a device.
>
> Suggested-by: Jason Gunthorpe
> Signed-off-by: Alex Williamson
I'd much prefer to just piggy back on the anon_inode fs rather than
duplicating this logic all over. Something like the trivial patch below
should be all that is needed:
diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index a280156138ed89..6fe404aab0f0dd 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -225,6 +225,12 @@ int anon_inode_getfd_secure(const char *name, const struct
file_operations *fops
}
EXPORT_SYMBOL_GPL(anon_inode_getfd_secure);
+struct inode *alloc_anon_inode_simple(void)
+{
+ return alloc_anon_inode(anon_inode_mnt->mnt_sb);
+}
+EXPORT_SYMBOL_GPL(alloc_anon_inode_simple);
+
static int __init anon_inode_init(void)
{
anon_inode_mnt = kern_mount(_inode_fs_type);
diff --git a/include/linux/anon_inodes.h b/include/linux/anon_inodes.h
index 71881a2b6f7860..6b2fb7d0abc57a 100644
--- a/include/linux/anon_inodes.h
+++ b/include/linux/anon_inodes.h
@@ -21,6 +21,7 @@ int anon_inode_getfd_secure(const char *name,
const struct file_operations *fops,
void *priv, int flags,
const struct inode *context_inode);
+struct inode *alloc_anon_inode_simple(void);
#endif /* _LINUX_ANON_INODES_H */
[PATCH v1 01/14] vfio: Create vfio_fs_type with inode per device
By linking all the device fds we provide to userspace to an
address space through a new pseudo fs, we can use tools like
unmap_mapping_range() to zap all vmas associated with a device.
Suggested-by: Jason Gunthorpe
Signed-off-by: Alex Williamson
---
drivers/vfio/vfio.c | 54 +++
1 file changed, 54 insertions(+)
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 38779e6fd80c..abdf8d52a911 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -32,11 +32,18 @@
#include
#include
#include
+#include
+#include
#define DRIVER_VERSION "0.3"
#define DRIVER_AUTHOR "Alex Williamson "
#define DRIVER_DESC"VFIO - User Level meta-driver"
+#define VFIO_MAGIC 0x5646494f /* "VFIO" */
+
+static int vfio_fs_cnt;
+static struct vfsmount *vfio_fs_mnt;
+
static struct vfio {
struct class*class;
struct list_headiommu_drivers_list;
@@ -97,6 +104,7 @@ struct vfio_device {
struct vfio_group *group;
struct list_headgroup_next;
void*device_data;
+ struct inode*inode;
};
#ifdef CONFIG_VFIO_NOIOMMU
@@ -529,6 +537,34 @@ static struct vfio_group *vfio_group_get_from_dev(struct
device *dev)
return group;
}
+static int vfio_fs_init_fs_context(struct fs_context *fc)
+{
+ return init_pseudo(fc, VFIO_MAGIC) ? 0 : -ENOMEM;
+}
+
+static struct file_system_type vfio_fs_type = {
+ .name = "vfio",
+ .owner = THIS_MODULE,
+ .init_fs_context = vfio_fs_init_fs_context,
+ .kill_sb = kill_anon_super,
+};
+
+static struct inode *vfio_fs_inode_new(void)
+{
+ struct inode *inode;
+ int ret;
+
+ ret = simple_pin_fs(_fs_type, _fs_mnt, _fs_cnt);
+ if (ret)
+ return ERR_PTR(ret);
+
+ inode = alloc_anon_inode(vfio_fs_mnt->mnt_sb);
+ if (IS_ERR(inode))
+ simple_release_fs(_fs_mnt, _fs_cnt);
+
+ return inode;
+}
+
/**
* Device objects - create, release, get, put, search
*/
@@ -539,11 +575,19 @@ struct vfio_device *vfio_group_create_device(struct
vfio_group *group,
void *device_data)
{
struct vfio_device *device;
+ struct inode *inode;
device = kzalloc(sizeof(*device), GFP_KERNEL);
if (!device)
return ERR_PTR(-ENOMEM);
+ inode = vfio_fs_inode_new();
+ if (IS_ERR(inode)) {
+ kfree(device);
+ return ERR_CAST(inode);
+ }
+ device->inode = inode;
+
kref_init(>kref);
device->dev = dev;
device->group = group;
@@ -574,6 +618,9 @@ static void vfio_device_release(struct kref *kref)
dev_set_drvdata(device->dev, NULL);
+ iput(device->inode);
+ simple_release_fs(_fs_mnt, _fs_cnt);
+
kfree(device);
/* vfio_del_group_dev may be waiting for this device */
@@ -1488,6 +1535,13 @@ static int vfio_group_get_device_fd(struct vfio_group
*group, char *buf)
*/
filep->f_mode |= (FMODE_LSEEK | FMODE_PREAD | FMODE_PWRITE);
+ /*
+* Use the pseudo fs inode on the device to link all mmaps
+* to the same address space, allowing us to unmap all vmas
+* associated to this device using unmap_mapping_range().
+*/
+ filep->f_mapping = device->inode->i_mapping;
+
atomic_inc(>container_users);
fd_install(ret, filep);
