linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/1] arm64: realm: Fix DMA address for devices
@ 2025-02-12 17:14 Suzuki K Poulose
  2025-02-12 17:14 ` [PATCH 1/1] arm64: realm: Use aliased addresses for device DMA to shared buffers Suzuki K Poulose
  0 siblings, 1 reply; 9+ messages in thread
From: Suzuki K Poulose @ 2025-02-12 17:14 UTC (permalink / raw)
  To: will, maz, catalin.marinas
  Cc: linux-arm-kernel, linux-kernel, gregkh, aneesh.kumar,
	steven.price, suzuki.poulose, Jean-Philippe Brucker, Robin Murphy,
	Christoph Hellwig, Tom Lendacky

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.

Thus 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 as attribute).

This patch implements this in Linux by hooking into the phys_to_dma and vice versa
for providing the appropriate address. 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 is not affected. 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 stable kernel releases to make
sure that they are compliant.

[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 (1):
  arm64: realm: Use aliased addresses for device DMA to shared buffers

 arch/arm64/Kconfig                  |  1 +
 arch/arm64/include/asm/dma-direct.h | 38 +++++++++++++++++++++++++++++
 include/linux/dma-direct.h          | 35 +++++++++++++++++---------
 3 files changed, 62 insertions(+), 12 deletions(-)
 create mode 100644 arch/arm64/include/asm/dma-direct.h

-- 
2.43.0



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/1] arm64: realm: Use aliased addresses for device DMA to shared buffers
  2025-02-12 17:14 [PATCH 0/1] arm64: realm: Fix DMA address for devices Suzuki K Poulose
@ 2025-02-12 17:14 ` Suzuki K Poulose
  2025-02-12 18:18   ` Robin Murphy
  2025-02-15 14:22   ` Aneesh Kumar K.V
  0 siblings, 2 replies; 9+ messages in thread
From: Suzuki K Poulose @ 2025-02-12 17:14 UTC (permalink / raw)
  To: will, maz, catalin.marinas
  Cc: linux-arm-kernel, linux-kernel, gregkh, aneesh.kumar,
	steven.price, suzuki.poulose, Jean-Philippe Brucker, Robin Murphy,
	Christoph Hellwig, Tom Lendacky

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.

Add the support for this by providing arm64 version of the phys_to_dma(). We
cannot use the __sme_mask as it assumes the "encryption" always "sets a bit",
which is the opposite for CCA. i.e., "set a bit" for "decrypted" address. So,
move the common code that can be reused by all - i.e., add __phys_to_dma() and
__dma_to_phys() - and do the arch specific processing.

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>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 arch/arm64/Kconfig                  |  1 +
 arch/arm64/include/asm/dma-direct.h | 38 +++++++++++++++++++++++++++++
 include/linux/dma-direct.h          | 35 +++++++++++++++++---------
 3 files changed, 62 insertions(+), 12 deletions(-)
 create mode 100644 arch/arm64/include/asm/dma-direct.h

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index fcdd0ed3eca8..7befe04106de 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -41,6 +41,7 @@ config ARM64
 	select ARCH_HAS_NMI_SAFE_THIS_CPU_OPS
 	select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
 	select ARCH_HAS_NONLEAF_PMD_YOUNG if ARM64_HAFT
+	select ARCH_HAS_PHYS_TO_DMA
 	select ARCH_HAS_PTE_DEVMAP
 	select ARCH_HAS_PTE_SPECIAL
 	select ARCH_HAS_HW_PTE_YOUNG
diff --git a/arch/arm64/include/asm/dma-direct.h b/arch/arm64/include/asm/dma-direct.h
new file mode 100644
index 000000000000..37c3270542b8
--- /dev/null
+++ b/arch/arm64/include/asm/dma-direct.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_DMA_DIRECT_H
+#define __ASM_DMA_DIRECT_H
+
+#include <asm/pgtable-prot.h>
+
+static inline unsigned long addr_to_shared(unsigned long addr)
+{
+	if (is_realm_world())
+		addr |= prot_ns_shared;
+	return addr;
+}
+
+static inline unsigned long addr_to_private(unsigned long addr)
+{
+	if (is_realm_world())
+		addr &= prot_ns_shared - 1;
+	return addr;
+}
+
+static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
+{
+	return __phys_to_dma(dev, paddr);
+}
+
+static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev,
+						 phys_addr_t paddr)
+{
+	return addr_to_shared(__phys_to_dma(dev, paddr));
+}
+#define phys_to_dma_unencrypted phys_to_dma_unencrypted
+
+static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dma_addr)
+{
+	return addr_to_private(__dma_to_phys(dev, dma_addr));
+}
+
+#endif	/* __ASM_DMA_DIRECT_H */
diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index d7e30d4f7503..3e9bf6ca640e 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -72,18 +72,36 @@ static inline dma_addr_t dma_range_map_max(const struct bus_dma_region *map)
 	return ret;
 }
 
+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 phys_addr_t __dma_to_phys(struct device *dev, dma_addr_t dma_addr)
+{
+	phys_addr_t paddr;
+
+	if (dev->dma_range_map)
+		paddr = translate_dma_to_phys(dev, dma_addr);
+	else
+		paddr = dma_addr;
+
+	return paddr;
+}
+
 #ifdef CONFIG_ARCH_HAS_PHYS_TO_DMA
 #include <asm/dma-direct.h>
 #ifndef phys_to_dma_unencrypted
 #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)
 {
-	if (dev->dma_range_map)
-		return translate_phys_to_dma(dev, paddr);
-	return paddr;
+	return __phys_to_dma(dev, paddr);
 }
 
 /*
@@ -94,19 +112,12 @@ 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 __sme_set(__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;
-
-	if (dev->dma_range_map)
-		paddr = translate_dma_to_phys(dev, dma_addr);
-	else
-		paddr = dma_addr;
-
-	return __sme_clr(paddr);
+	return __sme_clr(__dma_to_phys(dev, dma_addr));
 }
 #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
 
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/1] arm64: realm: Use aliased addresses for device DMA to shared buffers
  2025-02-12 17:14 ` [PATCH 1/1] arm64: realm: Use aliased addresses for device DMA to shared buffers Suzuki K Poulose
@ 2025-02-12 18:18   ` Robin Murphy
  2025-02-12 18:48     ` Suzuki K Poulose
  2025-02-15 14:22   ` Aneesh Kumar K.V
  1 sibling, 1 reply; 9+ messages in thread
From: Robin Murphy @ 2025-02-12 18:18 UTC (permalink / raw)
  To: Suzuki K Poulose, will, maz, catalin.marinas
  Cc: linux-arm-kernel, linux-kernel, gregkh, aneesh.kumar,
	steven.price, Jean-Philippe Brucker, Christoph Hellwig,
	Tom Lendacky

On 2025-02-12 5:14 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.
> 
> Add the support for this by providing arm64 version of the phys_to_dma(). We
> cannot use the __sme_mask as it assumes the "encryption" always "sets a bit",
> which is the opposite for CCA. i.e., "set a bit" for "decrypted" address. So,
> move the common code that can be reused by all - i.e., add __phys_to_dma() and
> __dma_to_phys() - and do the arch specific processing.
> 
> 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>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>   arch/arm64/Kconfig                  |  1 +
>   arch/arm64/include/asm/dma-direct.h | 38 +++++++++++++++++++++++++++++
>   include/linux/dma-direct.h          | 35 +++++++++++++++++---------
>   3 files changed, 62 insertions(+), 12 deletions(-)
>   create mode 100644 arch/arm64/include/asm/dma-direct.h
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index fcdd0ed3eca8..7befe04106de 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -41,6 +41,7 @@ config ARM64
>   	select ARCH_HAS_NMI_SAFE_THIS_CPU_OPS
>   	select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
>   	select ARCH_HAS_NONLEAF_PMD_YOUNG if ARM64_HAFT
> +	select ARCH_HAS_PHYS_TO_DMA
>   	select ARCH_HAS_PTE_DEVMAP
>   	select ARCH_HAS_PTE_SPECIAL
>   	select ARCH_HAS_HW_PTE_YOUNG
> diff --git a/arch/arm64/include/asm/dma-direct.h b/arch/arm64/include/asm/dma-direct.h
> new file mode 100644
> index 000000000000..37c3270542b8
> --- /dev/null
> +++ b/arch/arm64/include/asm/dma-direct.h
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef __ASM_DMA_DIRECT_H
> +#define __ASM_DMA_DIRECT_H
> +
> +#include <asm/pgtable-prot.h>
> +
> +static inline unsigned long addr_to_shared(unsigned long addr)
> +{
> +	if (is_realm_world())
> +		addr |= prot_ns_shared;
> +	return addr;
> +}
> +
> +static inline unsigned long addr_to_private(unsigned long addr)
> +{
> +	if (is_realm_world())
> +		addr &= prot_ns_shared - 1;
> +	return addr;
> +}
> +
> +static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
> +{
> +	return __phys_to_dma(dev, paddr);
> +}
> +
> +static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev,
> +						 phys_addr_t paddr)
> +{
> +	return addr_to_shared(__phys_to_dma(dev, paddr));
> +}
> +#define phys_to_dma_unencrypted phys_to_dma_unencrypted
> +
> +static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dma_addr)
> +{
> +	return addr_to_private(__dma_to_phys(dev, dma_addr));
> +}
> +
> +#endif	/* __ASM_DMA_DIRECT_H */
> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
> index d7e30d4f7503..3e9bf6ca640e 100644
> --- a/include/linux/dma-direct.h
> +++ b/include/linux/dma-direct.h
> @@ -72,18 +72,36 @@ static inline dma_addr_t dma_range_map_max(const struct bus_dma_region *map)
>   	return ret;
>   }
>   
> +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 phys_addr_t __dma_to_phys(struct device *dev, dma_addr_t dma_addr)
> +{
> +	phys_addr_t paddr;
> +
> +	if (dev->dma_range_map)
> +		paddr = translate_dma_to_phys(dev, dma_addr);
> +	else
> +		paddr = dma_addr;
> +
> +	return paddr;
> +}
> +
>   #ifdef CONFIG_ARCH_HAS_PHYS_TO_DMA
>   #include <asm/dma-direct.h>
>   #ifndef phys_to_dma_unencrypted
>   #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)
>   {
> -	if (dev->dma_range_map)
> -		return translate_phys_to_dma(dev, paddr);
> -	return paddr;
> +	return __phys_to_dma(dev, paddr);
>   }
>   
>   /*
> @@ -94,19 +112,12 @@ 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 __sme_set(__phys_to_dma(dev, paddr));

Hmm, it really feels like we should generalise __sme_{set,clr} at this 
level, rather than drag in the entire ARCH_HAS_PHYS_TO_DMA override for 
the purposes of setting/clearing an address bit just because the 
"generic" mechanism for doing that is unashamedly AMD-specific.

Maybe something like:

#define dma_shared(x)  sme_clr(x)
#define dma_private(x) sme_set(x)

for x86, then go from there?

Thanks,
Robin.

>   }
>   
>   static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dma_addr)
>   {
> -	phys_addr_t paddr;
> -
> -	if (dev->dma_range_map)
> -		paddr = translate_dma_to_phys(dev, dma_addr);
> -	else
> -		paddr = dma_addr;
> -
> -	return __sme_clr(paddr);
> +	return __sme_clr(__dma_to_phys(dev, dma_addr));
>   }
>   #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
>   



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/1] arm64: realm: Use aliased addresses for device DMA to shared buffers
  2025-02-12 18:18   ` Robin Murphy
@ 2025-02-12 18:48     ` Suzuki K Poulose
  0 siblings, 0 replies; 9+ messages in thread
From: Suzuki K Poulose @ 2025-02-12 18:48 UTC (permalink / raw)
  To: Robin Murphy, will, maz, catalin.marinas
  Cc: linux-arm-kernel, linux-kernel, gregkh, aneesh.kumar,
	steven.price, Jean-Philippe Brucker, Christoph Hellwig,
	Tom Lendacky

Hi Robin

On 12/02/2025 18:18, Robin Murphy wrote:
> On 2025-02-12 5:14 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.
>>
>> Add the support for this by providing arm64 version of the 
>> phys_to_dma(). We
>> cannot use the __sme_mask as it assumes the "encryption" always "sets 
>> a bit",
>> which is the opposite for CCA. i.e., "set a bit" for "decrypted" 
>> address. So,
>> move the common code that can be reused by all - i.e., add 
>> __phys_to_dma() and
>> __dma_to_phys() - and do the arch specific processing.
>>
>> 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>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>   arch/arm64/Kconfig                  |  1 +
>>   arch/arm64/include/asm/dma-direct.h | 38 +++++++++++++++++++++++++++++
>>   include/linux/dma-direct.h          | 35 +++++++++++++++++---------
>>   3 files changed, 62 insertions(+), 12 deletions(-)
>>   create mode 100644 arch/arm64/include/asm/dma-direct.h
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index fcdd0ed3eca8..7befe04106de 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -41,6 +41,7 @@ config ARM64
>>       select ARCH_HAS_NMI_SAFE_THIS_CPU_OPS
>>       select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
>>       select ARCH_HAS_NONLEAF_PMD_YOUNG if ARM64_HAFT
>> +    select ARCH_HAS_PHYS_TO_DMA
>>       select ARCH_HAS_PTE_DEVMAP
>>       select ARCH_HAS_PTE_SPECIAL
>>       select ARCH_HAS_HW_PTE_YOUNG
>> diff --git a/arch/arm64/include/asm/dma-direct.h b/arch/arm64/include/ 
>> asm/dma-direct.h
>> new file mode 100644
>> index 000000000000..37c3270542b8
>> --- /dev/null
>> +++ b/arch/arm64/include/asm/dma-direct.h
>> @@ -0,0 +1,38 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +#ifndef __ASM_DMA_DIRECT_H
>> +#define __ASM_DMA_DIRECT_H
>> +
>> +#include <asm/pgtable-prot.h>
>> +
>> +static inline unsigned long addr_to_shared(unsigned long addr)
>> +{
>> +    if (is_realm_world())
>> +        addr |= prot_ns_shared;
>> +    return addr;
>> +}
>> +
>> +static inline unsigned long addr_to_private(unsigned long addr)
>> +{
>> +    if (is_realm_world())
>> +        addr &= prot_ns_shared - 1;
>> +    return addr;
>> +}
>> +
>> +static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t 
>> paddr)
>> +{
>> +    return __phys_to_dma(dev, paddr);
>> +}
>> +
>> +static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev,
>> +                         phys_addr_t paddr)
>> +{
>> +    return addr_to_shared(__phys_to_dma(dev, paddr));
>> +}
>> +#define phys_to_dma_unencrypted phys_to_dma_unencrypted
>> +
>> +static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t 
>> dma_addr)
>> +{
>> +    return addr_to_private(__dma_to_phys(dev, dma_addr));
>> +}
>> +
>> +#endif    /* __ASM_DMA_DIRECT_H */
>> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
>> index d7e30d4f7503..3e9bf6ca640e 100644
>> --- a/include/linux/dma-direct.h
>> +++ b/include/linux/dma-direct.h
>> @@ -72,18 +72,36 @@ static inline dma_addr_t dma_range_map_max(const 
>> struct bus_dma_region *map)
>>       return ret;
>>   }
>> +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 phys_addr_t __dma_to_phys(struct device *dev, 
>> dma_addr_t dma_addr)
>> +{
>> +    phys_addr_t paddr;
>> +
>> +    if (dev->dma_range_map)
>> +        paddr = translate_dma_to_phys(dev, dma_addr);
>> +    else
>> +        paddr = dma_addr;
>> +
>> +    return paddr;
>> +}
>> +
>>   #ifdef CONFIG_ARCH_HAS_PHYS_TO_DMA
>>   #include <asm/dma-direct.h>
>>   #ifndef phys_to_dma_unencrypted
>>   #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)
>>   {
>> -    if (dev->dma_range_map)
>> -        return translate_phys_to_dma(dev, paddr);
>> -    return paddr;
>> +    return __phys_to_dma(dev, paddr);
>>   }
>>   /*
>> @@ -94,19 +112,12 @@ 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 __sme_set(__phys_to_dma(dev, paddr));
> 
> Hmm, it really feels like we should generalise __sme_{set,clr} at this 
> level, rather than drag in the entire ARCH_HAS_PHYS_TO_DMA override for 
> the purposes of setting/clearing an address bit just because the 
> "generic" mechanism for doing that is unashamedly AMD-specific.

I was planning to convert that to use ARCH_HAS_PHYS_TO_DMA ;-) too, in a
separate patch, to get rid of this "one approach" works for all hook.

> 
> Maybe something like:
> 
> #define dma_shared(x)  sme_clr(x)
> #define dma_private(x) sme_set(x)
> 
> for x86, then go from there?

But this looks sensible too, can switch to this approach.
Thanks for the feedback.

Suzuki


> 
> Thanks,
> Robin.
> 
>>   }
>>   static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t 
>> dma_addr)
>>   {
>> -    phys_addr_t paddr;
>> -
>> -    if (dev->dma_range_map)
>> -        paddr = translate_dma_to_phys(dev, dma_addr);
>> -    else
>> -        paddr = dma_addr;
>> -
>> -    return __sme_clr(paddr);
>> +    return __sme_clr(__dma_to_phys(dev, dma_addr));
>>   }
>>   #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
> 



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/1] arm64: realm: Use aliased addresses for device DMA to shared buffers
  2025-02-12 17:14 ` [PATCH 1/1] arm64: realm: Use aliased addresses for device DMA to shared buffers Suzuki K Poulose
  2025-02-12 18:18   ` Robin Murphy
@ 2025-02-15 14:22   ` Aneesh Kumar K.V
  2025-02-15 14:38     ` Aneesh Kumar K.V
  1 sibling, 1 reply; 9+ messages in thread
From: Aneesh Kumar K.V @ 2025-02-15 14:22 UTC (permalink / raw)
  To: Suzuki K Poulose, will, maz, catalin.marinas
  Cc: linux-arm-kernel, linux-kernel, gregkh, steven.price,
	suzuki.poulose, Jean-Philippe Brucker, Robin Murphy,
	Christoph Hellwig, Tom Lendacky

Suzuki K Poulose <suzuki.poulose@arm.com> writes:

> 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.
>
> Add the support for this by providing arm64 version of the phys_to_dma(). We
> cannot use the __sme_mask as it assumes the "encryption" always "sets a bit",
> which is the opposite for CCA. i.e., "set a bit" for "decrypted" address. So,
> move the common code that can be reused by all - i.e., add __phys_to_dma() and
> __dma_to_phys() - and do the arch specific processing.
>
> 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>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  arch/arm64/Kconfig                  |  1 +
>  arch/arm64/include/asm/dma-direct.h | 38 +++++++++++++++++++++++++++++
>  include/linux/dma-direct.h          | 35 +++++++++++++++++---------
>  3 files changed, 62 insertions(+), 12 deletions(-)
>  create mode 100644 arch/arm64/include/asm/dma-direct.h
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index fcdd0ed3eca8..7befe04106de 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -41,6 +41,7 @@ config ARM64
>  	select ARCH_HAS_NMI_SAFE_THIS_CPU_OPS
>  	select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
>  	select ARCH_HAS_NONLEAF_PMD_YOUNG if ARM64_HAFT
> +	select ARCH_HAS_PHYS_TO_DMA
>  	select ARCH_HAS_PTE_DEVMAP
>  	select ARCH_HAS_PTE_SPECIAL
>  	select ARCH_HAS_HW_PTE_YOUNG
> diff --git a/arch/arm64/include/asm/dma-direct.h b/arch/arm64/include/asm/dma-direct.h
> new file mode 100644
> index 000000000000..37c3270542b8
> --- /dev/null
> +++ b/arch/arm64/include/asm/dma-direct.h
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef __ASM_DMA_DIRECT_H
> +#define __ASM_DMA_DIRECT_H
> +
> +#include <asm/pgtable-prot.h>
> +
> +static inline unsigned long addr_to_shared(unsigned long addr)
> +{
> +	if (is_realm_world())
> +		addr |= prot_ns_shared;
> +	return addr;
> +}
> +
> +static inline unsigned long addr_to_private(unsigned long addr)
> +{
> +	if (is_realm_world())
> +		addr &= prot_ns_shared - 1;
> +	return addr;
> +}
> +
> +static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
> +{
> +	return __phys_to_dma(dev, paddr);
> +}
> +
> +static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev,
> +						 phys_addr_t paddr)
> +{
> +	return addr_to_shared(__phys_to_dma(dev, paddr));
> +}
> +#define phys_to_dma_unencrypted phys_to_dma_unencrypted
> +
> +static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dma_addr)
> +{
> +	return addr_to_private(__dma_to_phys(dev, dma_addr));
> +}
> +
> +#endif	/* __ASM_DMA_DIRECT_H */
> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
> index d7e30d4f7503..3e9bf6ca640e 100644
> --- a/include/linux/dma-direct.h
> +++ b/include/linux/dma-direct.h
> @@ -72,18 +72,36 @@ static inline dma_addr_t dma_range_map_max(const struct bus_dma_region *map)
>  	return ret;
>  }
>  
> +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 phys_addr_t __dma_to_phys(struct device *dev, dma_addr_t dma_addr)
> +{
> +	phys_addr_t paddr;
> +
> +	if (dev->dma_range_map)
> +		paddr = translate_dma_to_phys(dev, dma_addr);
> +	else
> +		paddr = dma_addr;
> +
> +	return paddr;
> +}
> +
>  #ifdef CONFIG_ARCH_HAS_PHYS_TO_DMA
>  #include <asm/dma-direct.h>
>  #ifndef phys_to_dma_unencrypted
>  #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)
>  {
> -	if (dev->dma_range_map)
> -		return translate_phys_to_dma(dev, paddr);
> -	return paddr;
> +	return __phys_to_dma(dev, paddr);
>  }
>  
>  /*
> @@ -94,19 +112,12 @@ 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 __sme_set(__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;
> -
> -	if (dev->dma_range_map)
> -		paddr = translate_dma_to_phys(dev, dma_addr);
> -	else
> -		paddr = dma_addr;
> -
> -	return __sme_clr(paddr);
> +	return __sme_clr(__dma_to_phys(dev, dma_addr));
>  }
>  #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
>  
> -- 
> 2.43.0

How about the below?

The function name addr_to_shared is too generic to be included in the
dma-direct.h header file. Since we don’t expect it to be called
directly, we can either inline it or find a more specific name.

Additionally, for dma_to_phys conversion, we first retrieve the private
address/alias before switching to the physical address. While both
approaches yield the correct result, this change more clearly defines the
conversion rules?

modified   arch/arm64/include/asm/dma-direct.h
@@ -4,14 +4,14 @@
 
 #include <asm/pgtable-prot.h>
 
-static inline unsigned long addr_to_shared(unsigned long addr)
+static inline unsigned long shared_dma_addr(unsigned long addr)
 {
 	if (is_realm_world())
 		addr |= prot_ns_shared;
 	return addr;
 }
 
-static inline unsigned long addr_to_private(unsigned long addr)
+static inline unsigned long private_dma_addr(unsigned long addr)
 {
 	if (is_realm_world())
 		addr &= prot_ns_shared - 1;
@@ -26,13 +26,14 @@ static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
 static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev,
 						 phys_addr_t paddr)
 {
-	return addr_to_shared(__phys_to_dma(dev, paddr));
+	return shared_dma_addr(__phys_to_dma(dev, paddr));
 }
 #define phys_to_dma_unencrypted phys_to_dma_unencrypted
 
 static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dma_addr)
 {
-	return addr_to_private(__dma_to_phys(dev, dma_addr));
+	/* if it is a shared dma addr convert to private before looking for phys_addr */
+	return __dma_to_phys(dev, private_dma_addr(dma_addr));
 }
 
 #endif	/* __ASM_DMA_DIRECT_H */


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/1] arm64: realm: Use aliased addresses for device DMA to shared buffers
  2025-02-15 14:22   ` Aneesh Kumar K.V
@ 2025-02-15 14:38     ` Aneesh Kumar K.V
  2025-02-19 14:51       ` Suzuki K Poulose
  0 siblings, 1 reply; 9+ messages in thread
From: Aneesh Kumar K.V @ 2025-02-15 14:38 UTC (permalink / raw)
  To: Suzuki K Poulose, will, maz, catalin.marinas
  Cc: linux-arm-kernel, linux-kernel, gregkh, steven.price,
	suzuki.poulose, Jean-Philippe Brucker, Robin Murphy,
	Christoph Hellwig, Tom Lendacky

Aneesh Kumar K.V <aneesh.kumar@kernel.org> writes:

> Suzuki K Poulose <suzuki.poulose@arm.com> writes:
>
>> 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.
>>
>> Add the support for this by providing arm64 version of the phys_to_dma(). We
>> cannot use the __sme_mask as it assumes the "encryption" always "sets a bit",
>> which is the opposite for CCA. i.e., "set a bit" for "decrypted" address. So,
>> move the common code that can be reused by all - i.e., add __phys_to_dma() and
>> __dma_to_phys() - and do the arch specific processing.
>>
>> 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>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>  arch/arm64/Kconfig                  |  1 +
>>  arch/arm64/include/asm/dma-direct.h | 38 +++++++++++++++++++++++++++++
>>  include/linux/dma-direct.h          | 35 +++++++++++++++++---------
>>  3 files changed, 62 insertions(+), 12 deletions(-)
>>  create mode 100644 arch/arm64/include/asm/dma-direct.h
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index fcdd0ed3eca8..7befe04106de 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -41,6 +41,7 @@ config ARM64
>>  	select ARCH_HAS_NMI_SAFE_THIS_CPU_OPS
>>  	select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
>>  	select ARCH_HAS_NONLEAF_PMD_YOUNG if ARM64_HAFT
>> +	select ARCH_HAS_PHYS_TO_DMA
>>  	select ARCH_HAS_PTE_DEVMAP
>>  	select ARCH_HAS_PTE_SPECIAL
>>  	select ARCH_HAS_HW_PTE_YOUNG
>> diff --git a/arch/arm64/include/asm/dma-direct.h b/arch/arm64/include/asm/dma-direct.h
>> new file mode 100644
>> index 000000000000..37c3270542b8
>> --- /dev/null
>> +++ b/arch/arm64/include/asm/dma-direct.h
>> @@ -0,0 +1,38 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +#ifndef __ASM_DMA_DIRECT_H
>> +#define __ASM_DMA_DIRECT_H
>> +
>> +#include <asm/pgtable-prot.h>
>> +
>> +static inline unsigned long addr_to_shared(unsigned long addr)
>> +{
>> +	if (is_realm_world())
>> +		addr |= prot_ns_shared;
>> +	return addr;
>> +}
>> +
>> +static inline unsigned long addr_to_private(unsigned long addr)
>> +{
>> +	if (is_realm_world())
>> +		addr &= prot_ns_shared - 1;
>> +	return addr;
>> +}
>> +
>> +static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
>> +{
>> +	return __phys_to_dma(dev, paddr);
>> +}
>> +
>> +static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev,
>> +						 phys_addr_t paddr)
>> +{
>> +	return addr_to_shared(__phys_to_dma(dev, paddr));
>> +}
>> +#define phys_to_dma_unencrypted phys_to_dma_unencrypted
>> +
>> +static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dma_addr)
>> +{
>> +	return addr_to_private(__dma_to_phys(dev, dma_addr));
>> +}
>> +
>> +#endif	/* __ASM_DMA_DIRECT_H */
>> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
>> index d7e30d4f7503..3e9bf6ca640e 100644
>> --- a/include/linux/dma-direct.h
>> +++ b/include/linux/dma-direct.h
>> @@ -72,18 +72,36 @@ static inline dma_addr_t dma_range_map_max(const struct bus_dma_region *map)
>>  	return ret;
>>  }
>>  
>> +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 phys_addr_t __dma_to_phys(struct device *dev, dma_addr_t dma_addr)
>> +{
>> +	phys_addr_t paddr;
>> +
>> +	if (dev->dma_range_map)
>> +		paddr = translate_dma_to_phys(dev, dma_addr);
>> +	else
>> +		paddr = dma_addr;
>> +
>> +	return paddr;
>> +}
>> +
>>  #ifdef CONFIG_ARCH_HAS_PHYS_TO_DMA
>>  #include <asm/dma-direct.h>
>>  #ifndef phys_to_dma_unencrypted
>>  #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)
>>  {
>> -	if (dev->dma_range_map)
>> -		return translate_phys_to_dma(dev, paddr);
>> -	return paddr;
>> +	return __phys_to_dma(dev, paddr);
>>  }
>>  
>>  /*
>> @@ -94,19 +112,12 @@ 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 __sme_set(__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;
>> -
>> -	if (dev->dma_range_map)
>> -		paddr = translate_dma_to_phys(dev, dma_addr);
>> -	else
>> -		paddr = dma_addr;
>> -
>> -	return __sme_clr(paddr);
>> +	return __sme_clr(__dma_to_phys(dev, dma_addr));
>>  }
>>  #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
>>  
>> -- 
>> 2.43.0
>
> How about the below?
>
> The function name addr_to_shared is too generic to be included in the
> dma-direct.h header file. Since we don’t expect it to be called
> directly, we can either inline it or find a more specific name.
>
> Additionally, for dma_to_phys conversion, we first retrieve the private
> address/alias before switching to the physical address. While both
> approaches yield the correct result, this change more clearly defines the
> conversion rules?
>

Also translate_dma_to_phys/translate_phys_to_dma work with private dma
addr/alias right?

>
> modified   arch/arm64/include/asm/dma-direct.h
> @@ -4,14 +4,14 @@
>  
>  #include <asm/pgtable-prot.h>
>  
> -static inline unsigned long addr_to_shared(unsigned long addr)
> +static inline unsigned long shared_dma_addr(unsigned long addr)
>  {
>  	if (is_realm_world())
>  		addr |= prot_ns_shared;
>  	return addr;
>  }
>  
> -static inline unsigned long addr_to_private(unsigned long addr)
> +static inline unsigned long private_dma_addr(unsigned long addr)
>  {
>  	if (is_realm_world())
>  		addr &= prot_ns_shared - 1;
> @@ -26,13 +26,14 @@ static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
>  static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev,
>  						 phys_addr_t paddr)
>  {
> -	return addr_to_shared(__phys_to_dma(dev, paddr));
> +	return shared_dma_addr(__phys_to_dma(dev, paddr));
>  }
>  #define phys_to_dma_unencrypted phys_to_dma_unencrypted
>  
>  static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dma_addr)
>  {
> -	return addr_to_private(__dma_to_phys(dev, dma_addr));
> +	/* if it is a shared dma addr convert to private before looking for phys_addr */
> +	return __dma_to_phys(dev, private_dma_addr(dma_addr));
>  }
>  
>  #endif	/* __ASM_DMA_DIRECT_H */


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/1] arm64: realm: Use aliased addresses for device DMA to shared buffers
  2025-02-15 14:38     ` Aneesh Kumar K.V
@ 2025-02-19 14:51       ` Suzuki K Poulose
  2025-02-19 15:12         ` Tom Lendacky
  0 siblings, 1 reply; 9+ messages in thread
From: Suzuki K Poulose @ 2025-02-19 14:51 UTC (permalink / raw)
  To: Aneesh Kumar K.V, will, maz, catalin.marinas
  Cc: linux-arm-kernel, linux-kernel, gregkh, steven.price,
	Jean-Philippe Brucker, Robin Murphy, Christoph Hellwig,
	Tom Lendacky

On 15/02/2025 14:38, Aneesh Kumar K.V wrote:
> Aneesh Kumar K.V <aneesh.kumar@kernel.org> writes:
> 
>> Suzuki K Poulose <suzuki.poulose@arm.com> writes:
>>
>>> 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.
>>>
>>> Add the support for this by providing arm64 version of the phys_to_dma(). We
>>> cannot use the __sme_mask as it assumes the "encryption" always "sets a bit",
>>> which is the opposite for CCA. i.e., "set a bit" for "decrypted" address. So,
>>> move the common code that can be reused by all - i.e., add __phys_to_dma() and
>>> __dma_to_phys() - and do the arch specific processing.
>>>
>>> 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>
>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> ---
>>>   arch/arm64/Kconfig                  |  1 +
>>>   arch/arm64/include/asm/dma-direct.h | 38 +++++++++++++++++++++++++++++
>>>   include/linux/dma-direct.h          | 35 +++++++++++++++++---------
>>>   3 files changed, 62 insertions(+), 12 deletions(-)
>>>   create mode 100644 arch/arm64/include/asm/dma-direct.h
>>>
>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>> index fcdd0ed3eca8..7befe04106de 100644
>>> --- a/arch/arm64/Kconfig
>>> +++ b/arch/arm64/Kconfig
>>> @@ -41,6 +41,7 @@ config ARM64
>>>   	select ARCH_HAS_NMI_SAFE_THIS_CPU_OPS
>>>   	select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
>>>   	select ARCH_HAS_NONLEAF_PMD_YOUNG if ARM64_HAFT
>>> +	select ARCH_HAS_PHYS_TO_DMA
>>>   	select ARCH_HAS_PTE_DEVMAP
>>>   	select ARCH_HAS_PTE_SPECIAL
>>>   	select ARCH_HAS_HW_PTE_YOUNG
>>> diff --git a/arch/arm64/include/asm/dma-direct.h b/arch/arm64/include/asm/dma-direct.h
>>> new file mode 100644
>>> index 000000000000..37c3270542b8
>>> --- /dev/null
>>> +++ b/arch/arm64/include/asm/dma-direct.h
>>> @@ -0,0 +1,38 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +#ifndef __ASM_DMA_DIRECT_H
>>> +#define __ASM_DMA_DIRECT_H
>>> +
>>> +#include <asm/pgtable-prot.h>
>>> +
>>> +static inline unsigned long addr_to_shared(unsigned long addr)
>>> +{
>>> +	if (is_realm_world())
>>> +		addr |= prot_ns_shared;
>>> +	return addr;
>>> +}
>>> +
>>> +static inline unsigned long addr_to_private(unsigned long addr)
>>> +{
>>> +	if (is_realm_world())
>>> +		addr &= prot_ns_shared - 1;
>>> +	return addr;
>>> +}
>>> +
>>> +static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
>>> +{
>>> +	return __phys_to_dma(dev, paddr);
>>> +}
>>> +
>>> +static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev,
>>> +						 phys_addr_t paddr)
>>> +{
>>> +	return addr_to_shared(__phys_to_dma(dev, paddr));
>>> +}
>>> +#define phys_to_dma_unencrypted phys_to_dma_unencrypted
>>> +
>>> +static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dma_addr)
>>> +{
>>> +	return addr_to_private(__dma_to_phys(dev, dma_addr));
>>> +}
>>> +
>>> +#endif	/* __ASM_DMA_DIRECT_H */
>>> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
>>> index d7e30d4f7503..3e9bf6ca640e 100644
>>> --- a/include/linux/dma-direct.h
>>> +++ b/include/linux/dma-direct.h
>>> @@ -72,18 +72,36 @@ static inline dma_addr_t dma_range_map_max(const struct bus_dma_region *map)
>>>   	return ret;
>>>   }
>>>   
>>> +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 phys_addr_t __dma_to_phys(struct device *dev, dma_addr_t dma_addr)
>>> +{
>>> +	phys_addr_t paddr;
>>> +
>>> +	if (dev->dma_range_map)
>>> +		paddr = translate_dma_to_phys(dev, dma_addr);
>>> +	else
>>> +		paddr = dma_addr;
>>> +
>>> +	return paddr;
>>> +}
>>> +
>>>   #ifdef CONFIG_ARCH_HAS_PHYS_TO_DMA
>>>   #include <asm/dma-direct.h>
>>>   #ifndef phys_to_dma_unencrypted
>>>   #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)
>>>   {
>>> -	if (dev->dma_range_map)
>>> -		return translate_phys_to_dma(dev, paddr);
>>> -	return paddr;
>>> +	return __phys_to_dma(dev, paddr);
>>>   }
>>>   
>>>   /*
>>> @@ -94,19 +112,12 @@ 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 __sme_set(__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;
>>> -
>>> -	if (dev->dma_range_map)
>>> -		paddr = translate_dma_to_phys(dev, dma_addr);
>>> -	else
>>> -		paddr = dma_addr;
>>> -
>>> -	return __sme_clr(paddr);
>>> +	return __sme_clr(__dma_to_phys(dev, dma_addr));
>>>   }
>>>   #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
>>>   
>>> -- 
>>> 2.43.0
>>
>> How about the below?
>>
>> The function name addr_to_shared is too generic to be included in the
>> dma-direct.h header file. Since we don’t expect it to be called
>> directly, we can either inline it or find a more specific name.
>>
>> Additionally, for dma_to_phys conversion, we first retrieve the private
>> address/alias before switching to the physical address. While both
>> approaches yield the correct result, this change more clearly defines the
>> conversion rules?
>>
> 
> Also translate_dma_to_phys/translate_phys_to_dma work with private dma
> addr/alias right?
> 

You're right, that needs to be fixed. Luckily (hopefully) DMA ranges
are not used by AMD SME.

Suzuki


>>
>> modified   arch/arm64/include/asm/dma-direct.h
>> @@ -4,14 +4,14 @@
>>   
>>   #include <asm/pgtable-prot.h>
>>   
>> -static inline unsigned long addr_to_shared(unsigned long addr)
>> +static inline unsigned long shared_dma_addr(unsigned long addr)
>>   {
>>   	if (is_realm_world())
>>   		addr |= prot_ns_shared;
>>   	return addr;
>>   }
>>   
>> -static inline unsigned long addr_to_private(unsigned long addr)
>> +static inline unsigned long private_dma_addr(unsigned long addr)
>>   {
>>   	if (is_realm_world())
>>   		addr &= prot_ns_shared - 1;
>> @@ -26,13 +26,14 @@ static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
>>   static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev,
>>   						 phys_addr_t paddr)
>>   {
>> -	return addr_to_shared(__phys_to_dma(dev, paddr));
>> +	return shared_dma_addr(__phys_to_dma(dev, paddr));
>>   }
>>   #define phys_to_dma_unencrypted phys_to_dma_unencrypted
>>   
>>   static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dma_addr)
>>   {
>> -	return addr_to_private(__dma_to_phys(dev, dma_addr));
>> +	/* if it is a shared dma addr convert to private before looking for phys_addr */
>> +	return __dma_to_phys(dev, private_dma_addr(dma_addr));
>>   }
>>   
>>   #endif	/* __ASM_DMA_DIRECT_H */



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/1] arm64: realm: Use aliased addresses for device DMA to shared buffers
  2025-02-19 14:51       ` Suzuki K Poulose
@ 2025-02-19 15:12         ` Tom Lendacky
  2025-02-19 15:50           ` Suzuki K Poulose
  0 siblings, 1 reply; 9+ messages in thread
From: Tom Lendacky @ 2025-02-19 15:12 UTC (permalink / raw)
  To: Suzuki K Poulose, Aneesh Kumar K.V, will, maz, catalin.marinas
  Cc: linux-arm-kernel, linux-kernel, gregkh, steven.price,
	Jean-Philippe Brucker, Robin Murphy, Christoph Hellwig

On 2/19/25 08:51, Suzuki K Poulose wrote:
> On 15/02/2025 14:38, Aneesh Kumar K.V wrote:
>> Aneesh Kumar K.V <aneesh.kumar@kernel.org> writes:
>>
>>> Suzuki K Poulose <suzuki.poulose@arm.com> writes:
>>>
>>>> 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.
>>>>
>>>> Add the support for this by providing arm64 version of the
>>>> phys_to_dma(). We
>>>> cannot use the __sme_mask as it assumes the "encryption" always "sets
>>>> a bit",
>>>> which is the opposite for CCA. i.e., "set a bit" for "decrypted"
>>>> address. So,
>>>> move the common code that can be reused by all - i.e., add
>>>> __phys_to_dma() and
>>>> __dma_to_phys() - and do the arch specific processing.
>>>>
>>>> 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>
>>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>>> ---
>>>>   arch/arm64/Kconfig                  |  1 +
>>>>   arch/arm64/include/asm/dma-direct.h | 38 +++++++++++++++++++++++++++++
>>>>   include/linux/dma-direct.h          | 35 +++++++++++++++++---------
>>>>   3 files changed, 62 insertions(+), 12 deletions(-)
>>>>   create mode 100644 arch/arm64/include/asm/dma-direct.h
>>>>
>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>>> index fcdd0ed3eca8..7befe04106de 100644
>>>> --- a/arch/arm64/Kconfig
>>>> +++ b/arch/arm64/Kconfig
>>>> @@ -41,6 +41,7 @@ config ARM64
>>>>       select ARCH_HAS_NMI_SAFE_THIS_CPU_OPS
>>>>       select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
>>>>       select ARCH_HAS_NONLEAF_PMD_YOUNG if ARM64_HAFT
>>>> +    select ARCH_HAS_PHYS_TO_DMA
>>>>       select ARCH_HAS_PTE_DEVMAP
>>>>       select ARCH_HAS_PTE_SPECIAL
>>>>       select ARCH_HAS_HW_PTE_YOUNG
>>>> diff --git a/arch/arm64/include/asm/dma-direct.h
>>>> b/arch/arm64/include/asm/dma-direct.h
>>>> new file mode 100644
>>>> index 000000000000..37c3270542b8
>>>> --- /dev/null
>>>> +++ b/arch/arm64/include/asm/dma-direct.h
>>>> @@ -0,0 +1,38 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>>> +#ifndef __ASM_DMA_DIRECT_H
>>>> +#define __ASM_DMA_DIRECT_H
>>>> +
>>>> +#include <asm/pgtable-prot.h>
>>>> +
>>>> +static inline unsigned long addr_to_shared(unsigned long addr)
>>>> +{
>>>> +    if (is_realm_world())
>>>> +        addr |= prot_ns_shared;
>>>> +    return addr;
>>>> +}
>>>> +
>>>> +static inline unsigned long addr_to_private(unsigned long addr)
>>>> +{
>>>> +    if (is_realm_world())
>>>> +        addr &= prot_ns_shared - 1;
>>>> +    return addr;
>>>> +}
>>>> +
>>>> +static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t
>>>> paddr)
>>>> +{
>>>> +    return __phys_to_dma(dev, paddr);
>>>> +}
>>>> +
>>>> +static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev,
>>>> +                         phys_addr_t paddr)
>>>> +{
>>>> +    return addr_to_shared(__phys_to_dma(dev, paddr));
>>>> +}
>>>> +#define phys_to_dma_unencrypted phys_to_dma_unencrypted
>>>> +
>>>> +static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t
>>>> dma_addr)
>>>> +{
>>>> +    return addr_to_private(__dma_to_phys(dev, dma_addr));
>>>> +}
>>>> +
>>>> +#endif    /* __ASM_DMA_DIRECT_H */
>>>> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
>>>> index d7e30d4f7503..3e9bf6ca640e 100644
>>>> --- a/include/linux/dma-direct.h
>>>> +++ b/include/linux/dma-direct.h
>>>> @@ -72,18 +72,36 @@ static inline dma_addr_t dma_range_map_max(const
>>>> struct bus_dma_region *map)
>>>>       return ret;
>>>>   }
>>>>   +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 phys_addr_t __dma_to_phys(struct device *dev,
>>>> dma_addr_t dma_addr)
>>>> +{
>>>> +    phys_addr_t paddr;
>>>> +
>>>> +    if (dev->dma_range_map)
>>>> +        paddr = translate_dma_to_phys(dev, dma_addr);
>>>> +    else
>>>> +        paddr = dma_addr;
>>>> +
>>>> +    return paddr;
>>>> +}
>>>> +
>>>>   #ifdef CONFIG_ARCH_HAS_PHYS_TO_DMA
>>>>   #include <asm/dma-direct.h>
>>>>   #ifndef phys_to_dma_unencrypted
>>>>   #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)
>>>>   {
>>>> -    if (dev->dma_range_map)
>>>> -        return translate_phys_to_dma(dev, paddr);
>>>> -    return paddr;
>>>> +    return __phys_to_dma(dev, paddr);
>>>>   }
>>>>     /*
>>>> @@ -94,19 +112,12 @@ 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 __sme_set(__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;
>>>> -
>>>> -    if (dev->dma_range_map)
>>>> -        paddr = translate_dma_to_phys(dev, dma_addr);
>>>> -    else
>>>> -        paddr = dma_addr;
>>>> -
>>>> -    return __sme_clr(paddr);
>>>> +    return __sme_clr(__dma_to_phys(dev, dma_addr));
>>>>   }
>>>>   #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
>>>>   -- 
>>>> 2.43.0
>>>
>>> How about the below?
>>>
>>> The function name addr_to_shared is too generic to be included in the
>>> dma-direct.h header file. Since we don’t expect it to be called
>>> directly, we can either inline it or find a more specific name.
>>>
>>> Additionally, for dma_to_phys conversion, we first retrieve the private
>>> address/alias before switching to the physical address. While both
>>> approaches yield the correct result, this change more clearly defines the
>>> conversion rules?
>>>
>>
>> Also translate_dma_to_phys/translate_phys_to_dma work with private dma
>> addr/alias right?
>>
> 
> You're right, that needs to be fixed. Luckily (hopefully) DMA ranges
> are not used by AMD SME.

I'm not sure what you mean by this, but DMA to an encrypted addresses is
allowed under SME and eventually will be allowed with TDISP support in an
SEV guest. So the DMA address (assuming no IOMMU) could have the
encryption bit set.

Thanks,
Tom

> 
> Suzuki
> 
> 
>>>
>>> modified   arch/arm64/include/asm/dma-direct.h
>>> @@ -4,14 +4,14 @@
>>>     #include <asm/pgtable-prot.h>
>>>   -static inline unsigned long addr_to_shared(unsigned long addr)
>>> +static inline unsigned long shared_dma_addr(unsigned long addr)
>>>   {
>>>       if (is_realm_world())
>>>           addr |= prot_ns_shared;
>>>       return addr;
>>>   }
>>>   -static inline unsigned long addr_to_private(unsigned long addr)
>>> +static inline unsigned long private_dma_addr(unsigned long addr)
>>>   {
>>>       if (is_realm_world())
>>>           addr &= prot_ns_shared - 1;
>>> @@ -26,13 +26,14 @@ static inline dma_addr_t phys_to_dma(struct device
>>> *dev, phys_addr_t paddr)
>>>   static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev,
>>>                            phys_addr_t paddr)
>>>   {
>>> -    return addr_to_shared(__phys_to_dma(dev, paddr));
>>> +    return shared_dma_addr(__phys_to_dma(dev, paddr));
>>>   }
>>>   #define phys_to_dma_unencrypted phys_to_dma_unencrypted
>>>     static inline phys_addr_t dma_to_phys(struct device *dev,
>>> dma_addr_t dma_addr)
>>>   {
>>> -    return addr_to_private(__dma_to_phys(dev, dma_addr));
>>> +    /* if it is a shared dma addr convert to private before looking
>>> for phys_addr */
>>> +    return __dma_to_phys(dev, private_dma_addr(dma_addr));
>>>   }
>>>     #endif    /* __ASM_DMA_DIRECT_H */
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/1] arm64: realm: Use aliased addresses for device DMA to shared buffers
  2025-02-19 15:12         ` Tom Lendacky
@ 2025-02-19 15:50           ` Suzuki K Poulose
  0 siblings, 0 replies; 9+ messages in thread
From: Suzuki K Poulose @ 2025-02-19 15:50 UTC (permalink / raw)
  To: Tom Lendacky, Aneesh Kumar K.V, will, maz, catalin.marinas
  Cc: linux-arm-kernel, linux-kernel, gregkh, steven.price,
	Jean-Philippe Brucker, Robin Murphy, Christoph Hellwig

On 19/02/2025 15:12, Tom Lendacky wrote:
> On 2/19/25 08:51, Suzuki K Poulose wrote:
>> On 15/02/2025 14:38, Aneesh Kumar K.V wrote:
>>> Aneesh Kumar K.V <aneesh.kumar@kernel.org> writes:
>>>
>>>> Suzuki K Poulose <suzuki.poulose@arm.com> writes:
>>>>
>>>>> 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.
>>>>>
>>>>> Add the support for this by providing arm64 version of the
>>>>> phys_to_dma(). We
>>>>> cannot use the __sme_mask as it assumes the "encryption" always "sets
>>>>> a bit",
>>>>> which is the opposite for CCA. i.e., "set a bit" for "decrypted"
>>>>> address. So,
>>>>> move the common code that can be reused by all - i.e., add
>>>>> __phys_to_dma() and
>>>>> __dma_to_phys() - and do the arch specific processing.
>>>>>
>>>>> 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>
>>>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>>>> ---
>>>>>    arch/arm64/Kconfig                  |  1 +
>>>>>    arch/arm64/include/asm/dma-direct.h | 38 +++++++++++++++++++++++++++++
>>>>>    include/linux/dma-direct.h          | 35 +++++++++++++++++---------
>>>>>    3 files changed, 62 insertions(+), 12 deletions(-)
>>>>>    create mode 100644 arch/arm64/include/asm/dma-direct.h
>>>>>
>>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>>>> index fcdd0ed3eca8..7befe04106de 100644
>>>>> --- a/arch/arm64/Kconfig
>>>>> +++ b/arch/arm64/Kconfig
>>>>> @@ -41,6 +41,7 @@ config ARM64
>>>>>        select ARCH_HAS_NMI_SAFE_THIS_CPU_OPS
>>>>>        select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
>>>>>        select ARCH_HAS_NONLEAF_PMD_YOUNG if ARM64_HAFT
>>>>> +    select ARCH_HAS_PHYS_TO_DMA
>>>>>        select ARCH_HAS_PTE_DEVMAP
>>>>>        select ARCH_HAS_PTE_SPECIAL
>>>>>        select ARCH_HAS_HW_PTE_YOUNG
>>>>> diff --git a/arch/arm64/include/asm/dma-direct.h
>>>>> b/arch/arm64/include/asm/dma-direct.h
>>>>> new file mode 100644
>>>>> index 000000000000..37c3270542b8
>>>>> --- /dev/null
>>>>> +++ b/arch/arm64/include/asm/dma-direct.h
>>>>> @@ -0,0 +1,38 @@
>>>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>>>> +#ifndef __ASM_DMA_DIRECT_H
>>>>> +#define __ASM_DMA_DIRECT_H
>>>>> +
>>>>> +#include <asm/pgtable-prot.h>
>>>>> +
>>>>> +static inline unsigned long addr_to_shared(unsigned long addr)
>>>>> +{
>>>>> +    if (is_realm_world())
>>>>> +        addr |= prot_ns_shared;
>>>>> +    return addr;
>>>>> +}
>>>>> +
>>>>> +static inline unsigned long addr_to_private(unsigned long addr)
>>>>> +{
>>>>> +    if (is_realm_world())
>>>>> +        addr &= prot_ns_shared - 1;
>>>>> +    return addr;
>>>>> +}
>>>>> +
>>>>> +static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t
>>>>> paddr)
>>>>> +{
>>>>> +    return __phys_to_dma(dev, paddr);
>>>>> +}
>>>>> +
>>>>> +static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev,
>>>>> +                         phys_addr_t paddr)
>>>>> +{
>>>>> +    return addr_to_shared(__phys_to_dma(dev, paddr));
>>>>> +}
>>>>> +#define phys_to_dma_unencrypted phys_to_dma_unencrypted
>>>>> +
>>>>> +static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t
>>>>> dma_addr)
>>>>> +{
>>>>> +    return addr_to_private(__dma_to_phys(dev, dma_addr));
>>>>> +}
>>>>> +
>>>>> +#endif    /* __ASM_DMA_DIRECT_H */
>>>>> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
>>>>> index d7e30d4f7503..3e9bf6ca640e 100644
>>>>> --- a/include/linux/dma-direct.h
>>>>> +++ b/include/linux/dma-direct.h
>>>>> @@ -72,18 +72,36 @@ static inline dma_addr_t dma_range_map_max(const
>>>>> struct bus_dma_region *map)
>>>>>        return ret;
>>>>>    }
>>>>>    +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 phys_addr_t __dma_to_phys(struct device *dev,
>>>>> dma_addr_t dma_addr)
>>>>> +{
>>>>> +    phys_addr_t paddr;
>>>>> +
>>>>> +    if (dev->dma_range_map)
>>>>> +        paddr = translate_dma_to_phys(dev, dma_addr);
>>>>> +    else
>>>>> +        paddr = dma_addr;
>>>>> +
>>>>> +    return paddr;
>>>>> +}
>>>>> +
>>>>>    #ifdef CONFIG_ARCH_HAS_PHYS_TO_DMA
>>>>>    #include <asm/dma-direct.h>
>>>>>    #ifndef phys_to_dma_unencrypted
>>>>>    #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)
>>>>>    {
>>>>> -    if (dev->dma_range_map)
>>>>> -        return translate_phys_to_dma(dev, paddr);
>>>>> -    return paddr;
>>>>> +    return __phys_to_dma(dev, paddr);
>>>>>    }
>>>>>      /*
>>>>> @@ -94,19 +112,12 @@ 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 __sme_set(__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;
>>>>> -
>>>>> -    if (dev->dma_range_map)
>>>>> -        paddr = translate_dma_to_phys(dev, dma_addr);
>>>>> -    else
>>>>> -        paddr = dma_addr;
>>>>> -
>>>>> -    return __sme_clr(paddr);
>>>>> +    return __sme_clr(__dma_to_phys(dev, dma_addr));
>>>>>    }
>>>>>    #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
>>>>>    --
>>>>> 2.43.0
>>>>
>>>> How about the below?
>>>>
>>>> The function name addr_to_shared is too generic to be included in the
>>>> dma-direct.h header file. Since we don’t expect it to be called
>>>> directly, we can either inline it or find a more specific name.
>>>>
>>>> Additionally, for dma_to_phys conversion, we first retrieve the private
>>>> address/alias before switching to the physical address. While both
>>>> approaches yield the correct result, this change more clearly defines the
>>>> conversion rules?
>>>>
>>>
>>> Also translate_dma_to_phys/translate_phys_to_dma work with private dma
>>> addr/alias right?
>>>
>>
>> You're right, that needs to be fixed. Luckily (hopefully) DMA ranges
>> are not used by AMD SME.
> 
> I'm not sure what you mean by this, but DMA to an encrypted addresses is
> allowed under SME and eventually will be allowed with TDISP support in an
> SEV guest. So the DMA address (assuming no IOMMU) could have the
> encryption bit set.
> 


We meant, something like this below, which I plan to add in v2.


---8>---


dma: Fix encryption bit clearing for dma_to_phys

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.


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





> Thanks,
> Tom
> 
>>
>> Suzuki
>>
>>
>>>>
>>>> modified   arch/arm64/include/asm/dma-direct.h
>>>> @@ -4,14 +4,14 @@
>>>>      #include <asm/pgtable-prot.h>
>>>>    -static inline unsigned long addr_to_shared(unsigned long addr)
>>>> +static inline unsigned long shared_dma_addr(unsigned long addr)
>>>>    {
>>>>        if (is_realm_world())
>>>>            addr |= prot_ns_shared;
>>>>        return addr;
>>>>    }
>>>>    -static inline unsigned long addr_to_private(unsigned long addr)
>>>> +static inline unsigned long private_dma_addr(unsigned long addr)
>>>>    {
>>>>        if (is_realm_world())
>>>>            addr &= prot_ns_shared - 1;
>>>> @@ -26,13 +26,14 @@ static inline dma_addr_t phys_to_dma(struct device
>>>> *dev, phys_addr_t paddr)
>>>>    static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev,
>>>>                             phys_addr_t paddr)
>>>>    {
>>>> -    return addr_to_shared(__phys_to_dma(dev, paddr));
>>>> +    return shared_dma_addr(__phys_to_dma(dev, paddr));
>>>>    }
>>>>    #define phys_to_dma_unencrypted phys_to_dma_unencrypted
>>>>      static inline phys_addr_t dma_to_phys(struct device *dev,
>>>> dma_addr_t dma_addr)
>>>>    {
>>>> -    return addr_to_private(__dma_to_phys(dev, dma_addr));
>>>> +    /* if it is a shared dma addr convert to private before looking
>>>> for phys_addr */
>>>> +    return __dma_to_phys(dev, private_dma_addr(dma_addr));
>>>>    }
>>>>      #endif    /* __ASM_DMA_DIRECT_H */
>>



^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-02-19 15:52 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-12 17:14 [PATCH 0/1] arm64: realm: Fix DMA address for devices Suzuki K Poulose
2025-02-12 17:14 ` [PATCH 1/1] arm64: realm: Use aliased addresses for device DMA to shared buffers Suzuki K Poulose
2025-02-12 18:18   ` Robin Murphy
2025-02-12 18:48     ` Suzuki K Poulose
2025-02-15 14:22   ` Aneesh Kumar K.V
2025-02-15 14:38     ` Aneesh Kumar K.V
2025-02-19 14:51       ` Suzuki K Poulose
2025-02-19 15:12         ` Tom Lendacky
2025-02-19 15:50           ` Suzuki K Poulose

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