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