Re: [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
Borislav Petkov writes:
> On Thu, Mar 19, 2020 at 06:25:49PM +0100, Thomas Gleixner wrote:
>> TBH, I don't see how
>>
>> if (force_dma_decrypted(dev))
>> set_memory_encrypted((unsigned long)cpu_addr, 1 << page_order);
>>
>> makes more sense than the above. It's both non-sensical unless there is
>
> 9087c37584fb ("dma-direct: Force unencrypted DMA under SME for certain DMA
> masks")
Reading the changelog again...
I have to say that force_dma_unencrypted() makes way more sense in that
context than force_dma_decrypted(). It still wants a comment.
Linguistical semantics and correctness matters a lot. Consistency is
required as well, but not for the price of ambiguous wording.
Thanks,
tglx
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
On Thu, Mar 19, 2020 at 06:25:49PM +0100, Thomas Gleixner wrote: > Borislav Petkov writes: > > > On Thu, Mar 19, 2020 at 11:06:15AM +, Robin Murphy wrote: > >> Let me add another vote from a native English speaker that "unencrypted" is > >> the appropriate term to imply the *absence* of encryption, whereas > >> "decrypted" implies the *reversal* of applied encryption. Even as a non-native speaker I can clearly see the distinction. > >> > >> Naming things is famously hard, for good reason - names are *important* for > >> understanding. Just because a decision was already made one way doesn't > >> mean > >> that that decision was necessarily right. Churning one area to be > >> consistently inaccurate just because it's less work than churning another > >> area to be consistently accurate isn't really the best excuse. > > > > Well, the reason we chose "decrypted" vs something else is so to be as > > different from "encrypted" as possible. If we called it "unencrypted" > > you'd have stuff like: > > > >if (force_dma_unencrypted(dev)) > > set_memory_encrypted((unsigned long)cpu_addr, 1 << > > page_order); If you want something with high edit distance from 'encrypted' meaning the opposite there is already 'cleartext' which was designed for this exact purpose. Thanks Michal ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
On Thu, Mar 19, 2020 at 06:25:49PM +0100, Thomas Gleixner wrote:
> TBH, I don't see how
>
> if (force_dma_decrypted(dev))
> set_memory_encrypted((unsigned long)cpu_addr, 1 << page_order);
>
> makes more sense than the above. It's both non-sensical unless there is
9087c37584fb ("dma-direct: Force unencrypted DMA under SME for certain DMA
masks")
> a big fat comment explaining what this is about.
ACK to that.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
Borislav Petkov writes: > On Thu, Mar 19, 2020 at 11:06:15AM +, Robin Murphy wrote: >> Let me add another vote from a native English speaker that "unencrypted" is >> the appropriate term to imply the *absence* of encryption, whereas >> "decrypted" implies the *reversal* of applied encryption. >> >> Naming things is famously hard, for good reason - names are *important* for >> understanding. Just because a decision was already made one way doesn't mean >> that that decision was necessarily right. Churning one area to be >> consistently inaccurate just because it's less work than churning another >> area to be consistently accurate isn't really the best excuse. > > Well, the reason we chose "decrypted" vs something else is so to be as > different from "encrypted" as possible. If we called it "unencrypted" > you'd have stuff like: > >if (force_dma_unencrypted(dev)) > set_memory_encrypted((unsigned long)cpu_addr, 1 << > page_order); TBH, I don't see how if (force_dma_decrypted(dev)) set_memory_encrypted((unsigned long)cpu_addr, 1 << page_order); makes more sense than the above. It's both non-sensical unless there is a big fat comment explaining what this is about. Thanks, tglx ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
On Thu, Mar 19, 2020 at 11:06:15AM +, Robin Murphy wrote: > Let me add another vote from a native English speaker that "unencrypted" is > the appropriate term to imply the *absence* of encryption, whereas > "decrypted" implies the *reversal* of applied encryption. > > Naming things is famously hard, for good reason - names are *important* for > understanding. Just because a decision was already made one way doesn't mean > that that decision was necessarily right. Churning one area to be > consistently inaccurate just because it's less work than churning another > area to be consistently accurate isn't really the best excuse. Well, the reason we chose "decrypted" vs something else is so to be as different from "encrypted" as possible. If we called it "unencrypted" you'd have stuff like: if (force_dma_unencrypted(dev)) set_memory_encrypted((unsigned long)cpu_addr, 1 << page_order); and I *betcha* people will misread this and maybe even introduce bugs. So I don't think renaming it to "unencrypted" is better. And yes, I'm deliberately putting the language semantics here on a second place because of readability examples like the one above. But ok, since people don't want this, we can leave it as is. It's not like I don't have anything better to do. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
[since this is in my inbox...] On 2020-03-19 10:28 am, Borislav Petkov wrote: On Thu, Mar 19, 2020 at 11:20:11AM +0100, Christoph Hellwig wrote: I thought we agreed that decrypted is absolutely the wrong term. I don't think we did. At least I don't know where we did that. So NAK - if you want to change things it needs to go the other way. We are already using "decrypted" everywhere in arch/x86/. Changing that would be a *lot* more churn. And it is just a term, for chrissakes, to denote memory which is not encrypted. And it would make our lifes easier if we had only *two* terms instead of three or more. Especially if the concept we denote with this is a binary one: encrypted memory and *not* encrypted memory. Let me add another vote from a native English speaker that "unencrypted" is the appropriate term to imply the *absence* of encryption, whereas "decrypted" implies the *reversal* of applied encryption. Naming things is famously hard, for good reason - names are *important* for understanding. Just because a decision was already made one way doesn't mean that that decision was necessarily right. Churning one area to be consistently inaccurate just because it's less work than churning another area to be consistently accurate isn't really the best excuse. Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
On Thu, Mar 19, 2020 at 11:20:11AM +0100, Christoph Hellwig wrote: > I thought we agreed that decrypted is absolutely the wrong term. I don't think we did. At least I don't know where we did that. > So NAK - if you want to change things it needs to go the other way. We are already using "decrypted" everywhere in arch/x86/. Changing that would be a *lot* more churn. And it is just a term, for chrissakes, to denote memory which is not encrypted. And it would make our lifes easier if we had only *two* terms instead of three or more. Especially if the concept we denote with this is a binary one: encrypted memory and *not* encrypted memory. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
On Thu, Mar 19, 2020 at 11:16:57AM +0100, Borislav Petkov wrote: > Hi, > > here's v2 with build breakage fixed on ppc and s390 (obviously I can't > grep :-\) and commit message extended to explain more why. I thought we agreed that decrypted is absolutely the wrong term. So NAK - if you want to change things it needs to go the other way. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
Hi,
here's v2 with build breakage fixed on ppc and s390 (obviously I can't
grep :-\) and commit message extended to explain more why.
Thx.
---
From: Borislav Petkov
Date: Tue, 17 Mar 2020 12:03:05 +0100
Back then when the whole SME machinery started getting mainlined, it
was agreed that for simplicity, clarity and sanity's sake, the terms
denoting encrypted and not-encrypted memory should be "encrypted" and
"decrypted". And the majority of the code sticks to that convention
except those two. So rename them.
The intent of "encrypted" and "decrypted" is to represent the binary
concept of memory which is encrypted and of memory which is not.
The further differentiation between decrypted and unencrypted memory is
not needed in the code (for now) and therefore keep things simple by
using solely the two terms.
No functional changes.
[ Convert forgotten s390 and ppc function variants. ]
Reported-by: kbuild test robot
Signed-off-by: Borislav Petkov
---
arch/powerpc/include/asm/mem_encrypt.h | 2 +-
arch/powerpc/platforms/pseries/Kconfig | 2 +-
arch/s390/Kconfig | 2 +-
arch/s390/mm/init.c| 2 +-
arch/x86/Kconfig | 2 +-
arch/x86/mm/mem_encrypt.c | 4 ++--
include/linux/dma-direct.h | 8
kernel/dma/Kconfig | 2 +-
kernel/dma/direct.c| 14 +++---
kernel/dma/mapping.c | 2 +-
10 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/include/asm/mem_encrypt.h
b/arch/powerpc/include/asm/mem_encrypt.h
index ba9dab07c1be..f0705cd3b079 100644
--- a/arch/powerpc/include/asm/mem_encrypt.h
+++ b/arch/powerpc/include/asm/mem_encrypt.h
@@ -15,7 +15,7 @@ static inline bool mem_encrypt_active(void)
return is_secure_guest();
}
-static inline bool force_dma_unencrypted(struct device *dev)
+static inline bool force_dma_decrypted(struct device *dev)
{
return is_secure_guest();
}
diff --git a/arch/powerpc/platforms/pseries/Kconfig
b/arch/powerpc/platforms/pseries/Kconfig
index 24c18362e5ea..a78e2c3e1d92 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -151,7 +151,7 @@ config PPC_SVM
depends on PPC_PSERIES
select SWIOTLB
select ARCH_HAS_MEM_ENCRYPT
- select ARCH_HAS_FORCE_DMA_UNENCRYPTED
+ select ARCH_HAS_FORCE_DMA_DECRYPTED
help
There are certain POWER platforms which support secure guests using
the Protected Execution Facility, with the help of an Ultravisor
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 8abe77536d9d..ab1dbb7415b4 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -192,7 +192,7 @@ config S390
select VIRT_CPU_ACCOUNTING
select ARCH_HAS_SCALED_CPUTIME
select HAVE_NMI
- select ARCH_HAS_FORCE_DMA_UNENCRYPTED
+ select ARCH_HAS_FORCE_DMA_DECRYPTED
select SWIOTLB
select GENERIC_ALLOCATOR
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index ac44bd76db4b..5fed85f9fea6 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -157,7 +157,7 @@ int set_memory_decrypted(unsigned long addr, int numpages)
}
/* are we a protected virtualization guest? */
-bool force_dma_unencrypted(struct device *dev)
+bool force_dma_decrypted(struct device *dev)
{
return is_prot_virt_guest();
}
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index beea77046f9b..2ae904f505e1 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1525,7 +1525,7 @@ config AMD_MEM_ENCRYPT
depends on X86_64 && CPU_SUP_AMD
select DYNAMIC_PHYSICAL_MASK
select ARCH_USE_MEMREMAP_PROT
- select ARCH_HAS_FORCE_DMA_UNENCRYPTED
+ select ARCH_HAS_FORCE_DMA_DECRYPTED
---help---
Say yes to enable support for the encryption of system memory.
This requires an AMD processor that supports Secure Memory
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index a03614bd3e1a..66d09f269e6d 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -350,8 +350,8 @@ bool sev_active(void)
return sme_me_mask && sev_enabled;
}
-/* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_UNENCRYPTED */
-bool force_dma_unencrypted(struct device *dev)
+/* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_DECRYPTED */
+bool force_dma_decrypted(struct device *dev)
{
/*
* For SEV, all DMA must be to unencrypted addresses.
diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index 24b8684aa21d..9f955844e9c7 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -26,14 +26,14 @@ static inline phys_addr_t __dma_to_phys(struct device *dev,
dma_addr_t dev_addr)
}
#endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
-#ifdef CONFIG_ARCH_HAS_FORCE_DMA_UNENCRYPTED
-bool force_dma_unencrypted(struct device
