From mboxrd@z Thu Jan 1 00:00:00 1970 From: robin.murphy@arm.com (Robin Murphy) Date: Fri, 21 Oct 2016 15:48:07 +0100 Subject: [RFC,v1,2/2] vfio/iommu-type1: set only stage 2 translation In-Reply-To: <20161021082703.4890c419@t450s.home> References: <1477024764-79882-1-git-send-email-songwenjun@huawei.com> <1477024764-79882-2-git-send-email-songwenjun@huawei.com> <20161021082703.4890c419@t450s.home> Message-ID: <1522b473-5979-0fa1-073f-be0fe7e1aa2c@arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 21/10/16 15:27, Alex Williamson wrote: > On Fri, 21 Oct 2016 12:39:24 +0800 > Rick Song 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 >> --- >> 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); >