[PATCH v2 0/3] arm64: realm: Fix DMA address for devices

[PATCH v2 0/3] arm64: realm: Fix DMA address for devices

[Suzuki K Poulose]

Linux can be run as a Confidential Guest in Arm CCA from Linux v6.13. The address
space (GPA or IPA) of a Realm VM is split into two halves, with private bottom
half and shared top half. In Linux we treat the "top" bit of the IPA space as
an attribute, to indicate whether it is shared or not (MSB == 1 implies shared).
Stage2 (GPA to PA) translations used by the CPU accesses, cover the full IPA space,
and are managed by RMM. The "top" bit as attribute is only a software construct.

At present any device passed through to a Realm is treated as untrusted and the
Realm uses bounce buffering for any DMA, using the "decrypted" (shared) DMA
buffers (i.e., IPA with top bit set). In Linux, we only send the "DMA" address
masking the "top" bit. In Arm CCA, SMMU for untrusted devices are managed by the
non-secure Host and thus it can be confusing for the host/device when an unmasked
address is provided. Given there could be other hypervisors than Linux/KVM
running Arm CCA guests, the Realm Guest must adhere to a single convention for
the DMA address. This gets further complicated when we add support for trusted
devices, which can DMA into the full Realm memory space, once accepted. Thus,
a DMA masked address (with "top" bit lost) will prevent a trusted device from
accessing a shared buffer.

To resolve this Arm has decided to standardise the DMA address used by the Realm
to include the full IPA address bits (including the "top" bit, which Linux uses
as an attribute). This implies, any DMA to a shared buffer must have the top bit
of the IPA space set.

There is already a provision to do this in phys_to_dma* and dma_to_phys(), but
that is specific to AMD SME and is quite the opposite of what we need for Arm CCA.
i.e., For Arm CCA we need to set the bit for "decrypted" DMA and clear the bit
for "encrypted".

This series converts the existing __sme_* helpers to a bit more generalised versions :
dma_decrypted() and dma_encrypted(). Also, while converting a DMA address back
to CPU physical address requires clearing off any "encryption/decryption" bits.
I have named this "dma_clear_encryption()", while this should be applied for
both encrypted/decrypted addresses. The better choice of names are already 
taken (dma_to_phys() ;-) and dma_clear()). Suggestions welcome. Please see Patch 2
for more details.

This also implies that the VMMs must take care to :

 1. Create the S2-SMMU mappings for VFIO at the "unprotected" alias.
 2. Always mask the "top" bit off any IPA it receives from the Realm for DMA.
    KVM already does that today and no changes are required.

A kvmtool branch with the changes above is available here [1]. There are two
patches [2] & [3], that are really required on top of the Arm CCA support.

Ideally it would be good to get this backported to v6.13 stable kernel releases
to make sure that they are compliant with this change.

Changes since v1
 Link: https://lkml.kernel.org/r/20250212171411.951874-1-suzuki.poulose@arm.com
 - Follow Robin's suggestion to generalise the DMA address conversion helpers
   to provide dma_{encrypte,decrypted,clear_encryption}. See PATCH 2 for more
   details.
 - Add a fix to the ordering of "__sme_clr" for dma_to_phys (PATCH 1)

[1] git@git.gitlab.arm.com:linux-arm/kvmtool-cca.git cca/guest-dma-alias/v1
[2] https://gitlab.arm.com/linux-arm/kvmtool-cca/-/commit/ea37a6eb968abe4c75be4a8a90808714657c2ef7
[3] https://gitlab.arm.com/linux-arm/kvmtool-cca/-/commit/8afd0d5e6a7ee444dd0c1565fe94ecd831054a29

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>


Suzuki K Poulose (3):
  dma: Fix encryption bit clearing for dma_to_phys
  dma: Introduce generic dma_decrypted/dma_encrypted helpers
  arm64: realm: Use aliased addresses for device DMA to shared buffers

 arch/arm64/include/asm/mem_encrypt.h | 22 ++++++++++++++++++++++
 include/linux/dma-direct.h           | 13 +++++++++----
 include/linux/mem_encrypt.h          | 23 +++++++++++++++++++++++
 3 files changed, 54 insertions(+), 4 deletions(-)

-- 
2.43.0

[PATCH v2 1/3] dma: Fix encryption bit clearing for dma_to_phys

[Suzuki K Poulose]

phys_to_dma() sets the encryption bit on the translated DMA address. But
dma_to_phys() clears the encryption bit after it has been translated back
to the physical address, which could fail if the device uses DMA ranges.

Hopefully, AMD SME doesn't use it. Anyways, let us fix it, before cleanup
the infrastructure for supporting other architectures.

Reported-by: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
Link: https://lkml.kernel.org/r/yq5amsen9stc.fsf@kernel.org
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>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 include/linux/dma-direct.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index d7e30d4f7503..d20ecc24cb0f 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -101,12 +101,13 @@ 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);
 	if (dev->dma_range_map)
 		paddr = translate_dma_to_phys(dev, dma_addr);
 	else
 		paddr = dma_addr;
 
-	return __sme_clr(paddr);
+	return paddr;
 }
 #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
 
-- 
2.43.0

Re: [PATCH v2 1/3] dma: Fix encryption bit clearing for dma_to_phys

[Robin Murphy]

On 2025-02-19 10:07 pm, Suzuki K Poulose wrote:
> phys_to_dma() sets the encryption bit on the translated DMA address. But
> dma_to_phys() clears the encryption bit after it has been translated back
> to the physical address, which could fail if the device uses DMA ranges.
> 
> Hopefully, AMD SME doesn't use it.

...by which you mean we don't think any AMD systems are using the ACPI 
_DMA method to constrain physical DMA ranges, otherwise SME with 
dma-direct would presumably already be broken by this lookup going wrong.

> Anyways, let us fix it, before cleanup
> the infrastructure for supporting other architectures.

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

> Reported-by: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
> Link: https://lkml.kernel.org/r/yq5amsen9stc.fsf@kernel.org
> 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>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>   include/linux/dma-direct.h | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
> index d7e30d4f7503..d20ecc24cb0f 100644
> --- a/include/linux/dma-direct.h
> +++ b/include/linux/dma-direct.h
> @@ -101,12 +101,13 @@ 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);
>   	if (dev->dma_range_map)
>   		paddr = translate_dma_to_phys(dev, dma_addr);
>   	else
>   		paddr = dma_addr;
>   
> -	return __sme_clr(paddr);
> +	return paddr;
>   }
>   #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
>   

Re: [PATCH v2 1/3] dma: Fix encryption bit clearing for dma_to_phys

[Suzuki K Poulose]

On 25/02/2025 11:25, Robin Murphy wrote:
> On 2025-02-19 10:07 pm, Suzuki K Poulose wrote:
>> phys_to_dma() sets the encryption bit on the translated DMA address. But
>> dma_to_phys() clears the encryption bit after it has been translated back
>> to the physical address, which could fail if the device uses DMA ranges.
>>
>> Hopefully, AMD SME doesn't use it.
> 
> ...by which you mean we don't think any AMD systems are using the ACPI 
> _DMA method to constrain physical DMA ranges, otherwise SME with dma- 
> direct would presumably already be broken by this lookup going wrong.

Yep, that AMD systems aren't using DMA ranges.


> 
>> Anyways, let us fix it, before cleanup
>> the infrastructure for supporting other architectures.
> 
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>

Thanks

Suzuki

> 
>> Reported-by: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
>> Link: https://lkml.kernel.org/r/yq5amsen9stc.fsf@kernel.org
>> 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>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>   include/linux/dma-direct.h | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
>> index d7e30d4f7503..d20ecc24cb0f 100644
>> --- a/include/linux/dma-direct.h
>> +++ b/include/linux/dma-direct.h
>> @@ -101,12 +101,13 @@ 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);
>>       if (dev->dma_range_map)
>>           paddr = translate_dma_to_phys(dev, dma_addr);
>>       else
>>           paddr = dma_addr;
>> -    return __sme_clr(paddr);
>> +    return paddr;
>>   }
>>   #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
> 

Re: [PATCH v2 1/3] dma: Fix encryption bit clearing for dma_to_phys

[Tom Lendacky]

On 2/19/25 16:07, Suzuki K Poulose wrote:
> phys_to_dma() sets the encryption bit on the translated DMA address. But
> dma_to_phys() clears the encryption bit after it has been translated back
> to the physical address, which could fail if the device uses DMA ranges.
> 
> Hopefully, AMD SME doesn't use it. Anyways, let us fix it, before cleanup
> the infrastructure for supporting other architectures.

I'm not aware of anything using DMA ranges on SME capable hardware. And
as you stated, this would have failed if there was.

Acked-by: Tom Lendacky <thomas.lendacky@amd.com>

> 
> Reported-by: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
> Link: https://lkml.kernel.org/r/yq5amsen9stc.fsf@kernel.org
> 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>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  include/linux/dma-direct.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
> index d7e30d4f7503..d20ecc24cb0f 100644
> --- a/include/linux/dma-direct.h
> +++ b/include/linux/dma-direct.h
> @@ -101,12 +101,13 @@ 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);
>  	if (dev->dma_range_map)
>  		paddr = translate_dma_to_phys(dev, dma_addr);
>  	else
>  		paddr = dma_addr;
>  
> -	return __sme_clr(paddr);
> +	return paddr;
>  }
>  #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
>  

[PATCH v2 2/3] dma: Introduce generic dma_decrypted/dma_encrypted helpers

[Suzuki K Poulose]

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(-)

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__ */
-- 
2.43.0

Re: [PATCH v2 2/3] dma: Introduce generic dma_decrypted/dma_encrypted helpers

[Robin Murphy]

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__ */

Re: [PATCH v2 2/3] dma: Introduce generic dma_decrypted/dma_encrypted helpers

[Suzuki K Poulose]

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__ */
> 

[PATCH v2 3/3] arm64: realm: Use aliased addresses for device DMA to shared buffers

[Suzuki K Poulose]

When a device performs DMA to a shared buffer using physical addresses,
(without Stage1 translation), the device must use the "{I}PA address" with the
top bit set in Realm. This is to make sure that a trusted device will be able
to write to shared buffers as well as the protected buffers. Thus, a Realm must
always program the full address including the "protection" bit, like AMD SME
encryption bits.

Enable this by providing arm64 specific dma_{encrypted,decrypted,clear_encryption}
helpers for Realms. Please note that the VMM needs to similarly make sure that
the SMMU Stage2 in the Non-secure world is setup accordingly to map IPA at the
unprotected alias.

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>
---
 arch/arm64/include/asm/mem_encrypt.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm64/include/asm/mem_encrypt.h b/arch/arm64/include/asm/mem_encrypt.h
index f8f78f622dd2..aeda3bba255e 100644
--- a/arch/arm64/include/asm/mem_encrypt.h
+++ b/arch/arm64/include/asm/mem_encrypt.h
@@ -21,4 +21,26 @@ static inline bool force_dma_unencrypted(struct device *dev)
 	return is_realm_world();
 }
 
+static inline dma_addr_t dma_decrypted(dma_addr_t daddr)
+{
+	if (is_realm_world())
+		daddr |= prot_ns_shared;
+	return daddr;
+}
+#define dma_decrypted dma_decrypted
+
+static inline dma_addr_t dma_encrypted(dma_addr_t daddr)
+{
+	if (is_realm_world())
+		daddr &= prot_ns_shared - 1;
+	return daddr;
+}
+#define dma_encrypted dma_encrypted
+
+static inline dma_addr_t dma_clear_encryption(dma_addr_t daddr)
+{
+	return dma_encrypted(daddr);
+}
+#define dma_clear_encryption dma_clear_encryption
+
 #endif	/* __ASM_MEM_ENCRYPT_H */
-- 
2.43.0

Re: [PATCH v2 3/3] arm64: realm: Use aliased addresses for device DMA to shared buffers

[Gavin Shan]

Hi Suzuki,

On 2/20/25 8:07 AM, Suzuki K Poulose wrote:
> When a device performs DMA to a shared buffer using physical addresses,
> (without Stage1 translation), the device must use the "{I}PA address" with the
> top bit set in Realm. This is to make sure that a trusted device will be able
> to write to shared buffers as well as the protected buffers. Thus, a Realm must
> always program the full address including the "protection" bit, like AMD SME
> encryption bits.
> 
> Enable this by providing arm64 specific dma_{encrypted,decrypted,clear_encryption}
> helpers for Realms. Please note that the VMM needs to similarly make sure that
> the SMMU Stage2 in the Non-secure world is setup accordingly to map IPA at the
> unprotected alias.
> 
> 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>
> ---
>   arch/arm64/include/asm/mem_encrypt.h | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/mem_encrypt.h b/arch/arm64/include/asm/mem_encrypt.h
> index f8f78f622dd2..aeda3bba255e 100644
> --- a/arch/arm64/include/asm/mem_encrypt.h
> +++ b/arch/arm64/include/asm/mem_encrypt.h
> @@ -21,4 +21,26 @@ static inline bool force_dma_unencrypted(struct device *dev)
>   	return is_realm_world();
>   }
>   
> +static inline dma_addr_t dma_decrypted(dma_addr_t daddr)
> +{
> +	if (is_realm_world())
> +		daddr |= prot_ns_shared;
> +	return daddr;
> +}
> +#define dma_decrypted dma_decrypted
> +

There is an existing macro (PROT_NS_SHARED), which is preferred to return
prot_ns_shared or 0 depending on the availability of the realm capability.
However, that macro needs to be improved a bit so that it can be used here.
We need to return 0UL to match with the type of prot_ns_shared (unsigned long)

-#define PROT_NS_SHARED         (is_realm_world() ? prot_ns_shared : 0)
+#define PROT_NS_SHARED         (is_realm_world() ? prot_ns_shared : 0UL)

After that, the chunk of code can be as below.

	return daddr | PROT_NS_SHARED;

> +static inline dma_addr_t dma_encrypted(dma_addr_t daddr)
> +{
> +	if (is_realm_world())
> +		daddr &= prot_ns_shared - 1;
> +	return daddr;
> +}
> +#define dma_encrypted dma_encrypted
> +

With PROT_NS_SHARED, it can become something like below. (PROT_NS_SHARED - 1)
is equivalent to -1UL, 'daddr & -1UL' should be fine since it does nothing.

	return daddr & (PROT_NS_SHARED - 1);

> +static inline dma_addr_t dma_clear_encryption(dma_addr_t daddr)
> +{
> +	return dma_encrypted(daddr);
> +}
> +#define dma_clear_encryption dma_clear_encryption
> +
>   #endif	/* __ASM_MEM_ENCRYPT_H */

Thanks,
Gavin

Re: [PATCH v2 3/3] arm64: realm: Use aliased addresses for device DMA to shared buffers

[Gavin Shan]

On 2/25/25 3:24 PM, Gavin Shan wrote: 
> On 2/20/25 8:07 AM, Suzuki K Poulose wrote:
>> When a device performs DMA to a shared buffer using physical addresses,
>> (without Stage1 translation), the device must use the "{I}PA address" with the
>> top bit set in Realm. This is to make sure that a trusted device will be able
>> to write to shared buffers as well as the protected buffers. Thus, a Realm must
>> always program the full address including the "protection" bit, like AMD SME
>> encryption bits.
>>
>> Enable this by providing arm64 specific dma_{encrypted,decrypted,clear_encryption}
>> helpers for Realms. Please note that the VMM needs to similarly make sure that
>> the SMMU Stage2 in the Non-secure world is setup accordingly to map IPA at the
>> unprotected alias.
>>
>> 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>
>> ---
>>   arch/arm64/include/asm/mem_encrypt.h | 22 ++++++++++++++++++++++
>>   1 file changed, 22 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/mem_encrypt.h b/arch/arm64/include/asm/mem_encrypt.h
>> index f8f78f622dd2..aeda3bba255e 100644
>> --- a/arch/arm64/include/asm/mem_encrypt.h
>> +++ b/arch/arm64/include/asm/mem_encrypt.h
>> @@ -21,4 +21,26 @@ static inline bool force_dma_unencrypted(struct device *dev)
>>       return is_realm_world();
>>   }
>> +static inline dma_addr_t dma_decrypted(dma_addr_t daddr)
>> +{
>> +    if (is_realm_world())
>> +        daddr |= prot_ns_shared;
>> +    return daddr;
>> +}
>> +#define dma_decrypted dma_decrypted
>> +
> 
> There is an existing macro (PROT_NS_SHARED), which is preferred to return
> prot_ns_shared or 0 depending on the availability of the realm capability.
> However, that macro needs to be improved a bit so that it can be used here.
> We need to return 0UL to match with the type of prot_ns_shared (unsigned long)
> 
> -#define PROT_NS_SHARED         (is_realm_world() ? prot_ns_shared : 0)
> +#define PROT_NS_SHARED         (is_realm_world() ? prot_ns_shared : 0UL)
> 
> After that, the chunk of code can be as below.
> 
>      return daddr | PROT_NS_SHARED;
> 
>> +static inline dma_addr_t dma_encrypted(dma_addr_t daddr)
>> +{
>> +    if (is_realm_world())
>> +        daddr &= prot_ns_shared - 1;
>> +    return daddr;
>> +}
>> +#define dma_encrypted dma_encrypted
>> +
> 
> With PROT_NS_SHARED, it can become something like below. (PROT_NS_SHARED - 1)
> is equivalent to -1UL, 'daddr & -1UL' should be fine since it does nothing.
> 

I meant (PROT_NS_SHARED - 1) is equivalent to -1UL when no realm capability
is around :)

>      return daddr & (PROT_NS_SHARED - 1);
> 
>> +static inline dma_addr_t dma_clear_encryption(dma_addr_t daddr)
>> +{
>> +    return dma_encrypted(daddr);
>> +}
>> +#define dma_clear_encryption dma_clear_encryption
>> +
>>   #endif    /* __ASM_MEM_ENCRYPT_H */

Thanks,
Gavin

Re: [PATCH v2 3/3] arm64: realm: Use aliased addresses for device DMA to shared buffers

[Suzuki K Poulose]

Hi Gavin

Thanks for the review.

On 25/02/2025 05:28, Gavin Shan wrote:
> On 2/25/25 3:24 PM, Gavin Shan wrote:
>> On 2/20/25 8:07 AM, Suzuki K Poulose wrote:
>>> When a device performs DMA to a shared buffer using physical addresses,
>>> (without Stage1 translation), the device must use the "{I}PA address" 
>>> with the
>>> top bit set in Realm. This is to make sure that a trusted device will 
>>> be able
>>> to write to shared buffers as well as the protected buffers. Thus, a 
>>> Realm must
>>> always program the full address including the "protection" bit, like 
>>> AMD SME
>>> encryption bits.
>>>
>>> Enable this by providing arm64 specific 
>>> dma_{encrypted,decrypted,clear_encryption}
>>> helpers for Realms. Please note that the VMM needs to similarly make 
>>> sure that
>>> the SMMU Stage2 in the Non-secure world is setup accordingly to map 
>>> IPA at the
>>> unprotected alias.
>>>
>>> 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>
>>> ---
>>>   arch/arm64/include/asm/mem_encrypt.h | 22 ++++++++++++++++++++++
>>>   1 file changed, 22 insertions(+)
>>>
>>> diff --git a/arch/arm64/include/asm/mem_encrypt.h b/arch/arm64/ 
>>> include/asm/mem_encrypt.h
>>> index f8f78f622dd2..aeda3bba255e 100644
>>> --- a/arch/arm64/include/asm/mem_encrypt.h
>>> +++ b/arch/arm64/include/asm/mem_encrypt.h
>>> @@ -21,4 +21,26 @@ static inline bool force_dma_unencrypted(struct 
>>> device *dev)
>>>       return is_realm_world();
>>>   }
>>> +static inline dma_addr_t dma_decrypted(dma_addr_t daddr)
>>> +{
>>> +    if (is_realm_world())
>>> +        daddr |= prot_ns_shared;
>>> +    return daddr;
>>> +}
>>> +#define dma_decrypted dma_decrypted
>>> +
>>
>> There is an existing macro (PROT_NS_SHARED), which is preferred to return
>> prot_ns_shared or 0 depending on the availability of the realm 
>> capability.
>> However, that macro needs to be improved a bit so that it can be used 
>> here.
>> We need to return 0UL to match with the type of prot_ns_shared 
>> (unsigned long)
>>
>> -#define PROT_NS_SHARED         (is_realm_world() ? prot_ns_shared : 0)
>> +#define PROT_NS_SHARED         (is_realm_world() ? prot_ns_shared : 0UL)
>>
>> After that, the chunk of code can be as below.
>>
>>      return daddr | PROT_NS_SHARED;
>>
>>> +static inline dma_addr_t dma_encrypted(dma_addr_t daddr)
>>> +{
>>> +    if (is_realm_world())
>>> +        daddr &= prot_ns_shared - 1;
>>> +    return daddr;
>>> +}
>>> +#define dma_encrypted dma_encrypted
>>> +
>>
>> With PROT_NS_SHARED, it can become something like below. 
>> (PROT_NS_SHARED - 1)
>> is equivalent to -1UL, 'daddr & -1UL' should be fine since it does 
>> nothing.
>>
> 
> I meant (PROT_NS_SHARED - 1) is equivalent to -1UL when no realm capability
> is around :)

I didn't want this to be there ;-). But with Robin's comment, I think we
can revert back to PROT_NS_SHARED.

Cheers
Suzuki

> 
>>      return daddr & (PROT_NS_SHARED - 1);
>>
>>> +static inline dma_addr_t dma_clear_encryption(dma_addr_t daddr)
>>> +{
>>> +    return dma_encrypted(daddr);
>>> +}
>>> +#define dma_clear_encryption dma_clear_encryption
>>> +
>>>   #endif    /* __ASM_MEM_ENCRYPT_H */
> 
> Thanks,
> Gavin
> 

Re: [PATCH v2 3/3] arm64: realm: Use aliased addresses for device DMA to shared buffers

[Robin Murphy]

On 2025-02-19 10:07 pm, Suzuki K Poulose wrote:
> When a device performs DMA to a shared buffer using physical addresses,
> (without Stage1 translation), the device must use the "{I}PA address" with the
> top bit set in Realm. This is to make sure that a trusted device will be able
> to write to shared buffers as well as the protected buffers. Thus, a Realm must
> always program the full address including the "protection" bit, like AMD SME
> encryption bits.
> 
> Enable this by providing arm64 specific dma_{encrypted,decrypted,clear_encryption}
> helpers for Realms. Please note that the VMM needs to similarly make sure that
> the SMMU Stage2 in the Non-secure world is setup accordingly to map IPA at the
> unprotected alias.
> 
> 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>
> ---
>   arch/arm64/include/asm/mem_encrypt.h | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/mem_encrypt.h b/arch/arm64/include/asm/mem_encrypt.h
> index f8f78f622dd2..aeda3bba255e 100644
> --- a/arch/arm64/include/asm/mem_encrypt.h
> +++ b/arch/arm64/include/asm/mem_encrypt.h
> @@ -21,4 +21,26 @@ static inline bool force_dma_unencrypted(struct device *dev)
>   	return is_realm_world();
>   }
>   
> +static inline dma_addr_t dma_decrypted(dma_addr_t daddr)
> +{
> +	if (is_realm_world())
> +		daddr |= prot_ns_shared;
> +	return daddr;
> +}
> +#define dma_decrypted dma_decrypted
> +
> +static inline dma_addr_t dma_encrypted(dma_addr_t daddr)
> +{
> +	if (is_realm_world())
> +		daddr &= prot_ns_shared - 1;

Nit: is there a reason this isn't the direct inverse of the other 
operation, i.e. "daddr &= ~prot_ns_shared"? If so, it might be worth 
dropping a comment why we're doing slightly unintuitive arithmetic on a 
pagetable attribute (and if not then maybe just do the more obvious 
thing). I doubt anyone's in a rush to support TBI for DMA, and this 
would be far from the only potential hiccup for that, but still... :)

Thanks,
Robin.

> +	return daddr;
> +}
> +#define dma_encrypted dma_encrypted
> +
> +static inline dma_addr_t dma_clear_encryption(dma_addr_t daddr)
> +{
> +	return dma_encrypted(daddr);
> +}
> +#define dma_clear_encryption dma_clear_encryption
> +
>   #endif	/* __ASM_MEM_ENCRYPT_H */

Re: [PATCH v2 3/3] arm64: realm: Use aliased addresses for device DMA to shared buffers

[Suzuki K Poulose]

On 25/02/2025 13:04, Robin Murphy wrote:
> On 2025-02-19 10:07 pm, Suzuki K Poulose wrote:
>> When a device performs DMA to a shared buffer using physical addresses,
>> (without Stage1 translation), the device must use the "{I}PA address" 
>> with the
>> top bit set in Realm. This is to make sure that a trusted device will 
>> be able
>> to write to shared buffers as well as the protected buffers. Thus, a 
>> Realm must
>> always program the full address including the "protection" bit, like 
>> AMD SME
>> encryption bits.
>>
>> Enable this by providing arm64 specific 
>> dma_{encrypted,decrypted,clear_encryption}
>> helpers for Realms. Please note that the VMM needs to similarly make 
>> sure that
>> the SMMU Stage2 in the Non-secure world is setup accordingly to map 
>> IPA at the
>> unprotected alias.
>>
>> 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>
>> ---
>>   arch/arm64/include/asm/mem_encrypt.h | 22 ++++++++++++++++++++++
>>   1 file changed, 22 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/mem_encrypt.h b/arch/arm64/ 
>> include/asm/mem_encrypt.h
>> index f8f78f622dd2..aeda3bba255e 100644
>> --- a/arch/arm64/include/asm/mem_encrypt.h
>> +++ b/arch/arm64/include/asm/mem_encrypt.h
>> @@ -21,4 +21,26 @@ static inline bool force_dma_unencrypted(struct 
>> device *dev)
>>       return is_realm_world();
>>   }
>> +static inline dma_addr_t dma_decrypted(dma_addr_t daddr)
>> +{
>> +    if (is_realm_world())
>> +        daddr |= prot_ns_shared;
>> +    return daddr;
>> +}
>> +#define dma_decrypted dma_decrypted
>> +
>> +static inline dma_addr_t dma_encrypted(dma_addr_t daddr)
>> +{
>> +    if (is_realm_world())
>> +        daddr &= prot_ns_shared - 1;
> 
> Nit: is there a reason this isn't the direct inverse of the other 
> operation, i.e. "daddr &= ~prot_ns_shared"? If so, it might be worth 

It could be. The IPA size for the realm is split into half with the
lower half protected/encrypted and anything above that unprotected. 
Technically any addr >= prot_ns_shared is "unencrypted" (even though it
may be invalid, if >= BIT(IPA_Size) - 1), so to cover that, I masked
anything above the MS. But now when I think of it, it is much better to
trigger a Stage2 fault if the address is illegal (i.e., > BIT(IPA_Size) 
- 1) than corrupting some valid memory, by masking the top bits (beyond 
prot_ns_shared).

Cheers
Suzuki




> dropping a comment why we're doing slightly unintuitive arithmetic on a 
> pagetable attribute (and if not then maybe just do the more obvious 
> thing). I doubt anyone's in a rush to support TBI for DMA, and this 
> would be far from the only potential hiccup for that, but still... :)
> 
> Thanks,
> Robin.
> 
>> +    return daddr;
>> +}
>> +#define dma_encrypted dma_encrypted
>> +
>> +static inline dma_addr_t dma_clear_encryption(dma_addr_t daddr)
>> +{
>> +    return dma_encrypted(daddr);
>> +}
>> +#define dma_clear_encryption dma_clear_encryption
>> +
>>   #endif    /* __ASM_MEM_ENCRYPT_H */
> 

Re: [PATCH v2 3/3] arm64: realm: Use aliased addresses for device DMA to shared buffers

[Suzuki K Poulose]

On 25/02/2025 13:04, Robin Murphy wrote:
> On 2025-02-19 10:07 pm, Suzuki K Poulose wrote:
>> When a device performs DMA to a shared buffer using physical addresses,
>> (without Stage1 translation), the device must use the "{I}PA address" 
>> with the
>> top bit set in Realm. This is to make sure that a trusted device will 
>> be able
>> to write to shared buffers as well as the protected buffers. Thus, a 
>> Realm must
>> always program the full address including the "protection" bit, like 
>> AMD SME
>> encryption bits.
>>
>> Enable this by providing arm64 specific 
>> dma_{encrypted,decrypted,clear_encryption}
>> helpers for Realms. Please note that the VMM needs to similarly make 
>> sure that
>> the SMMU Stage2 in the Non-secure world is setup accordingly to map 
>> IPA at the
>> unprotected alias.
>>
>> 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>
>> ---
>>   arch/arm64/include/asm/mem_encrypt.h | 22 ++++++++++++++++++++++
>>   1 file changed, 22 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/mem_encrypt.h b/arch/arm64/ 
>> include/asm/mem_encrypt.h
>> index f8f78f622dd2..aeda3bba255e 100644
>> --- a/arch/arm64/include/asm/mem_encrypt.h
>> +++ b/arch/arm64/include/asm/mem_encrypt.h
>> @@ -21,4 +21,26 @@ static inline bool force_dma_unencrypted(struct 
>> device *dev)
>>       return is_realm_world();
>>   }
>> +static inline dma_addr_t dma_decrypted(dma_addr_t daddr)
>> +{
>> +    if (is_realm_world())
>> +        daddr |= prot_ns_shared;
>> +    return daddr;
>> +}
>> +#define dma_decrypted dma_decrypted
>> +
>> +static inline dma_addr_t dma_encrypted(dma_addr_t daddr)
>> +{
>> +    if (is_realm_world())
>> +        daddr &= prot_ns_shared - 1;
> 
> Nit: is there a reason this isn't the direct inverse of the other 
> operation, i.e. "daddr &= ~prot_ns_shared"? If so, it might be worth 

It could be. The IPA size for the realm is split into half with the
lower half protected/encrypted and anything above that unprotected. 
Technically any addr >= prot_ns_shared is "unencrypted" (even though it
may be invalid, if >= BIT(IPA_Size) - 1). But now when I think of it, it
is much better to trigger a Stage2 fault if the address is illegal 
(i.e., > BIT(IPA_Size) - 1) than corrupting some valid memory, by
masking the top bits (beyond prot_ns_shared).

So, I will fix it.

Suzuki


> dropping a comment why we're doing slightly unintuitive arithmetic on a 
> pagetable attribute (and if not then maybe just do the more obvious 
> thing). I doubt anyone's in a rush to support TBI for DMA, and this 
> would be far from the only potential hiccup for that, but still... :)
> 
> Thanks,
> Robin.
> 
>> +    return daddr;
>> +}
>> +#define dma_encrypted dma_encrypted
>> +
>> +static inline dma_addr_t dma_clear_encryption(dma_addr_t daddr)
>> +{
>> +    return dma_encrypted(daddr);
>> +}
>> +#define dma_clear_encryption dma_clear_encryption
>> +
>>   #endif    /* __ASM_MEM_ENCRYPT_H */
>