* [RFC,v1,1/2] iommu/arm-smmu: support s2 domain attribute @ 2016-10-21 4:39 Rick Song 2016-10-21 4:39 ` [RFC,v1,2/2] vfio/iommu-type1: set only stage 2 translation Rick Song 0 siblings, 1 reply; 4+ messages in thread From: Rick Song @ 2016-10-21 4:39 UTC (permalink / raw) To: will.deacon, robin.murphy, joro, linux-arm-kernel, iommu, alex.williamson, kvm, songwenjun Under some condition, user want to use the only stage 2 translation ability, so smmu driver need support setting only s2 domain attribute. Signed-off-by: Rick Song <songwenjun@huawei.com> --- drivers/iommu/arm-smmu-v3.c | 12 ++++++++++++ include/linux/iommu.h | 1 + 2 files changed, 13 insertions(+) diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index 15c01c3..d3b19d2 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -1854,6 +1854,18 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain, mutex_lock(&smmu_domain->init_mutex); switch (attr) { + case DOMAIN_ATTR_S2: + if (smmu_domain->smmu) { + ret = -EPERM; + goto out_unlock; + } + + if (*(int *)data) + smmu_domain->stage = ARM_SMMU_DOMAIN_S2; + else + smmu_domain->stage = ARM_SMMU_DOMAIN_S1; + + break; case DOMAIN_ATTR_NESTING: if (smmu_domain->smmu) { ret = -EPERM; diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 436dc21..0b5a387 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -114,6 +114,7 @@ enum iommu_attr { DOMAIN_ATTR_FSL_PAMU_ENABLE, DOMAIN_ATTR_FSL_PAMUV1, DOMAIN_ATTR_NESTING, /* two stages of translation */ + DOMAIN_ATTR_S2, /* only stage 2 translation */ DOMAIN_ATTR_MAX, }; -- 1.9.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [RFC,v1,2/2] vfio/iommu-type1: set only stage 2 translation 2016-10-21 4:39 [RFC,v1,1/2] iommu/arm-smmu: support s2 domain attribute Rick Song @ 2016-10-21 4:39 ` Rick Song [not found] ` <1477024764-79882-2-git-send-email-songwenjun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 4+ messages in thread From: Rick Song @ 2016-10-21 4:39 UTC (permalink / raw) To: will.deacon, robin.murphy, joro, linux-arm-kernel, iommu, alex.williamson, kvm, songwenjun Normally, VFIO should use only stage 2 translation of iommu, to create the address mapping. If nesting translation is disabled from VFIO core, enable iommu domain only stage 2 attribute, otherwise, enable iommu domain nesting attribute. Signed-off-by: Rick Song <songwenjun@huawei.com> --- drivers/vfio/vfio_iommu_type1.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c index 2ba1942..c0265fe 100644 --- a/drivers/vfio/vfio_iommu_type1.c +++ b/drivers/vfio/vfio_iommu_type1.c @@ -741,7 +741,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, struct vfio_group *group, *g; struct vfio_domain *domain, *d; struct bus_type *bus = NULL; - int ret; + int attr, ret; mutex_lock(&iommu->lock); @@ -775,13 +775,22 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, goto out_free; } + /* + * Set iommu nesting domain attribute if nesting translation + * is enabled from iommu vfio, otherwise set iommu only stage + * 2 domain attribute. + */ + attr = 1; if (iommu->nesting) { - int attr = 1; - ret = iommu_domain_set_attr(domain->domain, DOMAIN_ATTR_NESTING, &attr); if (ret) goto out_domain; + } else { + ret = iommu_domain_set_attr(domain->domain, DOMAIN_ATTR_S2, + &attr); + if (ret) + goto out_domain; } ret = iommu_attach_group(domain->domain, iommu_group); -- 1.9.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
[parent not found: <1477024764-79882-2-git-send-email-songwenjun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>]
* Re: [RFC,v1,2/2] vfio/iommu-type1: set only stage 2 translation [not found] ` <1477024764-79882-2-git-send-email-songwenjun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> @ 2016-10-21 14:27 ` Alex Williamson [not found] ` <20161021082703.4890c419-1yVPhWWZRC1BDLzU/O5InQ@public.gmane.org> 0 siblings, 1 reply; 4+ messages in thread From: Alex Williamson @ 2016-10-21 14:27 UTC (permalink / raw) To: Rick Song Cc: kvm-u79uwXL29TY76Z2rM5mHXA, will.deacon-5wv7dgnIgG8, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r On Fri, 21 Oct 2016 12:39:24 +0800 Rick Song <songwenjun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote: > Normally, VFIO should use only stage 2 translation of > iommu, to create the address mapping. If nesting translation > is disabled from VFIO core, enable iommu domain only stage 2 > attribute, otherwise, enable iommu domain nesting attribute. > > Signed-off-by: Rick Song <songwenjun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> > --- > drivers/vfio/vfio_iommu_type1.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c > index 2ba1942..c0265fe 100644 > --- a/drivers/vfio/vfio_iommu_type1.c > +++ b/drivers/vfio/vfio_iommu_type1.c > @@ -741,7 +741,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, > struct vfio_group *group, *g; > struct vfio_domain *domain, *d; > struct bus_type *bus = NULL; > - int ret; > + int attr, ret; > > mutex_lock(&iommu->lock); > > @@ -775,13 +775,22 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, > goto out_free; > } > > + /* > + * Set iommu nesting domain attribute if nesting translation > + * is enabled from iommu vfio, otherwise set iommu only stage > + * 2 domain attribute. > + */ > + attr = 1; > if (iommu->nesting) { > - int attr = 1; > - > ret = iommu_domain_set_attr(domain->domain, DOMAIN_ATTR_NESTING, > &attr); > if (ret) > goto out_domain; > + } else { > + ret = iommu_domain_set_attr(domain->domain, DOMAIN_ATTR_S2, > + &attr); > + if (ret) > + goto out_domain; > } This attribute is not relevant to the majority of current users, why would we assume that we need to call it for all non-nesting cases? Why do we need to set the attribute at all, what benefit does it provide? If this is the normal case for an IOMMU API domain, why is there an option for it at all? Maybe this should be the default and S1 (whatever that means) should be the alternate option. Thanks, Alex > > ret = iommu_attach_group(domain->domain, iommu_group); ^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <20161021082703.4890c419-1yVPhWWZRC1BDLzU/O5InQ@public.gmane.org>]
* Re: [RFC,v1,2/2] vfio/iommu-type1: set only stage 2 translation [not found] ` <20161021082703.4890c419-1yVPhWWZRC1BDLzU/O5InQ@public.gmane.org> @ 2016-10-21 14:48 ` Robin Murphy 0 siblings, 0 replies; 4+ messages in thread From: Robin Murphy @ 2016-10-21 14:48 UTC (permalink / raw) To: Rick Song Cc: kvm-u79uwXL29TY76Z2rM5mHXA, will.deacon-5wv7dgnIgG8, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r On 21/10/16 15:27, Alex Williamson wrote: > On Fri, 21 Oct 2016 12:39:24 +0800 > Rick Song <songwenjun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote: > >> Normally, VFIO should use only stage 2 translation of >> iommu, to create the address mapping. If nesting translation >> is disabled from VFIO core, enable iommu domain only stage 2 >> attribute, otherwise, enable iommu domain nesting attribute. >> >> Signed-off-by: Rick Song <songwenjun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> >> --- >> drivers/vfio/vfio_iommu_type1.c | 15 ++++++++++++--- >> 1 file changed, 12 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c >> index 2ba1942..c0265fe 100644 >> --- a/drivers/vfio/vfio_iommu_type1.c >> +++ b/drivers/vfio/vfio_iommu_type1.c >> @@ -741,7 +741,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, >> struct vfio_group *group, *g; >> struct vfio_domain *domain, *d; >> struct bus_type *bus = NULL; >> - int ret; >> + int attr, ret; >> >> mutex_lock(&iommu->lock); >> >> @@ -775,13 +775,22 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, >> goto out_free; >> } >> >> + /* >> + * Set iommu nesting domain attribute if nesting translation >> + * is enabled from iommu vfio, otherwise set iommu only stage >> + * 2 domain attribute. >> + */ >> + attr = 1; >> if (iommu->nesting) { >> - int attr = 1; >> - >> ret = iommu_domain_set_attr(domain->domain, DOMAIN_ATTR_NESTING, >> &attr); >> if (ret) >> goto out_domain; >> + } else { >> + ret = iommu_domain_set_attr(domain->domain, DOMAIN_ATTR_S2, >> + &attr); >> + if (ret) >> + goto out_domain; >> } > > This attribute is not relevant to the majority of current users, why > would we assume that we need to call it for all non-nesting cases? Why > do we need to set the attribute at all, what benefit does it provide? > If this is the normal case for an IOMMU API domain, why is there an > option for it at all? Maybe this should be the default and S1 > (whatever that means) should be the alternate option. Thanks, Indeed, it should be fairly straightforward to make arm_smmu_domain_finalise() prefer stage 1/stage 2 based on domain->type in the case that both stages are implemented. That would be preferable to changing core VFIO code for something that really is SMMU-specific. To echo Alex, though, what's the motivation for this? Could it be addressed by simply implementing a force_stage parameter like the SMMUv2 driver has? Robin. > > Alex > >> >> ret = iommu_attach_group(domain->domain, iommu_group); > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-10-21 14:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-21 4:39 [RFC,v1,1/2] iommu/arm-smmu: support s2 domain attribute Rick Song
2016-10-21 4:39 ` [RFC,v1,2/2] vfio/iommu-type1: set only stage 2 translation Rick Song
[not found] ` <1477024764-79882-2-git-send-email-songwenjun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2016-10-21 14:27 ` Alex Williamson
[not found] ` <20161021082703.4890c419-1yVPhWWZRC1BDLzU/O5InQ@public.gmane.org>
2016-10-21 14:48 ` Robin Murphy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).