RE: [PATCH v6 07/15] vfio/type1: Add VFIO_IOMMU_PASID_REQUEST (alloc/free)
Hi Alex, > From: Alex Williamson > Sent: Friday, August 21, 2020 9:49 AM > > On Fri, 21 Aug 2020 00:37:19 + > "Liu, Yi L" wrote: > > > Hi Alex, > > > > > From: Alex Williamson > > > Sent: Friday, August 21, 2020 4:51 AM > > > > > > On Mon, 27 Jul 2020 23:27:36 -0700 > > > Liu Yi L wrote: > > > > > > > This patch allows userspace to request PASID allocation/free, e.g. > > > > when serving the request from the guest. > > > > > > > > PASIDs that are not freed by userspace are automatically freed when > > > > the IOASID set is destroyed when process exits. > > > > > > > > Cc: Kevin Tian > > > > CC: Jacob Pan > > > > Cc: Alex Williamson > > > > Cc: Eric Auger > > > > Cc: Jean-Philippe Brucker > > > > Cc: Joerg Roedel > > > > Cc: Lu Baolu > > > > Signed-off-by: Liu Yi L > > > > Signed-off-by: Yi Sun > > > > Signed-off-by: Jacob Pan > > > > --- > > > > v5 -> v6: > > > > *) address comments from Eric against v5. remove the alloc/free helper. > > > > > > > > v4 -> v5: > > > > *) address comments from Eric Auger. > > > > *) the comments for the PASID_FREE request is addressed in patch 5/15 of > > > >this series. > > > > > > > > v3 -> v4: > > > > *) address comments from v3, except the below comment against the range > > > >of PASID_FREE request. needs more help on it. > > > > "> +if (req.range.min > req.range.max) > > > > > > > > Is it exploitable that a user can spin the kernel for a long time > > > > in > > > > the case of a free by calling this with [0, MAX_UINT] regardless of > > > > their actual allocations?" > > > > > > > > https://lore.kernel.org/linux-iommu/20200702151832.048b4...@x1.home/ > > > > > > > > v1 -> v2: > > > > *) move the vfio_mm related code to be a seprate module > > > > *) use a single structure for alloc/free, could support a range of > > > > PASIDs > > > > *) fetch vfio_mm at group_attach time instead of at iommu driver open > > > > time > > > > --- > > > > drivers/vfio/Kconfig| 1 + > > > > drivers/vfio/vfio_iommu_type1.c | 69 > > > + > > > > drivers/vfio/vfio_pasid.c | 10 ++ > > > > include/linux/vfio.h| 6 > > > > include/uapi/linux/vfio.h | 37 ++ > > > > 5 files changed, 123 insertions(+) > > > > > > > > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig index > > > > 3d8a108..95d90c6 100644 > > > > --- a/drivers/vfio/Kconfig > > > > +++ b/drivers/vfio/Kconfig > > > > @@ -2,6 +2,7 @@ > > > > config VFIO_IOMMU_TYPE1 > > > > tristate > > > > depends on VFIO > > > > + select VFIO_PASID if (X86) > > > > default n > > > > > > > > config VFIO_IOMMU_SPAPR_TCE > > > > diff --git a/drivers/vfio/vfio_iommu_type1.c > > > > b/drivers/vfio/vfio_iommu_type1.c index 18ff0c3..ea89c7c 100644 > > > > --- a/drivers/vfio/vfio_iommu_type1.c > > > > +++ b/drivers/vfio/vfio_iommu_type1.c > > > > @@ -76,6 +76,7 @@ struct vfio_iommu { > > > > booldirty_page_tracking; > > > > boolpinned_page_dirty_scope; > > > > struct iommu_nesting_info *nesting_info; > > > > + struct vfio_mm *vmm; > > > > }; > > > > > > > > struct vfio_domain { > > > > @@ -1937,6 +1938,11 @@ static void vfio_iommu_iova_insert_copy(struct > > > > vfio_iommu *iommu, > > > > > > > > static void vfio_iommu_release_nesting_info(struct vfio_iommu *iommu) > > > > { > > > > + if (iommu->vmm) { > > > > + vfio_mm_put(iommu->vmm); > > > > + iommu->vmm = NULL; > > > > + } > > > > + > > > > kfree(iommu->nesting_info); > > > > iommu->nesting_info = NULL; > > > > } > > > > @@ -2071,6 +2077,26 @@ static int vfio_iommu_type1_attach_group(void > > > *iommu_data, > > > > iommu->nesting_info); > > > > if (ret) > > > > goto out_detach; > > > > + > > > > + if (iommu->nesting_info->features & > > > > + > > > > IOMMU_NESTING_FEAT_SYSWIDE_PASID) > > > { > > > > + struct vfio_mm *vmm; > > > > + int sid; > > > > + > > > > + vmm = vfio_mm_get_from_task(current); > > > > + if (IS_ERR(vmm)) { > > > > + ret = PTR_ERR(vmm); > > > > + goto out_detach; > > > > + } > > > > + iommu->vmm = vmm; > > > > + > > > > + sid = vfio_mm_ioasid_sid(vmm); > > > > + ret = iommu_domain_set_attr(domain->domain, > > > > + > > > > DOMAIN_ATTR_IOASID_SID, > > > > + &sid); > > > > + if (ret) > > > > + goto out_detach; > >
Re: [PATCH v6 07/15] vfio/type1: Add VFIO_IOMMU_PASID_REQUEST (alloc/free)
On Fri, 21 Aug 2020 00:37:19 + "Liu, Yi L" wrote: > Hi Alex, > > > From: Alex Williamson > > Sent: Friday, August 21, 2020 4:51 AM > > > > On Mon, 27 Jul 2020 23:27:36 -0700 > > Liu Yi L wrote: > > > > > This patch allows userspace to request PASID allocation/free, e.g. > > > when serving the request from the guest. > > > > > > PASIDs that are not freed by userspace are automatically freed when > > > the IOASID set is destroyed when process exits. > > > > > > Cc: Kevin Tian > > > CC: Jacob Pan > > > Cc: Alex Williamson > > > Cc: Eric Auger > > > Cc: Jean-Philippe Brucker > > > Cc: Joerg Roedel > > > Cc: Lu Baolu > > > Signed-off-by: Liu Yi L > > > Signed-off-by: Yi Sun > > > Signed-off-by: Jacob Pan > > > --- > > > v5 -> v6: > > > *) address comments from Eric against v5. remove the alloc/free helper. > > > > > > v4 -> v5: > > > *) address comments from Eric Auger. > > > *) the comments for the PASID_FREE request is addressed in patch 5/15 of > > >this series. > > > > > > v3 -> v4: > > > *) address comments from v3, except the below comment against the range > > >of PASID_FREE request. needs more help on it. > > > "> +if (req.range.min > req.range.max) > > > > > > Is it exploitable that a user can spin the kernel for a long time in > > > the case of a free by calling this with [0, MAX_UINT] regardless of > > > their actual allocations?" > > > > > > https://lore.kernel.org/linux-iommu/20200702151832.048b4...@x1.home/ > > > > > > v1 -> v2: > > > *) move the vfio_mm related code to be a seprate module > > > *) use a single structure for alloc/free, could support a range of > > > PASIDs > > > *) fetch vfio_mm at group_attach time instead of at iommu driver open > > > time > > > --- > > > drivers/vfio/Kconfig| 1 + > > > drivers/vfio/vfio_iommu_type1.c | 69 > > + > > > drivers/vfio/vfio_pasid.c | 10 ++ > > > include/linux/vfio.h| 6 > > > include/uapi/linux/vfio.h | 37 ++ > > > 5 files changed, 123 insertions(+) > > > > > > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig index > > > 3d8a108..95d90c6 100644 > > > --- a/drivers/vfio/Kconfig > > > +++ b/drivers/vfio/Kconfig > > > @@ -2,6 +2,7 @@ > > > config VFIO_IOMMU_TYPE1 > > > tristate > > > depends on VFIO > > > + select VFIO_PASID if (X86) > > > default n > > > > > > config VFIO_IOMMU_SPAPR_TCE > > > diff --git a/drivers/vfio/vfio_iommu_type1.c > > > b/drivers/vfio/vfio_iommu_type1.c index 18ff0c3..ea89c7c 100644 > > > --- a/drivers/vfio/vfio_iommu_type1.c > > > +++ b/drivers/vfio/vfio_iommu_type1.c > > > @@ -76,6 +76,7 @@ struct vfio_iommu { > > > booldirty_page_tracking; > > > boolpinned_page_dirty_scope; > > > struct iommu_nesting_info *nesting_info; > > > + struct vfio_mm *vmm; > > > }; > > > > > > struct vfio_domain { > > > @@ -1937,6 +1938,11 @@ static void vfio_iommu_iova_insert_copy(struct > > > vfio_iommu *iommu, > > > > > > static void vfio_iommu_release_nesting_info(struct vfio_iommu *iommu) > > > { > > > + if (iommu->vmm) { > > > + vfio_mm_put(iommu->vmm); > > > + iommu->vmm = NULL; > > > + } > > > + > > > kfree(iommu->nesting_info); > > > iommu->nesting_info = NULL; > > > } > > > @@ -2071,6 +2077,26 @@ static int vfio_iommu_type1_attach_group(void > > *iommu_data, > > > iommu->nesting_info); > > > if (ret) > > > goto out_detach; > > > + > > > + if (iommu->nesting_info->features & > > > + IOMMU_NESTING_FEAT_SYSWIDE_PASID) > > { > > > + struct vfio_mm *vmm; > > > + int sid; > > > + > > > + vmm = vfio_mm_get_from_task(current); > > > + if (IS_ERR(vmm)) { > > > + ret = PTR_ERR(vmm); > > > + goto out_detach; > > > + } > > > + iommu->vmm = vmm; > > > + > > > + sid = vfio_mm_ioasid_sid(vmm); > > > + ret = iommu_domain_set_attr(domain->domain, > > > + DOMAIN_ATTR_IOASID_SID, > > > + &sid); > > > + if (ret) > > > + goto out_detach; > > > + } > > > } > > > > > > /* Get aperture info */ > > > @@ -2859,6 +2885,47 @@ static int vfio_iommu_type1_dirty_pages(struct > > vfio_iommu *iommu, > > > return -EINVAL; > > > } > > > > > > +static int vfio_iommu_type1_pasid_request(struct vfio_iommu *iommu, > > > + unsigned long arg) > > > +{ > > > + struct vfio_iommu_type1_pasid_request req; > > > + unsigned long minsz; > > > + int ret; > > > + > > > + minsz = offsetofend(struct vfio_iommu_type1_pasid_request, range);
RE: [PATCH v6 07/15] vfio/type1: Add VFIO_IOMMU_PASID_REQUEST (alloc/free)
Hi Alex, > From: Alex Williamson > Sent: Friday, August 21, 2020 4:51 AM > > On Mon, 27 Jul 2020 23:27:36 -0700 > Liu Yi L wrote: > > > This patch allows userspace to request PASID allocation/free, e.g. > > when serving the request from the guest. > > > > PASIDs that are not freed by userspace are automatically freed when > > the IOASID set is destroyed when process exits. > > > > Cc: Kevin Tian > > CC: Jacob Pan > > Cc: Alex Williamson > > Cc: Eric Auger > > Cc: Jean-Philippe Brucker > > Cc: Joerg Roedel > > Cc: Lu Baolu > > Signed-off-by: Liu Yi L > > Signed-off-by: Yi Sun > > Signed-off-by: Jacob Pan > > --- > > v5 -> v6: > > *) address comments from Eric against v5. remove the alloc/free helper. > > > > v4 -> v5: > > *) address comments from Eric Auger. > > *) the comments for the PASID_FREE request is addressed in patch 5/15 of > >this series. > > > > v3 -> v4: > > *) address comments from v3, except the below comment against the range > >of PASID_FREE request. needs more help on it. > > "> +if (req.range.min > req.range.max) > > > > Is it exploitable that a user can spin the kernel for a long time in > > the case of a free by calling this with [0, MAX_UINT] regardless of > > their actual allocations?" > > > > https://lore.kernel.org/linux-iommu/20200702151832.048b4...@x1.home/ > > > > v1 -> v2: > > *) move the vfio_mm related code to be a seprate module > > *) use a single structure for alloc/free, could support a range of > > PASIDs > > *) fetch vfio_mm at group_attach time instead of at iommu driver open > > time > > --- > > drivers/vfio/Kconfig| 1 + > > drivers/vfio/vfio_iommu_type1.c | 69 > + > > drivers/vfio/vfio_pasid.c | 10 ++ > > include/linux/vfio.h| 6 > > include/uapi/linux/vfio.h | 37 ++ > > 5 files changed, 123 insertions(+) > > > > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig index > > 3d8a108..95d90c6 100644 > > --- a/drivers/vfio/Kconfig > > +++ b/drivers/vfio/Kconfig > > @@ -2,6 +2,7 @@ > > config VFIO_IOMMU_TYPE1 > > tristate > > depends on VFIO > > + select VFIO_PASID if (X86) > > default n > > > > config VFIO_IOMMU_SPAPR_TCE > > diff --git a/drivers/vfio/vfio_iommu_type1.c > > b/drivers/vfio/vfio_iommu_type1.c index 18ff0c3..ea89c7c 100644 > > --- a/drivers/vfio/vfio_iommu_type1.c > > +++ b/drivers/vfio/vfio_iommu_type1.c > > @@ -76,6 +76,7 @@ struct vfio_iommu { > > booldirty_page_tracking; > > boolpinned_page_dirty_scope; > > struct iommu_nesting_info *nesting_info; > > + struct vfio_mm *vmm; > > }; > > > > struct vfio_domain { > > @@ -1937,6 +1938,11 @@ static void vfio_iommu_iova_insert_copy(struct > > vfio_iommu *iommu, > > > > static void vfio_iommu_release_nesting_info(struct vfio_iommu *iommu) > > { > > + if (iommu->vmm) { > > + vfio_mm_put(iommu->vmm); > > + iommu->vmm = NULL; > > + } > > + > > kfree(iommu->nesting_info); > > iommu->nesting_info = NULL; > > } > > @@ -2071,6 +2077,26 @@ static int vfio_iommu_type1_attach_group(void > *iommu_data, > > iommu->nesting_info); > > if (ret) > > goto out_detach; > > + > > + if (iommu->nesting_info->features & > > + IOMMU_NESTING_FEAT_SYSWIDE_PASID) > { > > + struct vfio_mm *vmm; > > + int sid; > > + > > + vmm = vfio_mm_get_from_task(current); > > + if (IS_ERR(vmm)) { > > + ret = PTR_ERR(vmm); > > + goto out_detach; > > + } > > + iommu->vmm = vmm; > > + > > + sid = vfio_mm_ioasid_sid(vmm); > > + ret = iommu_domain_set_attr(domain->domain, > > + DOMAIN_ATTR_IOASID_SID, > > + &sid); > > + if (ret) > > + goto out_detach; > > + } > > } > > > > /* Get aperture info */ > > @@ -2859,6 +2885,47 @@ static int vfio_iommu_type1_dirty_pages(struct > vfio_iommu *iommu, > > return -EINVAL; > > } > > > > +static int vfio_iommu_type1_pasid_request(struct vfio_iommu *iommu, > > + unsigned long arg) > > +{ > > + struct vfio_iommu_type1_pasid_request req; > > + unsigned long minsz; > > + int ret; > > + > > + minsz = offsetofend(struct vfio_iommu_type1_pasid_request, range); > > + > > + if (copy_from_user(&req, (void __user *)arg, minsz)) > > + return -EFAULT; > > + > > + if (req.argsz < minsz || (req.flags & ~VFIO_PASID_REQUEST_MASK)) > > + return -EINVAL; > > + > > + if (req.range.min > req.range.max) > > +
Re: [PATCH v6 07/15] vfio/type1: Add VFIO_IOMMU_PASID_REQUEST (alloc/free)
On Mon, 27 Jul 2020 23:27:36 -0700 Liu Yi L wrote: > This patch allows userspace to request PASID allocation/free, e.g. when > serving the request from the guest. > > PASIDs that are not freed by userspace are automatically freed when the > IOASID set is destroyed when process exits. > > Cc: Kevin Tian > CC: Jacob Pan > Cc: Alex Williamson > Cc: Eric Auger > Cc: Jean-Philippe Brucker > Cc: Joerg Roedel > Cc: Lu Baolu > Signed-off-by: Liu Yi L > Signed-off-by: Yi Sun > Signed-off-by: Jacob Pan > --- > v5 -> v6: > *) address comments from Eric against v5. remove the alloc/free helper. > > v4 -> v5: > *) address comments from Eric Auger. > *) the comments for the PASID_FREE request is addressed in patch 5/15 of >this series. > > v3 -> v4: > *) address comments from v3, except the below comment against the range >of PASID_FREE request. needs more help on it. > "> +if (req.range.min > req.range.max) > > Is it exploitable that a user can spin the kernel for a long time in > the case of a free by calling this with [0, MAX_UINT] regardless of > their actual allocations?" > https://lore.kernel.org/linux-iommu/20200702151832.048b4...@x1.home/ > > v1 -> v2: > *) move the vfio_mm related code to be a seprate module > *) use a single structure for alloc/free, could support a range of PASIDs > *) fetch vfio_mm at group_attach time instead of at iommu driver open time > --- > drivers/vfio/Kconfig| 1 + > drivers/vfio/vfio_iommu_type1.c | 69 > + > drivers/vfio/vfio_pasid.c | 10 ++ > include/linux/vfio.h| 6 > include/uapi/linux/vfio.h | 37 ++ > 5 files changed, 123 insertions(+) > > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig > index 3d8a108..95d90c6 100644 > --- a/drivers/vfio/Kconfig > +++ b/drivers/vfio/Kconfig > @@ -2,6 +2,7 @@ > config VFIO_IOMMU_TYPE1 > tristate > depends on VFIO > + select VFIO_PASID if (X86) > default n > > config VFIO_IOMMU_SPAPR_TCE > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c > index 18ff0c3..ea89c7c 100644 > --- a/drivers/vfio/vfio_iommu_type1.c > +++ b/drivers/vfio/vfio_iommu_type1.c > @@ -76,6 +76,7 @@ struct vfio_iommu { > booldirty_page_tracking; > boolpinned_page_dirty_scope; > struct iommu_nesting_info *nesting_info; > + struct vfio_mm *vmm; > }; > > struct vfio_domain { > @@ -1937,6 +1938,11 @@ static void vfio_iommu_iova_insert_copy(struct > vfio_iommu *iommu, > > static void vfio_iommu_release_nesting_info(struct vfio_iommu *iommu) > { > + if (iommu->vmm) { > + vfio_mm_put(iommu->vmm); > + iommu->vmm = NULL; > + } > + > kfree(iommu->nesting_info); > iommu->nesting_info = NULL; > } > @@ -2071,6 +2077,26 @@ static int vfio_iommu_type1_attach_group(void > *iommu_data, > iommu->nesting_info); > if (ret) > goto out_detach; > + > + if (iommu->nesting_info->features & > + IOMMU_NESTING_FEAT_SYSWIDE_PASID) { > + struct vfio_mm *vmm; > + int sid; > + > + vmm = vfio_mm_get_from_task(current); > + if (IS_ERR(vmm)) { > + ret = PTR_ERR(vmm); > + goto out_detach; > + } > + iommu->vmm = vmm; > + > + sid = vfio_mm_ioasid_sid(vmm); > + ret = iommu_domain_set_attr(domain->domain, > + DOMAIN_ATTR_IOASID_SID, > + &sid); > + if (ret) > + goto out_detach; > + } > } > > /* Get aperture info */ > @@ -2859,6 +2885,47 @@ static int vfio_iommu_type1_dirty_pages(struct > vfio_iommu *iommu, > return -EINVAL; > } > > +static int vfio_iommu_type1_pasid_request(struct vfio_iommu *iommu, > + unsigned long arg) > +{ > + struct vfio_iommu_type1_pasid_request req; > + unsigned long minsz; > + int ret; > + > + minsz = offsetofend(struct vfio_iommu_type1_pasid_request, range); > + > + if (copy_from_user(&req, (void __user *)arg, minsz)) > + return -EFAULT; > + > + if (req.argsz < minsz || (req.flags & ~VFIO_PASID_REQUEST_MASK)) > + return -EINVAL; > + > + if (req.range.min > req.range.max) > + return -EINVAL; > + > + mutex_lock(&iommu->lock); > + if (!iommu->vmm) { > + mutex_unlock(&iommu->lock); > + return -EOPNOTSUPP; > + } > + > + switch (req.flags & VFIO_PASID_REQUEST_MASK) { > +
RE: [PATCH v6 07/15] vfio/type1: Add VFIO_IOMMU_PASID_REQUEST (alloc/free)
Thanks, Eric. Regards, Yi Liu > From: Auger Eric > Sent: Sunday, August 16, 2020 12:30 AM > > Yi, > > On 7/28/20 8:27 AM, Liu Yi L wrote: > > This patch allows userspace to request PASID allocation/free, e.g. > > when serving the request from the guest. > > > > PASIDs that are not freed by userspace are automatically freed when > > the IOASID set is destroyed when process exits. > > > > Cc: Kevin Tian > > CC: Jacob Pan > > Cc: Alex Williamson > > Cc: Eric Auger > > Cc: Jean-Philippe Brucker > > Cc: Joerg Roedel > > Cc: Lu Baolu > > Signed-off-by: Liu Yi L > > Signed-off-by: Yi Sun > > Signed-off-by: Jacob Pan > > --- > > v5 -> v6: > > *) address comments from Eric against v5. remove the alloc/free helper. > > > > v4 -> v5: > > *) address comments from Eric Auger. > > *) the comments for the PASID_FREE request is addressed in patch 5/15 of > >this series. > > > > v3 -> v4: > > *) address comments from v3, except the below comment against the range > >of PASID_FREE request. needs more help on it. > > "> +if (req.range.min > req.range.max) > > > > Is it exploitable that a user can spin the kernel for a long time in > > the case of a free by calling this with [0, MAX_UINT] regardless of > > their actual allocations?" > > > > https://lore.kernel.org/linux-iommu/20200702151832.048b4...@x1.home/ > > > > v1 -> v2: > > *) move the vfio_mm related code to be a seprate module > > *) use a single structure for alloc/free, could support a range of > > PASIDs > > *) fetch vfio_mm at group_attach time instead of at iommu driver open > > time > > --- > > drivers/vfio/Kconfig| 1 + > > drivers/vfio/vfio_iommu_type1.c | 69 > + > > drivers/vfio/vfio_pasid.c | 10 ++ > > include/linux/vfio.h| 6 > > include/uapi/linux/vfio.h | 37 ++ > > 5 files changed, 123 insertions(+) > > > > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig index > > 3d8a108..95d90c6 100644 > > --- a/drivers/vfio/Kconfig > > +++ b/drivers/vfio/Kconfig > > @@ -2,6 +2,7 @@ > > config VFIO_IOMMU_TYPE1 > > tristate > > depends on VFIO > > + select VFIO_PASID if (X86) > > default n > > > > config VFIO_IOMMU_SPAPR_TCE > > diff --git a/drivers/vfio/vfio_iommu_type1.c > > b/drivers/vfio/vfio_iommu_type1.c index 18ff0c3..ea89c7c 100644 > > --- a/drivers/vfio/vfio_iommu_type1.c > > +++ b/drivers/vfio/vfio_iommu_type1.c > > @@ -76,6 +76,7 @@ struct vfio_iommu { > > booldirty_page_tracking; > > boolpinned_page_dirty_scope; > > struct iommu_nesting_info *nesting_info; > > + struct vfio_mm *vmm; > > }; > > > > struct vfio_domain { > > @@ -1937,6 +1938,11 @@ static void vfio_iommu_iova_insert_copy(struct > > vfio_iommu *iommu, > > > > static void vfio_iommu_release_nesting_info(struct vfio_iommu *iommu) > > { > > + if (iommu->vmm) { > > + vfio_mm_put(iommu->vmm); > > + iommu->vmm = NULL; > > + } > > + > > kfree(iommu->nesting_info); > > iommu->nesting_info = NULL; > > } > > @@ -2071,6 +2077,26 @@ static int vfio_iommu_type1_attach_group(void > *iommu_data, > > iommu->nesting_info); > > if (ret) > > goto out_detach; > > + > > + if (iommu->nesting_info->features & > > + IOMMU_NESTING_FEAT_SYSWIDE_PASID) > { > > + struct vfio_mm *vmm; > > + int sid; > > + > > + vmm = vfio_mm_get_from_task(current); > > + if (IS_ERR(vmm)) { > > + ret = PTR_ERR(vmm); > > + goto out_detach; > > + } > > + iommu->vmm = vmm; > > + > > + sid = vfio_mm_ioasid_sid(vmm); > > + ret = iommu_domain_set_attr(domain->domain, > > + DOMAIN_ATTR_IOASID_SID, > > + &sid); > > + if (ret) > > + goto out_detach; > > + } > > } > > > > /* Get aperture info */ > > @@ -2859,6 +2885,47 @@ static int vfio_iommu_type1_dirty_pages(struct > vfio_iommu *iommu, > > return -EINVAL; > > } > > > > +static int vfio_iommu_type1_pasid_request(struct vfio_iommu *iommu, > > + unsigned long arg) > > +{ > > + struct vfio_iommu_type1_pasid_request req; > > + unsigned long minsz; > > + int ret; > > + > > + minsz = offsetofend(struct vfio_iommu_type1_pasid_request, range); > > + > > + if (copy_from_user(&req, (void __user *)arg, minsz)) > > + return -EFAULT; > > + > > + if (req.argsz < minsz || (req.flags & ~VFIO_PASID_REQUEST_MASK)) > > + return -EINVAL; > > + > > + if (req.range.min > req.range.max) > > +
Re: [PATCH v6 07/15] vfio/type1: Add VFIO_IOMMU_PASID_REQUEST (alloc/free)
Yi, On 7/28/20 8:27 AM, Liu Yi L wrote: > This patch allows userspace to request PASID allocation/free, e.g. when > serving the request from the guest. > > PASIDs that are not freed by userspace are automatically freed when the > IOASID set is destroyed when process exits. > > Cc: Kevin Tian > CC: Jacob Pan > Cc: Alex Williamson > Cc: Eric Auger > Cc: Jean-Philippe Brucker > Cc: Joerg Roedel > Cc: Lu Baolu > Signed-off-by: Liu Yi L > Signed-off-by: Yi Sun > Signed-off-by: Jacob Pan > --- > v5 -> v6: > *) address comments from Eric against v5. remove the alloc/free helper. > > v4 -> v5: > *) address comments from Eric Auger. > *) the comments for the PASID_FREE request is addressed in patch 5/15 of >this series. > > v3 -> v4: > *) address comments from v3, except the below comment against the range >of PASID_FREE request. needs more help on it. > "> +if (req.range.min > req.range.max) > > Is it exploitable that a user can spin the kernel for a long time in > the case of a free by calling this with [0, MAX_UINT] regardless of > their actual allocations?" > https://lore.kernel.org/linux-iommu/20200702151832.048b4...@x1.home/ > > v1 -> v2: > *) move the vfio_mm related code to be a seprate module > *) use a single structure for alloc/free, could support a range of PASIDs > *) fetch vfio_mm at group_attach time instead of at iommu driver open time > --- > drivers/vfio/Kconfig| 1 + > drivers/vfio/vfio_iommu_type1.c | 69 > + > drivers/vfio/vfio_pasid.c | 10 ++ > include/linux/vfio.h| 6 > include/uapi/linux/vfio.h | 37 ++ > 5 files changed, 123 insertions(+) > > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig > index 3d8a108..95d90c6 100644 > --- a/drivers/vfio/Kconfig > +++ b/drivers/vfio/Kconfig > @@ -2,6 +2,7 @@ > config VFIO_IOMMU_TYPE1 > tristate > depends on VFIO > + select VFIO_PASID if (X86) > default n > > config VFIO_IOMMU_SPAPR_TCE > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c > index 18ff0c3..ea89c7c 100644 > --- a/drivers/vfio/vfio_iommu_type1.c > +++ b/drivers/vfio/vfio_iommu_type1.c > @@ -76,6 +76,7 @@ struct vfio_iommu { > booldirty_page_tracking; > boolpinned_page_dirty_scope; > struct iommu_nesting_info *nesting_info; > + struct vfio_mm *vmm; > }; > > struct vfio_domain { > @@ -1937,6 +1938,11 @@ static void vfio_iommu_iova_insert_copy(struct > vfio_iommu *iommu, > > static void vfio_iommu_release_nesting_info(struct vfio_iommu *iommu) > { > + if (iommu->vmm) { > + vfio_mm_put(iommu->vmm); > + iommu->vmm = NULL; > + } > + > kfree(iommu->nesting_info); > iommu->nesting_info = NULL; > } > @@ -2071,6 +2077,26 @@ static int vfio_iommu_type1_attach_group(void > *iommu_data, > iommu->nesting_info); > if (ret) > goto out_detach; > + > + if (iommu->nesting_info->features & > + IOMMU_NESTING_FEAT_SYSWIDE_PASID) { > + struct vfio_mm *vmm; > + int sid; > + > + vmm = vfio_mm_get_from_task(current); > + if (IS_ERR(vmm)) { > + ret = PTR_ERR(vmm); > + goto out_detach; > + } > + iommu->vmm = vmm; > + > + sid = vfio_mm_ioasid_sid(vmm); > + ret = iommu_domain_set_attr(domain->domain, > + DOMAIN_ATTR_IOASID_SID, > + &sid); > + if (ret) > + goto out_detach; > + } > } > > /* Get aperture info */ > @@ -2859,6 +2885,47 @@ static int vfio_iommu_type1_dirty_pages(struct > vfio_iommu *iommu, > return -EINVAL; > } > > +static int vfio_iommu_type1_pasid_request(struct vfio_iommu *iommu, > + unsigned long arg) > +{ > + struct vfio_iommu_type1_pasid_request req; > + unsigned long minsz; > + int ret; > + > + minsz = offsetofend(struct vfio_iommu_type1_pasid_request, range); > + > + if (copy_from_user(&req, (void __user *)arg, minsz)) > + return -EFAULT; > + > + if (req.argsz < minsz || (req.flags & ~VFIO_PASID_REQUEST_MASK)) > + return -EINVAL; > + > + if (req.range.min > req.range.max) > + return -EINVAL; > + > + mutex_lock(&iommu->lock); > + if (!iommu->vmm) { > + mutex_unlock(&iommu->lock); > + return -EOPNOTSUPP; > + } > + > + switch (req.flags & VFIO_PASID_REQUEST_MASK) { > + case VFIO