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