iommu.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] treewide: Rename "unencrypted" to "decrypted"
@ 2020-03-17 11:18 Borislav Petkov
  2020-03-17 20:35 ` Dave Hansen
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Borislav Petkov @ 2020-03-17 11:18 UTC (permalink / raw)
  To: lkml
  Cc: linux-s390, Dave Hansen, Vasily Gorbik, Tom Lendacky,
	Peter Zijlstra, Benjamin Herrenschmidt, x86, Heiko Carstens,
	linuxppc-dev, linux-kernel, Christian Borntraeger, iommu,
	Ingo Molnar, Paul Mackerras, Andy Lutomirski, Michael Ellerman,
	Thomas Gleixner, Robin Murphy, Christoph Hellwig

Hi all,

this hasn't been fully tested yet but it is mechanical rename only so
there shouldn't be any problems (famous last words :-)).

I'll run it through the randconfig bench today and take it through tip if
there are no objections.

Thx.

---

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.

No functional changes.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/powerpc/platforms/pseries/Kconfig |  2 +-
 arch/s390/Kconfig                      |  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 +-
 8 files changed, 18 insertions(+), 18 deletions(-)

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/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 *dev);
+#ifdef CONFIG_ARCH_HAS_FORCE_DMA_DECRYPTED
+bool force_dma_decrypted(struct device *dev);
 #else
-static inline bool force_dma_unencrypted(struct device *dev)
+static inline bool force_dma_decrypted(struct device *dev)
 {
 	return false;
 }
-#endif /* CONFIG_ARCH_HAS_FORCE_DMA_UNENCRYPTED */
+#endif /* CONFIG_ARCH_HAS_FORCE_DMA_DECRYPTED */
 
 /*
  * If memory encryption is supported, phys_to_dma will set the memory encryption
diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
index 4c103a24e380..55c4147bb2b1 100644
--- a/kernel/dma/Kconfig
+++ b/kernel/dma/Kconfig
@@ -51,7 +51,7 @@ config ARCH_HAS_SYNC_DMA_FOR_CPU_ALL
 config ARCH_HAS_DMA_PREP_COHERENT
 	bool
 
-config ARCH_HAS_FORCE_DMA_UNENCRYPTED
+config ARCH_HAS_FORCE_DMA_DECRYPTED
 	bool
 
 config DMA_NONCOHERENT_CACHE_SYNC
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index ac7956c38f69..a0576c0ccacd 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -26,7 +26,7 @@ unsigned int zone_dma_bits __ro_after_init = 24;
 static inline dma_addr_t phys_to_dma_direct(struct device *dev,
 		phys_addr_t phys)
 {
-	if (force_dma_unencrypted(dev))
+	if (force_dma_decrypted(dev))
 		return __phys_to_dma(dev, phys);
 	return phys_to_dma(dev, phys);
 }
@@ -49,7 +49,7 @@ static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
 {
 	u64 dma_limit = min_not_zero(dma_mask, dev->bus_dma_limit);
 
-	if (force_dma_unencrypted(dev))
+	if (force_dma_decrypted(dev))
 		*phys_limit = __dma_to_phys(dev, dma_limit);
 	else
 		*phys_limit = dma_to_phys(dev, dma_limit);
@@ -138,7 +138,7 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
 		return NULL;
 
 	if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
-	    !force_dma_unencrypted(dev)) {
+	    !force_dma_decrypted(dev)) {
 		/* remove any dirty cache lines on the kernel alias */
 		if (!PageHighMem(page))
 			arch_dma_prep_coherent(page, size);
@@ -179,7 +179,7 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
 	}
 
 	ret = page_address(page);
-	if (force_dma_unencrypted(dev))
+	if (force_dma_decrypted(dev))
 		set_memory_decrypted((unsigned long)ret, 1 << get_order(size));
 
 	memset(ret, 0, size);
@@ -190,7 +190,7 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
 		ret = uncached_kernel_address(ret);
 	}
 done:
-	if (force_dma_unencrypted(dev))
+	if (force_dma_decrypted(dev))
 		*dma_handle = __phys_to_dma(dev, page_to_phys(page));
 	else
 		*dma_handle = phys_to_dma(dev, page_to_phys(page));
@@ -203,7 +203,7 @@ void dma_direct_free_pages(struct device *dev, size_t size, void *cpu_addr,
 	unsigned int page_order = get_order(size);
 
 	if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
-	    !force_dma_unencrypted(dev)) {
+	    !force_dma_decrypted(dev)) {
 		/* cpu_addr is a struct page cookie, not a kernel address */
 		dma_free_contiguous(dev, cpu_addr, size);
 		return;
@@ -213,7 +213,7 @@ void dma_direct_free_pages(struct device *dev, size_t size, void *cpu_addr,
 	    dma_free_from_pool(cpu_addr, PAGE_ALIGN(size)))
 		return;
 
-	if (force_dma_unencrypted(dev))
+	if (force_dma_decrypted(dev))
 		set_memory_encrypted((unsigned long)cpu_addr, 1 << page_order);
 
 	if (IS_ENABLED(CONFIG_DMA_REMAP) && is_vmalloc_addr(cpu_addr))
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index 98e3d873792e..dbd0605a39c5 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -154,7 +154,7 @@ EXPORT_SYMBOL(dma_get_sgtable_attrs);
  */
 pgprot_t dma_pgprot(struct device *dev, pgprot_t prot, unsigned long attrs)
 {
-	if (force_dma_unencrypted(dev))
+	if (force_dma_decrypted(dev))
 		prot = pgprot_decrypted(prot);
 	if (dev_is_dma_coherent(dev) ||
 	    (IS_ENABLED(CONFIG_DMA_NONCOHERENT_CACHE_SYNC) &&
-- 
2.21.0

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

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

* Re: [PATCH] treewide: Rename "unencrypted" to "decrypted"
  2020-03-17 11:18 [PATCH] treewide: Rename "unencrypted" to "decrypted" Borislav Petkov
@ 2020-03-17 20:35 ` Dave Hansen
  2020-03-17 21:06   ` Borislav Petkov
  2020-03-17 23:16 ` kbuild test robot
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Dave Hansen @ 2020-03-17 20:35 UTC (permalink / raw)
  To: Borislav Petkov, lkml, Schofield, Alison
  Cc: linux-s390, Dave Hansen, Vasily Gorbik, Tom Lendacky,
	Peter Zijlstra, Benjamin Herrenschmidt, x86, Heiko Carstens,
	linuxppc-dev, Shutemov, Kirill, Christian Borntraeger, iommu,
	Ingo Molnar, Paul Mackerras, Andy Lutomirski, Michael Ellerman,
	Thomas Gleixner, Robin Murphy, Christoph Hellwig

On 3/17/20 4:18 AM, Borislav Petkov wrote:
> 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.

Don't "unencrypted" and "decrypted" mean different things?

Unencrypted to me means "encryption was never used for this data".

Decrypted means "this was/is encrypted but here is a plaintext copy".

This, for instance:

> +++ b/kernel/dma/direct.c
> @@ -26,7 +26,7 @@ unsigned int zone_dma_bits __ro_after_init = 24;
>  static inline dma_addr_t phys_to_dma_direct(struct device *dev,
>  		phys_addr_t phys)
>  {
> -	if (force_dma_unencrypted(dev))
> +	if (force_dma_decrypted(dev))
>  		return __phys_to_dma(dev, phys);

is referring to DMA that is not and never was encrypted.  It's skipping
the encryption altogether.  There's no act of "decryption" anywhere.

This, on the other hand, seems named wrong to me:

> /*
>  * Macros to add or remove encryption attribute
>  */
> #define pgprot_encrypted(prot)  __pgprot(__sme_set(pgprot_val(prot)))
> #define pgprot_decrypted(prot)  __pgprot(__sme_clr(pgprot_val(prot)))

This seems like it would be better named pgprot_unencrypted().
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH] treewide: Rename "unencrypted" to "decrypted"
  2020-03-17 20:35 ` Dave Hansen
@ 2020-03-17 21:06   ` Borislav Petkov
  2020-03-17 21:24     ` Dave Hansen
  2020-03-17 22:01     ` Thomas Gleixner
  0 siblings, 2 replies; 18+ messages in thread
From: Borislav Petkov @ 2020-03-17 21:06 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Schofield, Alison, Peter Zijlstra, Benjamin Herrenschmidt,
	Dave Hansen, Heiko Carstens, Paul Mackerras, Christoph Hellwig,
	linux-s390, Michael Ellerman, x86, Christian Borntraeger,
	Ingo Molnar, Borislav Petkov, Tom Lendacky, Vasily Gorbik,
	Shutemov, Kirill, Andy Lutomirski, Thomas Gleixner, Robin Murphy,
	lkml, iommu, linuxppc-dev

On Tue, Mar 17, 2020 at 01:35:12PM -0700, Dave Hansen wrote:
> On 3/17/20 4:18 AM, Borislav Petkov wrote:
> > 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.
> 
> Don't "unencrypted" and "decrypted" mean different things?
> 
> Unencrypted to me means "encryption was never used for this data".
> 
> Decrypted means "this was/is encrypted but here is a plaintext copy".

Maybe but linguistical semantics is not the point here.

The idea is to represent a "binary" concept of memory being encrypted
or memory being not encrypted. And at the time we decided to use
"encrypted" and "decrypted" for those two things.

Do you see the need to differentiate a third "state", so to speak, of
memory which was never encrypted?

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

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

* Re: [PATCH] treewide: Rename "unencrypted" to "decrypted"
  2020-03-17 21:06   ` Borislav Petkov
@ 2020-03-17 21:24     ` Dave Hansen
  2020-03-17 21:31       ` Borislav Petkov
  2020-03-17 21:34       ` Joe Perches
  2020-03-17 22:01     ` Thomas Gleixner
  1 sibling, 2 replies; 18+ messages in thread
From: Dave Hansen @ 2020-03-17 21:24 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Schofield, Alison, Peter Zijlstra, Benjamin Herrenschmidt,
	Dave Hansen, Heiko Carstens, Paul Mackerras, Christoph Hellwig,
	linux-s390, Michael Ellerman, x86, Christian Borntraeger,
	Ingo Molnar, Borislav Petkov, Tom Lendacky, Vasily Gorbik,
	Shutemov, Kirill, Andy Lutomirski, Thomas Gleixner, Robin Murphy,
	lkml, iommu, linuxppc-dev

On 3/17/20 2:06 PM, Borislav Petkov wrote:
> On Tue, Mar 17, 2020 at 01:35:12PM -0700, Dave Hansen wrote:
>> On 3/17/20 4:18 AM, Borislav Petkov wrote:
>>> 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.
>> Don't "unencrypted" and "decrypted" mean different things?
>>
>> Unencrypted to me means "encryption was never used for this data".
>>
>> Decrypted means "this was/is encrypted but here is a plaintext copy".
> Maybe but linguistical semantics is not the point here.
> 
> The idea is to represent a "binary" concept of memory being encrypted
> or memory being not encrypted. And at the time we decided to use
> "encrypted" and "decrypted" for those two things.

Yeah, agreed.  We're basically trying to name "!encrypted".

> Do you see the need to differentiate a third "state", so to speak, of
> memory which was never encrypted?

No, there are just two states.  I just think the "!encrypted" case
should not be called "decrypted".
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH] treewide: Rename "unencrypted" to "decrypted"
  2020-03-17 21:24     ` Dave Hansen
@ 2020-03-17 21:31       ` Borislav Petkov
  2020-03-17 21:34       ` Joe Perches
  1 sibling, 0 replies; 18+ messages in thread
From: Borislav Petkov @ 2020-03-17 21:31 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Schofield, Alison, Peter Zijlstra, Benjamin Herrenschmidt,
	Dave Hansen, Heiko Carstens, Paul Mackerras, Christoph Hellwig,
	linux-s390, Michael Ellerman, x86, Christian Borntraeger,
	Ingo Molnar, Tom Lendacky, Vasily Gorbik, Shutemov, Kirill,
	Andy Lutomirski, Thomas Gleixner, Robin Murphy, lkml, iommu,
	linuxppc-dev

On Tue, Mar 17, 2020 at 02:24:59PM -0700, Dave Hansen wrote:
> No, there are just two states.  I just think the "!encrypted" case
> should not be called "decrypted".

Yeah, we suck at naming - news at 11! :-)

I believe we even considered things like "encrypted" vs "clear" but
that sucked too. ;-\

In any case, that ship has sailed now and having two as differently as
possible looking words to denote the two "states" should be good enough
for our purposes...

Oh well.

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

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

* Re: [PATCH] treewide: Rename "unencrypted" to "decrypted"
  2020-03-17 21:24     ` Dave Hansen
  2020-03-17 21:31       ` Borislav Petkov
@ 2020-03-17 21:34       ` Joe Perches
  1 sibling, 0 replies; 18+ messages in thread
From: Joe Perches @ 2020-03-17 21:34 UTC (permalink / raw)
  To: Dave Hansen, Borislav Petkov
  Cc: Schofield, Alison, Peter Zijlstra, Benjamin Herrenschmidt,
	Dave Hansen, Heiko Carstens, Paul Mackerras, Christoph Hellwig,
	linux-s390, Michael Ellerman, x86, Christian Borntraeger,
	Ingo Molnar, Borislav Petkov, Tom Lendacky, Vasily Gorbik,
	Shutemov, Kirill, Andy Lutomirski, Thomas Gleixner, Robin Murphy,
	lkml, iommu, linuxppc-dev

On Tue, 2020-03-17 at 14:24 -0700, Dave Hansen wrote:
> On 3/17/20 2:06 PM, Borislav Petkov wrote:
> > On Tue, Mar 17, 2020 at 01:35:12PM -0700, Dave Hansen wrote:
> > > On 3/17/20 4:18 AM, Borislav Petkov wrote:
> > > > 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.
> > > Don't "unencrypted" and "decrypted" mean different things?
> > > 
> > > Unencrypted to me means "encryption was never used for this data".
> > > 
> > > Decrypted means "this was/is encrypted but here is a plaintext copy".
> > Maybe but linguistical semantics is not the point here.
> > 
> > The idea is to represent a "binary" concept of memory being encrypted
> > or memory being not encrypted. And at the time we decided to use
> > "encrypted" and "decrypted" for those two things.
> 
> Yeah, agreed.  We're basically trying to name "!encrypted".
> 
> > Do you see the need to differentiate a third "state", so to speak, of
> > memory which was never encrypted?
> 
> No, there are just two states.  I just think the "!encrypted" case
> should not be called "decrypted".

Nor do I, it's completely misleading.


_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH] treewide: Rename "unencrypted" to "decrypted"
  2020-03-17 21:06   ` Borislav Petkov
  2020-03-17 21:24     ` Dave Hansen
@ 2020-03-17 22:01     ` Thomas Gleixner
  1 sibling, 0 replies; 18+ messages in thread
From: Thomas Gleixner @ 2020-03-17 22:01 UTC (permalink / raw)
  To: Borislav Petkov, Dave Hansen
  Cc: Schofield, Alison, Peter Zijlstra, Benjamin Herrenschmidt,
	Dave Hansen, Heiko Carstens, Paul Mackerras, Christoph Hellwig,
	linux-s390, Michael Ellerman, x86, Christian Borntraeger,
	Ingo Molnar, Borislav Petkov, Tom Lendacky, Vasily Gorbik,
	Shutemov, Kirill, Andy Lutomirski, Robin Murphy, lkml, iommu,
	linuxppc-dev

Borislav Petkov <bp@alien8.de> writes:

> On Tue, Mar 17, 2020 at 01:35:12PM -0700, Dave Hansen wrote:
>> On 3/17/20 4:18 AM, Borislav Petkov wrote:
>> > 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.
>> 
>> Don't "unencrypted" and "decrypted" mean different things?
>> 
>> Unencrypted to me means "encryption was never used for this data".
>> 
>> Decrypted means "this was/is encrypted but here is a plaintext copy".
>
> Maybe but linguistical semantics is not the point here.
>
> The idea is to represent a "binary" concept of memory being encrypted
> or memory being not encrypted. And at the time we decided to use
> "encrypted" and "decrypted" for those two things.
>
> Do you see the need to differentiate a third "state", so to speak, of
> memory which was never encrypted?

I think so.

encrypted data is something you can't use without having the key

decrypted data is the plaintext copy of something encrypted, so
it might be of sensible nature.

unencrypted data can still be sensible, but nothing ever bothered to
encrypt it in the first place.

So having this distinction is useful in terms of setting the context
straight.

Thanks,

        tglx
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH] treewide: Rename "unencrypted" to "decrypted"
  2020-03-17 11:18 [PATCH] treewide: Rename "unencrypted" to "decrypted" Borislav Petkov
  2020-03-17 20:35 ` Dave Hansen
@ 2020-03-17 23:16 ` kbuild test robot
  2020-03-17 23:51 ` kbuild test robot
  2020-03-19 10:16 ` [PATCH -v2] " Borislav Petkov
  3 siblings, 0 replies; 18+ messages in thread
From: kbuild test robot @ 2020-03-17 23:16 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: linux-s390, Dave Hansen, kbuild-all, Vasily Gorbik, linuxppc-dev,
	Peter Zijlstra, Benjamin Herrenschmidt, x86, Heiko Carstens,
	lkml, Christian Borntraeger, iommu, Ingo Molnar, Paul Mackerras,
	Tom Lendacky, Andy Lutomirski, Michael Ellerman, Thomas Gleixner,
	Robin Murphy, Christoph Hellwig

[-- Attachment #1: Type: text/plain, Size: 2065 bytes --]

Hi Borislav,

I love your patch! Yet something to improve:

[auto build test ERROR on tip/x86/mm]
[cannot apply to linux/master powerpc/next s390/features tip/x86/core linus/master v5.6-rc6 next-20200317]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Borislav-Petkov/treewide-Rename-unencrypted-to-decrypted/20200318-015738
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 17c4a2ae15a7aaefe84bdb271952678c5c9cd8e1
config: s390-zfcpdump_defconfig (attached as .config)
compiler: s390-linux-gcc (GCC) 9.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=9.2.0 make.cross ARCH=s390 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   s390-linux-ld: kernel/dma/mapping.o: in function `dma_pgprot':
   mapping.c:(.text+0x144): undefined reference to `force_dma_decrypted'
   s390-linux-ld: kernel/dma/mapping.o: in function `dma_common_mmap':
   mapping.c:(.text+0x1a4): undefined reference to `force_dma_decrypted'
   s390-linux-ld: kernel/dma/direct.o: in function `dma_direct_get_required_mask':
   direct.c:(.text+0xae): undefined reference to `force_dma_decrypted'
   s390-linux-ld: kernel/dma/direct.o: in function `__dma_direct_alloc_pages':
   direct.c:(.text+0x152): undefined reference to `force_dma_decrypted'
>> s390-linux-ld: direct.c:(.text+0x1e8): undefined reference to `force_dma_decrypted'
   s390-linux-ld: kernel/dma/direct.o:direct.c:(.text+0x2e0): more undefined references to `force_dma_decrypted' follow

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 7925 bytes --]

[-- Attachment #3: Type: text/plain, Size: 156 bytes --]

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH] treewide: Rename "unencrypted" to "decrypted"
  2020-03-17 11:18 [PATCH] treewide: Rename "unencrypted" to "decrypted" Borislav Petkov
  2020-03-17 20:35 ` Dave Hansen
  2020-03-17 23:16 ` kbuild test robot
@ 2020-03-17 23:51 ` kbuild test robot
  2020-03-19 10:16 ` [PATCH -v2] " Borislav Petkov
  3 siblings, 0 replies; 18+ messages in thread
From: kbuild test robot @ 2020-03-17 23:51 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: linux-s390, Dave Hansen, kbuild-all, Vasily Gorbik, linuxppc-dev,
	Peter Zijlstra, Benjamin Herrenschmidt, x86, Heiko Carstens,
	lkml, Christian Borntraeger, iommu, Ingo Molnar, Paul Mackerras,
	Tom Lendacky, Andy Lutomirski, Michael Ellerman, Thomas Gleixner,
	Robin Murphy, Christoph Hellwig

[-- Attachment #1: Type: text/plain, Size: 2223 bytes --]

Hi Borislav,

I love your patch! Yet something to improve:

[auto build test ERROR on tip/x86/mm]
[cannot apply to linux/master powerpc/next s390/features tip/x86/core linus/master v5.6-rc6 next-20200317]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Borislav-Petkov/treewide-Rename-unencrypted-to-decrypted/20200318-015738
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 17c4a2ae15a7aaefe84bdb271952678c5c9cd8e1
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 9.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=9.2.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   powerpc64-linux-ld: warning: orphan section `.gnu.hash' from `linker stubs' being placed in section `.gnu.hash'
   powerpc64-linux-ld: kernel/dma/mapping.o: in function `.dma_pgprot':
>> mapping.c:(.text+0xc5c): undefined reference to `.force_dma_decrypted'
   powerpc64-linux-ld: kernel/dma/mapping.o: in function `.dma_common_mmap':
   mapping.c:(.text+0xcfc): undefined reference to `.force_dma_decrypted'
   powerpc64-linux-ld: kernel/dma/direct.o: in function `.dma_direct_get_required_mask':
>> direct.c:(.text+0x920): undefined reference to `.force_dma_decrypted'
   powerpc64-linux-ld: kernel/dma/direct.o: in function `.__dma_direct_alloc_pages':
   direct.c:(.text+0x9fc): undefined reference to `.force_dma_decrypted'
>> powerpc64-linux-ld: direct.c:(.text+0xaa8): undefined reference to `.force_dma_decrypted'
   powerpc64-linux-ld: kernel/dma/direct.o:direct.c:(.text+0xb28): more undefined references to `.force_dma_decrypted' follow

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25776 bytes --]

[-- Attachment #3: Type: text/plain, Size: 156 bytes --]

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
  2020-03-17 11:18 [PATCH] treewide: Rename "unencrypted" to "decrypted" Borislav Petkov
                   ` (2 preceding siblings ...)
  2020-03-17 23:51 ` kbuild test robot
@ 2020-03-19 10:16 ` Borislav Petkov
  2020-03-19 10:20   ` Christoph Hellwig
  3 siblings, 1 reply; 18+ messages in thread
From: Borislav Petkov @ 2020-03-19 10:16 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: linux-s390, Dave Hansen, Vasily Gorbik, linuxppc-dev,
	Peter Zijlstra, Benjamin Herrenschmidt, x86, Heiko Carstens,
	lkml, Christian Borntraeger, iommu, Ingo Molnar, Paul Mackerras,
	Tom Lendacky, Andy Lutomirski, Michael Ellerman, Thomas Gleixner,
	Robin Murphy, Christoph Hellwig

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 <bp@suse.de>
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 <lkp@intel.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 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 *dev);
+#ifdef CONFIG_ARCH_HAS_FORCE_DMA_DECRYPTED
+bool force_dma_decrypted(struct device *dev);
 #else
-static inline bool force_dma_unencrypted(struct device *dev)
+static inline bool force_dma_decrypted(struct device *dev)
 {
 	return false;
 }
-#endif /* CONFIG_ARCH_HAS_FORCE_DMA_UNENCRYPTED */
+#endif /* CONFIG_ARCH_HAS_FORCE_DMA_DECRYPTED */
 
 /*
  * If memory encryption is supported, phys_to_dma will set the memory encryption
diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
index 4c103a24e380..55c4147bb2b1 100644
--- a/kernel/dma/Kconfig
+++ b/kernel/dma/Kconfig
@@ -51,7 +51,7 @@ config ARCH_HAS_SYNC_DMA_FOR_CPU_ALL
 config ARCH_HAS_DMA_PREP_COHERENT
 	bool
 
-config ARCH_HAS_FORCE_DMA_UNENCRYPTED
+config ARCH_HAS_FORCE_DMA_DECRYPTED
 	bool
 
 config DMA_NONCOHERENT_CACHE_SYNC
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index ac7956c38f69..a0576c0ccacd 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -26,7 +26,7 @@ unsigned int zone_dma_bits __ro_after_init = 24;
 static inline dma_addr_t phys_to_dma_direct(struct device *dev,
 		phys_addr_t phys)
 {
-	if (force_dma_unencrypted(dev))
+	if (force_dma_decrypted(dev))
 		return __phys_to_dma(dev, phys);
 	return phys_to_dma(dev, phys);
 }
@@ -49,7 +49,7 @@ static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
 {
 	u64 dma_limit = min_not_zero(dma_mask, dev->bus_dma_limit);
 
-	if (force_dma_unencrypted(dev))
+	if (force_dma_decrypted(dev))
 		*phys_limit = __dma_to_phys(dev, dma_limit);
 	else
 		*phys_limit = dma_to_phys(dev, dma_limit);
@@ -138,7 +138,7 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
 		return NULL;
 
 	if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
-	    !force_dma_unencrypted(dev)) {
+	    !force_dma_decrypted(dev)) {
 		/* remove any dirty cache lines on the kernel alias */
 		if (!PageHighMem(page))
 			arch_dma_prep_coherent(page, size);
@@ -179,7 +179,7 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
 	}
 
 	ret = page_address(page);
-	if (force_dma_unencrypted(dev))
+	if (force_dma_decrypted(dev))
 		set_memory_decrypted((unsigned long)ret, 1 << get_order(size));
 
 	memset(ret, 0, size);
@@ -190,7 +190,7 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
 		ret = uncached_kernel_address(ret);
 	}
 done:
-	if (force_dma_unencrypted(dev))
+	if (force_dma_decrypted(dev))
 		*dma_handle = __phys_to_dma(dev, page_to_phys(page));
 	else
 		*dma_handle = phys_to_dma(dev, page_to_phys(page));
@@ -203,7 +203,7 @@ void dma_direct_free_pages(struct device *dev, size_t size, void *cpu_addr,
 	unsigned int page_order = get_order(size);
 
 	if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
-	    !force_dma_unencrypted(dev)) {
+	    !force_dma_decrypted(dev)) {
 		/* cpu_addr is a struct page cookie, not a kernel address */
 		dma_free_contiguous(dev, cpu_addr, size);
 		return;
@@ -213,7 +213,7 @@ void dma_direct_free_pages(struct device *dev, size_t size, void *cpu_addr,
 	    dma_free_from_pool(cpu_addr, PAGE_ALIGN(size)))
 		return;
 
-	if (force_dma_unencrypted(dev))
+	if (force_dma_decrypted(dev))
 		set_memory_encrypted((unsigned long)cpu_addr, 1 << page_order);
 
 	if (IS_ENABLED(CONFIG_DMA_REMAP) && is_vmalloc_addr(cpu_addr))
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index 98e3d873792e..dbd0605a39c5 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -154,7 +154,7 @@ EXPORT_SYMBOL(dma_get_sgtable_attrs);
  */
 pgprot_t dma_pgprot(struct device *dev, pgprot_t prot, unsigned long attrs)
 {
-	if (force_dma_unencrypted(dev))
+	if (force_dma_decrypted(dev))
 		prot = pgprot_decrypted(prot);
 	if (dev_is_dma_coherent(dev) ||
 	    (IS_ENABLED(CONFIG_DMA_NONCOHERENT_CACHE_SYNC) &&
-- 
2.21.0


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

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

* Re: [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
  2020-03-19 10:16 ` [PATCH -v2] " Borislav Petkov
@ 2020-03-19 10:20   ` Christoph Hellwig
  2020-03-19 10:28     ` Borislav Petkov
  0 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2020-03-19 10:20 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: linux-s390, Dave Hansen, Vasily Gorbik, linuxppc-dev,
	Peter Zijlstra, Benjamin Herrenschmidt, x86, Heiko Carstens,
	lkml, Christian Borntraeger, iommu, Ingo Molnar, Paul Mackerras,
	Tom Lendacky, Andy Lutomirski, Michael Ellerman, Thomas Gleixner,
	Borislav Petkov, Robin Murphy, Christoph Hellwig

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

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

* Re: [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
  2020-03-19 10:20   ` Christoph Hellwig
@ 2020-03-19 10:28     ` Borislav Petkov
  2020-03-19 11:06       ` Robin Murphy
  0 siblings, 1 reply; 18+ messages in thread
From: Borislav Petkov @ 2020-03-19 10:28 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-s390, Dave Hansen, Vasily Gorbik, linuxppc-dev,
	Peter Zijlstra, Benjamin Herrenschmidt, x86, Heiko Carstens,
	lkml, Christian Borntraeger, iommu, Ingo Molnar, Paul Mackerras,
	Tom Lendacky, Andy Lutomirski, Michael Ellerman, Thomas Gleixner,
	Robin Murphy

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

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

* Re: [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
  2020-03-19 10:28     ` Borislav Petkov
@ 2020-03-19 11:06       ` Robin Murphy
  2020-03-19 11:20         ` Borislav Petkov
  0 siblings, 1 reply; 18+ messages in thread
From: Robin Murphy @ 2020-03-19 11:06 UTC (permalink / raw)
  To: Borislav Petkov, Christoph Hellwig
  Cc: linux-s390, Dave Hansen, Vasily Gorbik, Tom Lendacky,
	Peter Zijlstra, Benjamin Herrenschmidt, x86, Heiko Carstens,
	lkml, Christian Borntraeger, iommu, Ingo Molnar, Paul Mackerras,
	Andy Lutomirski, Michael Ellerman, Thomas Gleixner, linuxppc-dev

[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

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

* Re: [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
  2020-03-19 11:06       ` Robin Murphy
@ 2020-03-19 11:20         ` Borislav Petkov
  2020-03-19 17:25           ` Thomas Gleixner
  0 siblings, 1 reply; 18+ messages in thread
From: Borislav Petkov @ 2020-03-19 11:20 UTC (permalink / raw)
  To: Robin Murphy
  Cc: linux-s390, Dave Hansen, Vasily Gorbik, Tom Lendacky,
	Peter Zijlstra, Michael Ellerman, x86, Heiko Carstens, lkml,
	Christian Borntraeger, iommu, Ingo Molnar, Paul Mackerras,
	Andy Lutomirski, Benjamin Herrenschmidt, Thomas Gleixner,
	linuxppc-dev, Christoph Hellwig

On Thu, Mar 19, 2020 at 11:06:15AM +0000, 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

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

* Re: [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
  2020-03-19 11:20         ` Borislav Petkov
@ 2020-03-19 17:25           ` Thomas Gleixner
  2020-03-19 17:42             ` Borislav Petkov
  2020-03-19 21:59             ` Michal Suchánek
  0 siblings, 2 replies; 18+ messages in thread
From: Thomas Gleixner @ 2020-03-19 17:25 UTC (permalink / raw)
  To: Borislav Petkov, Robin Murphy
  Cc: linux-s390, Dave Hansen, Vasily Gorbik, Tom Lendacky,
	Peter Zijlstra, Michael Ellerman, x86, Heiko Carstens, lkml,
	Christian Borntraeger, iommu, Ingo Molnar, Paul Mackerras,
	Andy Lutomirski, Benjamin Herrenschmidt, linuxppc-dev,
	Christoph Hellwig

Borislav Petkov <bp@alien8.de> writes:

> On Thu, Mar 19, 2020 at 11:06:15AM +0000, 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

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

* Re: [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
  2020-03-19 17:25           ` Thomas Gleixner
@ 2020-03-19 17:42             ` Borislav Petkov
  2020-03-19 23:53               ` Thomas Gleixner
  2020-03-19 21:59             ` Michal Suchánek
  1 sibling, 1 reply; 18+ messages in thread
From: Borislav Petkov @ 2020-03-19 17:42 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-s390, Dave Hansen, Vasily Gorbik, linuxppc-dev,
	Peter Zijlstra, Michael Ellerman, x86, Heiko Carstens, lkml,
	Christian Borntraeger, iommu, Ingo Molnar, Paul Mackerras,
	Andy Lutomirski, Benjamin Herrenschmidt, Tom Lendacky,
	Robin Murphy, Christoph Hellwig

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

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

* Re: [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
  2020-03-19 17:25           ` Thomas Gleixner
  2020-03-19 17:42             ` Borislav Petkov
@ 2020-03-19 21:59             ` Michal Suchánek
  1 sibling, 0 replies; 18+ messages in thread
From: Michal Suchánek @ 2020-03-19 21:59 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Tom Lendacky, x86, Vasily Gorbik, linux-s390, Peter Zijlstra,
	linuxppc-dev, Dave Hansen, Heiko Carstens, lkml,
	Christian Borntraeger, iommu, Ingo Molnar, Borislav Petkov,
	Andy Lutomirski, Paul Mackerras, Robin Murphy, Christoph Hellwig

On Thu, Mar 19, 2020 at 06:25:49PM +0100, Thomas Gleixner wrote:
> Borislav Petkov <bp@alien8.de> writes:
> 
> > On Thu, Mar 19, 2020 at 11:06:15AM +0000, 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

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

* Re: [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
  2020-03-19 17:42             ` Borislav Petkov
@ 2020-03-19 23:53               ` Thomas Gleixner
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Gleixner @ 2020-03-19 23:53 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: linux-s390, Dave Hansen, Vasily Gorbik, linuxppc-dev,
	Peter Zijlstra, Michael Ellerman, x86, Heiko Carstens, lkml,
	Christian Borntraeger, iommu, Ingo Molnar, Paul Mackerras,
	Andy Lutomirski, Benjamin Herrenschmidt, Tom Lendacky,
	Robin Murphy, Christoph Hellwig

Borislav Petkov <bp@alien8.de> 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

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

end of thread, other threads:[~2020-03-19 23:54 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-17 11:18 [PATCH] treewide: Rename "unencrypted" to "decrypted" Borislav Petkov
2020-03-17 20:35 ` Dave Hansen
2020-03-17 21:06   ` Borislav Petkov
2020-03-17 21:24     ` Dave Hansen
2020-03-17 21:31       ` Borislav Petkov
2020-03-17 21:34       ` Joe Perches
2020-03-17 22:01     ` Thomas Gleixner
2020-03-17 23:16 ` kbuild test robot
2020-03-17 23:51 ` kbuild test robot
2020-03-19 10:16 ` [PATCH -v2] " Borislav Petkov
2020-03-19 10:20   ` Christoph Hellwig
2020-03-19 10:28     ` Borislav Petkov
2020-03-19 11:06       ` Robin Murphy
2020-03-19 11:20         ` Borislav Petkov
2020-03-19 17:25           ` Thomas Gleixner
2020-03-19 17:42             ` Borislav Petkov
2020-03-19 23:53               ` Thomas Gleixner
2020-03-19 21:59             ` Michal Suchánek

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