| Message ID | 1595917664-33276-7-git-send-email-yi.l.liu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfio: expose virtual Shared Virtual Addressing to VMs | expand |
Hi Yi, On 7/28/20 8:27 AM, Liu Yi L wrote: > From IOMMU p.o.v., PASIDs allocated and managed by external components > (e.g. VFIO) will be passed in for gpasid_bind/unbind operation. IOMMU > needs some knowledge to check the PASID ownership, hence add an interface > for those components to tell the PASID owner. > > In latest kernel design, PASID ownership is managed by IOASID set where > the PASID is allocated from. This patch adds support for setting ioasid > set ID to the domains used for nesting/vSVA. Subsequent SVA operations > will check the PASID against its IOASID set for proper ownership. > > Cc: Kevin Tian <kevin.tian@intel.com> > CC: Jacob Pan <jacob.jun.pan@linux.intel.com> > Cc: Alex Williamson <alex.williamson@redhat.com> > Cc: Eric Auger <eric.auger@redhat.com> > Cc: Jean-Philippe Brucker <jean-philippe@linaro.org> > Cc: Joerg Roedel <joro@8bytes.org> > Cc: Lu Baolu <baolu.lu@linux.intel.com> > Signed-off-by: Liu Yi L <yi.l.liu@intel.com> > Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com> > --- > v5 -> v6: > *) address comments against v5 from Eric Auger. > > v4 -> v5: > *) address comments from Eric Auger. > --- > drivers/iommu/intel/iommu.c | 23 +++++++++++++++++++++++ > include/linux/intel-iommu.h | 4 ++++ > include/linux/iommu.h | 1 + > 3 files changed, 28 insertions(+) > > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c > index ed4b71c..b2fe54e 100644 > --- a/drivers/iommu/intel/iommu.c > +++ b/drivers/iommu/intel/iommu.c > @@ -1793,6 +1793,7 @@ static struct dmar_domain *alloc_domain(int flags) > if (first_level_by_default()) > domain->flags |= DOMAIN_FLAG_USE_FIRST_LEVEL; > domain->has_iotlb_device = false; > + domain->ioasid_sid = INVALID_IOASID_SET; > INIT_LIST_HEAD(&domain->devices); > > return domain; > @@ -6040,6 +6041,28 @@ intel_iommu_domain_set_attr(struct iommu_domain *domain, > } > spin_unlock_irqrestore(&device_domain_lock, flags); > break; > + case DOMAIN_ATTR_IOASID_SID: > + { > + int sid = *(int *)data; > + > + spin_lock_irqsave(&device_domain_lock, flags); > + if (!(dmar_domain->flags & DOMAIN_FLAG_NESTING_MODE)) { > + ret = -ENODEV; > + spin_unlock_irqrestore(&device_domain_lock, flags); > + break; > + } > + if (dmar_domain->ioasid_sid != INVALID_IOASID_SET && > + dmar_domain->ioasid_sid != sid) { > + pr_warn_ratelimited("multi ioasid_set (%d:%d) setting", > + dmar_domain->ioasid_sid, sid); > + ret = -EBUSY; > + spin_unlock_irqrestore(&device_domain_lock, flags); > + break; > + } > + dmar_domain->ioasid_sid = sid; > + spin_unlock_irqrestore(&device_domain_lock, flags); > + break; nit: Adding a small helper int__set_ioasid_sid(struct dmar_domain *dmar_domain, int sid_id) may simplify the lock handling > + } > default: > ret = -EINVAL; > break; > diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h > index 3f23c26..0d0ab32 100644 > --- a/include/linux/intel-iommu.h > +++ b/include/linux/intel-iommu.h > @@ -549,6 +549,10 @@ struct dmar_domain { > 2 == 1GiB, 3 == 512GiB, 4 == 1TiB */ > u64 max_addr; /* maximum mapped address */ > > + int ioasid_sid; /* > + * the ioasid set which tracks all id of the ioasid set? > + * PASIDs used by the domain. > + */ > int default_pasid; /* > * The default pasid used for non-SVM > * traffic on mediated devices. > diff --git a/include/linux/iommu.h b/include/linux/iommu.h > index 4a02c9e..b1ff702 100644 > --- a/include/linux/iommu.h > +++ b/include/linux/iommu.h > @@ -124,6 +124,7 @@ enum iommu_attr { > DOMAIN_ATTR_FSL_PAMUV1, > DOMAIN_ATTR_NESTING, /* two stages of translation */ > DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE, > + DOMAIN_ATTR_IOASID_SID, > DOMAIN_ATTR_MAX, > }; > > Besides Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric
Hi Eric, > From: Auger Eric <eric.auger@redhat.com> > Sent: Thursday, August 13, 2020 11:07 PM > > Hi Yi, > > On 7/28/20 8:27 AM, Liu Yi L wrote: > > From IOMMU p.o.v., PASIDs allocated and managed by external components > > (e.g. VFIO) will be passed in for gpasid_bind/unbind operation. IOMMU > > needs some knowledge to check the PASID ownership, hence add an > > interface for those components to tell the PASID owner. > > > > In latest kernel design, PASID ownership is managed by IOASID set > > where the PASID is allocated from. This patch adds support for setting > > ioasid set ID to the domains used for nesting/vSVA. Subsequent SVA > > operations will check the PASID against its IOASID set for proper ownership. > > > > Cc: Kevin Tian <kevin.tian@intel.com> > > CC: Jacob Pan <jacob.jun.pan@linux.intel.com> > > Cc: Alex Williamson <alex.williamson@redhat.com> > > Cc: Eric Auger <eric.auger@redhat.com> > > Cc: Jean-Philippe Brucker <jean-philippe@linaro.org> > > Cc: Joerg Roedel <joro@8bytes.org> > > Cc: Lu Baolu <baolu.lu@linux.intel.com> > > Signed-off-by: Liu Yi L <yi.l.liu@intel.com> > > Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com> > > --- > > v5 -> v6: > > *) address comments against v5 from Eric Auger. > > > > v4 -> v5: > > *) address comments from Eric Auger. > > --- > > drivers/iommu/intel/iommu.c | 23 +++++++++++++++++++++++ > > include/linux/intel-iommu.h | 4 ++++ > > include/linux/iommu.h | 1 + > > 3 files changed, 28 insertions(+) > > > > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c > > index ed4b71c..b2fe54e 100644 > > --- a/drivers/iommu/intel/iommu.c > > +++ b/drivers/iommu/intel/iommu.c > > @@ -1793,6 +1793,7 @@ static struct dmar_domain *alloc_domain(int flags) > > if (first_level_by_default()) > > domain->flags |= DOMAIN_FLAG_USE_FIRST_LEVEL; > > domain->has_iotlb_device = false; > > + domain->ioasid_sid = INVALID_IOASID_SET; > > INIT_LIST_HEAD(&domain->devices); > > > > return domain; > > @@ -6040,6 +6041,28 @@ intel_iommu_domain_set_attr(struct iommu_domain > *domain, > > } > > spin_unlock_irqrestore(&device_domain_lock, flags); > > break; > > + case DOMAIN_ATTR_IOASID_SID: > > + { > > + int sid = *(int *)data; > > > + > > + spin_lock_irqsave(&device_domain_lock, flags); > > + if (!(dmar_domain->flags & DOMAIN_FLAG_NESTING_MODE)) { > > + ret = -ENODEV; > > + spin_unlock_irqrestore(&device_domain_lock, flags); > > + break; > > + } > > + if (dmar_domain->ioasid_sid != INVALID_IOASID_SET && > > + dmar_domain->ioasid_sid != sid) { > > + pr_warn_ratelimited("multi ioasid_set (%d:%d) setting", > > + dmar_domain->ioasid_sid, sid); > > + ret = -EBUSY; > > + spin_unlock_irqrestore(&device_domain_lock, flags); > > + break; > > + } > > + dmar_domain->ioasid_sid = sid; > > + spin_unlock_irqrestore(&device_domain_lock, flags); > > + break; > nit: Adding a small helper > int__set_ioasid_sid(struct dmar_domain *dmar_domain, int sid_id) > > may simplify the lock handling ok. will do. > > > + } > > default: > > ret = -EINVAL; > > break; > > diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h > > index 3f23c26..0d0ab32 100644 > > --- a/include/linux/intel-iommu.h > > +++ b/include/linux/intel-iommu.h > > @@ -549,6 +549,10 @@ struct dmar_domain { > > 2 == 1GiB, 3 == 512GiB, 4 == 1TiB */ > > u64 max_addr; /* maximum mapped address */ > > > > + int ioasid_sid; /* > > + * the ioasid set which tracks all > id of the ioasid set? should be ioasid_set. however, ioasid_alloc_set() returns sid in Jacob's series. but, I heard from Jacob, he will remove ioasid_sid, and return ioasid_set. so I will modify it once his patch is sent out. https://lore.kernel.org/linux-iommu/1585158931-1825-4-git-send-email-jacob.jun.pan@linux.intel.com/ > > + * PASIDs used by the domain. > > + */ > > int default_pasid; /* > > * The default pasid used for non-SVM > > * traffic on mediated devices. > > diff --git a/include/linux/iommu.h b/include/linux/iommu.h index > > 4a02c9e..b1ff702 100644 > > --- a/include/linux/iommu.h > > +++ b/include/linux/iommu.h > > @@ -124,6 +124,7 @@ enum iommu_attr { > > DOMAIN_ATTR_FSL_PAMUV1, > > DOMAIN_ATTR_NESTING, /* two stages of translation */ > > DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE, > > + DOMAIN_ATTR_IOASID_SID, > > DOMAIN_ATTR_MAX, > > }; > > > > > Besides > Reviewed-by: Eric Auger <eric.auger@redhat.com> thanks :-) Regards, Yi Liu > > Eric
On 8/14/20 10:04 AM, Liu, Yi L wrote: > Hi Eric, > >> From: Auger Eric <eric.auger@redhat.com> >> Sent: Thursday, August 13, 2020 11:07 PM >> >> Hi Yi, >> >> On 7/28/20 8:27 AM, Liu Yi L wrote: >>> From IOMMU p.o.v., PASIDs allocated and managed by external components >>> (e.g. VFIO) will be passed in for gpasid_bind/unbind operation. IOMMU >>> needs some knowledge to check the PASID ownership, hence add an >>> interface for those components to tell the PASID owner. >>> >>> In latest kernel design, PASID ownership is managed by IOASID set >>> where the PASID is allocated from. This patch adds support for setting >>> ioasid set ID to the domains used for nesting/vSVA. Subsequent SVA >>> operations will check the PASID against its IOASID set for proper ownership. >>> >>> Cc: Kevin Tian <kevin.tian@intel.com> >>> CC: Jacob Pan <jacob.jun.pan@linux.intel.com> >>> Cc: Alex Williamson <alex.williamson@redhat.com> >>> Cc: Eric Auger <eric.auger@redhat.com> >>> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org> >>> Cc: Joerg Roedel <joro@8bytes.org> >>> Cc: Lu Baolu <baolu.lu@linux.intel.com> >>> Signed-off-by: Liu Yi L <yi.l.liu@intel.com> >>> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com> >>> --- >>> v5 -> v6: >>> *) address comments against v5 from Eric Auger. >>> >>> v4 -> v5: >>> *) address comments from Eric Auger. >>> --- >>> drivers/iommu/intel/iommu.c | 23 +++++++++++++++++++++++ >>> include/linux/intel-iommu.h | 4 ++++ >>> include/linux/iommu.h | 1 + >>> 3 files changed, 28 insertions(+) >>> >>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c >>> index ed4b71c..b2fe54e 100644 >>> --- a/drivers/iommu/intel/iommu.c >>> +++ b/drivers/iommu/intel/iommu.c >>> @@ -1793,6 +1793,7 @@ static struct dmar_domain *alloc_domain(int flags) >>> if (first_level_by_default()) >>> domain->flags |= DOMAIN_FLAG_USE_FIRST_LEVEL; >>> domain->has_iotlb_device = false; >>> + domain->ioasid_sid = INVALID_IOASID_SET; >>> INIT_LIST_HEAD(&domain->devices); >>> >>> return domain; >>> @@ -6040,6 +6041,28 @@ intel_iommu_domain_set_attr(struct iommu_domain >> *domain, >>> } >>> spin_unlock_irqrestore(&device_domain_lock, flags); >>> break; >>> + case DOMAIN_ATTR_IOASID_SID: >>> + { >>> + int sid = *(int *)data; >> >>> + >>> + spin_lock_irqsave(&device_domain_lock, flags); >>> + if (!(dmar_domain->flags & DOMAIN_FLAG_NESTING_MODE)) { >>> + ret = -ENODEV; >>> + spin_unlock_irqrestore(&device_domain_lock, flags); >>> + break; >>> + } >>> + if (dmar_domain->ioasid_sid != INVALID_IOASID_SET && >>> + dmar_domain->ioasid_sid != sid) { >>> + pr_warn_ratelimited("multi ioasid_set (%d:%d) setting", >>> + dmar_domain->ioasid_sid, sid); >>> + ret = -EBUSY; >>> + spin_unlock_irqrestore(&device_domain_lock, flags); >>> + break; >>> + } >>> + dmar_domain->ioasid_sid = sid; >>> + spin_unlock_irqrestore(&device_domain_lock, flags); >>> + break; >> nit: Adding a small helper >> int__set_ioasid_sid(struct dmar_domain *dmar_domain, int sid_id) >> >> may simplify the lock handling > > ok. will do. > >> >>> + } >>> default: >>> ret = -EINVAL; >>> break; >>> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h >>> index 3f23c26..0d0ab32 100644 >>> --- a/include/linux/intel-iommu.h >>> +++ b/include/linux/intel-iommu.h >>> @@ -549,6 +549,10 @@ struct dmar_domain { >>> 2 == 1GiB, 3 == 512GiB, 4 == 1TiB */ >>> u64 max_addr; /* maximum mapped address */ >>> >>> + int ioasid_sid; /* >>> + * the ioasid set which tracks all >> id of the ioasid set? > > should be ioasid_set. however, ioasid_alloc_set() returns sid in Jacob's > series. but, I heard from Jacob, he will remove ioasid_sid, and return > ioasid_set. so I will modify it once his patch is sent out. > > https://lore.kernel.org/linux-iommu/1585158931-1825-4-git-send-email-jacob.jun.pan@linux.intel.com/ OK Thanks Eric > >>> + * PASIDs used by the domain. >>> + */ >>> int default_pasid; /* >>> * The default pasid used for non-SVM >>> * traffic on mediated devices. >>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h index >>> 4a02c9e..b1ff702 100644 >>> --- a/include/linux/iommu.h >>> +++ b/include/linux/iommu.h >>> @@ -124,6 +124,7 @@ enum iommu_attr { >>> DOMAIN_ATTR_FSL_PAMUV1, >>> DOMAIN_ATTR_NESTING, /* two stages of translation */ >>> DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE, >>> + DOMAIN_ATTR_IOASID_SID, >>> DOMAIN_ATTR_MAX, >>> }; >>> >>> >> Besides >> Reviewed-by: Eric Auger <eric.auger@redhat.com> > > thanks :-) > > Regards, > Yi Liu > >> >> Eric >
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index ed4b71c..b2fe54e 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -1793,6 +1793,7 @@ static struct dmar_domain *alloc_domain(int flags) if (first_level_by_default()) domain->flags |= DOMAIN_FLAG_USE_FIRST_LEVEL; domain->has_iotlb_device = false; + domain->ioasid_sid = INVALID_IOASID_SET; INIT_LIST_HEAD(&domain->devices); return domain; @@ -6040,6 +6041,28 @@ intel_iommu_domain_set_attr(struct iommu_domain *domain, } spin_unlock_irqrestore(&device_domain_lock, flags); break; + case DOMAIN_ATTR_IOASID_SID: + { + int sid = *(int *)data; + + spin_lock_irqsave(&device_domain_lock, flags); + if (!(dmar_domain->flags & DOMAIN_FLAG_NESTING_MODE)) { + ret = -ENODEV; + spin_unlock_irqrestore(&device_domain_lock, flags); + break; + } + if (dmar_domain->ioasid_sid != INVALID_IOASID_SET && + dmar_domain->ioasid_sid != sid) { + pr_warn_ratelimited("multi ioasid_set (%d:%d) setting", + dmar_domain->ioasid_sid, sid); + ret = -EBUSY; + spin_unlock_irqrestore(&device_domain_lock, flags); + break; + } + dmar_domain->ioasid_sid = sid; + spin_unlock_irqrestore(&device_domain_lock, flags); + break; + } default: ret = -EINVAL; break; diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h index 3f23c26..0d0ab32 100644 --- a/include/linux/intel-iommu.h +++ b/include/linux/intel-iommu.h @@ -549,6 +549,10 @@ struct dmar_domain { 2 == 1GiB, 3 == 512GiB, 4 == 1TiB */ u64 max_addr; /* maximum mapped address */ + int ioasid_sid; /* + * the ioasid set which tracks all + * PASIDs used by the domain. + */ int default_pasid; /* * The default pasid used for non-SVM * traffic on mediated devices. diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 4a02c9e..b1ff702 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -124,6 +124,7 @@ enum iommu_attr { DOMAIN_ATTR_FSL_PAMUV1, DOMAIN_ATTR_NESTING, /* two stages of translation */ DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE, + DOMAIN_ATTR_IOASID_SID, DOMAIN_ATTR_MAX, };