When a device performs DMA to a shared buffer using physical addresses,
(without Stage1 translation), the device must use the "{I}PA address" with the
top bit set in Realm. This is to make sure that a trusted device will be able
to write to shared buffers as well as the protected buffers. Thus, a Realm must
always program the full address including the "protection" bit, like AMD SME
encryption bits.
Enable this by providing arm64 specific dma_{encrypted,decrypted,clear_encryption}
helpers for Realms. Please note that the VMM needs to similarly make sure that
the SMMU Stage2 in the Non-secure world is setup accordingly to map IPA at the
unprotected alias.
Cc: Will Deacon <will@kernel.org>
Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Steven Price <steven.price@arm.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
arch/arm64/include/asm/mem_encrypt.h | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm64/include/asm/mem_encrypt.h b/arch/arm64/include/asm/mem_encrypt.h
index f8f78f622dd2..aeda3bba255e 100644
--- a/arch/arm64/include/asm/mem_encrypt.h
+++ b/arch/arm64/include/asm/mem_encrypt.h
@@ -21,4 +21,26 @@ static inline bool force_dma_unencrypted(struct device *dev)
return is_realm_world();
}
+static inline dma_addr_t dma_decrypted(dma_addr_t daddr)
+{
+ if (is_realm_world())
+ daddr |= prot_ns_shared;
+ return daddr;
+}
+#define dma_decrypted dma_decrypted
+
+static inline dma_addr_t dma_encrypted(dma_addr_t daddr)
+{
+ if (is_realm_world())
+ daddr &= prot_ns_shared - 1;
+ return daddr;
+}
+#define dma_encrypted dma_encrypted
+
+static inline dma_addr_t dma_clear_encryption(dma_addr_t daddr)
+{
+ return dma_encrypted(daddr);
+}
+#define dma_clear_encryption dma_clear_encryption
+
#endif /* __ASM_MEM_ENCRYPT_H */
--
2.43.0On 2025-02-19 10:07 pm, Suzuki K Poulose wrote:
> When a device performs DMA to a shared buffer using physical addresses,
> (without Stage1 translation), the device must use the "{I}PA address" with the
> top bit set in Realm. This is to make sure that a trusted device will be able
> to write to shared buffers as well as the protected buffers. Thus, a Realm must
> always program the full address including the "protection" bit, like AMD SME
> encryption bits.
>
> Enable this by providing arm64 specific dma_{encrypted,decrypted,clear_encryption}
> helpers for Realms. Please note that the VMM needs to similarly make sure that
> the SMMU Stage2 in the Non-secure world is setup accordingly to map IPA at the
> unprotected alias.
>
> Cc: Will Deacon <will@kernel.org>
> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> arch/arm64/include/asm/mem_encrypt.h | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/arch/arm64/include/asm/mem_encrypt.h b/arch/arm64/include/asm/mem_encrypt.h
> index f8f78f622dd2..aeda3bba255e 100644
> --- a/arch/arm64/include/asm/mem_encrypt.h
> +++ b/arch/arm64/include/asm/mem_encrypt.h
> @@ -21,4 +21,26 @@ static inline bool force_dma_unencrypted(struct device *dev)
> return is_realm_world();
> }
>
> +static inline dma_addr_t dma_decrypted(dma_addr_t daddr)
> +{
> + if (is_realm_world())
> + daddr |= prot_ns_shared;
> + return daddr;
> +}
> +#define dma_decrypted dma_decrypted
> +
> +static inline dma_addr_t dma_encrypted(dma_addr_t daddr)
> +{
> + if (is_realm_world())
> + daddr &= prot_ns_shared - 1;
Nit: is there a reason this isn't the direct inverse of the other
operation, i.e. "daddr &= ~prot_ns_shared"? If so, it might be worth
dropping a comment why we're doing slightly unintuitive arithmetic on a
pagetable attribute (and if not then maybe just do the more obvious
thing). I doubt anyone's in a rush to support TBI for DMA, and this
would be far from the only potential hiccup for that, but still... :)
Thanks,
Robin.
> + return daddr;
> +}
> +#define dma_encrypted dma_encrypted
> +
> +static inline dma_addr_t dma_clear_encryption(dma_addr_t daddr)
> +{
> + return dma_encrypted(daddr);
> +}
> +#define dma_clear_encryption dma_clear_encryption
> +
> #endif /* __ASM_MEM_ENCRYPT_H */
On 25/02/2025 13:04, Robin Murphy wrote:
> On 2025-02-19 10:07 pm, Suzuki K Poulose wrote:
>> When a device performs DMA to a shared buffer using physical addresses,
>> (without Stage1 translation), the device must use the "{I}PA address"
>> with the
>> top bit set in Realm. This is to make sure that a trusted device will
>> be able
>> to write to shared buffers as well as the protected buffers. Thus, a
>> Realm must
>> always program the full address including the "protection" bit, like
>> AMD SME
>> encryption bits.
>>
>> Enable this by providing arm64 specific
>> dma_{encrypted,decrypted,clear_encryption}
>> helpers for Realms. Please note that the VMM needs to similarly make
>> sure that
>> the SMMU Stage2 in the Non-secure world is setup accordingly to map
>> IPA at the
>> unprotected alias.
>>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Robin Murphy <robin.murphy@arm.com>
>> Cc: Steven Price <steven.price@arm.com>
>> Cc: Christoph Hellwig <hch@lst.de>
>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>> Cc: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>> arch/arm64/include/asm/mem_encrypt.h | 22 ++++++++++++++++++++++
>> 1 file changed, 22 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/mem_encrypt.h b/arch/arm64/
>> include/asm/mem_encrypt.h
>> index f8f78f622dd2..aeda3bba255e 100644
>> --- a/arch/arm64/include/asm/mem_encrypt.h
>> +++ b/arch/arm64/include/asm/mem_encrypt.h
>> @@ -21,4 +21,26 @@ static inline bool force_dma_unencrypted(struct
>> device *dev)
>> return is_realm_world();
>> }
>> +static inline dma_addr_t dma_decrypted(dma_addr_t daddr)
>> +{
>> + if (is_realm_world())
>> + daddr |= prot_ns_shared;
>> + return daddr;
>> +}
>> +#define dma_decrypted dma_decrypted
>> +
>> +static inline dma_addr_t dma_encrypted(dma_addr_t daddr)
>> +{
>> + if (is_realm_world())
>> + daddr &= prot_ns_shared - 1;
>
> Nit: is there a reason this isn't the direct inverse of the other
> operation, i.e. "daddr &= ~prot_ns_shared"? If so, it might be worth
It could be. The IPA size for the realm is split into half with the
lower half protected/encrypted and anything above that unprotected.
Technically any addr >= prot_ns_shared is "unencrypted" (even though it
may be invalid, if >= BIT(IPA_Size) - 1). But now when I think of it, it
is much better to trigger a Stage2 fault if the address is illegal
(i.e., > BIT(IPA_Size) - 1) than corrupting some valid memory, by
masking the top bits (beyond prot_ns_shared).
So, I will fix it.
Suzuki
> dropping a comment why we're doing slightly unintuitive arithmetic on a
> pagetable attribute (and if not then maybe just do the more obvious
> thing). I doubt anyone's in a rush to support TBI for DMA, and this
> would be far from the only potential hiccup for that, but still... :)
>
> Thanks,
> Robin.
>
>> + return daddr;
>> +}
>> +#define dma_encrypted dma_encrypted
>> +
>> +static inline dma_addr_t dma_clear_encryption(dma_addr_t daddr)
>> +{
>> + return dma_encrypted(daddr);
>> +}
>> +#define dma_clear_encryption dma_clear_encryption
>> +
>> #endif /* __ASM_MEM_ENCRYPT_H */
>
On 25/02/2025 13:04, Robin Murphy wrote:
> On 2025-02-19 10:07 pm, Suzuki K Poulose wrote:
>> When a device performs DMA to a shared buffer using physical addresses,
>> (without Stage1 translation), the device must use the "{I}PA address"
>> with the
>> top bit set in Realm. This is to make sure that a trusted device will
>> be able
>> to write to shared buffers as well as the protected buffers. Thus, a
>> Realm must
>> always program the full address including the "protection" bit, like
>> AMD SME
>> encryption bits.
>>
>> Enable this by providing arm64 specific
>> dma_{encrypted,decrypted,clear_encryption}
>> helpers for Realms. Please note that the VMM needs to similarly make
>> sure that
>> the SMMU Stage2 in the Non-secure world is setup accordingly to map
>> IPA at the
>> unprotected alias.
>>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Robin Murphy <robin.murphy@arm.com>
>> Cc: Steven Price <steven.price@arm.com>
>> Cc: Christoph Hellwig <hch@lst.de>
>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>> Cc: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>> arch/arm64/include/asm/mem_encrypt.h | 22 ++++++++++++++++++++++
>> 1 file changed, 22 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/mem_encrypt.h b/arch/arm64/
>> include/asm/mem_encrypt.h
>> index f8f78f622dd2..aeda3bba255e 100644
>> --- a/arch/arm64/include/asm/mem_encrypt.h
>> +++ b/arch/arm64/include/asm/mem_encrypt.h
>> @@ -21,4 +21,26 @@ static inline bool force_dma_unencrypted(struct
>> device *dev)
>> return is_realm_world();
>> }
>> +static inline dma_addr_t dma_decrypted(dma_addr_t daddr)
>> +{
>> + if (is_realm_world())
>> + daddr |= prot_ns_shared;
>> + return daddr;
>> +}
>> +#define dma_decrypted dma_decrypted
>> +
>> +static inline dma_addr_t dma_encrypted(dma_addr_t daddr)
>> +{
>> + if (is_realm_world())
>> + daddr &= prot_ns_shared - 1;
>
> Nit: is there a reason this isn't the direct inverse of the other
> operation, i.e. "daddr &= ~prot_ns_shared"? If so, it might be worth
It could be. The IPA size for the realm is split into half with the
lower half protected/encrypted and anything above that unprotected.
Technically any addr >= prot_ns_shared is "unencrypted" (even though it
may be invalid, if >= BIT(IPA_Size) - 1), so to cover that, I masked
anything above the MS. But now when I think of it, it is much better to
trigger a Stage2 fault if the address is illegal (i.e., > BIT(IPA_Size)
- 1) than corrupting some valid memory, by masking the top bits (beyond
prot_ns_shared).
Cheers
Suzuki
> dropping a comment why we're doing slightly unintuitive arithmetic on a
> pagetable attribute (and if not then maybe just do the more obvious
> thing). I doubt anyone's in a rush to support TBI for DMA, and this
> would be far from the only potential hiccup for that, but still... :)
>
> Thanks,
> Robin.
>
>> + return daddr;
>> +}
>> +#define dma_encrypted dma_encrypted
>> +
>> +static inline dma_addr_t dma_clear_encryption(dma_addr_t daddr)
>> +{
>> + return dma_encrypted(daddr);
>> +}
>> +#define dma_clear_encryption dma_clear_encryption
>> +
>> #endif /* __ASM_MEM_ENCRYPT_H */
>
Hi Suzuki,
On 2/20/25 8:07 AM, Suzuki K Poulose wrote:
> When a device performs DMA to a shared buffer using physical addresses,
> (without Stage1 translation), the device must use the "{I}PA address" with the
> top bit set in Realm. This is to make sure that a trusted device will be able
> to write to shared buffers as well as the protected buffers. Thus, a Realm must
> always program the full address including the "protection" bit, like AMD SME
> encryption bits.
>
> Enable this by providing arm64 specific dma_{encrypted,decrypted,clear_encryption}
> helpers for Realms. Please note that the VMM needs to similarly make sure that
> the SMMU Stage2 in the Non-secure world is setup accordingly to map IPA at the
> unprotected alias.
>
> Cc: Will Deacon <will@kernel.org>
> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> arch/arm64/include/asm/mem_encrypt.h | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/arch/arm64/include/asm/mem_encrypt.h b/arch/arm64/include/asm/mem_encrypt.h
> index f8f78f622dd2..aeda3bba255e 100644
> --- a/arch/arm64/include/asm/mem_encrypt.h
> +++ b/arch/arm64/include/asm/mem_encrypt.h
> @@ -21,4 +21,26 @@ static inline bool force_dma_unencrypted(struct device *dev)
> return is_realm_world();
> }
>
> +static inline dma_addr_t dma_decrypted(dma_addr_t daddr)
> +{
> + if (is_realm_world())
> + daddr |= prot_ns_shared;
> + return daddr;
> +}
> +#define dma_decrypted dma_decrypted
> +
There is an existing macro (PROT_NS_SHARED), which is preferred to return
prot_ns_shared or 0 depending on the availability of the realm capability.
However, that macro needs to be improved a bit so that it can be used here.
We need to return 0UL to match with the type of prot_ns_shared (unsigned long)
-#define PROT_NS_SHARED (is_realm_world() ? prot_ns_shared : 0)
+#define PROT_NS_SHARED (is_realm_world() ? prot_ns_shared : 0UL)
After that, the chunk of code can be as below.
return daddr | PROT_NS_SHARED;
> +static inline dma_addr_t dma_encrypted(dma_addr_t daddr)
> +{
> + if (is_realm_world())
> + daddr &= prot_ns_shared - 1;
> + return daddr;
> +}
> +#define dma_encrypted dma_encrypted
> +
With PROT_NS_SHARED, it can become something like below. (PROT_NS_SHARED - 1)
is equivalent to -1UL, 'daddr & -1UL' should be fine since it does nothing.
return daddr & (PROT_NS_SHARED - 1);
> +static inline dma_addr_t dma_clear_encryption(dma_addr_t daddr)
> +{
> + return dma_encrypted(daddr);
> +}
> +#define dma_clear_encryption dma_clear_encryption
> +
> #endif /* __ASM_MEM_ENCRYPT_H */
Thanks,
Gavin
On 2/25/25 3:24 PM, Gavin Shan wrote:
> On 2/20/25 8:07 AM, Suzuki K Poulose wrote:
>> When a device performs DMA to a shared buffer using physical addresses,
>> (without Stage1 translation), the device must use the "{I}PA address" with the
>> top bit set in Realm. This is to make sure that a trusted device will be able
>> to write to shared buffers as well as the protected buffers. Thus, a Realm must
>> always program the full address including the "protection" bit, like AMD SME
>> encryption bits.
>>
>> Enable this by providing arm64 specific dma_{encrypted,decrypted,clear_encryption}
>> helpers for Realms. Please note that the VMM needs to similarly make sure that
>> the SMMU Stage2 in the Non-secure world is setup accordingly to map IPA at the
>> unprotected alias.
>>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Robin Murphy <robin.murphy@arm.com>
>> Cc: Steven Price <steven.price@arm.com>
>> Cc: Christoph Hellwig <hch@lst.de>
>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>> Cc: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>> arch/arm64/include/asm/mem_encrypt.h | 22 ++++++++++++++++++++++
>> 1 file changed, 22 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/mem_encrypt.h b/arch/arm64/include/asm/mem_encrypt.h
>> index f8f78f622dd2..aeda3bba255e 100644
>> --- a/arch/arm64/include/asm/mem_encrypt.h
>> +++ b/arch/arm64/include/asm/mem_encrypt.h
>> @@ -21,4 +21,26 @@ static inline bool force_dma_unencrypted(struct device *dev)
>> return is_realm_world();
>> }
>> +static inline dma_addr_t dma_decrypted(dma_addr_t daddr)
>> +{
>> + if (is_realm_world())
>> + daddr |= prot_ns_shared;
>> + return daddr;
>> +}
>> +#define dma_decrypted dma_decrypted
>> +
>
> There is an existing macro (PROT_NS_SHARED), which is preferred to return
> prot_ns_shared or 0 depending on the availability of the realm capability.
> However, that macro needs to be improved a bit so that it can be used here.
> We need to return 0UL to match with the type of prot_ns_shared (unsigned long)
>
> -#define PROT_NS_SHARED (is_realm_world() ? prot_ns_shared : 0)
> +#define PROT_NS_SHARED (is_realm_world() ? prot_ns_shared : 0UL)
>
> After that, the chunk of code can be as below.
>
> return daddr | PROT_NS_SHARED;
>
>> +static inline dma_addr_t dma_encrypted(dma_addr_t daddr)
>> +{
>> + if (is_realm_world())
>> + daddr &= prot_ns_shared - 1;
>> + return daddr;
>> +}
>> +#define dma_encrypted dma_encrypted
>> +
>
> With PROT_NS_SHARED, it can become something like below. (PROT_NS_SHARED - 1)
> is equivalent to -1UL, 'daddr & -1UL' should be fine since it does nothing.
>
I meant (PROT_NS_SHARED - 1) is equivalent to -1UL when no realm capability
is around :)
> return daddr & (PROT_NS_SHARED - 1);
>
>> +static inline dma_addr_t dma_clear_encryption(dma_addr_t daddr)
>> +{
>> + return dma_encrypted(daddr);
>> +}
>> +#define dma_clear_encryption dma_clear_encryption
>> +
>> #endif /* __ASM_MEM_ENCRYPT_H */
Thanks,
Gavin
Hi Gavin
Thanks for the review.
On 25/02/2025 05:28, Gavin Shan wrote:
> On 2/25/25 3:24 PM, Gavin Shan wrote:
>> On 2/20/25 8:07 AM, Suzuki K Poulose wrote:
>>> When a device performs DMA to a shared buffer using physical addresses,
>>> (without Stage1 translation), the device must use the "{I}PA address"
>>> with the
>>> top bit set in Realm. This is to make sure that a trusted device will
>>> be able
>>> to write to shared buffers as well as the protected buffers. Thus, a
>>> Realm must
>>> always program the full address including the "protection" bit, like
>>> AMD SME
>>> encryption bits.
>>>
>>> Enable this by providing arm64 specific
>>> dma_{encrypted,decrypted,clear_encryption}
>>> helpers for Realms. Please note that the VMM needs to similarly make
>>> sure that
>>> the SMMU Stage2 in the Non-secure world is setup accordingly to map
>>> IPA at the
>>> unprotected alias.
>>>
>>> Cc: Will Deacon <will@kernel.org>
>>> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>> Cc: Robin Murphy <robin.murphy@arm.com>
>>> Cc: Steven Price <steven.price@arm.com>
>>> Cc: Christoph Hellwig <hch@lst.de>
>>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>>> Cc: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> ---
>>> arch/arm64/include/asm/mem_encrypt.h | 22 ++++++++++++++++++++++
>>> 1 file changed, 22 insertions(+)
>>>
>>> diff --git a/arch/arm64/include/asm/mem_encrypt.h b/arch/arm64/
>>> include/asm/mem_encrypt.h
>>> index f8f78f622dd2..aeda3bba255e 100644
>>> --- a/arch/arm64/include/asm/mem_encrypt.h
>>> +++ b/arch/arm64/include/asm/mem_encrypt.h
>>> @@ -21,4 +21,26 @@ static inline bool force_dma_unencrypted(struct
>>> device *dev)
>>> return is_realm_world();
>>> }
>>> +static inline dma_addr_t dma_decrypted(dma_addr_t daddr)
>>> +{
>>> + if (is_realm_world())
>>> + daddr |= prot_ns_shared;
>>> + return daddr;
>>> +}
>>> +#define dma_decrypted dma_decrypted
>>> +
>>
>> There is an existing macro (PROT_NS_SHARED), which is preferred to return
>> prot_ns_shared or 0 depending on the availability of the realm
>> capability.
>> However, that macro needs to be improved a bit so that it can be used
>> here.
>> We need to return 0UL to match with the type of prot_ns_shared
>> (unsigned long)
>>
>> -#define PROT_NS_SHARED (is_realm_world() ? prot_ns_shared : 0)
>> +#define PROT_NS_SHARED (is_realm_world() ? prot_ns_shared : 0UL)
>>
>> After that, the chunk of code can be as below.
>>
>> return daddr | PROT_NS_SHARED;
>>
>>> +static inline dma_addr_t dma_encrypted(dma_addr_t daddr)
>>> +{
>>> + if (is_realm_world())
>>> + daddr &= prot_ns_shared - 1;
>>> + return daddr;
>>> +}
>>> +#define dma_encrypted dma_encrypted
>>> +
>>
>> With PROT_NS_SHARED, it can become something like below.
>> (PROT_NS_SHARED - 1)
>> is equivalent to -1UL, 'daddr & -1UL' should be fine since it does
>> nothing.
>>
>
> I meant (PROT_NS_SHARED - 1) is equivalent to -1UL when no realm capability
> is around :)
I didn't want this to be there ;-). But with Robin's comment, I think we
can revert back to PROT_NS_SHARED.
Cheers
Suzuki
>
>> return daddr & (PROT_NS_SHARED - 1);
>>
>>> +static inline dma_addr_t dma_clear_encryption(dma_addr_t daddr)
>>> +{
>>> + return dma_encrypted(daddr);
>>> +}
>>> +#define dma_clear_encryption dma_clear_encryption
>>> +
>>> #endif /* __ASM_MEM_ENCRYPT_H */
>
> Thanks,
> Gavin
>
© 2016 - 2025 Red Hat, Inc.