Hi Eric, > From: Auger Eric <eric.auger@xxxxxxxxxx> > Sent: Sunday, July 19, 2020 11:38 PM > > Yi, > > On 7/12/20 1:21 PM, 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 on the PASID will be checked against its IOASID set for proper > ownership. > Subsequent SVA operations will check the PASID against its IOASID set for proper > ownership. got it. > > > > Cc: Kevin Tian <kevin.tian@xxxxxxxxx> > > CC: Jacob Pan <jacob.jun.pan@xxxxxxxxxxxxxxx> > > Cc: Alex Williamson <alex.williamson@xxxxxxxxxx> > > Cc: Eric Auger <eric.auger@xxxxxxxxxx> > > Cc: Jean-Philippe Brucker <jean-philippe@xxxxxxxxxx> > > Cc: Joerg Roedel <joro@xxxxxxxxxx> > > Cc: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx> > > Signed-off-by: Liu Yi L <yi.l.liu@xxxxxxxxx> > > Signed-off-by: Jacob Pan <jacob.jun.pan@xxxxxxxxxxxxxxx> > > --- > > v4 -> v5: > > *) address comments from Eric Auger. > > --- > > drivers/iommu/intel/iommu.c | 22 ++++++++++++++++++++++ > > include/linux/intel-iommu.h | 4 ++++ > > include/linux/iommu.h | 1 + > > 3 files changed, 27 insertions(+) > > > > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c > > index 72ae6a2..4d54198 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; > > @@ -6039,6 +6040,27 @@ 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; > > + > > + if (!(dmar_domain->flags & DOMAIN_FLAG_NESTING_MODE)) { > > + ret = -ENODEV; > > + break; > > + } > > + spin_lock_irqsave(&device_domain_lock, flags); > I think the lock should be taken before the DOMAIN_FLAG_NESTING_MODE check. > Otherwise, the flags can be theretically changed inbetween the check and the test > below? I see. will correct it. Thanks, Yi Liu > Thanks > > Eric > > + 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 > > 7ca9d48..e84a1d5 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, > > }; > > > >