* [PATCH 0/2] Add access-protected IOMMU mappings @ 2014-09-17 20:16 ` Mitchel Humpherys 0 siblings, 0 replies; 12+ messages in thread From: Mitchel Humpherys @ 2014-09-17 20:16 UTC (permalink / raw) To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Will Deacon, Joerg Roedel 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] 12+ messages in thread
* [PATCH 0/2] Add access-protected IOMMU mappings @ 2014-09-17 20:16 ` Mitchel Humpherys 0 siblings, 0 replies; 12+ 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] 12+ messages in thread
[parent not found: <1410984969-2340-1-git-send-email-mitchelh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]
* [PATCH 1/2] iommu: add IOMMU_PRIV flag for access-protected mappings 2014-09-17 20:16 ` Mitchel Humpherys @ 2014-09-17 20:16 ` Mitchel Humpherys -1 siblings, 0 replies; 12+ messages in thread From: Mitchel Humpherys @ 2014-09-17 20:16 UTC (permalink / raw) To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Will Deacon, Joerg Roedel Cc: Shubhraprakash Das 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-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> Signed-off-by: Mitchel Humpherys <mitchelh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.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] 12+ messages in thread
* [PATCH 1/2] iommu: add IOMMU_PRIV flag for access-protected mappings @ 2014-09-17 20:16 ` Mitchel Humpherys 0 siblings, 0 replies; 12+ 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] 12+ messages in thread
* [PATCH 2/2] iommu/arm-smmu: add support for access-protected mappings 2014-09-17 20:16 ` Mitchel Humpherys @ 2014-09-17 20:16 ` Mitchel Humpherys -1 siblings, 0 replies; 12+ messages in thread From: Mitchel Humpherys @ 2014-09-17 20:16 UTC (permalink / raw) To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Will Deacon, Joerg Roedel 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-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.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] 12+ messages in thread
* [PATCH 2/2] iommu/arm-smmu: add support for access-protected mappings @ 2014-09-17 20:16 ` Mitchel Humpherys 0 siblings, 0 replies; 12+ 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] 12+ messages in thread
[parent not found: <1410984969-2340-3-git-send-email-mitchelh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]
* Re: [PATCH 2/2] iommu/arm-smmu: add support for access-protected mappings 2014-09-17 20:16 ` Mitchel Humpherys @ 2014-09-19 22:05 ` Will Deacon -1 siblings, 0 replies; 12+ messages in thread From: Will Deacon @ 2014-09-19 22:05 UTC (permalink / raw) To: Mitchel Humpherys Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, Joerg Roedel, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org 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-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.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] 12+ messages in thread
* [PATCH 2/2] iommu/arm-smmu: add support for access-protected mappings @ 2014-09-19 22:05 ` Will Deacon 0 siblings, 0 replies; 12+ 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] 12+ messages in thread
[parent not found: <20140919220535.GM20773-5wv7dgnIgG8@public.gmane.org>]
* Re: [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 -1 siblings, 0 replies; 12+ messages in thread From: Mitchel Humpherys @ 2014-09-22 22:28 UTC (permalink / raw) To: Will Deacon Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, Joerg Roedel, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org On Fri, Sep 19 2014 at 03:05:36 PM, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> 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-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.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-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.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] 12+ messages in thread
* [PATCH 2/2] iommu/arm-smmu: add support for access-protected mappings @ 2014-09-22 22:28 ` Mitchel Humpherys 0 siblings, 0 replies; 12+ 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] 12+ messages in thread
[parent not found: <vnkwtx3zb885.fsf-Yf+dfxj6toJBVvN7MMdr1KRtKmQZhJ7pQQ4Iyu8u01E@public.gmane.org>]
* Re: [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 -1 siblings, 0 replies; 12+ messages in thread From: Will Deacon @ 2014-09-24 16:27 UTC (permalink / raw) To: Mitchel Humpherys Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, Joerg Roedel, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org 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-5wv7dgnIgG8@public.gmane.org> 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-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.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] 12+ messages in thread
* [PATCH 2/2] iommu/arm-smmu: add support for access-protected mappings @ 2014-09-24 16:27 ` Will Deacon 0 siblings, 0 replies; 12+ 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] 12+ messages in thread
end of thread, other threads:[~2014-09-24 16:27 UTC | newest]
Thread overview: 12+ 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 ` Mitchel Humpherys
[not found] ` <1410984969-2340-1-git-send-email-mitchelh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
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-17 20:16 ` [PATCH 2/2] iommu/arm-smmu: add support " Mitchel Humpherys
2014-09-17 20:16 ` Mitchel Humpherys
[not found] ` <1410984969-2340-3-git-send-email-mitchelh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2014-09-19 22:05 ` Will Deacon
2014-09-19 22:05 ` Will Deacon
[not found] ` <20140919220535.GM20773-5wv7dgnIgG8@public.gmane.org>
2014-09-22 22:28 ` Mitchel Humpherys
2014-09-22 22:28 ` Mitchel Humpherys
[not found] ` <vnkwtx3zb885.fsf-Yf+dfxj6toJBVvN7MMdr1KRtKmQZhJ7pQQ4Iyu8u01E@public.gmane.org>
2014-09-24 16:27 ` Will Deacon
2014-09-24 16:27 ` Will Deacon
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.