IOMMU Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/3] Remove x86-specific code from generic headers
@ 2019-07-13  4:45 Thiago Jung Bauermann
  2019-07-13  4:45 ` [PATCH 1/3] x86, s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig Thiago Jung Bauermann
                   ` (3 more replies)
  0 siblings, 4 replies; 21+ messages in thread
From: Thiago Jung Bauermann @ 2019-07-13  4:45 UTC (permalink / raw)
  To: x86
  Cc: linux-s390, Konrad Rzeszutek Wilk, Robin Murphy, Mike Anderson,
	Ram Pai, linux-kernel, Alexey Dobriyan, Halil Pasic, iommu,
	Ingo Molnar, Borislav Petkov, Thomas Lendacky, H. Peter Anvin,
	linux-fsdevel, Thomas Gleixner, linuxppc-dev, Christoph Hellwig

Hello,

This version mostly changes patch 2/3, removing dma_check_mask() from
kernel/dma/mapping.c as suggested by Christoph Hellwig, and also adapting
s390's <asm/mem_encrypt.h>. There's also a small change in patch 1/3 as
mentioned in the changelog below.

Patch 3/3 may or may not need to change s390 code depending on how Tom
Lendacky's patch is fixed to avoid breaking that architecture, so I haven't
made any changes for now.

These patches are applied on top of today's master which at the time was at
commit 9787aed57dd3 ("coresight: Make the coresight_device_fwnode_match
declaration's fwnode parameter const"), plus a cherry-pick of commit
e67a5ed1f86f ("dma-direct: Force unencrypted DMA under SME for certain DMA
masks"), which is in dma-mapping/for-next and comes from this patch:

https://lore.kernel.org/linux-iommu/10b83d9ff31bca88e94da2ff34e30619eb396078.1562785123.git.thomas.lendacky@amd.com/

I don't have a way to test SME, SEV, nor s390's PEF so the patches have only
been build tested.

Original cover letter below:

Both powerpc¹ and s390² are adding <asm/mem_encrypt.h> headers. Currently,
they have to supply definitions for functions and macros which only have a
meaning on x86: sme_me_mask, sme_active() and sev_active().

Christoph Hellwig made a suggestion to "clean up the Kconfig and generic
headers bits for memory encryption so that we don't need all this
boilerplate code", and this is what this patch does.

After this patch set, this is powerpc's <asm/mem_encrypt.h>:

    #ifndef _ASM_POWERPC_MEM_ENCRYPT_H
    #define _ASM_POWERPC_MEM_ENCRYPT_H

    #include <asm/svm.h>

    static inline bool mem_encrypt_active(void)
    {
	    return is_secure_guest();
    }

    static inline bool force_dma_unencrypted(struct device *dev)
    {
	    return is_secure_guest();
    }

    int set_memory_encrypted(unsigned long addr, int numpages);
    int set_memory_decrypted(unsigned long addr, int numpages);

    #endif /* _ASM_POWERPC_MEM_ENCRYPT_H */

Changelog

Since v1:

- Patch "x86,s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig"
  - Remove definition of ARCH_HAS_MEM_ENCRYPT from s390/Kconfig as well.
  - Reworded patch title and message a little bit.

- Patch "DMA mapping: Move SME handling to x86-specific files"
  - Adapt s390's <asm/mem_encrypt.h> as well.
  - Remove dma_check_mask() from kernel/dma/mapping.c. Suggested by
    Christoph Hellwig.

-- 

¹ https://lore.kernel.org/linuxppc-dev/20190521044912.1375-12-bauerman@linux.ibm.com/
² https://lore.kernel.org/kvm/20190612111236.99538-2-pasic@linux.ibm.com/

Thiago Jung Bauermann (3):
  x86,s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig
  DMA mapping: Move SME handling to x86-specific files
  fs/core/vmcore: Move sev_active() reference to x86 arch code

 arch/Kconfig                        |  3 +++
 arch/s390/Kconfig                   |  3 ---
 arch/s390/include/asm/mem_encrypt.h |  4 +---
 arch/x86/Kconfig                    |  4 +---
 arch/x86/include/asm/mem_encrypt.h  | 10 ++++++++++
 arch/x86/kernel/crash_dump_64.c     |  5 +++++
 fs/proc/vmcore.c                    |  8 ++++----
 include/linux/crash_dump.h          | 14 ++++++++++++++
 include/linux/mem_encrypt.h         | 15 +--------------
 kernel/dma/mapping.c                |  8 --------
 kernel/dma/swiotlb.c                |  3 +--
 11 files changed, 40 insertions(+), 37 deletions(-)

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

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

* [PATCH 1/3] x86, s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig
  2019-07-13  4:45 [PATCH 0/3] Remove x86-specific code from generic headers Thiago Jung Bauermann
@ 2019-07-13  4:45 ` Thiago Jung Bauermann
  2019-07-13  7:27   ` [PATCH 1/3] x86,s390: " Christoph Hellwig
  2019-07-15 15:23   ` [PATCH 1/3] x86, s390: " janani
  2019-07-13  4:45 ` [PATCH 2/3] DMA mapping: Move SME handling to x86-specific files Thiago Jung Bauermann
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 21+ messages in thread
From: Thiago Jung Bauermann @ 2019-07-13  4:45 UTC (permalink / raw)
  To: x86
  Cc: linux-s390, Konrad Rzeszutek Wilk, Robin Murphy, Mike Anderson,
	Ram Pai, linux-kernel, Alexey Dobriyan, Halil Pasic, iommu,
	Ingo Molnar, Borislav Petkov, Thomas Lendacky, H. Peter Anvin,
	linux-fsdevel, Thomas Gleixner, linuxppc-dev, Christoph Hellwig

powerpc is also going to use this feature, so put it in a generic location.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/Kconfig      | 3 +++
 arch/s390/Kconfig | 3 ---
 arch/x86/Kconfig  | 4 +---
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index c47b328eada0..4ef3499d4480 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -927,6 +927,9 @@ config LOCK_EVENT_COUNTS
 	  the chance of application behavior change because of timing
 	  differences. The counts are reported via debugfs.
 
+config ARCH_HAS_MEM_ENCRYPT
+	bool
+
 source "kernel/gcov/Kconfig"
 
 source "scripts/gcc-plugins/Kconfig"
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 5d8570ed6cab..f820e631bf89 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -1,7 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
-config ARCH_HAS_MEM_ENCRYPT
-        def_bool y
-
 config MMU
 	def_bool y
 
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index c9f331bb538b..5d3295f2df94 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -68,6 +68,7 @@ config X86
 	select ARCH_HAS_FORTIFY_SOURCE
 	select ARCH_HAS_GCOV_PROFILE_ALL
 	select ARCH_HAS_KCOV			if X86_64
+	select ARCH_HAS_MEM_ENCRYPT
 	select ARCH_HAS_MEMBARRIER_SYNC_CORE
 	select ARCH_HAS_PMEM_API		if X86_64
 	select ARCH_HAS_PTE_SPECIAL
@@ -1520,9 +1521,6 @@ config X86_CPA_STATISTICS
 	  helps to determine the effectiveness of preserving large and huge
 	  page mappings when mapping protections are changed.
 
-config ARCH_HAS_MEM_ENCRYPT
-	def_bool y
-
 config AMD_MEM_ENCRYPT
 	bool "AMD Secure Memory Encryption (SME) support"
 	depends on X86_64 && CPU_SUP_AMD

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

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

* [PATCH 2/3] DMA mapping: Move SME handling to x86-specific files
  2019-07-13  4:45 [PATCH 0/3] Remove x86-specific code from generic headers Thiago Jung Bauermann
  2019-07-13  4:45 ` [PATCH 1/3] x86, s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig Thiago Jung Bauermann
@ 2019-07-13  4:45 ` Thiago Jung Bauermann
  2019-07-13  7:29   ` Christoph Hellwig
  2019-07-13  4:45 ` [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code Thiago Jung Bauermann
  2019-07-13  5:08 ` [PATCH 0/3] Remove x86-specific code from generic headers Thiago Jung Bauermann
  3 siblings, 1 reply; 21+ messages in thread
From: Thiago Jung Bauermann @ 2019-07-13  4:45 UTC (permalink / raw)
  To: x86
  Cc: linux-s390, Konrad Rzeszutek Wilk, Robin Murphy, Mike Anderson,
	Ram Pai, linux-kernel, Alexey Dobriyan, Halil Pasic, iommu,
	Ingo Molnar, Borislav Petkov, Thomas Lendacky, H. Peter Anvin,
	linux-fsdevel, Thomas Gleixner, linuxppc-dev, Christoph Hellwig

Secure Memory Encryption is an x86-specific feature, so it shouldn't appear
in generic kernel code.

In DMA mapping code, Christoph Hellwig mentioned that "There is no reason
why we should have a special debug printk just for one specific reason why
there is a requirement for a large DMA mask.", so we just remove
dma_check_mask().

In SWIOTLB code, there's no need to mention which memory encryption feature
is active, so just use a more generic warning. Besides, other architectures
will have different names for similar technology.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
 arch/s390/include/asm/mem_encrypt.h |  4 +---
 arch/x86/include/asm/mem_encrypt.h  | 10 ++++++++++
 include/linux/mem_encrypt.h         | 14 +-------------
 kernel/dma/mapping.c                |  8 --------
 kernel/dma/swiotlb.c                |  3 +--
 5 files changed, 13 insertions(+), 26 deletions(-)

diff --git a/arch/s390/include/asm/mem_encrypt.h b/arch/s390/include/asm/mem_encrypt.h
index 3eb018508190..ff813a56bc30 100644
--- a/arch/s390/include/asm/mem_encrypt.h
+++ b/arch/s390/include/asm/mem_encrypt.h
@@ -4,9 +4,7 @@
 
 #ifndef __ASSEMBLY__
 
-#define sme_me_mask	0ULL
-
-static inline bool sme_active(void) { return false; }
+static inline bool mem_encrypt_active(void) { return false; }
 extern bool sev_active(void);
 
 int set_memory_encrypted(unsigned long addr, int numpages);
diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
index 0c196c47d621..848ce43b9040 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -92,6 +92,16 @@ early_set_memory_encrypted(unsigned long vaddr, unsigned long size) { return 0;
 
 extern char __start_bss_decrypted[], __end_bss_decrypted[], __start_bss_decrypted_unused[];
 
+static inline bool mem_encrypt_active(void)
+{
+	return sme_me_mask;
+}
+
+static inline u64 sme_get_me_mask(void)
+{
+	return sme_me_mask;
+}
+
 #endif	/* __ASSEMBLY__ */
 
 #endif	/* __X86_MEM_ENCRYPT_H__ */
diff --git a/include/linux/mem_encrypt.h b/include/linux/mem_encrypt.h
index 470bd53a89df..0c5b0ff9eb29 100644
--- a/include/linux/mem_encrypt.h
+++ b/include/linux/mem_encrypt.h
@@ -18,23 +18,11 @@
 
 #else	/* !CONFIG_ARCH_HAS_MEM_ENCRYPT */
 
-#define sme_me_mask	0ULL
-
-static inline bool sme_active(void) { return false; }
+static inline bool mem_encrypt_active(void) { return false; }
 static inline bool sev_active(void) { return false; }
 
 #endif	/* CONFIG_ARCH_HAS_MEM_ENCRYPT */
 
-static inline bool mem_encrypt_active(void)
-{
-	return sme_me_mask;
-}
-
-static inline u64 sme_get_me_mask(void)
-{
-	return sme_me_mask;
-}
-
 #ifdef CONFIG_AMD_MEM_ENCRYPT
 /*
  * The __sme_set() and __sme_clr() macros are useful for adding or removing
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index f7afdadb6770..b53fc7ec4914 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -291,12 +291,6 @@ void dma_free_attrs(struct device *dev, size_t size, void *cpu_addr,
 }
 EXPORT_SYMBOL(dma_free_attrs);
 
-static inline void dma_check_mask(struct device *dev, u64 mask)
-{
-	if (sme_active() && (mask < (((u64)sme_get_me_mask() << 1) - 1)))
-		dev_warn(dev, "SME is active, device will require DMA bounce buffers\n");
-}
-
 int dma_supported(struct device *dev, u64 mask)
 {
 	const struct dma_map_ops *ops = get_dma_ops(dev);
@@ -321,7 +315,6 @@ int dma_set_mask(struct device *dev, u64 mask)
 		return -EIO;
 
 	arch_dma_set_mask(dev, mask);
-	dma_check_mask(dev, mask);
 	*dev->dma_mask = mask;
 	return 0;
 }
@@ -333,7 +326,6 @@ int dma_set_coherent_mask(struct device *dev, u64 mask)
 	if (!dma_supported(dev, mask))
 		return -EIO;
 
-	dma_check_mask(dev, mask);
 	dev->coherent_dma_mask = mask;
 	return 0;
 }
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 62fa5a82a065..e52401f94e91 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -459,8 +459,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *hwdev,
 		panic("Can not allocate SWIOTLB buffer earlier and can't now provide you with the DMA bounce buffer");
 
 	if (mem_encrypt_active())
-		pr_warn_once("%s is active and system is using DMA bounce buffers\n",
-			     sme_active() ? "SME" : "SEV");
+		pr_warn_once("Memory encryption is active and system is using DMA bounce buffers\n");
 
 	mask = dma_get_seg_boundary(hwdev);
 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
  2019-07-13  4:45 [PATCH 0/3] Remove x86-specific code from generic headers Thiago Jung Bauermann
  2019-07-13  4:45 ` [PATCH 1/3] x86, s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig Thiago Jung Bauermann
  2019-07-13  4:45 ` [PATCH 2/3] DMA mapping: Move SME handling to x86-specific files Thiago Jung Bauermann
@ 2019-07-13  4:45 ` Thiago Jung Bauermann
  2019-07-13  5:08 ` [PATCH 0/3] Remove x86-specific code from generic headers Thiago Jung Bauermann
  3 siblings, 0 replies; 21+ messages in thread
From: Thiago Jung Bauermann @ 2019-07-13  4:45 UTC (permalink / raw)
  To: x86
  Cc: linux-s390, Konrad Rzeszutek Wilk, Robin Murphy, Mike Anderson,
	Ram Pai, linux-kernel, Alexey Dobriyan, Halil Pasic, iommu,
	Ingo Molnar, Borislav Petkov, Thomas Lendacky, H. Peter Anvin,
	linux-fsdevel, Thomas Gleixner, linuxppc-dev, Christoph Hellwig

Secure Encrypted Virtualization is an x86-specific feature, so it shouldn't
appear in generic kernel code because it forces non-x86 architectures to
define the sev_active() function, which doesn't make a lot of sense.

To solve this problem, add an x86 elfcorehdr_read() function to override
the generic weak implementation. To do that, it's necessary to make
read_from_oldmem() public so that it can be used outside of vmcore.c.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
 arch/x86/kernel/crash_dump_64.c |  5 +++++
 fs/proc/vmcore.c                |  8 ++++----
 include/linux/crash_dump.h      | 14 ++++++++++++++
 include/linux/mem_encrypt.h     |  1 -
 4 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/crash_dump_64.c b/arch/x86/kernel/crash_dump_64.c
index 22369dd5de3b..045e82e8945b 100644
--- a/arch/x86/kernel/crash_dump_64.c
+++ b/arch/x86/kernel/crash_dump_64.c
@@ -70,3 +70,8 @@ ssize_t copy_oldmem_page_encrypted(unsigned long pfn, char *buf, size_t csize,
 {
 	return __copy_oldmem_page(pfn, buf, csize, offset, userbuf, true);
 }
+
+ssize_t elfcorehdr_read(char *buf, size_t count, u64 *ppos)
+{
+	return read_from_oldmem(buf, count, ppos, 0, sev_active());
+}
diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index 57957c91c6df..ca1f20bedd8c 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -100,9 +100,9 @@ static int pfn_is_ram(unsigned long pfn)
 }
 
 /* Reads a page from the oldmem device from given offset. */
-static ssize_t read_from_oldmem(char *buf, size_t count,
-				u64 *ppos, int userbuf,
-				bool encrypted)
+ssize_t read_from_oldmem(char *buf, size_t count,
+			 u64 *ppos, int userbuf,
+			 bool encrypted)
 {
 	unsigned long pfn, offset;
 	size_t nr_bytes;
@@ -166,7 +166,7 @@ void __weak elfcorehdr_free(unsigned long long addr)
  */
 ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 *ppos)
 {
-	return read_from_oldmem(buf, count, ppos, 0, sev_active());
+	return read_from_oldmem(buf, count, ppos, 0, false);
 }
 
 /*
diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
index f774c5eb9e3c..4664fc1871de 100644
--- a/include/linux/crash_dump.h
+++ b/include/linux/crash_dump.h
@@ -115,4 +115,18 @@ static inline int vmcore_add_device_dump(struct vmcoredd_data *data)
 	return -EOPNOTSUPP;
 }
 #endif /* CONFIG_PROC_VMCORE_DEVICE_DUMP */
+
+#ifdef CONFIG_PROC_VMCORE
+ssize_t read_from_oldmem(char *buf, size_t count,
+			 u64 *ppos, int userbuf,
+			 bool encrypted);
+#else
+static inline ssize_t read_from_oldmem(char *buf, size_t count,
+				       u64 *ppos, int userbuf,
+				       bool encrypted)
+{
+	return -EOPNOTSUPP;
+}
+#endif /* CONFIG_PROC_VMCORE */
+
 #endif /* LINUX_CRASHDUMP_H */
diff --git a/include/linux/mem_encrypt.h b/include/linux/mem_encrypt.h
index 0c5b0ff9eb29..5c4a18a91f89 100644
--- a/include/linux/mem_encrypt.h
+++ b/include/linux/mem_encrypt.h
@@ -19,7 +19,6 @@
 #else	/* !CONFIG_ARCH_HAS_MEM_ENCRYPT */
 
 static inline bool mem_encrypt_active(void) { return false; }
-static inline bool sev_active(void) { return false; }
 
 #endif	/* CONFIG_ARCH_HAS_MEM_ENCRYPT */
 

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

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

* Re: [PATCH 0/3] Remove x86-specific code from generic headers
  2019-07-13  4:45 [PATCH 0/3] Remove x86-specific code from generic headers Thiago Jung Bauermann
                   ` (2 preceding siblings ...)
  2019-07-13  4:45 ` [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code Thiago Jung Bauermann
@ 2019-07-13  5:08 ` Thiago Jung Bauermann
  3 siblings, 0 replies; 21+ messages in thread
From: Thiago Jung Bauermann @ 2019-07-13  5:08 UTC (permalink / raw)
  To: x86
  Cc: linux-s390, Konrad Rzeszutek Wilk, Robin Murphy, Mike Anderson,
	Ram Pai, linux-kernel, Alexey Dobriyan, Halil Pasic, iommu,
	Ingo Molnar, Borislav Petkov, Thomas Lendacky, H. Peter Anvin,
	linux-fsdevel, Thomas Gleixner, linuxppc-dev, Christoph Hellwig


I forgot to mark this series as v2 when generating the patches.
Sorry for the confusion.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH 1/3] x86,s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig
  2019-07-13  4:45 ` [PATCH 1/3] x86, s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig Thiago Jung Bauermann
@ 2019-07-13  7:27   ` " Christoph Hellwig
  2019-07-15 15:23   ` [PATCH 1/3] x86, s390: " janani
  1 sibling, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2019-07-13  7:27 UTC (permalink / raw)
  To: Thiago Jung Bauermann
  Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
	x86, Ram Pai, linux-kernel, Alexey Dobriyan, Halil Pasic, iommu,
	Ingo Molnar, Borislav Petkov, Thomas Lendacky, H. Peter Anvin,
	linux-fsdevel, Thomas Gleixner, linuxppc-dev, Christoph Hellwig

On Sat, Jul 13, 2019 at 01:45:52AM -0300, Thiago Jung Bauermann wrote:
> powerpc is also going to use this feature, so put it in a generic location.

Looks good,

even without a third arch using it we should never habe symbols defined
under arch/$(ARCH) that are used in common code to start with.

Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH 2/3] DMA mapping: Move SME handling to x86-specific files
  2019-07-13  4:45 ` [PATCH 2/3] DMA mapping: Move SME handling to x86-specific files Thiago Jung Bauermann
@ 2019-07-13  7:29   ` Christoph Hellwig
  0 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2019-07-13  7:29 UTC (permalink / raw)
  To: Thiago Jung Bauermann
  Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
	x86, Ram Pai, linux-kernel, Alexey Dobriyan, Halil Pasic, iommu,
	Ingo Molnar, Borislav Petkov, Thomas Lendacky, H. Peter Anvin,
	linux-fsdevel, Thomas Gleixner, linuxppc-dev, Christoph Hellwig

While this looks generally good to me, I think we want to split this
into three patches:

 1) update the swiotlb printk
 2) removing the dma-mapping check and printk
 3) clean up the mem_encrypt.h interface.

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

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

* Re: [PATCH 1/3] x86, s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig
  2019-07-13  4:45 ` [PATCH 1/3] x86, s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig Thiago Jung Bauermann
  2019-07-13  7:27   ` [PATCH 1/3] x86,s390: " Christoph Hellwig
@ 2019-07-15 15:23   ` " janani
  2019-07-15 20:00     ` Thiago Jung Bauermann
  1 sibling, 1 reply; 21+ messages in thread
From: janani @ 2019-07-15 15:23 UTC (permalink / raw)
  To: Thiago Jung Bauermann
  Cc: linux-s390, x86, Thomas Gleixner, Linuxppc-dev,
	Konrad Rzeszutek Wilk, linuxppc-dev, Mike Anderson, Ram Pai,
	linux-kernel, Christoph Hellwig, Halil Pasic, iommu, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, linux-fsdevel, Thomas Lendacky,
	Robin Murphy, Alexey Dobriyan

On 2019-07-12 23:45, Thiago Jung Bauermann wrote:
> powerpc is also going to use this feature, so put it in a generic 
> location.
> 
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  arch/Kconfig      | 3 +++
>  arch/s390/Kconfig | 3 ---
>  arch/x86/Kconfig  | 4 +---
>  3 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/Kconfig b/arch/Kconfig
> index c47b328eada0..4ef3499d4480 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -927,6 +927,9 @@ config LOCK_EVENT_COUNTS
>  	  the chance of application behavior change because of timing
>  	  differences. The counts are reported via debugfs.
> 
> +config ARCH_HAS_MEM_ENCRYPT
> +	bool
> +
>  source "kernel/gcov/Kconfig"
> 
>  source "scripts/gcc-plugins/Kconfig"
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 5d8570ed6cab..f820e631bf89 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -1,7 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0
> -config ARCH_HAS_MEM_ENCRYPT
> -        def_bool y
> -

  Since you are removing the "def_bool y" when ARCH_HAS_MEM_ENCRYPT is 
moved to arch/Kconfig, does the s390/Kconfig need "select 
ARCH_HAS_MEM_ENCRYPT" added like you do for x86/Kconfig?

  - Janani

>  config MMU
>  	def_bool y
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index c9f331bb538b..5d3295f2df94 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -68,6 +68,7 @@ config X86
>  	select ARCH_HAS_FORTIFY_SOURCE
>  	select ARCH_HAS_GCOV_PROFILE_ALL
>  	select ARCH_HAS_KCOV			if X86_64
> +	select ARCH_HAS_MEM_ENCRYPT
>  	select ARCH_HAS_MEMBARRIER_SYNC_CORE
>  	select ARCH_HAS_PMEM_API		if X86_64
>  	select ARCH_HAS_PTE_SPECIAL
> @@ -1520,9 +1521,6 @@ config X86_CPA_STATISTICS
>  	  helps to determine the effectiveness of preserving large and huge
>  	  page mappings when mapping protections are changed.
> 
> -config ARCH_HAS_MEM_ENCRYPT
> -	def_bool y
> -
>  config AMD_MEM_ENCRYPT
>  	bool "AMD Secure Memory Encryption (SME) support"
>  	depends on X86_64 && CPU_SUP_AMD
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH 1/3] x86, s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig
  2019-07-15 15:23   ` [PATCH 1/3] x86, s390: " janani
@ 2019-07-15 20:00     ` Thiago Jung Bauermann
  0 siblings, 0 replies; 21+ messages in thread
From: Thiago Jung Bauermann @ 2019-07-15 20:00 UTC (permalink / raw)
  To: janani
  Cc: linux-s390, x86, Thomas Gleixner, Linuxppc-dev,
	Konrad Rzeszutek Wilk, linuxppc-dev, Mike Anderson, Ram Pai,
	linux-kernel, Christoph Hellwig, Halil Pasic, iommu, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, linux-fsdevel, Thomas Lendacky,
	Robin Murphy, Alexey Dobriyan


Hello Janani,

Thanks for reviewing the patch.

janani <janani@linux.ibm.com> writes:

> On 2019-07-12 23:45, Thiago Jung Bauermann wrote:
>> powerpc is also going to use this feature, so put it in a generic location.
>>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
>> ---
>>  arch/Kconfig      | 3 +++
>>  arch/s390/Kconfig | 3 ---
>>  arch/x86/Kconfig  | 4 +---
>>  3 files changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/Kconfig b/arch/Kconfig
>> index c47b328eada0..4ef3499d4480 100644
>> --- a/arch/Kconfig
>> +++ b/arch/Kconfig
>> @@ -927,6 +927,9 @@ config LOCK_EVENT_COUNTS
>>  	  the chance of application behavior change because of timing
>>  	  differences. The counts are reported via debugfs.
>>
>> +config ARCH_HAS_MEM_ENCRYPT
>> +	bool
>> +
>>  source "kernel/gcov/Kconfig"
>>
>>  source "scripts/gcc-plugins/Kconfig"
>> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
>> index 5d8570ed6cab..f820e631bf89 100644
>> --- a/arch/s390/Kconfig
>> +++ b/arch/s390/Kconfig
>> @@ -1,7 +1,4 @@
>>  # SPDX-License-Identifier: GPL-2.0
>> -config ARCH_HAS_MEM_ENCRYPT
>> -        def_bool y
>> -
>
>  Since you are removing the "def_bool y" when ARCH_HAS_MEM_ENCRYPT is moved to
> arch/Kconfig, does the s390/Kconfig need "select ARCH_HAS_MEM_ENCRYPT" added
> like you do for x86/Kconfig?

Indeed, I missed that. Thanks for spotting it!

>
>  - Janani
>
>>  config MMU
>>  	def_bool y
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index c9f331bb538b..5d3295f2df94 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -68,6 +68,7 @@ config X86
>>  	select ARCH_HAS_FORTIFY_SOURCE
>>  	select ARCH_HAS_GCOV_PROFILE_ALL
>>  	select ARCH_HAS_KCOV			if X86_64
>> +	select ARCH_HAS_MEM_ENCRYPT
>>  	select ARCH_HAS_MEMBARRIER_SYNC_CORE
>>  	select ARCH_HAS_PMEM_API		if X86_64
>>  	select ARCH_HAS_PTE_SPECIAL
>> @@ -1520,9 +1521,6 @@ config X86_CPA_STATISTICS
>>  	  helps to determine the effectiveness of preserving large and huge
>>  	  page mappings when mapping protections are changed.
>>
>> -config ARCH_HAS_MEM_ENCRYPT
>> -	def_bool y
>> -
>>  config AMD_MEM_ENCRYPT
>>  	bool "AMD Secure Memory Encryption (SME) support"
>>  	depends on X86_64 && CPU_SUP_AMD


-- 
Thiago Jung Bauermann
IBM Linux Technology Center

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

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

* Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
  2019-07-15 14:30         ` Christoph Hellwig
  2019-07-15 15:44           ` Lendacky, Thomas
@ 2019-07-15 20:14           ` Thiago Jung Bauermann
  1 sibling, 0 replies; 21+ messages in thread
From: Thiago Jung Bauermann @ 2019-07-15 20:14 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-s390, Mike Anderson, Janosch Frank, Konrad Rzeszutek Wilk,
	Robin Murphy, x86, Ram Pai, linux-kernel, Halil Pasic, iommu,
	Ingo Molnar, Borislav Petkov, Lendacky, Thomas, H. Peter Anvin,
	linux-fsdevel, Thomas Gleixner, linuxppc-dev, Alexey Dobriyan


Christoph Hellwig <hch@lst.de> writes:

> On Mon, Jul 15, 2019 at 04:03:17PM +0200, Halil Pasic wrote:
>> > I thought about that but couldn't put my finger on a general concept.
>> > Is it "guest with memory inaccessible to the host"?
>> >
>>
>> Well, force_dma_unencrypted() is a much better name thatn sev_active():
>> s390 has no AMD SEV, that is sure, but for virtio to work we do need to
>> make our dma accessible to the hypervisor. Yes, your "guest with memory
>> inaccessible to the host" shows into the right direction IMHO.
>> Unfortunately I don't have too many cycles to spend on this right now.
>
> In x86 it means that we need to remove dma encryption using
> set_memory_decrypted before using it for DMA purposes.  In the SEV
> case that seems to be so that the hypervisor can access it, in the SME
> case that Tom just fixes it is because there is an encrypted bit set
> in the physical address, and if the device doesn't support a large
> enough DMA address the direct mapping code has to encrypt the pages
> used for the contigous allocation.
>
>> Being on cc for your patch made me realize that things got broken on
>> s390. Thanks! I've sent out a patch that fixes protvirt, but we are going
>> to benefit from your cleanups. I think with your cleanups and that patch
>> of mine both sev_active() and sme_active() can be removed. Feel free to
>> do so. If not, I can attend to it as well.
>
> Yes, I think with the dma-mapping fix and this series sme_active and
> sev_active should be gone from common code.  We should also be able
> to remove the exports x86 has for them.
>
> I'll wait a few days and will then feed the dma-mapping fix to Linus,
> it might make sense to either rebase Thiagos series on top of the
> dma-mapping for-next branch, or wait a few days before reposting.

I'll rebase on top of dma-mapping/for-next and do the break up of patch
2 that you mentioned as well.

--
Thiago Jung Bauermann
IBM Linux Technology Center

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

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

* Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
  2019-07-15 14:30         ` Christoph Hellwig
@ 2019-07-15 15:44           ` Lendacky, Thomas
  2019-07-15 20:14           ` Thiago Jung Bauermann
  1 sibling, 0 replies; 21+ messages in thread
From: Lendacky, Thomas @ 2019-07-15 15:44 UTC (permalink / raw)
  To: Christoph Hellwig, Halil Pasic
  Cc: linux-s390, Mike Anderson, Janosch Frank, Konrad Rzeszutek Wilk,
	Robin Murphy, x86, Ram Pai, linux-kernel, iommu, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, linux-fsdevel, Thomas Gleixner,
	linuxppc-dev, Alexey Dobriyan

On 7/15/19 9:30 AM, Christoph Hellwig wrote:
> On Mon, Jul 15, 2019 at 04:03:17PM +0200, Halil Pasic wrote:
>>> I thought about that but couldn't put my finger on a general concept.
>>> Is it "guest with memory inaccessible to the host"?
>>>
>>
>> Well, force_dma_unencrypted() is a much better name thatn sev_active():
>> s390 has no AMD SEV, that is sure, but for virtio to work we do need to
>> make our dma accessible to the hypervisor. Yes, your "guest with memory
>> inaccessible to the host" shows into the right direction IMHO.
>> Unfortunately I don't have too many cycles to spend on this right now.
> 
> In x86 it means that we need to remove dma encryption using
> set_memory_decrypted before using it for DMA purposes.  In the SEV
> case that seems to be so that the hypervisor can access it, in the SME
> case that Tom just fixes it is because there is an encrypted bit set
> in the physical address, and if the device doesn't support a large
> enough DMA address the direct mapping code has to encrypt the pages
> used for the contigous allocation.

Just a correction/clarification...

For SME, when a device doesn't support a large enough DMA address to
accommodate the encryption bit as part of the DMA address, the direct
mapping code has to provide un-encrypted pages. For un-encrypted pages,
the DMA address now does not include the encryption bit, making it
acceptable to the device. Since the device is now using a DMA address
without the encryption bit, the physical address in the CPU page table
must match (the call to set_memory_decrypted) so that both the device and
the CPU interact in the same way with the memory.

Thanks,
Tom

> 
>> Being on cc for your patch made me realize that things got broken on
>> s390. Thanks! I've sent out a patch that fixes protvirt, but we are going
>> to benefit from your cleanups. I think with your cleanups and that patch
>> of mine both sev_active() and sme_active() can be removed. Feel free to
>> do so. If not, I can attend to it as well.
> 
> Yes, I think with the dma-mapping fix and this series sme_active and
> sev_active should be gone from common code.  We should also be able
> to remove the exports x86 has for them.
> 
> I'll wait a few days and will then feed the dma-mapping fix to Linus,
> it might make sense to either rebase Thiagos series on top of the
> dma-mapping for-next branch, or wait a few days before reposting.
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
  2019-07-15 14:03       ` Halil Pasic
@ 2019-07-15 14:30         ` Christoph Hellwig
  2019-07-15 15:44           ` Lendacky, Thomas
  2019-07-15 20:14           ` Thiago Jung Bauermann
  0 siblings, 2 replies; 21+ messages in thread
From: Christoph Hellwig @ 2019-07-15 14:30 UTC (permalink / raw)
  To: Halil Pasic
  Cc: linux-s390, Mike Anderson, Janosch Frank, Konrad Rzeszutek Wilk,
	Robin Murphy, x86, Ram Pai, linux-kernel, Alexey Dobriyan, iommu,
	Ingo Molnar, Borislav Petkov, Lendacky, Thomas, H. Peter Anvin,
	linux-fsdevel, Thomas Gleixner, linuxppc-dev, Christoph Hellwig

On Mon, Jul 15, 2019 at 04:03:17PM +0200, Halil Pasic wrote:
> > I thought about that but couldn't put my finger on a general concept.
> > Is it "guest with memory inaccessible to the host"?
> > 
> 
> Well, force_dma_unencrypted() is a much better name thatn sev_active():
> s390 has no AMD SEV, that is sure, but for virtio to work we do need to
> make our dma accessible to the hypervisor. Yes, your "guest with memory
> inaccessible to the host" shows into the right direction IMHO.
> Unfortunately I don't have too many cycles to spend on this right now.

In x86 it means that we need to remove dma encryption using
set_memory_decrypted before using it for DMA purposes.  In the SEV
case that seems to be so that the hypervisor can access it, in the SME
case that Tom just fixes it is because there is an encrypted bit set
in the physical address, and if the device doesn't support a large
enough DMA address the direct mapping code has to encrypt the pages
used for the contigous allocation.

> Being on cc for your patch made me realize that things got broken on
> s390. Thanks! I've sent out a patch that fixes protvirt, but we are going
> to benefit from your cleanups. I think with your cleanups and that patch
> of mine both sev_active() and sme_active() can be removed. Feel free to
> do so. If not, I can attend to it as well.

Yes, I think with the dma-mapping fix and this series sme_active and
sev_active should be gone from common code.  We should also be able
to remove the exports x86 has for them.

I'll wait a few days and will then feed the dma-mapping fix to Linus,
it might make sense to either rebase Thiagos series on top of the
dma-mapping for-next branch, or wait a few days before reposting.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
  2019-07-12 21:55     ` Thiago Jung Bauermann
@ 2019-07-15 14:03       ` Halil Pasic
  2019-07-15 14:30         ` Christoph Hellwig
  0 siblings, 1 reply; 21+ messages in thread
From: Halil Pasic @ 2019-07-15 14:03 UTC (permalink / raw)
  To: Thiago Jung Bauermann, Janosch Frank
  Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
	x86, Ram Pai, linux-kernel, Alexey Dobriyan, iommu, Ingo Molnar,
	Borislav Petkov, Lendacky, Thomas, H. Peter Anvin, linux-fsdevel,
	Thomas Gleixner, linuxppc-dev, Christoph Hellwig

On Fri, 12 Jul 2019 18:55:47 -0300
Thiago Jung Bauermann <bauerman@linux.ibm.com> wrote:

> 
> [ Cc'ing Tom Lendacky which I forgot to do earlier. Sorry about that. ]
> 
> Hello Halil,
> 
> Thanks for the quick review.
> 
> Halil Pasic <pasic@linux.ibm.com> writes:
> 
> > On Fri, 12 Jul 2019 02:36:31 -0300
> > Thiago Jung Bauermann <bauerman@linux.ibm.com> wrote:
> >
> >> Secure Encrypted Virtualization is an x86-specific feature, so it shouldn't
> >> appear in generic kernel code because it forces non-x86 architectures to
> >> define the sev_active() function, which doesn't make a lot of sense.
> >
> > sev_active() might be just bad (too specific) name for a general
> > concept. s390 code defines it drives the right behavior in
> > kernel/dma/direct.c (which uses it).
> 
> I thought about that but couldn't put my finger on a general concept.
> Is it "guest with memory inaccessible to the host"?
> 

Well, force_dma_unencrypted() is a much better name thatn sev_active():
s390 has no AMD SEV, that is sure, but for virtio to work we do need to
make our dma accessible to the hypervisor. Yes, your "guest with memory
inaccessible to the host" shows into the right direction IMHO.
Unfortunately I don't have too many cycles to spend on this right now.

> Since your proposed definiton for force_dma_unencrypted() is simply to
> make it equivalent to sev_active(), I thought it was more
> straightforward to make each arch define force_dma_unencrypted()
> directly.

I did not mean to propose equivalence. I intended to say the name
sev_active() is not suitable for a common concept. On the other hand
we do have a common concept -- as common code needs to do or not do
things depending on whether "memory is protected/encrypted" or not. I'm
fine with the name force_dma_unencrypted(), especially because I don't
have a better name.

> 
> Also, does sev_active() drive the right behavior for s390 in
> elfcorehdr_read() as well?
> 

AFAIU, since s390 does not override it boils down to the same, whether
sev_active() returns true or false. I'm no expert in that area, but I
strongly hope that is the right behavior. @Janosch: can you help me
out with this one?

> >> To solve this problem, add an x86 elfcorehdr_read() function to override
> >> the generic weak implementation. To do that, it's necessary to make
> >> read_from_oldmem() public so that it can be used outside of vmcore.c.
> >>
> >> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> >> ---
> >>  arch/x86/kernel/crash_dump_64.c |  5 +++++
> >>  fs/proc/vmcore.c                |  8 ++++----
> >>  include/linux/crash_dump.h      | 14 ++++++++++++++
> >>  include/linux/mem_encrypt.h     |  1 -
> >>  4 files changed, 23 insertions(+), 5 deletions(-)
> >
> > Does not seem to apply to today's or yesterdays master.
> 
> It assumes the presence of the two patches I mentioned in the cover
> letter. Only one of them is in master.
> 
> I hadn't realized the s390 virtio patches were on their way to upstream.
> I was keeping an eye on the email thread but didn't see they were picked
> up in the s390 pull request. I'll add a new patch to this series making
> the corresponding changes to s390's <asm/mem_encrypt.h> as well.
> 

Being on cc for your patch made me realize that things got broken on
s390. Thanks! I've sent out a patch that fixes protvirt, but we are going
to benefit from your cleanups. I think with your cleanups and that patch
of mine both sev_active() and sme_active() can be removed. Feel free to
do so. If not, I can attend to it as well.

Regards,
Halil

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

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

* Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
  2019-07-12 15:42           ` Halil Pasic
@ 2019-07-13  8:08             ` Christoph Hellwig
  0 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2019-07-13  8:08 UTC (permalink / raw)
  To: Halil Pasic
  Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
	x86, Ram Pai, linux-kernel, Alexey Dobriyan, iommu, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, linux-fsdevel, Thomas Gleixner,
	linuxppc-dev, Christoph Hellwig

On Fri, Jul 12, 2019 at 05:42:49PM +0200, Halil Pasic wrote:
> 
> Will do! I guess I should do the patch against the for-next branch of the
> dma-mapping tree. But that branch does not have the s390 support patches (yet?).
> To fix it I need both e67a5ed1f86f and 64e1f0c531d1 "s390/mm: force
> swiotlb for protected virtualization" (Halil Pasic, 2018-09-13). Or
> should I wait for e67a5ed1f86f landing in mainline?

I've rebased the dma-mapping for-next branch to latest mainline as of
today that has both commits.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
  2019-07-12 13:09   ` Halil Pasic
  2019-07-12 14:08     ` Christoph Hellwig
@ 2019-07-12 21:55     ` Thiago Jung Bauermann
  2019-07-15 14:03       ` Halil Pasic
  1 sibling, 1 reply; 21+ messages in thread
From: Thiago Jung Bauermann @ 2019-07-12 21:55 UTC (permalink / raw)
  To: Halil Pasic
  Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
	x86, Ram Pai, linux-kernel, Alexey Dobriyan, iommu, Ingo Molnar,
	Borislav Petkov, Lendacky, Thomas, H. Peter Anvin, linux-fsdevel,
	Thomas Gleixner, linuxppc-dev, Christoph Hellwig


[ Cc'ing Tom Lendacky which I forgot to do earlier. Sorry about that. ]

Hello Halil,

Thanks for the quick review.

Halil Pasic <pasic@linux.ibm.com> writes:

> On Fri, 12 Jul 2019 02:36:31 -0300
> Thiago Jung Bauermann <bauerman@linux.ibm.com> wrote:
>
>> Secure Encrypted Virtualization is an x86-specific feature, so it shouldn't
>> appear in generic kernel code because it forces non-x86 architectures to
>> define the sev_active() function, which doesn't make a lot of sense.
>
> sev_active() might be just bad (too specific) name for a general
> concept. s390 code defines it drives the right behavior in
> kernel/dma/direct.c (which uses it).

I thought about that but couldn't put my finger on a general concept.
Is it "guest with memory inaccessible to the host"?

Since your proposed definiton for force_dma_unencrypted() is simply to
make it equivalent to sev_active(), I thought it was more
straightforward to make each arch define force_dma_unencrypted()
directly.

Also, does sev_active() drive the right behavior for s390 in
elfcorehdr_read() as well?

>> To solve this problem, add an x86 elfcorehdr_read() function to override
>> the generic weak implementation. To do that, it's necessary to make
>> read_from_oldmem() public so that it can be used outside of vmcore.c.
>>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>> ---
>>  arch/x86/kernel/crash_dump_64.c |  5 +++++
>>  fs/proc/vmcore.c                |  8 ++++----
>>  include/linux/crash_dump.h      | 14 ++++++++++++++
>>  include/linux/mem_encrypt.h     |  1 -
>>  4 files changed, 23 insertions(+), 5 deletions(-)
>
> Does not seem to apply to today's or yesterdays master.

It assumes the presence of the two patches I mentioned in the cover
letter. Only one of them is in master.

I hadn't realized the s390 virtio patches were on their way to upstream.
I was keeping an eye on the email thread but didn't see they were picked
up in the s390 pull request. I'll add a new patch to this series making
the corresponding changes to s390's <asm/mem_encrypt.h> as well.

--
Thiago Jung Bauermann
IBM Linux Technology Center

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

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

* Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
  2019-07-12 15:11         ` Christoph Hellwig
@ 2019-07-12 15:42           ` Halil Pasic
  2019-07-13  8:08             ` Christoph Hellwig
  0 siblings, 1 reply; 21+ messages in thread
From: Halil Pasic @ 2019-07-12 15:42 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
	x86, Ram Pai, linux-kernel, iommu, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, linux-fsdevel, Thomas Gleixner, linuxppc-dev,
	Alexey Dobriyan

On Fri, 12 Jul 2019 17:11:29 +0200
Christoph Hellwig <hch@lst.de> wrote:

> On Fri, Jul 12, 2019 at 04:51:53PM +0200, Halil Pasic wrote:
> > Thank you very much! I will have another look, but it seems to me,
> > without further measures taken, this would break protected virtualization
> > support on s390. The effect of the che for s390 is that
> > force_dma_unencrypted() will always return false instead calling into
> > the platform code like it did before the patch, right?
> > 
> > Should I send a  Fixes: e67a5ed1f86f "dma-direct: Force unencrypted DMA
> > under SME for certain DMA masks" (Tom Lendacky, 2019-07-10) patch that
> > rectifies things for s390 or how do we want handle this?
> 
> Yes, please do.  I hadn't noticed the s390 support had landed in
> mainline already.
> 

Will do! I guess I should do the patch against the for-next branch of the
dma-mapping tree. But that branch does not have the s390 support patches (yet?).
To fix it I need both e67a5ed1f86f and 64e1f0c531d1 "s390/mm: force
swiotlb for protected virtualization" (Halil Pasic, 2018-09-13). Or
should I wait for e67a5ed1f86f landing in mainline?

Regards,
Halil

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

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

* Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
  2019-07-12 14:51       ` Halil Pasic
@ 2019-07-12 15:11         ` Christoph Hellwig
  2019-07-12 15:42           ` Halil Pasic
  0 siblings, 1 reply; 21+ messages in thread
From: Christoph Hellwig @ 2019-07-12 15:11 UTC (permalink / raw)
  To: Halil Pasic
  Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
	x86, Ram Pai, linux-kernel, Alexey Dobriyan, iommu, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, linux-fsdevel, Thomas Gleixner,
	linuxppc-dev, Christoph Hellwig

On Fri, Jul 12, 2019 at 04:51:53PM +0200, Halil Pasic wrote:
> Thank you very much! I will have another look, but it seems to me,
> without further measures taken, this would break protected virtualization
> support on s390. The effect of the che for s390 is that
> force_dma_unencrypted() will always return false instead calling into
> the platform code like it did before the patch, right?
> 
> Should I send a  Fixes: e67a5ed1f86f "dma-direct: Force unencrypted DMA
> under SME for certain DMA masks" (Tom Lendacky, 2019-07-10) patch that
> rectifies things for s390 or how do we want handle this?

Yes, please do.  I hadn't noticed the s390 support had landed in
mainline already.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
  2019-07-12 14:08     ` Christoph Hellwig
@ 2019-07-12 14:51       ` Halil Pasic
  2019-07-12 15:11         ` Christoph Hellwig
  0 siblings, 1 reply; 21+ messages in thread
From: Halil Pasic @ 2019-07-12 14:51 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
	x86, Ram Pai, linux-kernel, iommu, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, linux-fsdevel, Thomas Gleixner, linuxppc-dev,
	Alexey Dobriyan

On Fri, 12 Jul 2019 16:08:12 +0200
Christoph Hellwig <hch@lst.de> wrote:

> On Fri, Jul 12, 2019 at 03:09:12PM +0200, Halil Pasic wrote:
> > This is the implementation for the guys that don't
> > have ARCH_HAS_MEM_ENCRYPT.
> > 
> > Means sev_active() may not be used in such code after this
> > patch. What about 
> > 
> > static inline bool force_dma_unencrypted(void)
> > {
> >         return sev_active();
> > }
> > 
> > in kernel/dma/direct.c?
> 
> FYI, I have this pending in the dma-mapping tree:
> 
> http://git.infradead.org/users/hch/dma-mapping.git/commitdiff/e67a5ed1f86f4370991c601f2fcad9ebf9e1eebb

Thank you very much! I will have another look, but it seems to me,
without further measures taken, this would break protected virtualization
support on s390. The effect of the che for s390 is that
force_dma_unencrypted() will always return false instead calling into
the platform code like it did before the patch, right?

Should I send a  Fixes: e67a5ed1f86f "dma-direct: Force unencrypted DMA
under SME for certain DMA masks" (Tom Lendacky, 2019-07-10) patch that
rectifies things for s390 or how do we want handle this?

Regards,
Halil

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

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

* Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
  2019-07-12 13:09   ` Halil Pasic
@ 2019-07-12 14:08     ` Christoph Hellwig
  2019-07-12 14:51       ` Halil Pasic
  2019-07-12 21:55     ` Thiago Jung Bauermann
  1 sibling, 1 reply; 21+ messages in thread
From: Christoph Hellwig @ 2019-07-12 14:08 UTC (permalink / raw)
  To: Halil Pasic
  Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
	x86, Ram Pai, linux-kernel, Alexey Dobriyan, iommu, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, linux-fsdevel, Thomas Gleixner,
	linuxppc-dev, Christoph Hellwig

On Fri, Jul 12, 2019 at 03:09:12PM +0200, Halil Pasic wrote:
> This is the implementation for the guys that don't
> have ARCH_HAS_MEM_ENCRYPT.
> 
> Means sev_active() may not be used in such code after this
> patch. What about 
> 
> static inline bool force_dma_unencrypted(void)
> {
>         return sev_active();
> }
> 
> in kernel/dma/direct.c?

FYI, I have this pending in the dma-mapping tree:

http://git.infradead.org/users/hch/dma-mapping.git/commitdiff/e67a5ed1f86f4370991c601f2fcad9ebf9e1eebb
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
  2019-07-12  5:36 ` [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code Thiago Jung Bauermann
@ 2019-07-12 13:09   ` Halil Pasic
  2019-07-12 14:08     ` Christoph Hellwig
  2019-07-12 21:55     ` Thiago Jung Bauermann
  0 siblings, 2 replies; 21+ messages in thread
From: Halil Pasic @ 2019-07-12 13:09 UTC (permalink / raw)
  To: Thiago Jung Bauermann
  Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
	x86, Ram Pai, linux-kernel, Alexey Dobriyan, iommu, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, linux-fsdevel, Thomas Gleixner,
	linuxppc-dev, Christoph Hellwig

On Fri, 12 Jul 2019 02:36:31 -0300
Thiago Jung Bauermann <bauerman@linux.ibm.com> wrote:

> Secure Encrypted Virtualization is an x86-specific feature, so it shouldn't
> appear in generic kernel code because it forces non-x86 architectures to
> define the sev_active() function, which doesn't make a lot of sense.

sev_active() might be just bad (too specific) name for a general
concept. s390 code defines it drives the right behavior in
kernel/dma/direct.c (which uses it).

> 
> To solve this problem, add an x86 elfcorehdr_read() function to override
> the generic weak implementation. To do that, it's necessary to make
> read_from_oldmem() public so that it can be used outside of vmcore.c.
> 
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> ---
>  arch/x86/kernel/crash_dump_64.c |  5 +++++
>  fs/proc/vmcore.c                |  8 ++++----
>  include/linux/crash_dump.h      | 14 ++++++++++++++
>  include/linux/mem_encrypt.h     |  1 -
>  4 files changed, 23 insertions(+), 5 deletions(-)

Does not seem to apply to today's or yesterdays master.

> 
> diff --git a/arch/x86/kernel/crash_dump_64.c b/arch/x86/kernel/crash_dump_64.c
> index 22369dd5de3b..045e82e8945b 100644
> --- a/arch/x86/kernel/crash_dump_64.c
> +++ b/arch/x86/kernel/crash_dump_64.c
> @@ -70,3 +70,8 @@ ssize_t copy_oldmem_page_encrypted(unsigned long pfn, char *buf, size_t csize,
>  {
>  	return __copy_oldmem_page(pfn, buf, csize, offset, userbuf, true);
>  }
> +
> +ssize_t elfcorehdr_read(char *buf, size_t count, u64 *ppos)
> +{
> +	return read_from_oldmem(buf, count, ppos, 0, sev_active());
> +}
> diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
> index 57957c91c6df..ca1f20bedd8c 100644
> --- a/fs/proc/vmcore.c
> +++ b/fs/proc/vmcore.c
> @@ -100,9 +100,9 @@ static int pfn_is_ram(unsigned long pfn)
>  }
>  
>  /* Reads a page from the oldmem device from given offset. */
> -static ssize_t read_from_oldmem(char *buf, size_t count,
> -				u64 *ppos, int userbuf,
> -				bool encrypted)
> +ssize_t read_from_oldmem(char *buf, size_t count,
> +			 u64 *ppos, int userbuf,
> +			 bool encrypted)
>  {
>  	unsigned long pfn, offset;
>  	size_t nr_bytes;
> @@ -166,7 +166,7 @@ void __weak elfcorehdr_free(unsigned long long addr)
>   */
>  ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 *ppos)
>  {
> -	return read_from_oldmem(buf, count, ppos, 0, sev_active());
> +	return read_from_oldmem(buf, count, ppos, 0, false);
>  }
>  
>  /*
> diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
> index f774c5eb9e3c..4664fc1871de 100644
> --- a/include/linux/crash_dump.h
> +++ b/include/linux/crash_dump.h
> @@ -115,4 +115,18 @@ static inline int vmcore_add_device_dump(struct vmcoredd_data *data)
>  	return -EOPNOTSUPP;
>  }
>  #endif /* CONFIG_PROC_VMCORE_DEVICE_DUMP */
> +
> +#ifdef CONFIG_PROC_VMCORE
> +ssize_t read_from_oldmem(char *buf, size_t count,
> +			 u64 *ppos, int userbuf,
> +			 bool encrypted);
> +#else
> +static inline ssize_t read_from_oldmem(char *buf, size_t count,
> +				       u64 *ppos, int userbuf,
> +				       bool encrypted)
> +{
> +	return -EOPNOTSUPP;
> +}
> +#endif /* CONFIG_PROC_VMCORE */
> +
>  #endif /* LINUX_CRASHDUMP_H */
> diff --git a/include/linux/mem_encrypt.h b/include/linux/mem_encrypt.h
> index f2e399fb626b..a3747fcae466 100644
> --- a/include/linux/mem_encrypt.h
> +++ b/include/linux/mem_encrypt.h
> @@ -21,7 +21,6 @@
>  
>  #else	/* !CONFIG_ARCH_HAS_MEM_ENCRYPT */
>  
> -static inline bool sev_active(void) { return false; }

This is the implementation for the guys that don't
have ARCH_HAS_MEM_ENCRYPT.

Means sev_active() may not be used in such code after this
patch. What about 

static inline bool force_dma_unencrypted(void)
{
        return sev_active();
}

in kernel/dma/direct.c?

Regards,
Halil

>  static inline bool mem_encrypt_active(void) { return false; }
>  
>  #endif	/* CONFIG_ARCH_HAS_MEM_ENCRYPT */

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

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

* [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
  2019-07-12  5:36 Thiago Jung Bauermann
@ 2019-07-12  5:36 ` Thiago Jung Bauermann
  2019-07-12 13:09   ` Halil Pasic
  0 siblings, 1 reply; 21+ messages in thread
From: Thiago Jung Bauermann @ 2019-07-12  5:36 UTC (permalink / raw)
  To: x86
  Cc: linux-s390, Konrad Rzeszutek Wilk, Robin Murphy, Mike Anderson,
	Ram Pai, linux-kernel, Alexey Dobriyan, Halil Pasic, iommu,
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, linux-fsdevel,
	Thomas Gleixner, linuxppc-dev, Christoph Hellwig

Secure Encrypted Virtualization is an x86-specific feature, so it shouldn't
appear in generic kernel code because it forces non-x86 architectures to
define the sev_active() function, which doesn't make a lot of sense.

To solve this problem, add an x86 elfcorehdr_read() function to override
the generic weak implementation. To do that, it's necessary to make
read_from_oldmem() public so that it can be used outside of vmcore.c.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
 arch/x86/kernel/crash_dump_64.c |  5 +++++
 fs/proc/vmcore.c                |  8 ++++----
 include/linux/crash_dump.h      | 14 ++++++++++++++
 include/linux/mem_encrypt.h     |  1 -
 4 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/crash_dump_64.c b/arch/x86/kernel/crash_dump_64.c
index 22369dd5de3b..045e82e8945b 100644
--- a/arch/x86/kernel/crash_dump_64.c
+++ b/arch/x86/kernel/crash_dump_64.c
@@ -70,3 +70,8 @@ ssize_t copy_oldmem_page_encrypted(unsigned long pfn, char *buf, size_t csize,
 {
 	return __copy_oldmem_page(pfn, buf, csize, offset, userbuf, true);
 }
+
+ssize_t elfcorehdr_read(char *buf, size_t count, u64 *ppos)
+{
+	return read_from_oldmem(buf, count, ppos, 0, sev_active());
+}
diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index 57957c91c6df..ca1f20bedd8c 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -100,9 +100,9 @@ static int pfn_is_ram(unsigned long pfn)
 }
 
 /* Reads a page from the oldmem device from given offset. */
-static ssize_t read_from_oldmem(char *buf, size_t count,
-				u64 *ppos, int userbuf,
-				bool encrypted)
+ssize_t read_from_oldmem(char *buf, size_t count,
+			 u64 *ppos, int userbuf,
+			 bool encrypted)
 {
 	unsigned long pfn, offset;
 	size_t nr_bytes;
@@ -166,7 +166,7 @@ void __weak elfcorehdr_free(unsigned long long addr)
  */
 ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 *ppos)
 {
-	return read_from_oldmem(buf, count, ppos, 0, sev_active());
+	return read_from_oldmem(buf, count, ppos, 0, false);
 }
 
 /*
diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
index f774c5eb9e3c..4664fc1871de 100644
--- a/include/linux/crash_dump.h
+++ b/include/linux/crash_dump.h
@@ -115,4 +115,18 @@ static inline int vmcore_add_device_dump(struct vmcoredd_data *data)
 	return -EOPNOTSUPP;
 }
 #endif /* CONFIG_PROC_VMCORE_DEVICE_DUMP */
+
+#ifdef CONFIG_PROC_VMCORE
+ssize_t read_from_oldmem(char *buf, size_t count,
+			 u64 *ppos, int userbuf,
+			 bool encrypted);
+#else
+static inline ssize_t read_from_oldmem(char *buf, size_t count,
+				       u64 *ppos, int userbuf,
+				       bool encrypted)
+{
+	return -EOPNOTSUPP;
+}
+#endif /* CONFIG_PROC_VMCORE */
+
 #endif /* LINUX_CRASHDUMP_H */
diff --git a/include/linux/mem_encrypt.h b/include/linux/mem_encrypt.h
index f2e399fb626b..a3747fcae466 100644
--- a/include/linux/mem_encrypt.h
+++ b/include/linux/mem_encrypt.h
@@ -21,7 +21,6 @@
 
 #else	/* !CONFIG_ARCH_HAS_MEM_ENCRYPT */
 
-static inline bool sev_active(void) { return false; }
 static inline bool mem_encrypt_active(void) { return false; }
 
 #endif	/* CONFIG_ARCH_HAS_MEM_ENCRYPT */
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

end of thread, back to index

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-13  4:45 [PATCH 0/3] Remove x86-specific code from generic headers Thiago Jung Bauermann
2019-07-13  4:45 ` [PATCH 1/3] x86, s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig Thiago Jung Bauermann
2019-07-13  7:27   ` [PATCH 1/3] x86,s390: " Christoph Hellwig
2019-07-15 15:23   ` [PATCH 1/3] x86, s390: " janani
2019-07-15 20:00     ` Thiago Jung Bauermann
2019-07-13  4:45 ` [PATCH 2/3] DMA mapping: Move SME handling to x86-specific files Thiago Jung Bauermann
2019-07-13  7:29   ` Christoph Hellwig
2019-07-13  4:45 ` [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code Thiago Jung Bauermann
2019-07-13  5:08 ` [PATCH 0/3] Remove x86-specific code from generic headers Thiago Jung Bauermann
  -- strict thread matches above, loose matches on Subject: below --
2019-07-12  5:36 Thiago Jung Bauermann
2019-07-12  5:36 ` [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code Thiago Jung Bauermann
2019-07-12 13:09   ` Halil Pasic
2019-07-12 14:08     ` Christoph Hellwig
2019-07-12 14:51       ` Halil Pasic
2019-07-12 15:11         ` Christoph Hellwig
2019-07-12 15:42           ` Halil Pasic
2019-07-13  8:08             ` Christoph Hellwig
2019-07-12 21:55     ` Thiago Jung Bauermann
2019-07-15 14:03       ` Halil Pasic
2019-07-15 14:30         ` Christoph Hellwig
2019-07-15 15:44           ` Lendacky, Thomas
2019-07-15 20:14           ` Thiago Jung Bauermann

IOMMU Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-iommu/0 linux-iommu/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-iommu linux-iommu/ https://lore.kernel.org/linux-iommu \
		iommu@lists.linux-foundation.org iommu@archiver.kernel.org
	public-inbox-index linux-iommu


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.linux-foundation.lists.iommu


AGPL code for this site: git clone https://public-inbox.org/ public-inbox