* [PATCH 0/2] Add access-protected IOMMU mappings
@ 2014-09-17 20:16 Mitchel Humpherys
2014-09-17 20:16 ` [PATCH 1/2] iommu: add IOMMU_PRIV flag for access-protected mappings Mitchel Humpherys
2014-09-17 20:16 ` [PATCH 2/2] iommu/arm-smmu: add support " Mitchel Humpherys
0 siblings, 2 replies; 6+ messages in thread
From: Mitchel Humpherys @ 2014-09-17 20:16 UTC (permalink / raw)
To: linux-arm-kernel
This series introduces a new mapping flag to indicate that the mapping
should be created with access protection applied. Support for this new flag
is then added to the ARM SMMU driver.
Mitchel Humpherys (2):
iommu: add IOMMU_PRIV flag for access-protected mappings
iommu/arm-smmu: add support for access-protected mappings
drivers/iommu/arm-smmu.c | 5 +++--
include/linux/iommu.h | 1 +
2 files changed, 4 insertions(+), 2 deletions(-)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] iommu: add IOMMU_PRIV flag for access-protected mappings
2014-09-17 20:16 [PATCH 0/2] Add access-protected IOMMU mappings Mitchel Humpherys
@ 2014-09-17 20:16 ` Mitchel Humpherys
2014-09-17 20:16 ` [PATCH 2/2] iommu/arm-smmu: add support " Mitchel Humpherys
1 sibling, 0 replies; 6+ messages in thread
From: Mitchel Humpherys @ 2014-09-17 20:16 UTC (permalink / raw)
To: linux-arm-kernel
Some IOMMUs support access-protected mappings. Add a mapping flag to
indicate that the mapping should be created with access protection
configured.
Cc: Shubhraprakash Das <sadas@codeaurora.org>
Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
---
include/linux/iommu.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 20f9a52792..44101c9332 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -28,6 +28,7 @@
#define IOMMU_WRITE (1 << 1)
#define IOMMU_CACHE (1 << 2) /* DMA cache coherency */
#define IOMMU_EXEC (1 << 3)
+#define IOMMU_PRIV (1 << 4)
struct iommu_ops;
struct iommu_group;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] iommu/arm-smmu: add support for access-protected mappings
2014-09-17 20:16 [PATCH 0/2] Add access-protected IOMMU mappings Mitchel Humpherys
2014-09-17 20:16 ` [PATCH 1/2] iommu: add IOMMU_PRIV flag for access-protected mappings Mitchel Humpherys
@ 2014-09-17 20:16 ` Mitchel Humpherys
2014-09-19 22:05 ` Will Deacon
1 sibling, 1 reply; 6+ messages in thread
From: Mitchel Humpherys @ 2014-09-17 20:16 UTC (permalink / raw)
To: linux-arm-kernel
ARM SMMUs support memory access control via some bits in the translation
table descriptor memory attributes. Currently we assume all translations
are "unprivileged". Add support for privileged mappings, controlled by
the IOMMU_PRIV prot flag.
Also sneak in a whitespace change for consistency with nearby code.
Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
---
drivers/iommu/arm-smmu.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index ca18d6d42a..93999ec22c 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1256,10 +1256,11 @@ static int arm_smmu_alloc_init_pte(struct arm_smmu_device *smmu, pmd_t *pmd,
}
if (stage == 1) {
- pteval |= ARM_SMMU_PTE_AP_UNPRIV | ARM_SMMU_PTE_nG;
+ pteval |= ARM_SMMU_PTE_nG;
+ if (!(prot & IOMMU_PRIV))
+ pteval |= ARM_SMMU_PTE_AP_UNPRIV;
if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
pteval |= ARM_SMMU_PTE_AP_RDONLY;
-
if (prot & IOMMU_CACHE)
pteval |= (MAIR_ATTR_IDX_CACHE <<
ARM_SMMU_PTE_ATTRINDX_SHIFT);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] iommu/arm-smmu: add support for access-protected mappings
2014-09-17 20:16 ` [PATCH 2/2] iommu/arm-smmu: add support " Mitchel Humpherys
@ 2014-09-19 22:05 ` Will Deacon
2014-09-22 22:28 ` Mitchel Humpherys
0 siblings, 1 reply; 6+ messages in thread
From: Will Deacon @ 2014-09-19 22:05 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Sep 17, 2014 at 09:16:09PM +0100, Mitchel Humpherys wrote:
> ARM SMMUs support memory access control via some bits in the translation
> table descriptor memory attributes. Currently we assume all translations
> are "unprivileged". Add support for privileged mappings, controlled by
> the IOMMU_PRIV prot flag.
>
> Also sneak in a whitespace change for consistency with nearby code.
>
> Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
> ---
> drivers/iommu/arm-smmu.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index ca18d6d42a..93999ec22c 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -1256,10 +1256,11 @@ static int arm_smmu_alloc_init_pte(struct arm_smmu_device *smmu, pmd_t *pmd,
> }
>
> if (stage == 1) {
> - pteval |= ARM_SMMU_PTE_AP_UNPRIV | ARM_SMMU_PTE_nG;
> + pteval |= ARM_SMMU_PTE_nG;
> + if (!(prot & IOMMU_PRIV))
> + pteval |= ARM_SMMU_PTE_AP_UNPRIV;
I think this actually makes more sense if we invert the logic, i.e. have
IOMMU_USER as a flag which sets the UNPRIV bit in the pte.
I don't have the spec to hand, but I guess you can't enforce this at
stage-2? If so, do we also need a new IOMMU capability so people don't try
to use this for stage-2 only SMMUs?
Will
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] iommu/arm-smmu: add support for access-protected mappings
2014-09-19 22:05 ` Will Deacon
@ 2014-09-22 22:28 ` Mitchel Humpherys
2014-09-24 16:27 ` Will Deacon
0 siblings, 1 reply; 6+ messages in thread
From: Mitchel Humpherys @ 2014-09-22 22:28 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Sep 19 2014 at 03:05:36 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Wed, Sep 17, 2014 at 09:16:09PM +0100, Mitchel Humpherys wrote:
>> ARM SMMUs support memory access control via some bits in the translation
>> table descriptor memory attributes. Currently we assume all translations
>> are "unprivileged". Add support for privileged mappings, controlled by
>> the IOMMU_PRIV prot flag.
>>
>> Also sneak in a whitespace change for consistency with nearby code.
>>
>> Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
>> ---
>> drivers/iommu/arm-smmu.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index ca18d6d42a..93999ec22c 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -1256,10 +1256,11 @@ static int arm_smmu_alloc_init_pte(struct arm_smmu_device *smmu, pmd_t *pmd,
>> }
>>
>> if (stage == 1) {
>> - pteval |= ARM_SMMU_PTE_AP_UNPRIV | ARM_SMMU_PTE_nG;
>> + pteval |= ARM_SMMU_PTE_nG;
>> + if (!(prot & IOMMU_PRIV))
>> + pteval |= ARM_SMMU_PTE_AP_UNPRIV;
>
> I think this actually makes more sense if we invert the logic, i.e. have
> IOMMU_USER as a flag which sets the UNPRIV bit in the pte.
I'm fine either way but the common case seems to be unprivileged
mappings (at least in our system). We have one user of this flag out of
a dozen or so users.
>
> I don't have the spec to hand, but I guess you can't enforce this at
> stage-2? If so, do we also need a new IOMMU capability so people don't try
> to use this for stage-2 only SMMUs?
Hmm, actually we do have S2AP although it doesn't make a distinction
between accesses from EL0 and EL1. But maybe it would make sense to
make the `IOMMU_PRIV' mean `no access from EL0 or EL1' for stage 2
mappings? Something like:
-- >8 --
Subject: iommu/arm-smmu: add support for access-protected mappings
ARM SMMUs support memory access control via some bits in the translation
table descriptor memory attributes. Currently we assume all translations
are "unprivileged". Add support for privileged mappings, controlled by
the IOMMU_PRIV prot flag.
Also sneak in a whitespace change for consistency with nearby code.
Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
---
drivers/iommu/arm-smmu.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index ca18d6d42a..4f85b64f74 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1256,18 +1256,19 @@ static int arm_smmu_alloc_init_pte(struct arm_smmu_device *smmu, pmd_t *pmd,
}
if (stage == 1) {
- pteval |= ARM_SMMU_PTE_AP_UNPRIV | ARM_SMMU_PTE_nG;
+ pteval |= ARM_SMMU_PTE_nG;
+ if (!(prot & IOMMU_PRIV))
+ pteval |= ARM_SMMU_PTE_AP_UNPRIV;
if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
pteval |= ARM_SMMU_PTE_AP_RDONLY;
-
if (prot & IOMMU_CACHE)
pteval |= (MAIR_ATTR_IDX_CACHE <<
ARM_SMMU_PTE_ATTRINDX_SHIFT);
} else {
pteval |= ARM_SMMU_PTE_HAP_FAULT;
- if (prot & IOMMU_READ)
+ if (prot & IOMMU_READ && !(prot & IOMMU_PRIV))
pteval |= ARM_SMMU_PTE_HAP_READ;
- if (prot & IOMMU_WRITE)
+ if (prot & IOMMU_WRITE && !(prot & IOMMU_PRIV))
pteval |= ARM_SMMU_PTE_HAP_WRITE;
if (prot & IOMMU_CACHE)
pteval |= ARM_SMMU_PTE_MEMATTR_OIWB;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] iommu/arm-smmu: add support for access-protected mappings
2014-09-22 22:28 ` Mitchel Humpherys
@ 2014-09-24 16:27 ` Will Deacon
0 siblings, 0 replies; 6+ messages in thread
From: Will Deacon @ 2014-09-24 16:27 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Sep 22, 2014 at 11:28:42PM +0100, Mitchel Humpherys wrote:
> On Fri, Sep 19 2014 at 03:05:36 PM, Will Deacon <will.deacon@arm.com> wrote:
> > On Wed, Sep 17, 2014 at 09:16:09PM +0100, Mitchel Humpherys wrote:
> >> ARM SMMUs support memory access control via some bits in the translation
> >> table descriptor memory attributes. Currently we assume all translations
> >> are "unprivileged". Add support for privileged mappings, controlled by
> >> the IOMMU_PRIV prot flag.
> >>
> >> Also sneak in a whitespace change for consistency with nearby code.
> >>
> >> Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
> >> ---
> >> drivers/iommu/arm-smmu.c | 5 +++--
> >> 1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> >> index ca18d6d42a..93999ec22c 100644
> >> --- a/drivers/iommu/arm-smmu.c
> >> +++ b/drivers/iommu/arm-smmu.c
> >> @@ -1256,10 +1256,11 @@ static int arm_smmu_alloc_init_pte(struct arm_smmu_device *smmu, pmd_t *pmd,
> >> }
> >>
> >> if (stage == 1) {
> >> - pteval |= ARM_SMMU_PTE_AP_UNPRIV | ARM_SMMU_PTE_nG;
> >> + pteval |= ARM_SMMU_PTE_nG;
> >> + if (!(prot & IOMMU_PRIV))
> >> + pteval |= ARM_SMMU_PTE_AP_UNPRIV;
> >
> > I think this actually makes more sense if we invert the logic, i.e. have
> > IOMMU_USER as a flag which sets the UNPRIV bit in the pte.
>
> I'm fine either way but the common case seems to be unprivileged
> mappings (at least in our system). We have one user of this flag out of
> a dozen or so users.
Well, I think the common case in reality (ie. in upstream) is that we don't
care, since there's no page table sharing with the CPU. Given that our DMA
buffers are privilege on the kernel side, it's consistent to copy that as
the default.
> >
> > I don't have the spec to hand, but I guess you can't enforce this at
> > stage-2? If so, do we also need a new IOMMU capability so people don't try
> > to use this for stage-2 only SMMUs?
>
> Hmm, actually we do have S2AP although it doesn't make a distinction
> between accesses from EL0 and EL1. But maybe it would make sense to
> make the `IOMMU_PRIV' mean `no access from EL0 or EL1' for stage 2
> mappings? Something like:
Hmm, that really doesn't match up with what KVM is doing on the CPU side and
would probably break DMA on stage-2 only SMMUs.
A better bet is to add a capability bit to the SMMU, which only advertises
support for IOMMU_USER mappings if stage-1 is supported.
Will
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-09-24 16:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-17 20:16 [PATCH 0/2] Add access-protected IOMMU mappings Mitchel Humpherys
2014-09-17 20:16 ` [PATCH 1/2] iommu: add IOMMU_PRIV flag for access-protected mappings Mitchel Humpherys
2014-09-17 20:16 ` [PATCH 2/2] iommu/arm-smmu: add support " Mitchel Humpherys
2014-09-19 22:05 ` Will Deacon
2014-09-22 22:28 ` Mitchel Humpherys
2014-09-24 16:27 ` Will Deacon
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).