| Message ID | 1484849955-1871-2-git-send-email-will.deacon@arm.com |
|---|---|
| State | Accepted |
| Commit | 0834cc28fa56c65887c614b6c045be2ba06fdcb0 |
| Headers | show |
| Series | Implement SMMU passthrough using the default domain | expand |
On 19/01/17 18:19, Will Deacon wrote: > The ARM SMMU drivers provide a DOMAIN_ATTR_NESTING domain attribute, > which allows callers of the IOMMU API to request that the page table > for a domain is installed at stage-2, if supported by the hardware. > > Since setting this attribute only makes sense for UNMANAGED domains, > this patch returns -ENODEV if the domain_{get,set}_attr operations are > called on other domain types. For the sake of discussion, would it make sense to enforce this in domain_set_attr() itself? The intersection of drivers providing these callbacks and drivers supporting anything other than unmanaged domains is currently these two below, so it clearly wouldn't break anything to put this check in core code today. Looking forward, is there likely to be any plausible situation where users of a managed domain would be legitimate in mucking about with its attrs, on any platform? Robin. > Signed-off-by: Will Deacon <will.deacon@arm.com> > --- > drivers/iommu/arm-smmu-v3.c | 6 ++++++ > drivers/iommu/arm-smmu.c | 6 ++++++ > 2 files changed, 12 insertions(+) > > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c > index 4d6ec444a9d6..c254325b0c7a 100644 > --- a/drivers/iommu/arm-smmu-v3.c > +++ b/drivers/iommu/arm-smmu-v3.c > @@ -1839,6 +1839,9 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain, > { > struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); > > + if (domain->type != IOMMU_DOMAIN_UNMANAGED) > + return -ENODEV; > + > switch (attr) { > case DOMAIN_ATTR_NESTING: > *(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED); > @@ -1854,6 +1857,9 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain, > int ret = 0; > struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); > > + if (domain->type != IOMMU_DOMAIN_UNMANAGED) > + return -ENODEV; > + > mutex_lock(&smmu_domain->init_mutex); > > switch (attr) { > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > index a60cded8a6ed..a328ffb75509 100644 > --- a/drivers/iommu/arm-smmu.c > +++ b/drivers/iommu/arm-smmu.c > @@ -1497,6 +1497,9 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain, > { > struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); > > + if (domain->type != IOMMU_DOMAIN_UNMANAGED) > + return -ENODEV; > + > switch (attr) { > case DOMAIN_ATTR_NESTING: > *(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED); > @@ -1512,6 +1515,9 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain, > int ret = 0; > struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); > > + if (domain->type != IOMMU_DOMAIN_UNMANAGED) > + return -ENODEV; > + > mutex_lock(&smmu_domain->init_mutex); > > switch (attr) { > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thu, Jan 19, 2017 at 06:19:11PM +0000, Will Deacon wrote: > The ARM SMMU drivers provide a DOMAIN_ATTR_NESTING domain attribute, > which allows callers of the IOMMU API to request that the page table > for a domain is installed at stage-2, if supported by the hardware. > > Since setting this attribute only makes sense for UNMANAGED domains, > this patch returns -ENODEV if the domain_{get,set}_attr operations are > called on other domain types. Isn't -EINVAL more suitable here? In the end the domain passed in is invalid because it does not support attributes, no? Joerg _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thu, Jan 19, 2017 at 06:41:34PM +0000, Robin Murphy wrote: > For the sake of discussion, would it make sense to enforce this in > domain_set_attr() itself? The intersection of drivers providing these > callbacks and drivers supporting anything other than unmanaged domains > is currently these two below, so it clearly wouldn't break anything to > put this check in core code today. Looking forward, is there likely to > be any plausible situation where users of a managed domain would be > legitimate in mucking about with its attrs, on any platform? No, this belongs in driver code. I am pretty certain there will be a use for attributes in unmanaged domains at some point. Crazy things happen all the time :) Joerg _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thu, Jan 26, 2017 at 06:03:30PM +0100, Joerg Roedel wrote: > On Thu, Jan 19, 2017 at 06:19:11PM +0000, Will Deacon wrote: > > The ARM SMMU drivers provide a DOMAIN_ATTR_NESTING domain attribute, > > which allows callers of the IOMMU API to request that the page table > > for a domain is installed at stage-2, if supported by the hardware. > > > > Since setting this attribute only makes sense for UNMANAGED domains, > > this patch returns -ENODEV if the domain_{get,set}_attr operations are > > called on other domain types. > > Isn't -EINVAL more suitable here? In the end the domain passed in is > invalid because it does not support attributes, no? Sure, I can change that. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index 4d6ec444a9d6..c254325b0c7a 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -1839,6 +1839,9 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain, { struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); + if (domain->type != IOMMU_DOMAIN_UNMANAGED) + return -ENODEV; + switch (attr) { case DOMAIN_ATTR_NESTING: *(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED); @@ -1854,6 +1857,9 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain, int ret = 0; struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); + if (domain->type != IOMMU_DOMAIN_UNMANAGED) + return -ENODEV; + mutex_lock(&smmu_domain->init_mutex); switch (attr) { diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index a60cded8a6ed..a328ffb75509 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -1497,6 +1497,9 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain, { struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); + if (domain->type != IOMMU_DOMAIN_UNMANAGED) + return -ENODEV; + switch (attr) { case DOMAIN_ATTR_NESTING: *(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED); @@ -1512,6 +1515,9 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain, int ret = 0; struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); + if (domain->type != IOMMU_DOMAIN_UNMANAGED) + return -ENODEV; + mutex_lock(&smmu_domain->init_mutex); switch (attr) {
The ARM SMMU drivers provide a DOMAIN_ATTR_NESTING domain attribute, which allows callers of the IOMMU API to request that the page table for a domain is installed at stage-2, if supported by the hardware. Since setting this attribute only makes sense for UNMANAGED domains, this patch returns -ENODEV if the domain_{get,set}_attr operations are called on other domain types. Signed-off-by: Will Deacon <will.deacon@arm.com> --- drivers/iommu/arm-smmu-v3.c | 6 ++++++ drivers/iommu/arm-smmu.c | 6 ++++++ 2 files changed, 12 insertions(+) -- 2.1.4 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel