| Message ID | 20250219220751.1276854-3-suzuki.poulose@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: realm: Fix DMA address for devices | expand |
On 2025-02-19 10:07 pm, Suzuki K Poulose wrote: > AMD SME added __sme_set/__sme_clr primitives to modify the DMA address for > encrypted/decrypted traffic. However this doesn't fit in with other models, > e.g., Arm CCA where the meanings are the opposite. i.e., "decrypted" traffic > has a bit set and "encrypted" traffic has the top bit cleared. > > In preparation for adding the support for Arm CCA DMA conversions, convert the > existing primitives to more generic ones that can be provided by the backends. > i.e., add helpers to > 1. dma_encrypted - Convert a DMA address to "encrypted" [ == __sme_set() ] > 2. dma_decrypted - Convert a DMA address to "decrypted" [ None exists today ] > 3. dma_clear_encryption - Clear any "encryption"/"decryption" bits from DMA > address [ SME uses __sme_clr() ] Nit: I'd still prefer to have as much distinction as possible between manipulation of the address representation itself, and any other aspects such as the manipulation of underlying pagetable attributes also implied by force_dma_unencrypted() on x86. So how about: dma_addr_encrypted dma_addr_unencrypted dma_addr_clear_encryption ? Note that we intentionally avoided "decrypted" in the existing APIs to minimise confusion, the only time any actual decryption is involved is for an "encrypted" address. (Stuff like that being why I personally would prefer to generalise away from encryption as an implementation detail at all, but since everyone else seems to be accustomed to the existing terminology and not complaining, I'm prepared to compromise that far!) Otherwise, the overall shape looks good to me. Thanks, Robin. > Since the original __sme_xxx helpers come from linux/mem_encrypt.h, use that > as the home for the new definitions and provide dummy ones when none is provided > by the architectures. > > With the above, phys_to_dma_unencrypted() uses the newly added dma_decrypted() > helper and to make it a bit more easier to read and avoid double conversion, > provide __phys_to_dma(). > > No functional changes intended. Compile tested on x86 defconfig with > CONFIG_AMD_MEM_ENCRYPT. > > Suggested-by: Robin Murphy <robin.murphy@arm.com> > Cc: Will Deacon <will@kernel.org> > Cc: Jean-Philippe Brucker <jean-philippe@linaro.org> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Robin Murphy <robin.murphy@arm.com> > Cc: Steven Price <steven.price@arm.com> > Cc: Christoph Hellwig <hch@lst.de> > Cc: Tom Lendacky <thomas.lendacky@amd.com> > Cc: Aneesh Kumar K.V <aneesh.kumar@kernel.org> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > --- > include/linux/dma-direct.h | 12 ++++++++---- > include/linux/mem_encrypt.h | 23 +++++++++++++++++++++++ > 2 files changed, 31 insertions(+), 4 deletions(-) > > diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h > index d20ecc24cb0f..9b5cc0ee86d5 100644 > --- a/include/linux/dma-direct.h > +++ b/include/linux/dma-direct.h > @@ -78,14 +78,18 @@ static inline dma_addr_t dma_range_map_max(const struct bus_dma_region *map) > #define phys_to_dma_unencrypted phys_to_dma > #endif > #else > -static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev, > - phys_addr_t paddr) > +static inline dma_addr_t __phys_to_dma(struct device *dev, phys_addr_t paddr) > { > if (dev->dma_range_map) > return translate_phys_to_dma(dev, paddr); > return paddr; > } > > +static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev, > + phys_addr_t paddr) > +{ > + return dma_decrypted(__phys_to_dma(dev, paddr)); > +} > /* > * If memory encryption is supported, phys_to_dma will set the memory encryption > * bit in the DMA address, and dma_to_phys will clear it. > @@ -94,14 +98,14 @@ static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev, > */ > static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr) > { > - return __sme_set(phys_to_dma_unencrypted(dev, paddr)); > + return dma_encrypted(__phys_to_dma(dev, paddr)); > } > > static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dma_addr) > { > phys_addr_t paddr; > > - dma_addr = __sme_clr(dma_addr); > + dma_addr = dma_clear_encryption(dma_addr); > if (dev->dma_range_map) > paddr = translate_dma_to_phys(dev, dma_addr); > else > diff --git a/include/linux/mem_encrypt.h b/include/linux/mem_encrypt.h > index ae4526389261..c8dcc1be695a 100644 > --- a/include/linux/mem_encrypt.h > +++ b/include/linux/mem_encrypt.h > @@ -26,11 +26,34 @@ > */ > #define __sme_set(x) ((x) | sme_me_mask) > #define __sme_clr(x) ((x) & ~sme_me_mask) > + > +#define dma_encrypted(x) __sme_set(x) > +#define dma_clear_encryption(x) __sme_clr(x) > + > #else > #define __sme_set(x) (x) > #define __sme_clr(x) (x) > #endif > > +/* > + * dma_encrypted() and dma_decrypted() are for converting a given DMA > + * address to the respective type of addressing. > + * > + * dma_clear_encryption() is used to reverse the conversion back to "normal" > + * DMA address. > + */ > +#ifndef dma_encrypted > +#define dma_encrypted(x) (x) > +#endif > + > +#ifndef dma_decrypted > +#define dma_decrypted(x) (x) > +#endif > + > +#ifndef dma_clear_encryption > +#define dma_clear_encryption(x) (x) > +#endif > + > #endif /* __ASSEMBLY__ */ > > #endif /* __MEM_ENCRYPT_H__ */
Hi Robin Thanks for your comments, response below. On 25/02/2025 12:49, Robin Murphy wrote: > On 2025-02-19 10:07 pm, Suzuki K Poulose wrote: >> AMD SME added __sme_set/__sme_clr primitives to modify the DMA address >> for >> encrypted/decrypted traffic. However this doesn't fit in with other >> models, >> e.g., Arm CCA where the meanings are the opposite. i.e., "decrypted" >> traffic >> has a bit set and "encrypted" traffic has the top bit cleared. >> >> In preparation for adding the support for Arm CCA DMA conversions, >> convert the >> existing primitives to more generic ones that can be provided by the >> backends. >> i.e., add helpers to >> 1. dma_encrypted - Convert a DMA address to "encrypted" [ == >> __sme_set() ] >> 2. dma_decrypted - Convert a DMA address to "decrypted" [ None >> exists today ] >> 3. dma_clear_encryption - Clear any "encryption"/"decryption" bits >> from DMA >> address [ SME uses __sme_clr() ] > > Nit: I'd still prefer to have as much distinction as possible between > manipulation of the address representation itself, and any other aspects > such as the manipulation of underlying pagetable attributes also implied > by force_dma_unencrypted() on x86. So how about: > > dma_addr_encrypted > dma_addr_unencrypted > dma_addr_clear_encryption > > ? Sure, sounds better. I am still not convinced about the dma_addr_clear_encryption() as it really applies to "clear" decryption too. The options we have are : dma_addr_canonical(), dma_addr_normal() or any others. > > Note that we intentionally avoided "decrypted" in the existing APIs to > minimise confusion, the only time any actual decryption is involved is > for an "encrypted" address. Agreed. > > (Stuff like that being why I personally would prefer to generalise away > from encryption as an implementation detail at all, but since everyone > else seems to be accustomed to the existing terminology and not > complaining, I'm prepared to compromise that far!) > > Otherwise, the overall shape looks good to me. I will respin this, with the changes. Suzuki > > Thanks, > Robin. > >> Since the original __sme_xxx helpers come from linux/mem_encrypt.h, >> use that >> as the home for the new definitions and provide dummy ones when none >> is provided >> by the architectures. >> >> With the above, phys_to_dma_unencrypted() uses the newly added >> dma_decrypted() >> helper and to make it a bit more easier to read and avoid double >> conversion, >> provide __phys_to_dma(). >> >> No functional changes intended. Compile tested on x86 defconfig with >> CONFIG_AMD_MEM_ENCRYPT. >> >> Suggested-by: Robin Murphy <robin.murphy@arm.com> >> Cc: Will Deacon <will@kernel.org> >> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org> >> Cc: Catalin Marinas <catalin.marinas@arm.com> >> Cc: Robin Murphy <robin.murphy@arm.com> >> Cc: Steven Price <steven.price@arm.com> >> Cc: Christoph Hellwig <hch@lst.de> >> Cc: Tom Lendacky <thomas.lendacky@amd.com> >> Cc: Aneesh Kumar K.V <aneesh.kumar@kernel.org> >> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> >> --- >> include/linux/dma-direct.h | 12 ++++++++---- >> include/linux/mem_encrypt.h | 23 +++++++++++++++++++++++ >> 2 files changed, 31 insertions(+), 4 deletions(-) >> >> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h >> index d20ecc24cb0f..9b5cc0ee86d5 100644 >> --- a/include/linux/dma-direct.h >> +++ b/include/linux/dma-direct.h >> @@ -78,14 +78,18 @@ static inline dma_addr_t dma_range_map_max(const >> struct bus_dma_region *map) >> #define phys_to_dma_unencrypted phys_to_dma >> #endif >> #else >> -static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev, >> - phys_addr_t paddr) >> +static inline dma_addr_t __phys_to_dma(struct device *dev, >> phys_addr_t paddr) >> { >> if (dev->dma_range_map) >> return translate_phys_to_dma(dev, paddr); >> return paddr; >> } >> +static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev, >> + phys_addr_t paddr) >> +{ >> + return dma_decrypted(__phys_to_dma(dev, paddr)); >> +} >> /* >> * If memory encryption is supported, phys_to_dma will set the >> memory encryption >> * bit in the DMA address, and dma_to_phys will clear it. >> @@ -94,14 +98,14 @@ static inline dma_addr_t >> phys_to_dma_unencrypted(struct device *dev, >> */ >> static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t >> paddr) >> { >> - return __sme_set(phys_to_dma_unencrypted(dev, paddr)); >> + return dma_encrypted(__phys_to_dma(dev, paddr)); >> } >> static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t >> dma_addr) >> { >> phys_addr_t paddr; >> - dma_addr = __sme_clr(dma_addr); >> + dma_addr = dma_clear_encryption(dma_addr); >> if (dev->dma_range_map) >> paddr = translate_dma_to_phys(dev, dma_addr); >> else >> diff --git a/include/linux/mem_encrypt.h b/include/linux/mem_encrypt.h >> index ae4526389261..c8dcc1be695a 100644 >> --- a/include/linux/mem_encrypt.h >> +++ b/include/linux/mem_encrypt.h >> @@ -26,11 +26,34 @@ >> */ >> #define __sme_set(x) ((x) | sme_me_mask) >> #define __sme_clr(x) ((x) & ~sme_me_mask) >> + >> +#define dma_encrypted(x) __sme_set(x) >> +#define dma_clear_encryption(x) __sme_clr(x) >> + >> #else >> #define __sme_set(x) (x) >> #define __sme_clr(x) (x) >> #endif >> +/* >> + * dma_encrypted() and dma_decrypted() are for converting a given DMA >> + * address to the respective type of addressing. >> + * >> + * dma_clear_encryption() is used to reverse the conversion back to >> "normal" >> + * DMA address. >> + */ >> +#ifndef dma_encrypted >> +#define dma_encrypted(x) (x) >> +#endif >> + >> +#ifndef dma_decrypted >> +#define dma_decrypted(x) (x) >> +#endif >> + >> +#ifndef dma_clear_encryption >> +#define dma_clear_encryption(x) (x) >> +#endif >> + >> #endif /* __ASSEMBLY__ */ >> #endif /* __MEM_ENCRYPT_H__ */ >
diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h index d20ecc24cb0f..9b5cc0ee86d5 100644 --- a/include/linux/dma-direct.h +++ b/include/linux/dma-direct.h @@ -78,14 +78,18 @@ static inline dma_addr_t dma_range_map_max(const struct bus_dma_region *map) #define phys_to_dma_unencrypted phys_to_dma #endif #else -static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev, - phys_addr_t paddr) +static inline dma_addr_t __phys_to_dma(struct device *dev, phys_addr_t paddr) { if (dev->dma_range_map) return translate_phys_to_dma(dev, paddr); return paddr; } +static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev, + phys_addr_t paddr) +{ + return dma_decrypted(__phys_to_dma(dev, paddr)); +} /* * If memory encryption is supported, phys_to_dma will set the memory encryption * bit in the DMA address, and dma_to_phys will clear it. @@ -94,14 +98,14 @@ static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev, */ static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr) { - return __sme_set(phys_to_dma_unencrypted(dev, paddr)); + return dma_encrypted(__phys_to_dma(dev, paddr)); } static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dma_addr) { phys_addr_t paddr; - dma_addr = __sme_clr(dma_addr); + dma_addr = dma_clear_encryption(dma_addr); if (dev->dma_range_map) paddr = translate_dma_to_phys(dev, dma_addr); else diff --git a/include/linux/mem_encrypt.h b/include/linux/mem_encrypt.h index ae4526389261..c8dcc1be695a 100644 --- a/include/linux/mem_encrypt.h +++ b/include/linux/mem_encrypt.h @@ -26,11 +26,34 @@ */ #define __sme_set(x) ((x) | sme_me_mask) #define __sme_clr(x) ((x) & ~sme_me_mask) + +#define dma_encrypted(x) __sme_set(x) +#define dma_clear_encryption(x) __sme_clr(x) + #else #define __sme_set(x) (x) #define __sme_clr(x) (x) #endif +/* + * dma_encrypted() and dma_decrypted() are for converting a given DMA + * address to the respective type of addressing. + * + * dma_clear_encryption() is used to reverse the conversion back to "normal" + * DMA address. + */ +#ifndef dma_encrypted +#define dma_encrypted(x) (x) +#endif + +#ifndef dma_decrypted +#define dma_decrypted(x) (x) +#endif + +#ifndef dma_clear_encryption +#define dma_clear_encryption(x) (x) +#endif + #endif /* __ASSEMBLY__ */ #endif /* __MEM_ENCRYPT_H__ */
AMD SME added __sme_set/__sme_clr primitives to modify the DMA address for encrypted/decrypted traffic. However this doesn't fit in with other models, e.g., Arm CCA where the meanings are the opposite. i.e., "decrypted" traffic has a bit set and "encrypted" traffic has the top bit cleared. In preparation for adding the support for Arm CCA DMA conversions, convert the existing primitives to more generic ones that can be provided by the backends. i.e., add helpers to 1. dma_encrypted - Convert a DMA address to "encrypted" [ == __sme_set() ] 2. dma_decrypted - Convert a DMA address to "decrypted" [ None exists today ] 3. dma_clear_encryption - Clear any "encryption"/"decryption" bits from DMA address [ SME uses __sme_clr() ] Since the original __sme_xxx helpers come from linux/mem_encrypt.h, use that as the home for the new definitions and provide dummy ones when none is provided by the architectures. With the above, phys_to_dma_unencrypted() uses the newly added dma_decrypted() helper and to make it a bit more easier to read and avoid double conversion, provide __phys_to_dma(). No functional changes intended. Compile tested on x86 defconfig with CONFIG_AMD_MEM_ENCRYPT. Suggested-by: Robin Murphy <robin.murphy@arm.com> Cc: Will Deacon <will@kernel.org> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Robin Murphy <robin.murphy@arm.com> Cc: Steven Price <steven.price@arm.com> Cc: Christoph Hellwig <hch@lst.de> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Aneesh Kumar K.V <aneesh.kumar@kernel.org> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> --- include/linux/dma-direct.h | 12 ++++++++---- include/linux/mem_encrypt.h | 23 +++++++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-)