Re: [PATCH v3] powerpc/pseries/svm: Allocate SWIOTLB buffer anywhere in memory
Konrad Rzeszutek Wilk writes: > On Tue, Aug 18, 2020 at 07:11:26PM -0300, Thiago Jung Bauermann wrote: >> POWER secure guests (i.e., guests which use the Protection Execution >> Facility) need to use SWIOTLB to be able to do I/O with the hypervisor, but >> they don't need the SWIOTLB memory to be in low addresses since the >> hypervisor doesn't have any addressing limitation. >> >> This solves a SWIOTLB initialization problem we are seeing in secure guests >> with 128 GB of RAM: they are configured with 4 GB of crashkernel reserved >> memory, which leaves no space for SWIOTLB in low addresses. >> >> To do this, we use mostly the same code as swiotlb_init(), but allocate the >> buffer using memblock_alloc() instead of memblock_alloc_low(). >> >> Signed-off-by: Thiago Jung Bauermann > > Reviewed-by: Konrad Rzeszutek Wilk Thanks! -- Thiago Jung Bauermann IBM Linux Technology Center ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3] powerpc/pseries/svm: Allocate SWIOTLB buffer anywhere in memory
Christoph Hellwig writes: > On Tue, Aug 18, 2020 at 07:11:26PM -0300, Thiago Jung Bauermann wrote: >> POWER secure guests (i.e., guests which use the Protection Execution >> Facility) need to use SWIOTLB to be able to do I/O with the hypervisor, but >> they don't need the SWIOTLB memory to be in low addresses since the >> hypervisor doesn't have any addressing limitation. >> >> This solves a SWIOTLB initialization problem we are seeing in secure guests >> with 128 GB of RAM: they are configured with 4 GB of crashkernel reserved >> memory, which leaves no space for SWIOTLB in low addresses. >> >> To do this, we use mostly the same code as swiotlb_init(), but allocate the >> buffer using memblock_alloc() instead of memblock_alloc_low(). >> >> Signed-off-by: Thiago Jung Bauermann > > Looks fine to me (except for the pointlessly long comment lines, but I've > been told that's the powerpc way). Thanks! Do I have your Reviewed-by? -- Thiago Jung Bauermann IBM Linux Technology Center ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3] powerpc/pseries/svm: Allocate SWIOTLB buffer anywhere in memory
POWER secure guests (i.e., guests which use the Protection Execution
Facility) need to use SWIOTLB to be able to do I/O with the hypervisor, but
they don't need the SWIOTLB memory to be in low addresses since the
hypervisor doesn't have any addressing limitation.
This solves a SWIOTLB initialization problem we are seeing in secure guests
with 128 GB of RAM: they are configured with 4 GB of crashkernel reserved
memory, which leaves no space for SWIOTLB in low addresses.
To do this, we use mostly the same code as swiotlb_init(), but allocate the
buffer using memblock_alloc() instead of memblock_alloc_low().
Signed-off-by: Thiago Jung Bauermann
---
arch/powerpc/include/asm/svm.h | 4
arch/powerpc/mm/mem.c| 6 +-
arch/powerpc/platforms/pseries/svm.c | 26 ++
3 files changed, 35 insertions(+), 1 deletion(-)
Changes from v2:
- Panic if unable to allocate buffer, as suggested by Christoph.
Changes from v1:
- Open-code swiotlb_init() in arch-specific code, as suggested by
Christoph.
diff --git a/arch/powerpc/include/asm/svm.h b/arch/powerpc/include/asm/svm.h
index 85580b30aba4..7546402d796a 100644
--- a/arch/powerpc/include/asm/svm.h
+++ b/arch/powerpc/include/asm/svm.h
@@ -15,6 +15,8 @@ static inline bool is_secure_guest(void)
return mfmsr() & MSR_S;
}
+void __init svm_swiotlb_init(void);
+
void dtl_cache_ctor(void *addr);
#define get_dtl_cache_ctor() (is_secure_guest() ? dtl_cache_ctor : NULL)
@@ -25,6 +27,8 @@ static inline bool is_secure_guest(void)
return false;
}
+static inline void svm_swiotlb_init(void) {}
+
#define get_dtl_cache_ctor() NULL
#endif /* CONFIG_PPC_SVM */
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index c2c11eb8dcfc..0f21bcb16405 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -50,6 +50,7 @@
#include
#include
#include
+#include
#include
@@ -290,7 +291,10 @@ void __init mem_init(void)
* back to to-down.
*/
memblock_set_bottom_up(true);
- swiotlb_init(0);
+ if (is_secure_guest())
+ svm_swiotlb_init();
+ else
+ swiotlb_init(0);
#endif
high_memory = (void *) __va(max_low_pfn * PAGE_SIZE);
diff --git a/arch/powerpc/platforms/pseries/svm.c
b/arch/powerpc/platforms/pseries/svm.c
index 40c0637203d5..81085eb8f225 100644
--- a/arch/powerpc/platforms/pseries/svm.c
+++ b/arch/powerpc/platforms/pseries/svm.c
@@ -7,6 +7,7 @@
*/
#include
+#include
#include
#include
#include
@@ -34,6 +35,31 @@ static int __init init_svm(void)
}
machine_early_initcall(pseries, init_svm);
+/*
+ * Initialize SWIOTLB. Essentially the same as swiotlb_init(), except that it
+ * can allocate the buffer anywhere in memory. Since the hypervisor doesn't
have
+ * any addressing limitation, we don't need to allocate it in low addresses.
+ */
+void __init svm_swiotlb_init(void)
+{
+ unsigned char *vstart;
+ unsigned long bytes, io_tlb_nslabs;
+
+ io_tlb_nslabs = (swiotlb_size_or_default() >> IO_TLB_SHIFT);
+ io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
+
+ bytes = io_tlb_nslabs << IO_TLB_SHIFT;
+
+ vstart = memblock_alloc(PAGE_ALIGN(bytes), PAGE_SIZE);
+ if (vstart && !swiotlb_init_with_tbl(vstart, io_tlb_nslabs, false))
+ return;
+
+ if (io_tlb_start)
+ memblock_free_early(io_tlb_start,
+ PAGE_ALIGN(io_tlb_nslabs << IO_TLB_SHIFT));
+ panic("SVM: Cannot allocate SWIOTLB buffer");
+}
+
int set_memory_encrypted(unsigned long addr, int numpages)
{
if (!PAGE_ALIGNED(addr))
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] powerpc/pseries/svm: Allocate SWIOTLB buffer anywhere in memory
Christoph Hellwig writes: > On Mon, Aug 17, 2020 at 06:46:58PM -0300, Thiago Jung Bauermann wrote: >> POWER secure guests (i.e., guests which use the Protection Execution >> Facility) need to use SWIOTLB to be able to do I/O with the hypervisor, but >> they don't need the SWIOTLB memory to be in low addresses since the >> hypervisor doesn't have any addressing limitation. >> >> This solves a SWIOTLB initialization problem we are seeing in secure guests >> with 128 GB of RAM: they are configured with 4 GB of crashkernel reserved >> memory, which leaves no space for SWIOTLB in low addresses. >> >> To do this, we use mostly the same code as swiotlb_init(), but allocate the >> buffer using memblock_alloc() instead of memblock_alloc_low(). >> >> We also need to add swiotlb_set_no_iotlb_memory() in order to set the >> no_iotlb_memory flag if initialization fails. > > Do you really need the helper? As far as I can tell the secure guests > very much rely on swiotlb for all I/O, so you might as well panic if > you fail to allocate it. That is true. Ok, I will do that. -- Thiago Jung Bauermann IBM Linux Technology Center ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] swiotlb: Allow allocating buffer anywhere in memory
Hello Christoph, Christoph Hellwig writes: > On Sat, Aug 15, 2020 at 05:45:36PM -0300, Thiago Jung Bauermann wrote: >> POWER secure guests (i.e., guests which use the Protection Execution >> Facility) need to use SWIOTLB to be able to do I/O with the hypervisor, but >> they don't need the SWIOTLB memory to be in low addresses since the >> hypervisor doesn't have any addressing limitation. >> >> This solves a SWIOTLB initialization problem we are seeing in secure guests >> with 128 GB of RAM: they are configured with 4 GB of crashkernel reserved >> memory, which leaves no space for SWIOTLB in low addresses. > > What about just open coding the allocation and using > swiotlb_init_with_tbl? Yes, that works too. I just sent a v2 implementing that change. I just had to add a small accessor function so that I could set no_iotlb_memory from outside swiotlb.c. Thank you for the quick review. -- Thiago Jung Bauermann IBM Linux Technology Center ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2] powerpc/pseries/svm: Allocate SWIOTLB buffer anywhere in memory
POWER secure guests (i.e., guests which use the Protection Execution
Facility) need to use SWIOTLB to be able to do I/O with the hypervisor, but
they don't need the SWIOTLB memory to be in low addresses since the
hypervisor doesn't have any addressing limitation.
This solves a SWIOTLB initialization problem we are seeing in secure guests
with 128 GB of RAM: they are configured with 4 GB of crashkernel reserved
memory, which leaves no space for SWIOTLB in low addresses.
To do this, we use mostly the same code as swiotlb_init(), but allocate the
buffer using memblock_alloc() instead of memblock_alloc_low().
We also need to add swiotlb_set_no_iotlb_memory() in order to set the
no_iotlb_memory flag if initialization fails.
Signed-off-by: Thiago Jung Bauermann
---
arch/powerpc/include/asm/svm.h | 4
arch/powerpc/mm/mem.c| 6 +-
arch/powerpc/platforms/pseries/svm.c | 27 +++
include/linux/swiotlb.h | 1 +
kernel/dma/swiotlb.c | 5 +
5 files changed, 42 insertions(+), 1 deletion(-)
Changes from v1:
- Open-code swiotlb_init() in arch-specific code, as suggested by
Christoph.
diff --git a/arch/powerpc/include/asm/svm.h b/arch/powerpc/include/asm/svm.h
index 85580b30aba4..7546402d796a 100644
--- a/arch/powerpc/include/asm/svm.h
+++ b/arch/powerpc/include/asm/svm.h
@@ -15,6 +15,8 @@ static inline bool is_secure_guest(void)
return mfmsr() & MSR_S;
}
+void __init svm_swiotlb_init(void);
+
void dtl_cache_ctor(void *addr);
#define get_dtl_cache_ctor() (is_secure_guest() ? dtl_cache_ctor : NULL)
@@ -25,6 +27,8 @@ static inline bool is_secure_guest(void)
return false;
}
+static inline void svm_swiotlb_init(void) {}
+
#define get_dtl_cache_ctor() NULL
#endif /* CONFIG_PPC_SVM */
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index c2c11eb8dcfc..0f21bcb16405 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -50,6 +50,7 @@
#include
#include
#include
+#include
#include
@@ -290,7 +291,10 @@ void __init mem_init(void)
* back to to-down.
*/
memblock_set_bottom_up(true);
- swiotlb_init(0);
+ if (is_secure_guest())
+ svm_swiotlb_init();
+ else
+ swiotlb_init(0);
#endif
high_memory = (void *) __va(max_low_pfn * PAGE_SIZE);
diff --git a/arch/powerpc/platforms/pseries/svm.c
b/arch/powerpc/platforms/pseries/svm.c
index 40c0637203d5..d592e663a8d6 100644
--- a/arch/powerpc/platforms/pseries/svm.c
+++ b/arch/powerpc/platforms/pseries/svm.c
@@ -7,6 +7,7 @@
*/
#include
+#include
#include
#include
#include
@@ -34,6 +35,32 @@ static int __init init_svm(void)
}
machine_early_initcall(pseries, init_svm);
+/*
+ * Initialize SWIOTLB. Essentially the same as swiotlb_init(), except that it
+ * can allocate the buffer anywhere in memory. Since the hypervisor doesn't
have
+ * any addressing limitation, we don't need to allocate it in low addresses.
+ */
+void __init svm_swiotlb_init(void)
+{
+ unsigned char *vstart;
+ unsigned long bytes, io_tlb_nslabs;
+
+ io_tlb_nslabs = (swiotlb_size_or_default() >> IO_TLB_SHIFT);
+ io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
+
+ bytes = io_tlb_nslabs << IO_TLB_SHIFT;
+
+ vstart = memblock_alloc(PAGE_ALIGN(bytes), PAGE_SIZE);
+ if (vstart && !swiotlb_init_with_tbl(vstart, io_tlb_nslabs, false))
+ return;
+
+ if (io_tlb_start)
+ memblock_free_early(io_tlb_start,
+ PAGE_ALIGN(io_tlb_nslabs << IO_TLB_SHIFT));
+ pr_warn("Cannot allocate SWIOTLB buffer");
+ swiotlb_set_no_iotlb_memory(true);
+}
+
int set_memory_encrypted(unsigned long addr, int numpages)
{
if (!PAGE_ALIGNED(addr))
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 046bb94bd4d6..991e9f13e663 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -35,6 +35,7 @@ extern unsigned long swiotlb_nr_tbl(void);
unsigned long swiotlb_size_or_default(void);
extern int swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs);
extern void __init swiotlb_update_mem_attributes(void);
+void __init swiotlb_set_no_iotlb_memory(bool value);
/*
* Enumeration for sync targets
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index c19379fabd20..ed2b8818ff67 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -132,6 +132,11 @@ early_param("swiotlb", setup_io_tlb_npages);
static bool no_iotlb_memory;
+void __init swiotlb_set_no_iotlb_memory(bool value)
+{
+ no_iotlb_memory = value;
+}
+
unsigned long swiotlb_nr_tbl(void)
{
return unlikely(no_iotlb_memory) ? 0 : io_tlb_nslabs;
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] swiotlb: Allow allocating buffer anywhere in memory
POWER secure guests (i.e., guests which use the Protection Execution
Facility) need to use SWIOTLB to be able to do I/O with the hypervisor, but
they don't need the SWIOTLB memory to be in low addresses since the
hypervisor doesn't have any addressing limitation.
This solves a SWIOTLB initialization problem we are seeing in secure guests
with 128 GB of RAM: they are configured with 4 GB of crashkernel reserved
memory, which leaves no space for SWIOTLB in low addresses.
Signed-off-by: Thiago Jung Bauermann
---
arch/powerpc/mm/mem.c | 7 ++-
include/linux/swiotlb.h | 8 +++-
kernel/dma/swiotlb.c| 10 +++---
3 files changed, 20 insertions(+), 5 deletions(-)
Normally I would split changes like this into one patch touching generic
code and another for the arch-specific part, but in this case I thought it
would be unneeded complexity. I can split though if people prefer it that
way.
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index c2c11eb8dcfc..13f2e3aff8b5 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -50,6 +50,7 @@
#include
#include
#include
+#include
#include
@@ -290,7 +291,11 @@ void __init mem_init(void)
* back to to-down.
*/
memblock_set_bottom_up(true);
- swiotlb_init(0);
+ /*
+* SVM guests can use the SWIOTLB wherever it is in memory,
+* even if not DMA-able.
+*/
+ swiotlb_init_anywhere(0, is_secure_guest());
#endif
high_memory = (void *) __va(max_low_pfn * PAGE_SIZE);
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 046bb94bd4d6..433f3dbb35b5 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -29,7 +29,13 @@ enum swiotlb_force {
*/
#define IO_TLB_SHIFT 11
-extern void swiotlb_init(int verbose);
+void __init swiotlb_init_anywhere(int verbose, bool allocate_anywhere);
+
+static inline void swiotlb_init(int verbose)
+{
+ swiotlb_init_anywhere(verbose, false);
+}
+
int swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose);
extern unsigned long swiotlb_nr_tbl(void);
unsigned long swiotlb_size_or_default(void);
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index c19379fabd20..27070aa59e34 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -244,7 +244,7 @@ int __init swiotlb_init_with_tbl(char *tlb, unsigned long
nslabs, int verbose)
* structures for the software IO TLB used to implement the DMA API.
*/
void __init
-swiotlb_init(int verbose)
+swiotlb_init_anywhere(int verbose, bool allocate_anywhere)
{
size_t default_size = IO_TLB_DEFAULT_SIZE;
unsigned char *vstart;
@@ -257,8 +257,12 @@ swiotlb_init(int verbose)
bytes = io_tlb_nslabs << IO_TLB_SHIFT;
- /* Get IO TLB memory from the low pages */
- vstart = memblock_alloc_low(PAGE_ALIGN(bytes), PAGE_SIZE);
+ if (allocate_anywhere)
+ vstart = memblock_alloc(PAGE_ALIGN(bytes), PAGE_SIZE);
+ else
+ /* Get IO TLB memory from the low pages */
+ vstart = memblock_alloc_low(PAGE_ALIGN(bytes), PAGE_SIZE);
+
if (vstart && !swiotlb_init_with_tbl(vstart, io_tlb_nslabs, verbose))
return;
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/6] x86, s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig
Michael Ellerman writes: > On Tue, 2019-08-06 at 04:49:14 UTC, 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 >> Reviewed-by: Thomas Gleixner >> Reviewed-by: Christoph Hellwig > > Series applied to powerpc topic/mem-encrypt, thanks. > > https://git.kernel.org/powerpc/c/0c9c1d56397518eb823d458b00b06bcccd956794 Thank you! -- Thiago Jung Bauermann IBM Linux Technology Center
Re: [PATCH v4 0/6] Remove x86-specific code from generic headers
m...@ellerman.id.au writes: > Thiago Jung Bauermann writes: >> Hello, >> >> This version has only a small change in the last patch as requested by >> Christoph and Halil, and collects Reviewed-by's. >> >> These patches are applied on top of v5.3-rc2. >> >> I don't have a way to test SME, SEV, nor s390's PEF so the patches have only >> been build tested. > > I need to take this series via the powerpc tree because there is another > fairly large powerpc specific series dependent on it. > > I think this series already has pretty much all the acks it needs, which > almost never happens, amazing work! Yes, thank you very much to everyone who reviewed the patches! > I'll put the series in a topic branch, just in case there's any bad > conflicts and other folks want to merge it later on. I'll then merge the > topic branch into my next, and so this series will be tested in > linux-next that way. That's awesome. Thank you very much! -- Thiago Jung Bauermann IBM Linux Technology Center ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 5/6] fs/core/vmcore: Move sev_active() reference to x86 arch code
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.
Also, remove the export for sev_active() since it's only used in files that
won't be built as modules.
Signed-off-by: Thiago Jung Bauermann
Reviewed-by: Christoph Hellwig
Reviewed-by: Lianbo Jiang
---
arch/x86/kernel/crash_dump_64.c | 5 +
arch/x86/mm/mem_encrypt.c | 1 -
fs/proc/vmcore.c| 8
include/linux/crash_dump.h | 14 ++
include/linux/mem_encrypt.h | 1 -
5 files changed, 23 insertions(+), 6 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/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index 94da5a88abe6..9268c12458c8 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -349,7 +349,6 @@ bool sev_active(void)
{
return sme_me_mask && sev_enabled;
}
-EXPORT_SYMBOL(sev_active);
/* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_UNENCRYPTED */
bool force_dma_unencrypted(struct device *dev)
diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index 7bcc92add72c..7b13988796e1 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -104,9 +104,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;
@@ -170,7 +170,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
[PATCH v4 3/6] dma-mapping: Remove dma_check_mask()
sme_active() is an x86-specific function so it's better not to call it from
generic 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 just remove dma_check_mask().
Signed-off-by: Thiago Jung Bauermann
Reviewed-by: Christoph Hellwig
Reviewed-by: Tom Lendacky
---
kernel/dma/mapping.c | 8
1 file changed, 8 deletions(-)
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index 1f628e7ac709..61eeefbfcb36 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);
@@ -327,7 +321,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;
}
@@ -345,7 +338,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;
}
[PATCH v4 6/6] s390/mm: Remove sev_active() function
All references to sev_active() were moved to arch/x86 so we don't need to
define it for s390 anymore.
Signed-off-by: Thiago Jung Bauermann
Reviewed-by: Christoph Hellwig
Reviewed-by: Halil Pasic
---
arch/s390/include/asm/mem_encrypt.h | 1 -
arch/s390/mm/init.c | 7 +--
2 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/arch/s390/include/asm/mem_encrypt.h
b/arch/s390/include/asm/mem_encrypt.h
index ff813a56bc30..2542cbf7e2d1 100644
--- a/arch/s390/include/asm/mem_encrypt.h
+++ b/arch/s390/include/asm/mem_encrypt.h
@@ -5,7 +5,6 @@
#ifndef __ASSEMBLY__
static inline bool mem_encrypt_active(void) { return false; }
-extern bool sev_active(void);
int set_memory_encrypted(unsigned long addr, int numpages);
int set_memory_decrypted(unsigned long addr, int numpages);
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index 20340a03ad90..a124f19f7b3c 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -156,14 +156,9 @@ int set_memory_decrypted(unsigned long addr, int numpages)
}
/* are we a protected virtualization guest? */
-bool sev_active(void)
-{
- return is_prot_virt_guest();
-}
-
bool force_dma_unencrypted(struct device *dev)
{
- return sev_active();
+ return is_prot_virt_guest();
}
/* protected virtualization */
[PATCH v4 2/6] swiotlb: Remove call to sme_active()
sme_active() is an x86-specific function so it's better not to call it from
generic code.
There's no need to mention which memory encryption feature is active, so
just use a more generic message. Besides, other architectures will have
different names for similar technology.
Signed-off-by: Thiago Jung Bauermann
Reviewed-by: Christoph Hellwig
Reviewed-by: Tom Lendacky
---
kernel/dma/swiotlb.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 9de232229063..f29caad71e13 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -461,8 +461,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);
[PATCH v4 1/6] x86,s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig
powerpc is also going to use this feature, so put it in a generic location. Signed-off-by: Thiago Jung Bauermann Reviewed-by: Thomas Gleixner Reviewed-by: Christoph Hellwig --- arch/Kconfig | 3 +++ arch/s390/Kconfig | 4 +--- arch/x86/Kconfig | 4 +--- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/arch/Kconfig b/arch/Kconfig index a7b57dd42c26..89e2e3f64f79 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -925,6 +925,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 a4ad2733eedf..f43319c44454 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 @@ -68,6 +65,7 @@ config S390 select ARCH_HAS_GCOV_PROFILE_ALL select ARCH_HAS_GIGANTIC_PAGE select ARCH_HAS_KCOV + select ARCH_HAS_MEM_ENCRYPT select ARCH_HAS_PTE_SPECIAL select ARCH_HAS_SET_MEMORY select ARCH_HAS_STRICT_KERNEL_RWX diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 222855cc0158..06027809c599 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_KCOVif X86_64 + select ARCH_HAS_MEM_ENCRYPT select ARCH_HAS_MEMBARRIER_SYNC_CORE select ARCH_HAS_PMEM_APIif X86_64 select ARCH_HAS_PTE_DEVMAP if X86_64 @@ -1518,9 +1519,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
[PATCH v4 4/6] x86,s390/mm: Move sme_active() and sme_me_mask to x86-specific header
Now that generic code doesn't reference them, move sme_active() and
sme_me_mask to x86's .
Also remove the export for sme_active() since it's only used in files that
won't be built as modules. sme_me_mask on the other hand is used in
arch/x86/kvm/svm.c (via __sme_set() and __psp_pa()) which can be built as a
module so its export needs to stay.
Signed-off-by: Thiago Jung Bauermann
Reviewed-by: Christoph Hellwig
Reviewed-by: Tom Lendacky
---
arch/s390/include/asm/mem_encrypt.h | 4 +---
arch/x86/include/asm/mem_encrypt.h | 10 ++
arch/x86/mm/mem_encrypt.c | 1 -
include/linux/mem_encrypt.h | 14 +-
4 files changed, 12 insertions(+), 17 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_mask0ULL
-
-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/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index fece30ca8b0c..94da5a88abe6 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -344,7 +344,6 @@ bool sme_active(void)
{
return sme_me_mask && !sev_enabled;
}
-EXPORT_SYMBOL(sme_active);
bool sev_active(void)
{
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_mask0ULL
-
-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
[PATCH v4 0/6] Remove x86-specific code from generic headers
Hello, This version has only a small change in the last patch as requested by Christoph and Halil, and collects Reviewed-by's. These patches are applied on top of v5.3-rc2. I don't have a way to test SME, SEV, nor s390's PEF so the patches have only been build tested. Changelog Since v3: - Patch "s390/mm: Remove sev_active() function" - Preserve comment from sev_active() in force_dma_unencrypted(). Suggested by Christoph Hellwig. Since v2: - Patch "x86,s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig" - Added "select ARCH_HAS_MEM_ENCRYPT" to config S390. Suggested by Janani. - Patch "DMA mapping: Move SME handling to x86-specific files" - Split up into 3 new patches. Suggested by Christoph Hellwig. - Patch "swiotlb: Remove call to sme_active()" - New patch. - Patch "dma-mapping: Remove dma_check_mask()" - New patch. - Patch "x86,s390/mm: Move sme_active() and sme_me_mask to x86-specific header" - New patch. - Removed export of sme_active symbol. Suggested by Christoph Hellwig. - Patch "fs/core/vmcore: Move sev_active() reference to x86 arch code" - Removed export of sev_active symbol. Suggested by Christoph Hellwig. - Patch "s390/mm: Remove sev_active() function" - New patch. 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 as well. - Remove dma_check_mask() from kernel/dma/mapping.c. Suggested by Christoph Hellwig. Thiago Jung Bauermann (6): x86,s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig swiotlb: Remove call to sme_active() dma-mapping: Remove dma_check_mask() x86,s390/mm: Move sme_active() and sme_me_mask to x86-specific header fs/core/vmcore: Move sev_active() reference to x86 arch code s390/mm: Remove sev_active() function arch/Kconfig| 3 +++ arch/s390/Kconfig | 4 +--- arch/s390/include/asm/mem_encrypt.h | 5 + arch/s390/mm/init.c | 7 +-- arch/x86/Kconfig| 4 +--- arch/x86/include/asm/mem_encrypt.h | 10 ++ arch/x86/kernel/crash_dump_64.c | 5 + arch/x86/mm/mem_encrypt.c | 2 -- 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 +-- 13 files changed, 42 insertions(+), 46 deletions(-)
Re: [PATCH 2/3] DMA mapping: Move SME handling to x86-specific files
kbuild test robot writes: > Hi Thiago, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on linus/master] > [cannot apply to v5.2 next-20190718] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] > > url: > https://github.com/0day-ci/linux/commits/Thiago-Jung-Bauermann/Remove-x86-specific-code-from-generic-headers/20190715-063006 > config: s390-allnoconfig (attached as .config) > compiler: s390-linux-gcc (GCC) 7.4.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=7.4.0 make.cross ARCH=s390 > > If you fix the issue, kindly add following tag > Reported-by: kbuild test robot > > All errors (new ones prefixed by >>): > >kernel/dma/swiotlb.c: In function 'swiotlb_tbl_map_single': >>> kernel/dma/swiotlb.c:461:6: error: implicit declaration of function >>> 'mem_encrypt_active'; did you mean 'set_cpu_active'? >>> [-Werror=implicit-function-declaration] > if (mem_encrypt_active()) > ^~ > set_cpu_active >cc1: some warnings being treated as errors This error was reported for v1 of the patch series. I wasn't able to reproduce this problem on v1 but found a similar issue on v2. I just did a build test of each patch of the latest version (v3) with an s390 cross-toolchain and the config file from this report and didn't find any build issues, so I believe this problem is solved. -- Thiago Jung Bauermann IBM Linux Technology Center ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 5/6] fs/core/vmcore: Move sev_active() reference to x86 arch code
Hello Lianbo, lijiang writes: > 在 2019年07月19日 01:47, Lendacky, Thomas 写道: >> On 7/17/19 10:28 PM, Thiago Jung Bauermann 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. >>> >>> 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. >>> >>> Also, remove the export for sev_active() since it's only used in files that >>> won't be built as modules. >>> >>> Signed-off-by: Thiago Jung Bauermann >> >> Adding Lianbo and Baoquan, who recently worked on this, for their review. >> > > This change looks good to me. > > Reviewed-by: Lianbo Jiang Thanks for your review! -- Thiago Jung Bauermann IBM Linux Technology Center ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 0/6] Remove x86-specific code from generic headers
Lendacky, Thomas writes: > On 7/18/19 2:44 PM, Thiago Jung Bauermann wrote: >> >> Lendacky, Thomas writes: >> >>> On 7/17/19 10:28 PM, Thiago Jung Bauermann wrote: >>>> Hello, >>>> >>>> This version is mostly about splitting up patch 2/3 into three separate >>>> patches, as suggested by Christoph Hellwig. Two other changes are a fix in >>>> patch 1 which wasn't selecting ARCH_HAS_MEM_ENCRYPT for s390 spotted by >>>> Janani and removal of sme_active and sev_active symbol exports as suggested >>>> by Christoph Hellwig. >>>> >>>> These patches are applied on top of today's dma-mapping/for-next. >>>> >>>> I don't have a way to test SME, SEV, nor s390's PEF so the patches have >>>> only >>>> been build tested. >>> >>> I'll try and get this tested quickly to be sure everything works for SME >>> and SEV. > > Built and tested both SME and SEV and everything appears to be working > well (not extensive testing, but should be good enough). Great news. Thanks for testing! -- Thiago Jung Bauermann IBM Linux Technology Center
Re: [PATCH 2/3] DMA mapping: Move SME handling to x86-specific files
Thomas Gleixner writes:
> On Fri, 12 Jul 2019, Thiago Jung Bauermann wrote:
>> diff --git a/include/linux/mem_encrypt.h b/include/linux/mem_encrypt.h
>> index b310a9c18113..f2e399fb626b 100644
>> --- a/include/linux/mem_encrypt.h
>> +++ b/include/linux/mem_encrypt.h
>> @@ -21,23 +21,11 @@
>>
>> #else /* !CONFIG_ARCH_HAS_MEM_ENCRYPT */
>>
>> -#define sme_me_mask 0ULL
>> -
>> -static inline bool sme_active(void) { return false; }
>> static inline bool sev_active(void) { return false; }
>
> You want to move out sev_active as well, the only relevant thing is
> mem_encrypt_active(). Everything SME/SEV is an architecture detail.
I'm sure you saw it. I addressed sev_active in a separate patch.
Thanks for reviewing this series!
>> +static inline bool mem_encrypt_active(void) { return false; }
>
> Thanks,
>
> tglx
--
Thiago Jung Bauermann
IBM Linux Technology Center
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 0/6] Remove x86-specific code from generic headers
Lendacky, Thomas writes: > On 7/17/19 10:28 PM, Thiago Jung Bauermann wrote: >> Hello, >> >> This version is mostly about splitting up patch 2/3 into three separate >> patches, as suggested by Christoph Hellwig. Two other changes are a fix in >> patch 1 which wasn't selecting ARCH_HAS_MEM_ENCRYPT for s390 spotted by >> Janani and removal of sme_active and sev_active symbol exports as suggested >> by Christoph Hellwig. >> >> These patches are applied on top of today's dma-mapping/for-next. >> >> I don't have a way to test SME, SEV, nor s390's PEF so the patches have only >> been build tested. > > I'll try and get this tested quickly to be sure everything works for SME > and SEV. Thanks! And thanks for reviewing the patches. -- Thiago Jung Bauermann IBM Linux Technology Center ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 6/6] s390/mm: Remove sev_active() function
Halil Pasic writes:
> On Thu, 18 Jul 2019 10:44:56 +0200
> Christoph Hellwig wrote:
>
>> > -/* are we a protected virtualization guest? */
>> > -bool sev_active(void)
>> > -{
>> > - return is_prot_virt_guest();
>> > -}
>> > -
>> > bool force_dma_unencrypted(struct device *dev)
>> > {
>> > - return sev_active();
>> > + return is_prot_virt_guest();
>> > }
>>
>> Do we want to keep the comment for force_dma_unencrypted?
>
> Yes we do. With the comment transferred:
>
> Reviewed-by: Halil Pasic
Thanks for your review.
Here is the new version. Should I send a new patch series with this
patch and the Reviewed-by on the other ones?
--
Thiago Jung Bauermann
IBM Linux Technology Center
>From 1726205c73fb9e29feaa3d8909c5a1b0f2054c04 Mon Sep 17 00:00:00 2001
From: Thiago Jung Bauermann
Date: Mon, 15 Jul 2019 20:50:43 -0300
Subject: [PATCH v4] s390/mm: Remove sev_active() function
All references to sev_active() were moved to arch/x86 so we don't need to
define it for s390 anymore.
Signed-off-by: Thiago Jung Bauermann
Reviewed-by: Christoph Hellwig
Reviewed-by: Halil Pasic
---
arch/s390/include/asm/mem_encrypt.h | 1 -
arch/s390/mm/init.c | 7 +--
2 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/arch/s390/include/asm/mem_encrypt.h
b/arch/s390/include/asm/mem_encrypt.h
index ff813a56bc30..2542cbf7e2d1 100644
--- a/arch/s390/include/asm/mem_encrypt.h
+++ b/arch/s390/include/asm/mem_encrypt.h
@@ -5,7 +5,6 @@
#ifndef __ASSEMBLY__
static inline bool mem_encrypt_active(void) { return false; }
-extern bool sev_active(void);
int set_memory_encrypted(unsigned long addr, int numpages);
int set_memory_decrypted(unsigned long addr, int numpages);
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index 78c319c5ce48..6c43a1ed1beb 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -156,14 +156,9 @@ int set_memory_decrypted(unsigned long addr, int numpages)
}
/* are we a protected virtualization guest? */
-bool sev_active(void)
-{
- return is_prot_virt_guest();
-}
-
bool force_dma_unencrypted(struct device *dev)
{
- return sev_active();
+ return is_prot_virt_guest();
}
/* protected virtualization */
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 6/6] s390/mm: Remove sev_active() function
Christoph Hellwig writes:
>> -/* are we a protected virtualization guest? */
>> -bool sev_active(void)
>> -{
>> -return is_prot_virt_guest();
>> -}
>> -
>> bool force_dma_unencrypted(struct device *dev)
>> {
>> -return sev_active();
>> +return is_prot_virt_guest();
>> }
>
> Do we want to keep the comment for force_dma_unencrypted?
>
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig
Thank you for your review on al these patches.
--
Thiago Jung Bauermann
IBM Linux Technology Center
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
Hello, Just going back to this question which I wasn't able to answer. Thiago Jung Bauermann writes: > Michael S. Tsirkin writes: > >> So far so good, but now a question: >> >> how are we handling guest address width limitations? >> Is VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS subject to >> guest address width limitations? >> I am guessing we can make them so ... >> This needs to be documented. > > I'm not sure. I will get back to you on this. We don't have address width limitations between host and guest. -- Thiago Jung Bauermann IBM Linux Technology Center
[PATCH v3 0/6] Remove x86-specific code from generic headers
Hello, This version is mostly about splitting up patch 2/3 into three separate patches, as suggested by Christoph Hellwig. Two other changes are a fix in patch 1 which wasn't selecting ARCH_HAS_MEM_ENCRYPT for s390 spotted by Janani and removal of sme_active and sev_active symbol exports as suggested by Christoph Hellwig. These patches are applied on top of today's dma-mapping/for-next. I don't have a way to test SME, SEV, nor s390's PEF so the patches have only been build tested. Changelog Since v2: - Patch "x86,s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig" - Added "select ARCH_HAS_MEM_ENCRYPT" to config S390. Suggested by Janani. - Patch "DMA mapping: Move SME handling to x86-specific files" - Split up into 3 new patches. Suggested by Christoph Hellwig. - Patch "swiotlb: Remove call to sme_active()" - New patch. - Patch "dma-mapping: Remove dma_check_mask()" - New patch. - Patch "x86,s390/mm: Move sme_active() and sme_me_mask to x86-specific header" - New patch. - Removed export of sme_active symbol. Suggested by Christoph Hellwig. - Patch "fs/core/vmcore: Move sev_active() reference to x86 arch code" - Removed export of sev_active symbol. Suggested by Christoph Hellwig. - Patch "s390/mm: Remove sev_active() function" - New patch. 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 as well. - Remove dma_check_mask() from kernel/dma/mapping.c. Suggested by Christoph Hellwig. Thiago Jung Bauermann (6): x86,s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig swiotlb: Remove call to sme_active() dma-mapping: Remove dma_check_mask() x86,s390/mm: Move sme_active() and sme_me_mask to x86-specific header fs/core/vmcore: Move sev_active() reference to x86 arch code s390/mm: Remove sev_active() function arch/Kconfig| 3 +++ arch/s390/Kconfig | 4 +--- arch/s390/include/asm/mem_encrypt.h | 5 + arch/s390/mm/init.c | 8 +--- arch/x86/Kconfig| 4 +--- arch/x86/include/asm/mem_encrypt.h | 10 ++ arch/x86/kernel/crash_dump_64.c | 5 + arch/x86/mm/mem_encrypt.c | 2 -- 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 +-- 13 files changed, 42 insertions(+), 47 deletions(-) ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 5/6] fs/core/vmcore: Move sev_active() reference to x86 arch code
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.
Also, remove the export for sev_active() since it's only used in files that
won't be built as modules.
Signed-off-by: Thiago Jung Bauermann
---
arch/x86/kernel/crash_dump_64.c | 5 +
arch/x86/mm/mem_encrypt.c | 1 -
fs/proc/vmcore.c| 8
include/linux/crash_dump.h | 14 ++
include/linux/mem_encrypt.h | 1 -
5 files changed, 23 insertions(+), 6 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/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index 7139f2f43955..b1e823441093 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -349,7 +349,6 @@ bool sev_active(void)
{
return sme_me_mask && sev_enabled;
}
-EXPORT_SYMBOL(sev_active);
/* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_UNENCRYPTED */
bool force_dma_unencrypted(struct device *dev)
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
[PATCH v3 3/6] dma-mapping: Remove dma_check_mask()
sme_active() is an x86-specific function so it's better not to call it from
generic 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 just remove dma_check_mask().
Signed-off-by: Thiago Jung Bauermann
---
kernel/dma/mapping.c | 8
1 file changed, 8 deletions(-)
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index 1f628e7ac709..61eeefbfcb36 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);
@@ -327,7 +321,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;
}
@@ -345,7 +338,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;
}
[PATCH v3 4/6] x86, s390/mm: Move sme_active() and sme_me_mask to x86-specific header
Now that generic code doesn't reference them, move sme_active() and
sme_me_mask to x86's .
Also remove the export for sme_active() since it's only used in files that
won't be built as modules. sme_me_mask on the other hand is used in
arch/x86/kvm/svm.c (via __sme_set() and __psp_pa()) which can be built as a
module so its export needs to stay.
Signed-off-by: Thiago Jung Bauermann
---
arch/s390/include/asm/mem_encrypt.h | 4 +---
arch/x86/include/asm/mem_encrypt.h | 10 ++
arch/x86/mm/mem_encrypt.c | 1 -
include/linux/mem_encrypt.h | 14 +-
4 files changed, 12 insertions(+), 17 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_mask0ULL
-
-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/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index c805f0a5c16e..7139f2f43955 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -344,7 +344,6 @@ bool sme_active(void)
{
return sme_me_mask && !sev_enabled;
}
-EXPORT_SYMBOL(sme_active);
bool sev_active(void)
{
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_mask0ULL
-
-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
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 6/6] s390/mm: Remove sev_active() function
All references to sev_active() were moved to arch/x86 so we don't need to
define it for s390 anymore.
Signed-off-by: Thiago Jung Bauermann
---
arch/s390/include/asm/mem_encrypt.h | 1 -
arch/s390/mm/init.c | 8 +---
2 files changed, 1 insertion(+), 8 deletions(-)
diff --git a/arch/s390/include/asm/mem_encrypt.h
b/arch/s390/include/asm/mem_encrypt.h
index ff813a56bc30..2542cbf7e2d1 100644
--- a/arch/s390/include/asm/mem_encrypt.h
+++ b/arch/s390/include/asm/mem_encrypt.h
@@ -5,7 +5,6 @@
#ifndef __ASSEMBLY__
static inline bool mem_encrypt_active(void) { return false; }
-extern bool sev_active(void);
int set_memory_encrypted(unsigned long addr, int numpages);
int set_memory_decrypted(unsigned long addr, int numpages);
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index 78c319c5ce48..6286eb3e815b 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -155,15 +155,9 @@ int set_memory_decrypted(unsigned long addr, int numpages)
return 0;
}
-/* are we a protected virtualization guest? */
-bool sev_active(void)
-{
- return is_prot_virt_guest();
-}
-
bool force_dma_unencrypted(struct device *dev)
{
- return sev_active();
+ return is_prot_virt_guest();
}
/* protected virtualization */
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 2/6] swiotlb: Remove call to sme_active()
sme_active() is an x86-specific function so it's better not to call it from
generic code.
There's no need to mention which memory encryption feature is active, so
just use a more generic message. Besides, other architectures will have
different names for similar technology.
Signed-off-by: Thiago Jung Bauermann
---
kernel/dma/swiotlb.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
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);
[PATCH v3 1/6] x86, s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig
powerpc is also going to use this feature, so put it in a generic location. Signed-off-by: Thiago Jung Bauermann Reviewed-by: Thomas Gleixner Reviewed-by: Christoph Hellwig --- arch/Kconfig | 3 +++ arch/s390/Kconfig | 4 +--- arch/x86/Kconfig | 4 +--- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/arch/Kconfig b/arch/Kconfig index e8d19c3cb91f..8fc285180848 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -935,6 +935,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 a4ad2733eedf..f43319c44454 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 @@ -68,6 +65,7 @@ config S390 select ARCH_HAS_GCOV_PROFILE_ALL select ARCH_HAS_GIGANTIC_PAGE select ARCH_HAS_KCOV + select ARCH_HAS_MEM_ENCRYPT select ARCH_HAS_PTE_SPECIAL select ARCH_HAS_SET_MEMORY select ARCH_HAS_STRICT_KERNEL_RWX 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_KCOVif X86_64 + select ARCH_HAS_MEM_ENCRYPT select ARCH_HAS_MEMBARRIER_SYNC_CORE select ARCH_HAS_PMEM_APIif 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
Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
Michael S. Tsirkin writes: > On Mon, Jul 15, 2019 at 07:03:03PM -0300, Thiago Jung Bauermann wrote: >> >> Michael S. Tsirkin writes: >> >> > On Mon, Jul 15, 2019 at 05:29:06PM -0300, Thiago Jung Bauermann wrote: >> >> >> >> Michael S. Tsirkin writes: >> >> >> >> > On Sun, Jul 14, 2019 at 02:51:18AM -0300, Thiago Jung Bauermann wrote: >> >> >> >> >> >> >> >> >> Michael S. Tsirkin writes: >> >> >> >> >> >> > So this is what I would call this option: >> >> >> > >> >> >> > VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS >> >> >> > >> >> >> > and the explanation should state that all device >> >> >> > addresses are translated by the platform to identical >> >> >> > addresses. >> >> >> > >> >> >> > In fact this option then becomes more, not less restrictive >> >> >> > than VIRTIO_F_ACCESS_PLATFORM - it's a promise >> >> >> > by guest to only create identity mappings, >> >> >> > and only before driver_ok is set. >> >> >> > This option then would always be negotiated together with >> >> >> > VIRTIO_F_ACCESS_PLATFORM. >> >> >> > >> >> >> > Host then must verify that >> >> >> > 1. full 1:1 mappings are created before driver_ok >> >> >> > or can we make sure this happens before features_ok? >> >> >> > that would be ideal as we could require that features_ok fails >> >> >> > 2. mappings are not modified between driver_ok and reset >> >> >> > i guess attempts to change them will fail - >> >> >> > possibly by causing a guest crash >> >> >> > or some other kind of platform-specific error >> >> >> >> >> >> I think VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS is good, but >> >> >> requiring >> >> >> it to be accompanied by ACCESS_PLATFORM can be a problem. One reason is >> >> >> SLOF as I mentioned above, another is that we would be requiring all >> >> >> guests running on the machine (secure guests or not, since we would use >> >> >> the same configuration for all guests) to support it. But >> >> >> ACCESS_PLATFORM is relatively recent so it's a bit early for that. For >> >> >> instance, Ubuntu 16.04 LTS (which is still supported) doesn't know >> >> >> about >> >> >> it and wouldn't be able to use the device. >> >> > >> >> > OK and your target is to enable use with kernel drivers within >> >> > guests, right? >> >> >> >> Right. >> >> >> >> > My question is, we are defining a new flag here, I guess old guests >> >> > then do not set it. How does it help old guests? Or maybe it's >> >> > not designed to ... >> >> >> >> Indeed. The idea is that QEMU can offer the flag, old guests can reject >> >> it (or even new guests can reject it, if they decide not to convert into >> >> secure VMs) and the feature negotiation will succeed with the flag >> >> unset. >> > >> > OK. And then what does QEMU do? Assume guest is not encrypted I guess? >> >> There's nothing different that QEMU needs to do, with or without the >> flag. the perspective of the host, a secure guest and a regular guest >> work the same way with respect to virtio. > > OK. So now let's get back to implementation. What will > Linux guest driver do? It can't activate DMA API blindly since that > will assume translation also works, right? It can on pseries, because we always have a 1:1 window mapping the whole guest memory. > Or do we somehow limit it to just a specific platform? Yes, we want to accept the new flag only on secure pseries guests. -- Thiago Jung Bauermann IBM Linux Technology Center ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
Michael S. Tsirkin writes: > On Mon, Jul 15, 2019 at 05:29:06PM -0300, Thiago Jung Bauermann wrote: >> >> Michael S. Tsirkin writes: >> >> > On Sun, Jul 14, 2019 at 02:51:18AM -0300, Thiago Jung Bauermann wrote: >> >> >> >> >> >> Michael S. Tsirkin writes: >> >> >> >> > So this is what I would call this option: >> >> > >> >> > VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS >> >> > >> >> > and the explanation should state that all device >> >> > addresses are translated by the platform to identical >> >> > addresses. >> >> > >> >> > In fact this option then becomes more, not less restrictive >> >> > than VIRTIO_F_ACCESS_PLATFORM - it's a promise >> >> > by guest to only create identity mappings, >> >> > and only before driver_ok is set. >> >> > This option then would always be negotiated together with >> >> > VIRTIO_F_ACCESS_PLATFORM. >> >> > >> >> > Host then must verify that >> >> > 1. full 1:1 mappings are created before driver_ok >> >> > or can we make sure this happens before features_ok? >> >> > that would be ideal as we could require that features_ok fails >> >> > 2. mappings are not modified between driver_ok and reset >> >> > i guess attempts to change them will fail - >> >> > possibly by causing a guest crash >> >> > or some other kind of platform-specific error >> >> >> >> I think VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS is good, but requiring >> >> it to be accompanied by ACCESS_PLATFORM can be a problem. One reason is >> >> SLOF as I mentioned above, another is that we would be requiring all >> >> guests running on the machine (secure guests or not, since we would use >> >> the same configuration for all guests) to support it. But >> >> ACCESS_PLATFORM is relatively recent so it's a bit early for that. For >> >> instance, Ubuntu 16.04 LTS (which is still supported) doesn't know about >> >> it and wouldn't be able to use the device. >> > >> > OK and your target is to enable use with kernel drivers within >> > guests, right? >> >> Right. >> >> > My question is, we are defining a new flag here, I guess old guests >> > then do not set it. How does it help old guests? Or maybe it's >> > not designed to ... >> >> Indeed. The idea is that QEMU can offer the flag, old guests can reject >> it (or even new guests can reject it, if they decide not to convert into >> secure VMs) and the feature negotiation will succeed with the flag >> unset. > > OK. And then what does QEMU do? Assume guest is not encrypted I guess? There's nothing different that QEMU needs to do, with or without the flag. the perspective of the host, a secure guest and a regular guest work the same way with respect to virtio. -- Thiago Jung Bauermann IBM Linux Technology Center ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
Michael S. Tsirkin writes: > On Sun, Jul 14, 2019 at 02:51:18AM -0300, Thiago Jung Bauermann wrote: >> >> >> Michael S. Tsirkin writes: >> >> > So this is what I would call this option: >> > >> > VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS >> > >> > and the explanation should state that all device >> > addresses are translated by the platform to identical >> > addresses. >> > >> > In fact this option then becomes more, not less restrictive >> > than VIRTIO_F_ACCESS_PLATFORM - it's a promise >> > by guest to only create identity mappings, >> > and only before driver_ok is set. >> > This option then would always be negotiated together with >> > VIRTIO_F_ACCESS_PLATFORM. >> > >> > Host then must verify that >> > 1. full 1:1 mappings are created before driver_ok >> > or can we make sure this happens before features_ok? >> > that would be ideal as we could require that features_ok fails >> > 2. mappings are not modified between driver_ok and reset >> > i guess attempts to change them will fail - >> > possibly by causing a guest crash >> > or some other kind of platform-specific error >> >> I think VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS is good, but requiring >> it to be accompanied by ACCESS_PLATFORM can be a problem. One reason is >> SLOF as I mentioned above, another is that we would be requiring all >> guests running on the machine (secure guests or not, since we would use >> the same configuration for all guests) to support it. But >> ACCESS_PLATFORM is relatively recent so it's a bit early for that. For >> instance, Ubuntu 16.04 LTS (which is still supported) doesn't know about >> it and wouldn't be able to use the device. > > OK and your target is to enable use with kernel drivers within > guests, right? Right. > My question is, we are defining a new flag here, I guess old guests > then do not set it. How does it help old guests? Or maybe it's > not designed to ... Indeed. The idea is that QEMU can offer the flag, old guests can reject it (or even new guests can reject it, if they decide not to convert into secure VMs) and the feature negotiation will succeed with the flag unset. -- Thiago Jung Bauermann IBM Linux Technology Center ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
Christoph Hellwig 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
Re: [PATCH 1/3] x86, s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig
Hello Janani, Thanks for reviewing the patch. janani 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 >> Reviewed-by: Thomas Gleixner >> --- >> 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_KCOVif X86_64 >> +select ARCH_HAS_MEM_ENCRYPT >> select ARCH_HAS_MEMBARRIER_SYNC_CORE >> select ARCH_HAS_PMEM_APIif 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
Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
Michael S. Tsirkin writes: > On Thu, Jun 27, 2019 at 10:58:40PM -0300, Thiago Jung Bauermann wrote: >> >> Michael S. Tsirkin writes: >> >> > On Mon, Jun 03, 2019 at 10:13:59PM -0300, Thiago Jung Bauermann wrote: >> >> >> >> >> >> Michael S. Tsirkin writes: >> >> >> >> > On Wed, Apr 17, 2019 at 06:42:00PM -0300, Thiago Jung Bauermann wrote: >> >> >> I rephrased it in terms of address translation. What do you think of >> >> >> this version? The flag name is slightly different too: >> >> >> >> >> >> >> >> >> VIRTIO_F_ACCESS_PLATFORM_NO_TRANSLATION This feature has the same >> >> >> meaning as VIRTIO_F_ACCESS_PLATFORM both when set and when not set, >> >> >> with the exception that address translation is guaranteed to be >> >> >> unnecessary when accessing memory addresses supplied to the device >> >> >> by the driver. Which is to say, the device will always use physical >> >> >> addresses matching addresses used by the driver (typically meaning >> >> >> physical addresses used by the CPU) and not translated further. >> >> >> This >> >> >> flag should be set by the guest if offered, but to allow for >> >> >> backward-compatibility device implementations allow for it to be >> >> >> left unset by the guest. It is an error to set both this flag and >> >> >> VIRTIO_F_ACCESS_PLATFORM. >> >> > >> >> > >> >> > >> >> > >> >> > OK so VIRTIO_F_ACCESS_PLATFORM is designed to allow unpriveledged >> >> > drivers. This is why devices fail when it's not negotiated. >> >> >> >> Just to clarify, what do you mean by unprivileged drivers? Is it drivers >> >> implemented in guest userspace such as with VFIO? Or unprivileged in >> >> some other sense such as needing to use bounce buffers for some reason? >> > >> > I had drivers in guest userspace in mind. >> >> Great. Thanks for clarifying. >> >> I don't think this flag would work for guest userspace drivers. Should I >> add a note about that in the flag definition? > > I think you need to clarify access protection rules. Is it only > translation that is bypassed or is any platform-specific > protection mechanism bypassed too? It is only translation. In a secure guest, if the device tries to access a memory address that wasn't provided by the driver then the architecture will deny that access. If the device accesses addresses provided to it by the driver, then there's no protection mechanism or translation to get in the way. >> >> > This confuses me. >> >> > If driver is unpriveledged then what happens with this flag? >> >> > It can supply any address it wants. Will that corrupt kernel >> >> > memory? >> >> >> >> Not needing address translation doesn't necessarily mean that there's no >> >> IOMMU. On powerpc we don't use VIRTIO_F_ACCESS_PLATFORM but there's >> >> always an IOMMU present. And we also support VFIO drivers. The VFIO API >> >> for pseries (sPAPR section in Documentation/vfio.txt) has extra ioctls >> >> to program the IOMMU. >> >> >> >> For our use case, we don't need address translation because we set up an >> >> identity mapping in the IOMMU so that the device can use guest physical >> >> addresses. > > OK so I think I am beginning to see it in a different light. Right now the > specific > platform creates an identity mapping. That in turn means DMA API can be > fast - it does not need to do anything. What you are looking for is a > way to tell host it's an identity mapping - just as an optimization. > > Is that right? Almost. Theoretically it is just an optimization. But in practice the pseries boot firmware (SLOF) doesn't support IOMMU_PLATFORM so it's not possible to boot a guest from a device with that flag set. > So this is what I would call this option: > > VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS > > and the explanation should state that all device > addresses are translated by the platform to identical > addresses. > > In fact this option then becomes more, not less restrictive > than VIRTIO_F_ACCESS_PLATFORM - it's a promise > by guest to only create identity mappings, > and only before driver_ok is set. > This option then would always be negotiated together with > VIRTIO_F_ACCES
Re: [PATCH 0/3] Remove x86-specific code from generic headers
I forgot to mark this series as v2 when generating the patches. Sorry for the confusion. -- Thiago Jung Bauermann IBM Linux Technology Center
[PATCH 2/3] DMA mapping: Move SME handling to x86-specific files
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
---
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_mask0ULL
-
-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_mask0ULL
-
-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);
[PATCH 1/3] x86,s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig
powerpc is also going to use this feature, so put it in a generic location. Signed-off-by: Thiago Jung Bauermann Reviewed-by: Thomas Gleixner --- 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_KCOVif X86_64 + select ARCH_HAS_MEM_ENCRYPT select ARCH_HAS_MEMBARRIER_SYNC_CORE select ARCH_HAS_PMEM_APIif 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
[PATCH 0/3] Remove x86-specific code from generic headers
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 . 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.lenda...@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 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 :
#ifndef _ASM_POWERPC_MEM_ENCRYPT_H
#define _ASM_POWERPC_MEM_ENCRYPT_H
#include
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 as well.
- Remove dma_check_mask() from kernel/dma/mapping.c. Suggested by
Christoph Hellwig.
--
¹
https://lore.kernel.org/linuxppc-dev/20190521044912.1375-12-bauer...@linux.ibm.com/
² https://lore.kernel.org/kvm/20190612111236.99538-2-pa...@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(-)
[PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
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
---
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 */
Re: [PATCH 2/3] DMA mapping: Move SME handling to x86-specific files
[ Cc'ing Tom Lendacky which I forgot to do earlier. Sorry about that. ] Hello Christoph, Christoph Hellwig writes: > Honestly I think this code should go away without any replacement. > 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. Makes sense. I'll submit a v2 which just removes this code. -- Thiago Jung Bauermann IBM Linux Technology Center
Re: [PATCH 1/3] x86/Kconfig: Move ARCH_HAS_MEM_ENCRYPT to arch/Kconfig
Hello Thomas, Thanks for quickly reviewing the patches. Thomas Gleixner writes: > On Fri, 12 Jul 2019, Thiago Jung Bauermann wrote: > >> powerpc and s390 are going to use this feature as well, so put it in a >> generic location. >> >> Signed-off-by: Thiago Jung Bauermann > > Reviewed-by: Thomas Gleixner Thanks! -- Thiago Jung Bauermann IBM Linux Technology Center
Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
[ Cc'ing Tom Lendacky which I forgot to do earlier. Sorry about that. ] Hello Halil, Thanks for the quick review. Halil Pasic writes: > On Fri, 12 Jul 2019 02:36:31 -0300 > Thiago Jung Bauermann 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 >> --- >> 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 as well. -- Thiago Jung Bauermann IBM Linux Technology Center
[PATCH 2/3] DMA mapping: Move SME handling to x86-specific files
Secure Memory Encryption is an x86-specific feature, so it shouldn't appear
in generic kernel code.
Introduce ARCH_HAS_DMA_CHECK_MASK so that x86 can define its own
dma_check_mask() for the SME check.
In SWIOTLB code, there's no need to mention which memory encryption
feature is active. Also, other architectures will have different names so
this gets unwieldy quickly.
Signed-off-by: Thiago Jung Bauermann
---
arch/x86/Kconfig | 1 +
arch/x86/include/asm/dma-mapping.h | 7 +++
arch/x86/include/asm/mem_encrypt.h | 10 ++
include/linux/mem_encrypt.h| 14 +-
kernel/dma/Kconfig | 3 +++
kernel/dma/mapping.c | 4 ++--
kernel/dma/swiotlb.c | 3 +--
7 files changed, 25 insertions(+), 17 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 7f4d28da8fe3..dbabe42e7f1c 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -61,6 +61,7 @@ config X86
select ARCH_HAS_ACPI_TABLE_UPGRADE if ACPI
select ARCH_HAS_DEBUG_VIRTUAL
select ARCH_HAS_DEVMEM_IS_ALLOWED
+ select ARCH_HAS_DMA_CHECK_MASK
select ARCH_HAS_ELF_RANDOMIZE
select ARCH_HAS_FAST_MULTIPLIER
select ARCH_HAS_FILTER_PGPROT
diff --git a/arch/x86/include/asm/dma-mapping.h
b/arch/x86/include/asm/dma-mapping.h
index 6b15a24930e0..55e710ba95a5 100644
--- a/arch/x86/include/asm/dma-mapping.h
+++ b/arch/x86/include/asm/dma-mapping.h
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
extern int iommu_merge;
extern int panic_on_overflow;
@@ -23,4 +24,10 @@ static inline const struct dma_map_ops
*get_arch_dma_ops(struct bus_type *bus)
return dma_ops;
}
+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");
+}
+
#endif
diff --git a/arch/x86/include/asm/mem_encrypt.h
b/arch/x86/include/asm/mem_encrypt.h
index 616f8e637bc3..e4c9e1a57d25 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -95,6 +95,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 b310a9c18113..f2e399fb626b 100644
--- a/include/linux/mem_encrypt.h
+++ b/include/linux/mem_encrypt.h
@@ -21,23 +21,11 @@
#else /* !CONFIG_ARCH_HAS_MEM_ENCRYPT */
-#define sme_me_mask0ULL
-
-static inline bool sme_active(void) { return false; }
static inline bool sev_active(void) { return false; }
+static inline bool mem_encrypt_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/Kconfig b/kernel/dma/Kconfig
index 9decbba255fc..34b44bfba372 100644
--- a/kernel/dma/Kconfig
+++ b/kernel/dma/Kconfig
@@ -17,6 +17,9 @@ config ARCH_DMA_ADDR_T_64BIT
config ARCH_HAS_DMA_COHERENCE_H
bool
+config ARCH_HAS_DMA_CHECK_MASK
+ bool
+
config ARCH_HAS_DMA_SET_MASK
bool
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index f7afdadb6770..ed46f88378d4 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -291,11 +291,11 @@ void dma_free_attrs(struct device *dev, size_t size, void
*cpu_addr,
}
EXPORT_SYMBOL(dma_free_attrs);
+#ifndef CONFIG_ARCH_HAS_DMA_CHECK_MASK
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");
}
+#endif
int dma_supported(struct device *dev, u64 mask)
{
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 13f0cb080a4d..67482ad6aab2 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
[PATCH 1/3] x86/Kconfig: Move ARCH_HAS_MEM_ENCRYPT to arch/Kconfig
powerpc and s390 are going to use this feature as well, so put it in a generic location. Signed-off-by: Thiago Jung Bauermann --- arch/Kconfig | 3 +++ arch/x86/Kconfig | 4 +--- 2 files changed, 4 insertions(+), 3 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/x86/Kconfig b/arch/x86/Kconfig index 12e02a8f9de7..7f4d28da8fe3 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -67,6 +67,7 @@ config X86 select ARCH_HAS_FORTIFY_SOURCE select ARCH_HAS_GCOV_PROFILE_ALL select ARCH_HAS_KCOVif X86_64 + select ARCH_HAS_MEM_ENCRYPT select ARCH_HAS_MEMBARRIER_SYNC_CORE select ARCH_HAS_PMEM_APIif X86_64 select ARCH_HAS_PTE_SPECIAL @@ -1500,9 +1501,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
[PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
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
---
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 */
[PATCH 0/3] Remove x86-specific code from generic headers
Hello,
Both powerpc¹ and s390² are adding 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 series does.
After this patch set, this is powerpc's :
#ifndef _ASM_POWERPC_MEM_ENCRYPT_H
#define _ASM_POWERPC_MEM_ENCRYPT_H
#include
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 */
I don't have a way to test SME nor SEV, so the patches have only been build
tested. They assume the presence of the following two commits:
Commit 4eb5fec31e61 ("fs/proc/vmcore: Enable dumping of encrypted memory
when SEV was active"), which is now in Linus' master branch;
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.lenda...@amd.com/
Thiago Jung Bauermann (3):
x86/Kconfig: Move ARCH_HAS_MEM_ENCRYPT 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/x86/Kconfig | 5 ++---
arch/x86/include/asm/dma-mapping.h | 7 +++
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/Kconfig | 3 +++
kernel/dma/mapping.c | 4 ++--
kernel/dma/swiotlb.c | 3 +--
11 files changed, 52 insertions(+), 25 deletions(-)
--
¹
https://lore.kernel.org/linuxppc-dev/20190521044912.1375-12-bauer...@linux.ibm.com/
² https://lore.kernel.org/kvm/20190612111236.99538-2-pa...@linux.ibm.com/
Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
Michael S. Tsirkin writes: > On Mon, Jun 03, 2019 at 10:13:59PM -0300, Thiago Jung Bauermann wrote: >> >> >> Michael S. Tsirkin writes: >> >> > On Wed, Apr 17, 2019 at 06:42:00PM -0300, Thiago Jung Bauermann wrote: >> >> I rephrased it in terms of address translation. What do you think of >> >> this version? The flag name is slightly different too: >> >> >> >> >> >> VIRTIO_F_ACCESS_PLATFORM_NO_TRANSLATION This feature has the same >> >> meaning as VIRTIO_F_ACCESS_PLATFORM both when set and when not set, >> >> with the exception that address translation is guaranteed to be >> >> unnecessary when accessing memory addresses supplied to the device >> >> by the driver. Which is to say, the device will always use physical >> >> addresses matching addresses used by the driver (typically meaning >> >> physical addresses used by the CPU) and not translated further. This >> >> flag should be set by the guest if offered, but to allow for >> >> backward-compatibility device implementations allow for it to be >> >> left unset by the guest. It is an error to set both this flag and >> >> VIRTIO_F_ACCESS_PLATFORM. >> > >> > >> > OK so VIRTIO_F_ACCESS_PLATFORM is designed to allow unpriveledged >> > drivers. This is why devices fail when it's not negotiated. >> >> Just to clarify, what do you mean by unprivileged drivers? Is it drivers >> implemented in guest userspace such as with VFIO? Or unprivileged in >> some other sense such as needing to use bounce buffers for some reason? > > I had drivers in guest userspace in mind. Great. Thanks for clarifying. I don't think this flag would work for guest userspace drivers. Should I add a note about that in the flag definition? >> > This confuses me. >> > If driver is unpriveledged then what happens with this flag? >> > It can supply any address it wants. Will that corrupt kernel >> > memory? >> >> Not needing address translation doesn't necessarily mean that there's no >> IOMMU. On powerpc we don't use VIRTIO_F_ACCESS_PLATFORM but there's >> always an IOMMU present. And we also support VFIO drivers. The VFIO API >> for pseries (sPAPR section in Documentation/vfio.txt) has extra ioctls >> to program the IOMMU. >> >> For our use case, we don't need address translation because we set up an >> identity mapping in the IOMMU so that the device can use guest physical >> addresses. > > And can it access any guest physical address? Sorry, I was mistaken. We do support VFIO in guests but not for virtio devices, only for regular PCI devices. In which case they will use address translation. >> If the guest kernel is concerned that an unprivileged driver could >> jeopardize its integrity it should not negotiate this feature flag. > > Unfortunately flag negotiation is done through config space > and so can be overwritten by the driver. Ok, so the guest kernel has to forbid VFIO access on devices where this flag is advertised. >> Perhaps there should be a note about this in the flag definition? This >> concern is platform-dependant though. I don't believe it's an issue in >> pseries. > > Again ACCESS_PLATFORM has a pretty open definition. It does actually > say it's all up to the platform. > > Specifically how will VIRTIO_F_ACCESS_PLATFORM_NO_TRANSLATION be > implemented portably? virtio has no portable way to know > whether DMA API bypasses translation. The fact that VIRTIO_F_ACCESS_PLATFORM_NO_TRANSLATION is set communicates that knowledge to virtio. There is a shared understanding between the guest and the host about what this flag being set means. -- Thiago Jung Bauermann IBM Linux Technology Center ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
Michael S. Tsirkin writes: > On Wed, Apr 17, 2019 at 06:42:00PM -0300, Thiago Jung Bauermann wrote: >> I rephrased it in terms of address translation. What do you think of >> this version? The flag name is slightly different too: >> >> >> VIRTIO_F_ACCESS_PLATFORM_NO_TRANSLATION This feature has the same >> meaning as VIRTIO_F_ACCESS_PLATFORM both when set and when not set, >> with the exception that address translation is guaranteed to be >> unnecessary when accessing memory addresses supplied to the device >> by the driver. Which is to say, the device will always use physical >> addresses matching addresses used by the driver (typically meaning >> physical addresses used by the CPU) and not translated further. This >> flag should be set by the guest if offered, but to allow for >> backward-compatibility device implementations allow for it to be >> left unset by the guest. It is an error to set both this flag and >> VIRTIO_F_ACCESS_PLATFORM. > > > OK so VIRTIO_F_ACCESS_PLATFORM is designed to allow unpriveledged > drivers. This is why devices fail when it's not negotiated. Just to clarify, what do you mean by unprivileged drivers? Is it drivers implemented in guest userspace such as with VFIO? Or unprivileged in some other sense such as needing to use bounce buffers for some reason? > This confuses me. > If driver is unpriveledged then what happens with this flag? > It can supply any address it wants. Will that corrupt kernel > memory? Not needing address translation doesn't necessarily mean that there's no IOMMU. On powerpc we don't use VIRTIO_F_ACCESS_PLATFORM but there's always an IOMMU present. And we also support VFIO drivers. The VFIO API for pseries (sPAPR section in Documentation/vfio.txt) has extra ioctls to program the IOMMU. For our use case, we don't need address translation because we set up an identity mapping in the IOMMU so that the device can use guest physical addresses. If the guest kernel is concerned that an unprivileged driver could jeopardize its integrity it should not negotiate this feature flag. Perhaps there should be a note about this in the flag definition? This concern is platform-dependant though. I don't believe it's an issue in pseries. -- Thiago Jung Bauermann IBM Linux Technology Center
Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
Michael S. Tsirkin writes: > On Wed, Apr 24, 2019 at 10:01:56PM -0300, Thiago Jung Bauermann wrote: >> >> Michael S. Tsirkin writes: >> >> > On Wed, Apr 17, 2019 at 06:42:00PM -0300, Thiago Jung Bauermann wrote: >> >> >> >> Michael S. Tsirkin writes: >> >> >> >> > On Thu, Mar 21, 2019 at 09:05:04PM -0300, Thiago Jung Bauermann wrote: >> >> >> >> >> >> Michael S. Tsirkin writes: >> >> >> >> >> >> > On Wed, Mar 20, 2019 at 01:13:41PM -0300, Thiago Jung Bauermann >> >> >> > wrote: >> >> >> >> >From what I understand of the ACCESS_PLATFORM definition, the host >> >> >> >> >will >> >> >> >> only ever try to access memory addresses that are supplied to it by >> >> >> >> the >> >> >> >> guest, so all of the secure guest memory that the host cares about >> >> >> >> is >> >> >> >> accessible: >> >> >> >> >> >> >> >> If this feature bit is set to 0, then the device has same >> >> >> >> access to >> >> >> >> memory addresses supplied to it as the driver has. In >> >> >> >> particular, >> >> >> >> the device will always use physical addresses matching addresses >> >> >> >> used by the driver (typically meaning physical addresses used >> >> >> >> by the >> >> >> >> CPU) and not translated further, and can access any address >> >> >> >> supplied >> >> >> >> to it by the driver. When clear, this overrides any >> >> >> >> platform-specific description of whether device access is >> >> >> >> limited or >> >> >> >> translated in any way, e.g. whether an IOMMU may be present. >> >> >> >> >> >> >> >> All of the above is true for POWER guests, whether they are secure >> >> >> >> guests or not. >> >> >> >> >> >> >> >> Or are you saying that a virtio device may want to access memory >> >> >> >> addresses that weren't supplied to it by the driver? >> >> >> > >> >> >> > Your logic would apply to IOMMUs as well. For your mode, there are >> >> >> > specific encrypted memory regions that driver has access to but >> >> >> > device >> >> >> > does not. that seems to violate the constraint. >> >> >> >> >> >> Right, if there's a pre-configured 1:1 mapping in the IOMMU such that >> >> >> the device can ignore the IOMMU for all practical purposes I would >> >> >> indeed say that the logic would apply to IOMMUs as well. :-) >> >> >> >> >> >> I guess I'm still struggling with the purpose of signalling to the >> >> >> driver that the host may not have access to memory addresses that it >> >> >> will never try to access. >> >> > >> >> > For example, one of the benefits is to signal to host that driver does >> >> > not expect ability to access all memory. If it does, host can >> >> > fail initialization gracefully. >> >> >> >> But why would the ability to access all memory be necessary or even >> >> useful? When would the host access memory that the driver didn't tell it >> >> to access? >> > >> > When I say all memory I mean even memory not allowed by the IOMMU. >> >> Yes, but why? How is that memory relevant? > > It's relevant when driver is not trusted to only supply correct > addresses. The feature was originally designed to support userspace > drivers within guests. Ah, thanks for clarifying. I don't think that's a problem in our case. If the guest provides an incorrect address, the hardware simply won't allow the host to access it. >> >> >> > Another idea is maybe something like virtio-iommu? >> >> >> >> >> >> You mean, have legacy guests use virtio-iommu to request an IOMMU >> >> >> bypass? If so, it's an interesting idea for new guests but it doesn't >> >> >> help with guests that are out today in the field, which don't have A >> >> >> virtio-iommu driver.
Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
Michael S. Tsirkin writes: > On Wed, Apr 17, 2019 at 06:42:00PM -0300, Thiago Jung Bauermann wrote: >> >> Michael S. Tsirkin writes: >> >> > On Thu, Mar 21, 2019 at 09:05:04PM -0300, Thiago Jung Bauermann wrote: >> >> >> >> Michael S. Tsirkin writes: >> >> >> >> > On Wed, Mar 20, 2019 at 01:13:41PM -0300, Thiago Jung Bauermann wrote: >> >> >> >From what I understand of the ACCESS_PLATFORM definition, the host >> >> >> >will >> >> >> only ever try to access memory addresses that are supplied to it by the >> >> >> guest, so all of the secure guest memory that the host cares about is >> >> >> accessible: >> >> >> >> >> >> If this feature bit is set to 0, then the device has same access to >> >> >> memory addresses supplied to it as the driver has. In particular, >> >> >> the device will always use physical addresses matching addresses >> >> >> used by the driver (typically meaning physical addresses used by >> >> >> the >> >> >> CPU) and not translated further, and can access any address >> >> >> supplied >> >> >> to it by the driver. When clear, this overrides any >> >> >> platform-specific description of whether device access is limited >> >> >> or >> >> >> translated in any way, e.g. whether an IOMMU may be present. >> >> >> >> >> >> All of the above is true for POWER guests, whether they are secure >> >> >> guests or not. >> >> >> >> >> >> Or are you saying that a virtio device may want to access memory >> >> >> addresses that weren't supplied to it by the driver? >> >> > >> >> > Your logic would apply to IOMMUs as well. For your mode, there are >> >> > specific encrypted memory regions that driver has access to but device >> >> > does not. that seems to violate the constraint. >> >> >> >> Right, if there's a pre-configured 1:1 mapping in the IOMMU such that >> >> the device can ignore the IOMMU for all practical purposes I would >> >> indeed say that the logic would apply to IOMMUs as well. :-) >> >> >> >> I guess I'm still struggling with the purpose of signalling to the >> >> driver that the host may not have access to memory addresses that it >> >> will never try to access. >> > >> > For example, one of the benefits is to signal to host that driver does >> > not expect ability to access all memory. If it does, host can >> > fail initialization gracefully. >> >> But why would the ability to access all memory be necessary or even >> useful? When would the host access memory that the driver didn't tell it >> to access? > > When I say all memory I mean even memory not allowed by the IOMMU. Yes, but why? How is that memory relevant? >> >> >> >> > But the name "sev_active" makes me scared because at least AMD >> >> >> >> > guys who >> >> >> >> > were doing the sensible thing and setting ACCESS_PLATFORM >> >> >> >> >> >> >> >> My understanding is, AMD guest-platform knows in advance that their >> >> >> >> guest will run in secure mode and hence sets the flag at the time >> >> >> >> of VM >> >> >> >> instantiation. Unfortunately we dont have that luxury on our >> >> >> >> platforms. >> >> >> > >> >> >> > Well you do have that luxury. It looks like that there are existing >> >> >> > guests that already acknowledge ACCESS_PLATFORM and you are not happy >> >> >> > with how that path is slow. So you are trying to optimize for >> >> >> > them by clearing ACCESS_PLATFORM and then you have lost ability >> >> >> > to invoke DMA API. >> >> >> > >> >> >> > For example if there was another flag just like ACCESS_PLATFORM >> >> >> > just not yet used by anyone, you would be all fine using that right? >> >> >> >> >> >> Yes, a new flag sounds like a great idea. What about the definition >> >> >> below? >> >> >> >> >> >> VIRTIO_F_ACCESS_PLATFORM_NO_IOMMU This fe
Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
David Gibson writes:
> On Sat, Mar 23, 2019 at 05:01:35PM -0400, Michael S. Tsirkin wrote:
>> On Thu, Mar 21, 2019 at 09:05:04PM -0300, Thiago Jung Bauermann wrote:
>> > Michael S. Tsirkin writes:
> [snip]
>> > >> > Is there any justification to doing that beyond someone putting
>> > >> > out slow code in the past?
>> > >>
>> > >> The definition of the ACCESS_PLATFORM flag is generic and captures the
>> > >> notion of memory access restrictions for the device. Unfortunately, on
>> > >> powerpc pSeries guests it also implies that the IOMMU is turned on
>> > >
>> > > IIUC that's really because on pSeries IOMMU is *always* turned on.
>> > > Platform has no way to say what you want it to say
>> > > which is bypass the iommu for the specific device.
>> >
>> > Yes, that's correct. pSeries guests running on KVM are in a gray area
>> > where theoretically they use an IOMMU but in practice KVM ignores it.
>> > It's unfortunate but it's the reality on the ground today. :-/
>
> Um.. I'm not sure what you mean by this. As far as I'm concerned
> there is always a guest-visible (paravirtualized) IOMMU, and that will
> be backed onto the host IOMMU when necessary.
There is, but vhost will ignore it and directly map the guest memory
when ACCESS_PLATFORM (the flag previously known as IOMMU_PLATFORM) isn't
set. From QEMU's hw/virtio/vhost.c:
static int vhost_dev_has_iommu(struct vhost_dev *dev)
{
VirtIODevice *vdev = dev->vdev;
return virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
}
static void *vhost_memory_map(struct vhost_dev *dev, hwaddr addr,
hwaddr *plen, int is_write)
{
if (!vhost_dev_has_iommu(dev)) {
return cpu_physical_memory_map(addr, plen, is_write);
} else {
return (void *)(uintptr_t)addr;
}
}
--
Thiago Jung Bauermann
IBM Linux Technology Center
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
Michael S. Tsirkin writes: > On Thu, Mar 21, 2019 at 09:05:04PM -0300, Thiago Jung Bauermann wrote: >> >> Michael S. Tsirkin writes: >> >> > On Wed, Mar 20, 2019 at 01:13:41PM -0300, Thiago Jung Bauermann wrote: >> >> >From what I understand of the ACCESS_PLATFORM definition, the host will >> >> only ever try to access memory addresses that are supplied to it by the >> >> guest, so all of the secure guest memory that the host cares about is >> >> accessible: >> >> >> >> If this feature bit is set to 0, then the device has same access to >> >> memory addresses supplied to it as the driver has. In particular, >> >> the device will always use physical addresses matching addresses >> >> used by the driver (typically meaning physical addresses used by the >> >> CPU) and not translated further, and can access any address supplied >> >> to it by the driver. When clear, this overrides any >> >> platform-specific description of whether device access is limited or >> >> translated in any way, e.g. whether an IOMMU may be present. >> >> >> >> All of the above is true for POWER guests, whether they are secure >> >> guests or not. >> >> >> >> Or are you saying that a virtio device may want to access memory >> >> addresses that weren't supplied to it by the driver? >> > >> > Your logic would apply to IOMMUs as well. For your mode, there are >> > specific encrypted memory regions that driver has access to but device >> > does not. that seems to violate the constraint. >> >> Right, if there's a pre-configured 1:1 mapping in the IOMMU such that >> the device can ignore the IOMMU for all practical purposes I would >> indeed say that the logic would apply to IOMMUs as well. :-) >> >> I guess I'm still struggling with the purpose of signalling to the >> driver that the host may not have access to memory addresses that it >> will never try to access. > > For example, one of the benefits is to signal to host that driver does > not expect ability to access all memory. If it does, host can > fail initialization gracefully. But why would the ability to access all memory be necessary or even useful? When would the host access memory that the driver didn't tell it to access? >> >> >> > But the name "sev_active" makes me scared because at least AMD guys >> >> >> > who >> >> >> > were doing the sensible thing and setting ACCESS_PLATFORM >> >> >> >> >> >> My understanding is, AMD guest-platform knows in advance that their >> >> >> guest will run in secure mode and hence sets the flag at the time of VM >> >> >> instantiation. Unfortunately we dont have that luxury on our platforms. >> >> > >> >> > Well you do have that luxury. It looks like that there are existing >> >> > guests that already acknowledge ACCESS_PLATFORM and you are not happy >> >> > with how that path is slow. So you are trying to optimize for >> >> > them by clearing ACCESS_PLATFORM and then you have lost ability >> >> > to invoke DMA API. >> >> > >> >> > For example if there was another flag just like ACCESS_PLATFORM >> >> > just not yet used by anyone, you would be all fine using that right? >> >> >> >> Yes, a new flag sounds like a great idea. What about the definition >> >> below? >> >> >> >> VIRTIO_F_ACCESS_PLATFORM_NO_IOMMU This feature has the same meaning as >> >> VIRTIO_F_ACCESS_PLATFORM both when set and when not set, with the >> >> exception that the IOMMU is explicitly defined to be off or bypassed >> >> when accessing memory addresses supplied to the device by the >> >> driver. This flag should be set by the guest if offered, but to >> >> allow for backward-compatibility device implementations allow for it >> >> to be left unset by the guest. It is an error to set both this flag >> >> and VIRTIO_F_ACCESS_PLATFORM. >> > >> > It looks kind of narrow but it's an option. >> >> Great! >> >> > I wonder how we'll define what's an iommu though. >> >> Hm, it didn't occur to me it could be an issue. I'll try. I rephrased it in terms of address translation. What do you think of this version? The flag name is slightly different too: VIRTIO_F_ACCESS_PLATFORM_NO_TRANSLATION
Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
Michael S. Tsirkin writes: > On Wed, Mar 20, 2019 at 01:13:41PM -0300, Thiago Jung Bauermann wrote: >> >> Another way of looking at this issue which also explains our reluctance >> >> is that the only difference between a secure guest and a regular guest >> >> (at least regarding virtio) is that the former uses swiotlb while the >> >> latter doens't. >> > >> > But swiotlb is just one implementation. It's a guest internal thing. The >> > issue is that memory isn't host accessible. >> >> >From what I understand of the ACCESS_PLATFORM definition, the host will >> only ever try to access memory addresses that are supplied to it by the >> guest, so all of the secure guest memory that the host cares about is >> accessible: >> >> If this feature bit is set to 0, then the device has same access to >> memory addresses supplied to it as the driver has. In particular, >> the device will always use physical addresses matching addresses >> used by the driver (typically meaning physical addresses used by the >> CPU) and not translated further, and can access any address supplied >> to it by the driver. When clear, this overrides any >> platform-specific description of whether device access is limited or >> translated in any way, e.g. whether an IOMMU may be present. >> >> All of the above is true for POWER guests, whether they are secure >> guests or not. >> >> Or are you saying that a virtio device may want to access memory >> addresses that weren't supplied to it by the driver? > > Your logic would apply to IOMMUs as well. For your mode, there are > specific encrypted memory regions that driver has access to but device > does not. that seems to violate the constraint. Right, if there's a pre-configured 1:1 mapping in the IOMMU such that the device can ignore the IOMMU for all practical purposes I would indeed say that the logic would apply to IOMMUs as well. :-) I guess I'm still struggling with the purpose of signalling to the driver that the host may not have access to memory addresses that it will never try to access. >> >> And from the device's point of view they're >> >> indistinguishable. It can't tell one guest that is using swiotlb from >> >> one that isn't. And that implies that secure guest vs regular guest >> >> isn't a virtio interface issue, it's "guest internal affairs". So >> >> there's no reason to reflect that in the feature flags. >> > >> > So don't. The way not to reflect that in the feature flags is >> > to set ACCESS_PLATFORM. Then you say *I don't care let platform device*. >> > >> > >> > Without ACCESS_PLATFORM >> > virtio has a very specific opinion about the security of the >> > device, and that opinion is that device is part of the guest >> > supervisor security domain. >> >> Sorry for being a bit dense, but not sure what "the device is part of >> the guest supervisor security domain" means. In powerpc-speak, >> "supervisor" is the operating system so perhaps that explains my >> confusion. Are you saying that without ACCESS_PLATFORM, the guest >> considers the host to be part of the guest operating system's security >> domain? > > I think so. The spec says "device has same access as driver". Ok, makes sense. >> If so, does that have any other implication besides "the host >> can access any address supplied to it by the driver"? If that is the >> case, perhaps the definition of ACCESS_PLATFORM needs to be amended to >> include that information because it's not part of the current >> definition. >> >> >> > But the name "sev_active" makes me scared because at least AMD guys who >> >> > were doing the sensible thing and setting ACCESS_PLATFORM >> >> >> >> My understanding is, AMD guest-platform knows in advance that their >> >> guest will run in secure mode and hence sets the flag at the time of VM >> >> instantiation. Unfortunately we dont have that luxury on our platforms. >> > >> > Well you do have that luxury. It looks like that there are existing >> > guests that already acknowledge ACCESS_PLATFORM and you are not happy >> > with how that path is slow. So you are trying to optimize for >> > them by clearing ACCESS_PLATFORM and then you have lost ability >> > to invoke DMA API. >> > >> > For example if there was another flag just like ACCESS_PLATFORM >> > just not yet used by anyone, you would be all fi
Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
Hello Michael, Sorry for the delay in responding. We had some internal discussions on this. Michael S. Tsirkin writes: > On Mon, Feb 04, 2019 at 04:14:20PM -0200, Thiago Jung Bauermann wrote: >> >> Hello Michael, >> >> Michael S. Tsirkin writes: >> >> > On Tue, Jan 29, 2019 at 03:42:44PM -0200, Thiago Jung Bauermann wrote: >> So while ACCESS_PLATFORM solves our problems for secure guests, we can't >> turn it on by default because we can't affect legacy systems. Doing so >> would penalize existing systems that can access all memory. They would >> all have to unnecessarily go through address translations, and take a >> performance hit. > > So as step one, you just give hypervisor admin an option to run legacy > systems faster by blocking secure mode. I don't see why that is > so terrible. There are a few reasons why: 1. It's bad user experience to require people to fiddle with knobs for obscure reasons if it's possible to design things such that they Just Work. 2. "User" in this case can be a human directly calling QEMU, but could also be libvirt or one of its users, or some other framework. This means having to adjust and/or educate an open-ended number of people and software. It's best avoided if possible. 3. The hypervisor admin and the admin of the guest system don't necessarily belong to the same organization (e.g., cloud provider and cloud customer), so there may be some friction when they need to coordinate to get this right. 4. A feature of our design is that the guest may or may not decide to "go secure" at boot time, so it's best not to depend on flags that may or may not have been set at the time QEMU was started. >> The semantics of ACCESS_PLATFORM assume that the hypervisor/QEMU knows >> in advance - right when the VM is instantiated - that it will not have >> access to all guest memory. > > Not quite. It just means that hypervisor can live with not having > access to all memory. If platform wants to give it access > to all memory that is quite all right. Except that on powerpc it also means "there's an IOMMU present" and there's no way to say "bypass IOMMU translation". :-/ >> Another way of looking at this issue which also explains our reluctance >> is that the only difference between a secure guest and a regular guest >> (at least regarding virtio) is that the former uses swiotlb while the >> latter doens't. > > But swiotlb is just one implementation. It's a guest internal thing. The > issue is that memory isn't host accessible. >From what I understand of the ACCESS_PLATFORM definition, the host will only ever try to access memory addresses that are supplied to it by the guest, so all of the secure guest memory that the host cares about is accessible: If this feature bit is set to 0, then the device has same access to memory addresses supplied to it as the driver has. In particular, the device will always use physical addresses matching addresses used by the driver (typically meaning physical addresses used by the CPU) and not translated further, and can access any address supplied to it by the driver. When clear, this overrides any platform-specific description of whether device access is limited or translated in any way, e.g. whether an IOMMU may be present. All of the above is true for POWER guests, whether they are secure guests or not. Or are you saying that a virtio device may want to access memory addresses that weren't supplied to it by the driver? >> And from the device's point of view they're >> indistinguishable. It can't tell one guest that is using swiotlb from >> one that isn't. And that implies that secure guest vs regular guest >> isn't a virtio interface issue, it's "guest internal affairs". So >> there's no reason to reflect that in the feature flags. > > So don't. The way not to reflect that in the feature flags is > to set ACCESS_PLATFORM. Then you say *I don't care let platform device*. > > > Without ACCESS_PLATFORM > virtio has a very specific opinion about the security of the > device, and that opinion is that device is part of the guest > supervisor security domain. Sorry for being a bit dense, but not sure what "the device is part of the guest supervisor security domain" means. In powerpc-speak, "supervisor" is the operating system so perhaps that explains my confusion. Are you saying that without ACCESS_PLATFORM, the guest considers the host to be part of the guest operating system's security domain? If so, does that have any other implication besides "the host can access any address supplied to it by the driver"? If that is the case, perhaps the definition of ACCESS_PLATFORM needs to be amended to include that information becau
Re: [PATCH v7 0/7] Add virtio-iommu driver
Hello Jean-Philippe, Jean-Philippe Brucker writes: > Makes sense, though I think other virtio devices have been developed a > little more organically: device and driver code got upstreamed first, > and then the specification describing their interface got merged into > the standard. For example I believe that code for crypto, input and GPU > devices were upstreamed long before the specification was merged. Once > an implementation is upstream, the interface is expected to be > backward-compatible (all subsequent changes are introduced using feature > bits). > > So I've been working with this process in mind, also described by Jens > at KVM forum 2017 [3]: > (1) Reserve a device ID, and get that merged into virtio (ID 23 for > virtio-iommu was reserved last year) > (2) Open-source an implementation (this driver and Eric's device) > (3) Formalize and upstream the device specification > > But I get that some overlap between (2) and (3) would have been better. > So far the spec document has been reviewed mainly from the IOMMU point > of view, and might require more changes to be in line with the other > virtio devices -- hopefully just wording changes. I'll kick off step > (3), but I think the virtio folks are a bit busy with finalizing the 1.1 > spec so I expect it to take a while. I read v0.9 of the spec and have some minor comments, hope this is a good place to send them: 1. In section 2.6.2, one reads If the VIRTIO_IOMMU_F_INPUT_RANGE feature is offered and the range described by fields virt_start and virt_end doesn’t fit in the range described by input_range, the device MAY set status to VIRTIO_- IOMMU_S_RANGE and ignore the request. Shouldn't int say "If the VIRTIO_IOMMU_F_INPUT_RANGE feature is negotiated" instead? 2. There's a typo at the end of section 2.6.5: The VIRTIO_IOMMU_MAP_F_MMIO flag is a memory type rather than a protection lag. s/lag/flag/ 3. In section 3.1.2.1.1, the viommu compatible field says "virtio,mmio". Shouldn't it say "virtio,mmio-iommu" instead, to be consistent with "virtio,pci-iommu"? 4. There's a typo in section 3.3: A host bridge may limit the input address space – transaction accessing some addresses won’t reach the physical IOMMU. s/transaction/transactions/ I also have one last comment which you may freely ignore, considering it's clearly just personal opinion and also considering that the specification is mature at this point: it specifies memory ranges by specifying start and end addresses. My experience has been that this is error prone, leading to confusion and bugs regarding whether the end address is inclusive or exclusive. I tend to prefer expressing memory ranges by specifying a start address and a length, which eliminates ambiguity. -- Thiago Jung Bauermann IBM Linux Technology Center ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 0/7] Add virtio-iommu driver
making a small module that >> > loads early and pokes at the IOMMU sufficiently to get the data about >> > which devices use the IOMMU out of it using standard virtio config >> > space. IIUC it's claimed to be impossible without messy changes to the >> > boot sequence. >> > >> > If I succeed at least on some platforms I'll ask that this design is >> > worked into this device, minimizing info that goes through DT/ACPI. If >> > I see I can't make it in time to meet the next merge window, I plan >> > merging the existing patches using DT (barring surprises). >> > >> > As I only have a very small amount of time to spend on this attempt, If >> > someone else wants to try doing that in parallel, that would be great! -- Thiago Jung Bauermann IBM Linux Technology Center ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
Christoph Hellwig writes: > On Tue, Jan 29, 2019 at 09:36:08PM -0500, Michael S. Tsirkin wrote: >> This has been discussed ad nauseum. virtio is all about compatibility. >> Losing a couple of lines of code isn't worth breaking working setups. >> People that want "just use DMA API no tricks" now have the option. >> Setting a flag in a feature bit map is literally a single line >> of code in the hypervisor. So stop pushing for breaking working >> legacy setups and just fix it in the right place. > > I agree with the legacy aspect. What I am missing is an extremely > strong wording that says you SHOULD always set this flag for new > hosts, including an explanation why. My understanding of ACCESS_PLATFORM is that it means "this device will behave in all aspects like a regular device attached to this bus". Is that it? Therefore it should be set because it's the sane thing to do? -- Thiago Jung Bauermann IBM Linux Technology Center ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
Hello Michael, Michael S. Tsirkin writes: > On Tue, Jan 29, 2019 at 03:42:44PM -0200, Thiago Jung Bauermann wrote: >> >> Fixing address of powerpc mailing list. >> >> Thiago Jung Bauermann writes: >> >> > Hello, >> > >> > With Christoph's rework of the DMA API that recently landed, the patch >> > below is the only change needed in virtio to make it work in a POWER >> > secure guest under the ultravisor. >> > >> > The other change we need (making sure the device's dma_map_ops is NULL >> > so that the dma-direct/swiotlb code is used) can be made in >> > powerpc-specific code. >> > >> > Of course, I also have patches (soon to be posted as RFC) which hook up >> > to the powerpc secure guest support code. >> > >> > What do you think? >> > >> > From d0629a36a75c678b4a72b853f8f7f8c17eedd6b3 Mon Sep 17 00:00:00 2001 >> > From: Thiago Jung Bauermann >> > Date: Thu, 24 Jan 2019 22:08:02 -0200 >> > Subject: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted >> > >> > The host can't access the guest memory when it's encrypted, so using >> > regular memory pages for the ring isn't an option. Go through the DMA API. >> > >> > Signed-off-by: Thiago Jung Bauermann > > Well I think this will come back to bite us (witness xen which is now > reworking precisely this path - but at least they aren't to blame, xen > came before ACCESS_PLATFORM). > > I also still think the right thing would have been to set > ACCESS_PLATFORM for all systems where device can't access all memory. I understand. The problem with that approach for us is that because we don't know which guests will become secure guests and which will remain regular guests, QEMU would need to offer ACCESS_PLATFORM to all guests. And the problem with that is that for QEMU on POWER, having ACCESS_PLATFORM turned off means that it can bypass the IOMMU for the device (which makes sense considering that the name of the flag was IOMMU_PLATFORM). And we need that for regular guests to avoid performance degradation. So while ACCESS_PLATFORM solves our problems for secure guests, we can't turn it on by default because we can't affect legacy systems. Doing so would penalize existing systems that can access all memory. They would all have to unnecessarily go through address translations, and take a performance hit. The semantics of ACCESS_PLATFORM assume that the hypervisor/QEMU knows in advance - right when the VM is instantiated - that it will not have access to all guest memory. Unfortunately that assumption is subtly broken on our secure-platform. The hypervisor/QEMU realizes that the platform is going secure only *after the VM is instantiated*. It's the kernel running in the VM that determines that it wants to switch the platform to secure-mode. Another way of looking at this issue which also explains our reluctance is that the only difference between a secure guest and a regular guest (at least regarding virtio) is that the former uses swiotlb while the latter doens't. And from the device's point of view they're indistinguishable. It can't tell one guest that is using swiotlb from one that isn't. And that implies that secure guest vs regular guest isn't a virtio interface issue, it's "guest internal affairs". So there's no reason to reflect that in the feature flags. That said, we still would like to arrive at a proper design for this rather than add yet another hack if we can avoid it. So here's another proposal: considering that the dma-direct code (in kernel/dma/direct.c) automatically uses swiotlb when necessary (thanks to Christoph's recent DMA work), would it be ok to replace virtio's own direct-memory code that is used in the !ACCESS_PLATFORM case with the dma-direct code? That way we'll get swiotlb even with !ACCESS_PLATFORM, and virtio will get a code cleanup (replace open-coded stuff with calls to existing infrastructure). > But I also think I don't have the energy to argue about power secure > guest anymore. So be it for power secure guest since the involved > engineers disagree with me. Hey I've been wrong in the past ;). Yeah, it's been a difficult discussion. Thanks for still engaging! I honestly thought that this patch was a good solution (if the guest has encrypted memory it means that the DMA API needs to be used), but I can see where you are coming from. As I said, we'd like to arrive at a good solution if possible. > But the name "sev_active" makes me scared because at least AMD guys who > were doing the sensible thing and setting ACCESS_PLATFORM My understanding is, AMD guest-platform knows in advance that their guest will run in secure mode and hence sets the flag at the time of VM instantiation. Unfortunately
Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
Fixing address of powerpc mailing list. Thiago Jung Bauermann writes: > Hello, > > With Christoph's rework of the DMA API that recently landed, the patch > below is the only change needed in virtio to make it work in a POWER > secure guest under the ultravisor. > > The other change we need (making sure the device's dma_map_ops is NULL > so that the dma-direct/swiotlb code is used) can be made in > powerpc-specific code. > > Of course, I also have patches (soon to be posted as RFC) which hook up > to the powerpc secure guest support code. > > What do you think? > > From d0629a36a75c678b4a72b853f8f7f8c17eedd6b3 Mon Sep 17 00:00:00 2001 > From: Thiago Jung Bauermann > Date: Thu, 24 Jan 2019 22:08:02 -0200 > Subject: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted > > The host can't access the guest memory when it's encrypted, so using > regular memory pages for the ring isn't an option. Go through the DMA API. > > Signed-off-by: Thiago Jung Bauermann > --- > drivers/virtio/virtio_ring.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > index cd7e755484e3..321a27075380 100644 > --- a/drivers/virtio/virtio_ring.c > +++ b/drivers/virtio/virtio_ring.c > @@ -259,8 +259,11 @@ static bool vring_use_dma_api(struct virtio_device *vdev) >* not work without an even larger kludge. Instead, enable >* the DMA API if we're a Xen guest, which at least allows >* all of the sensible Xen configurations to work correctly. > + * > + * Also, if guest memory is encrypted the host can't access > + * it directly. In this case, we'll need to use the DMA API. >*/ > - if (xen_domain()) > + if (xen_domain() || sev_active()) > return true; > > return false; -- Thiago Jung Bauermann IBM Linux Technology Center ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
Hello, With Christoph's rework of the DMA API that recently landed, the patch below is the only change needed in virtio to make it work in a POWER secure guest under the ultravisor. The other change we need (making sure the device's dma_map_ops is NULL so that the dma-direct/swiotlb code is used) can be made in powerpc-specific code. Of course, I also have patches (soon to be posted as RFC) which hook up to the powerpc secure guest support code. What do you think? >From d0629a36a75c678b4a72b853f8f7f8c17eedd6b3 Mon Sep 17 00:00:00 2001 From: Thiago Jung Bauermann Date: Thu, 24 Jan 2019 22:08:02 -0200 Subject: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted The host can't access the guest memory when it's encrypted, so using regular memory pages for the ring isn't an option. Go through the DMA API. Signed-off-by: Thiago Jung Bauermann --- drivers/virtio/virtio_ring.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index cd7e755484e3..321a27075380 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -259,8 +259,11 @@ static bool vring_use_dma_api(struct virtio_device *vdev) * not work without an even larger kludge. Instead, enable * the DMA API if we're a Xen guest, which at least allows * all of the sensible Xen configurations to work correctly. +* +* Also, if guest memory is encrypted the host can't access +* it directly. In this case, we'll need to use the DMA API. */ - if (xen_domain()) + if (xen_domain() || sev_active()) return true; return false; ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] dma-direct: set_memory_{en, de}crypted() take number of pages
Lendacky, Thomas writes:
> On 1/22/19 3:17 PM, Thiago Jung Bauermann wrote:
>> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
>> index 355d16acee6d..bc78c37220ba 100644
>> --- a/kernel/dma/direct.c
>> +++ b/kernel/dma/direct.c
>> @@ -166,7 +166,7 @@ void *dma_direct_alloc_pages(struct device *dev, size_t
>> size,
>>
>> ret = page_address(page);
>> if (force_dma_unencrypted()) {
>> -set_memory_decrypted((unsigned long)ret, 1 << get_order(size));
>> +set_memory_decrypted((unsigned long)ret, 1);
>
> The get_order() function will return the order for the specified size. To
> then get the number of pages you perform the shift as is being done. The
> change is definitely wrong since you are now hardcoding the page count to
> 1. The call to __dma_direct_alloc_pages() will allocate more than one page
> if the size is greater than a page.
You are correct, of course. Sorry for the noise and thanks for explaining.
--
Thiago Jung Bauermann
IBM Linux Technology Center
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/2] x86/kvmclock: set_memory_decrypted() takes number of pages
From: Ram Pai
set_memory_decrypted() expects the number of PAGE_SIZE pages to decrypt.
kvmclock_init_mem() instead passes number of bytes. This decrypts a huge
number of pages resulting in data corruption.
Fixed it.
[ bauermann: Slightly reworded commit message and added Fixes: tag. ]
Fixes: 6a1cac56f41f ("x86/kvm: Use __bss_decrypted attribute in shared
variables")
Signed-off-by: Ram Pai
Signed-off-by: Thiago Jung Bauermann
---
arch/x86/kernel/kvmclock.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Note: Found by code inspection. I don't have a way to test.
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index e811d4d1c824..b5c867dd2c8d 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -251,8 +251,7 @@ static void __init kvmclock_init_mem(void)
* be mapped decrypted.
*/
if (sev_active()) {
- r = set_memory_decrypted((unsigned long) hvclock_mem,
-1UL << order);
+ r = set_memory_decrypted((unsigned long) hvclock_mem, 1);
if (r) {
__free_pages(p, order);
hvclock_mem = NULL;
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/2] dma-direct: set_memory_{en, de}crypted() take number of pages
From: Ram Pai
set_memory_encrypted() and set_memory_decrypted() expect the number of
PAGE_SIZE pages to encrypt or decrypt. dma_direct_alloc() and
dma_direct_free() instead pass number of bytes. This encrypts/decrypts a
huge number of pages resulting in data corruption.
Fixed it.
[ bauermann: Slightly reworded commit message and added Fixes: tag. ]
Fixes: d7b417fa08d1 ("x86/mm: Add DMA support for SEV memory encryption")
Signed-off-by: Ram Pai
Signed-off-by: Thiago Jung Bauermann
---
kernel/dma/direct.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
Notes:
1. This was tested on powerpc with patches adding support for running
under the ultravisor, which are not yet upstream.
2. The lines changed in this patch were added by commit c10f07aa27da
("dma/direct: Handle force decryption for DMA coherent buffers in
common code"), but it only moves the code from an x86-specific file.
Therefore the Fixes tag references the commit that first introduced
the code.
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 355d16acee6d..bc78c37220ba 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -166,7 +166,7 @@ void *dma_direct_alloc_pages(struct device *dev, size_t
size,
ret = page_address(page);
if (force_dma_unencrypted()) {
- set_memory_decrypted((unsigned long)ret, 1 << get_order(size));
+ set_memory_decrypted((unsigned long)ret, 1);
*dma_handle = __phys_to_dma(dev, page_to_phys(page));
} else {
*dma_handle = phys_to_dma(dev, page_to_phys(page));
@@ -186,10 +186,8 @@ void __dma_direct_free_pages(struct device *dev, size_t
size, struct page *page)
void dma_direct_free_pages(struct device *dev, size_t size, void *cpu_addr,
dma_addr_t dma_addr, unsigned long attrs)
{
- unsigned int page_order = get_order(size);
-
if (force_dma_unencrypted())
- set_memory_encrypted((unsigned long)cpu_addr, 1 << page_order);
+ set_memory_encrypted((unsigned long)cpu_addr, 1);
__dma_direct_free_pages(dev, size, virt_to_page(cpu_addr));
}
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 11/11] powerpc/svm: Increase SWIOTLB buffer size
Hello Randy, Thanks for your quick review. Randy Dunlap writes: > On 08/24/2018 09:25 AM, Thiago Jung Bauermann wrote: >> From: Anshuman Khandual >> >> SWIOTLB buffer default size (64MB) is not enough for large sequential write >> operations which eventually leads to kernel crash like here. >> >> virtio-pci :00:05.0: swiotlb buffer is full (sz: 327680 bytes) >> virtio-pci :00:05.0: DMA: Out of SW-IOMMU space for 327680 bytes >> Kernel panic - not syncing: DMA: Random memory could be DMA read >> CPU: 12 PID: 3985 Comm: mkfs.ext4 Not tainted 4.18.0-rc4+ #285 >> Call Trace: >> [c007d2a27020] [c0cfdffc] dump_stack+0xb0/0xf4 (unreliable) >> [c007d2a27060] [c0112a98] panic+0x140/0x328 >> [c007d2a270f0] [c01b4f88] swiotlb_full+0x108/0x130 >> [c007d2a27180] [c01b5f6c] swiotlb_map_page+0x25c/0x2c0 >> [c007d2a271e0] [c07bfaf8] vring_map_one_sg.isra.0+0x58/0x70 >> [c007d2a27200] [c07c08dc] virtqueue_add_sgs+0x1bc/0x690 >> [c007d2a272f0] [d42a1280] virtio_queue_rq+0x358/0x4a0 >> [virtio_blk] >> [c007d2a273d0] [c06b5d68] blk_mq_dispatch_rq_list+0x1f8/0x6d0 >> .. >> >> Increase the SWIOTLB size to 1GB on Ultravisor based secure guests. >> >> Signed-off-by: Anshuman Khandual >> Signed-off-by: Thiago Jung Bauermann >> --- >> arch/powerpc/Kconfig | 5 + >> kernel/dma/swiotlb.c | 5 + >> 2 files changed, 10 insertions(+) >> >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig >> index 1466d1234723..fee7194ce9e4 100644 >> --- a/arch/powerpc/Kconfig >> +++ b/arch/powerpc/Kconfig >> @@ -457,6 +457,11 @@ config PPC_SVM >> >> If unsure, say "N". >> >> +config SWIOTLB_DEFAULT_SIZE >> + int "Size of Software I/O TLB buffer (in MiB)" >> + default "1024" > > I would add a "range" to limit (restrict) how small or large that can be. > E.g.: > > range 16 102400 > > or even smaller for the maximum value... That's an interesting idea. Since this config option is restricted to PPC_SVM it may be possible to find out what reasonable limits would be. I'll have to experiment a bit to find out. Though I can say that 16 MB is too small since even with 64 MB the kernel panics with the simple task of formatting a filesystem... -- Thiago Jung Bauermann IBM Linux Technology Center ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 10/11] powerpc/svm: Force the use of bounce buffers
Hello Christoph, Thanks for your quick review. Christoph Hellwig writes: > On Thu, Aug 23, 2018 at 11:59:32PM -0300, Thiago Jung Bauermann wrote: >> From: Anshuman Khandual >> >> SWIOTLB checks range of incoming CPU addresses to be bounced and see if the >> device can access it through it's DMA window without requiring bouncing. In >> such cases it just chooses to skip bouncing. But for cases like secure >> guests on powerpc platform all addresses need to be bounced into the shared >> pool of memory because the host cannot access it otherwise. Hence the need >> to do the bouncing is not related to device's DMA window. Hence force the >> use of bouncing by setting the swiotlb_force variable on secure guests. >> >> Signed-off-by: Anshuman Khandual >> Signed-off-by: Thiago Jung Bauermann >> --- >> arch/powerpc/kernel/svm.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/arch/powerpc/kernel/svm.c b/arch/powerpc/kernel/svm.c >> index 1af5caa955f5..f0576ad65cd0 100644 >> --- a/arch/powerpc/kernel/svm.c >> +++ b/arch/powerpc/kernel/svm.c >> @@ -17,6 +17,7 @@ static int __init init_svm(void) >> return 0; >> >> ppc_swiotlb_enable = 1; >> +swiotlb_force = SWIOTLB_FORCE; >> swiotlb_update_mem_attributes(); > > This needs a comment. Good point. Will add one. If in the new scheme of things (with bus_dma_mask?) dma_capable() on a virtio device running in a secure guest returns false, then this patch becomes unnecessary and can be dropped. -- Thiago Jung Bauermann IBM Linux Technology Center ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 08/11] powerpc: Add and use LPPACA_SIZE constant
Helps document what the hard-coded number means.
Suggested-by: Alexey Kardashevskiy
Signed-off-by: Thiago Jung Bauermann
---
arch/powerpc/kernel/paca.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/paca.c b/arch/powerpc/kernel/paca.c
index 0ee3e6d50f28..1edf8695019d 100644
--- a/arch/powerpc/kernel/paca.c
+++ b/arch/powerpc/kernel/paca.c
@@ -24,6 +24,8 @@
#define boot_cpuid 0
#endif
+#define LPPACA_SIZE 0x400
+
static void *__init alloc_paca_data(unsigned long size, unsigned long align,
unsigned long limit, int cpu)
{
@@ -70,7 +72,7 @@ static inline void init_lppaca(struct lppaca *lppaca)
*lppaca = (struct lppaca) {
.desc = cpu_to_be32(0xd397d781),/* "LpPa" */
- .size = cpu_to_be16(0x400),
+ .size = cpu_to_be16(LPPACA_SIZE),
.fpregs_in_use = 1,
.slb_count = cpu_to_be16(64),
.vmxregs_in_use = 0,
@@ -80,14 +82,13 @@ static inline void init_lppaca(struct lppaca *lppaca)
static struct lppaca * __init new_lppaca(int cpu, unsigned long limit)
{
struct lppaca *lp;
- size_t size = 0x400;
- BUILD_BUG_ON(size < sizeof(struct lppaca));
+ BUILD_BUG_ON(LPPACA_SIZE < sizeof(struct lppaca));
if (early_cpu_has_feature(CPU_FTR_HVMODE))
return NULL;
- lp = alloc_paca_data(size, 0x400, limit, cpu);
+ lp = alloc_paca_data(LPPACA_SIZE, 0x400, limit, cpu);
init_lppaca(lp);
return lp;
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 09/11] powerpc/svm: Use shared memory for LPPACA structures
From: Anshuman Khandual
LPPACA structures need to be shared with the host. Hence they need to be on
shared memory. Instead of allocating individual chunks of memory for given
structure from memblock, a contiguous chunk of memory is allocated and then
converted into shared memory. Subsequent allocation requests will come from
the contiguous chunk which will be always shared memory for all structures.
While we were able to use a kmem_cache constructor for the Debug Trace Log,
LPPACAs are allocated very early in the boot process (before SLUB is
available) so we need to use a simpler scheme here.
Signed-off-by: Anshuman Khandual
Signed-off-by: Thiago Jung Bauermann
---
arch/powerpc/kernel/paca.c | 34 +-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/paca.c b/arch/powerpc/kernel/paca.c
index 1edf8695019d..3e2aca150ad2 100644
--- a/arch/powerpc/kernel/paca.c
+++ b/arch/powerpc/kernel/paca.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include "setup.h"
@@ -25,6 +26,33 @@
#endif
#define LPPACA_SIZE 0x400
+#define SHARED_LPPACA_SIZE PAGE_ALIGN(LPPACA_SIZE * CONFIG_NR_CPUS)
+
+static phys_addr_t shared_lppaca_pa;
+static unsigned long shared_lppaca_size;
+
+static void *__init alloc_shared_lppaca(unsigned long size, unsigned long
align,
+ unsigned long limit, int cpu)
+{
+ unsigned long pa;
+
+ if (!shared_lppaca_pa) {
+ memblock_set_bottom_up(true);
+ shared_lppaca_pa = memblock_alloc_base_nid(SHARED_LPPACA_SIZE,
+ PAGE_SIZE, limit, -1, MEMBLOCK_NONE);
+ if (!shared_lppaca_pa)
+ panic("cannot allocate shared data");
+ memblock_set_bottom_up(false);
+ mem_convert_shared(PHYS_PFN(shared_lppaca_pa),
+ SHARED_LPPACA_SIZE / PAGE_SIZE);
+ }
+
+ pa = shared_lppaca_pa + shared_lppaca_size;
+ shared_lppaca_size += size;
+
+ BUG_ON(shared_lppaca_size >= SHARED_LPPACA_SIZE);
+ return __va(pa);
+}
static void *__init alloc_paca_data(unsigned long size, unsigned long align,
unsigned long limit, int cpu)
@@ -88,7 +116,11 @@ static struct lppaca * __init new_lppaca(int cpu, unsigned
long limit)
if (early_cpu_has_feature(CPU_FTR_HVMODE))
return NULL;
- lp = alloc_paca_data(LPPACA_SIZE, 0x400, limit, cpu);
+ if (is_svm_platform())
+ lp = alloc_shared_lppaca(LPPACA_SIZE, 0x400, limit, cpu);
+ else
+ lp = alloc_paca_data(LPPACA_SIZE, 0x400, limit, cpu);
+
init_lppaca(lp);
return lp;
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 11/11] powerpc/svm: Increase SWIOTLB buffer size
From: Anshuman Khandual
SWIOTLB buffer default size (64MB) is not enough for large sequential write
operations which eventually leads to kernel crash like here.
virtio-pci :00:05.0: swiotlb buffer is full (sz: 327680 bytes)
virtio-pci :00:05.0: DMA: Out of SW-IOMMU space for 327680 bytes
Kernel panic - not syncing: DMA: Random memory could be DMA read
CPU: 12 PID: 3985 Comm: mkfs.ext4 Not tainted 4.18.0-rc4+ #285
Call Trace:
[c007d2a27020] [c0cfdffc] dump_stack+0xb0/0xf4 (unreliable)
[c007d2a27060] [c0112a98] panic+0x140/0x328
[c007d2a270f0] [c01b4f88] swiotlb_full+0x108/0x130
[c007d2a27180] [c01b5f6c] swiotlb_map_page+0x25c/0x2c0
[c007d2a271e0] [c07bfaf8] vring_map_one_sg.isra.0+0x58/0x70
[c007d2a27200] [c07c08dc] virtqueue_add_sgs+0x1bc/0x690
[c007d2a272f0] [d42a1280] virtio_queue_rq+0x358/0x4a0 [virtio_blk]
[c007d2a273d0] [c06b5d68] blk_mq_dispatch_rq_list+0x1f8/0x6d0
..
Increase the SWIOTLB size to 1GB on Ultravisor based secure guests.
Signed-off-by: Anshuman Khandual
Signed-off-by: Thiago Jung Bauermann
---
arch/powerpc/Kconfig | 5 +
kernel/dma/swiotlb.c | 5 +
2 files changed, 10 insertions(+)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 1466d1234723..fee7194ce9e4 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -457,6 +457,11 @@ config PPC_SVM
If unsure, say "N".
+config SWIOTLB_DEFAULT_SIZE
+ int "Size of Software I/O TLB buffer (in MiB)"
+ default "1024"
+ depends on PPC_SVM
+
config PPC_TRANSACTIONAL_MEM
bool "Transactional Memory support for POWERPC"
depends on PPC_BOOK3S_64
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 04b68d9dffac..32dc67422d8a 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -146,8 +146,13 @@ void swiotlb_set_max_segment(unsigned int val)
max_segment = rounddown(val, PAGE_SIZE);
}
+#ifdef CONFIG_SWIOTLB_DEFAULT_SIZE
+#define IO_TLB_DEFAULT_SIZE ((unsigned long) CONFIG_SWIOTLB_DEFAULT_SIZE << 20)
+#else
/* default to 64MB */
#define IO_TLB_DEFAULT_SIZE (64UL<<20)
+#endif
+
unsigned long swiotlb_size_or_default(void)
{
unsigned long size;
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 10/11] powerpc/svm: Force the use of bounce buffers
From: Anshuman Khandual SWIOTLB checks range of incoming CPU addresses to be bounced and see if the device can access it through it's DMA window without requiring bouncing. In such cases it just chooses to skip bouncing. But for cases like secure guests on powerpc platform all addresses need to be bounced into the shared pool of memory because the host cannot access it otherwise. Hence the need to do the bouncing is not related to device's DMA window. Hence force the use of bouncing by setting the swiotlb_force variable on secure guests. Signed-off-by: Anshuman Khandual Signed-off-by: Thiago Jung Bauermann --- arch/powerpc/kernel/svm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/kernel/svm.c b/arch/powerpc/kernel/svm.c index 1af5caa955f5..f0576ad65cd0 100644 --- a/arch/powerpc/kernel/svm.c +++ b/arch/powerpc/kernel/svm.c @@ -17,6 +17,7 @@ static int __init init_svm(void) return 0; ppc_swiotlb_enable = 1; + swiotlb_force = SWIOTLB_FORCE; swiotlb_update_mem_attributes(); return 0; ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 06/11] powerpc/svm: Use SWIOTLB DMA API for all virtio devices
From: Anshuman Khandual
Secure guest memory (GPA range) is isolated and inaccessible from the host.
But virtio ring transactions require the back end drivers to process
incoming scatter gather buffers which require their access in host. Hence a
portion of guest memory can be converted to shared memory and all buffers
need to be bounced into the pool before being passed on to the host.
SWIOTLB buffers can be pre-allocated and converted as shared memory during
early boot. Generic SWIOTLB DMA API (swiotlb_dma_ops) callbacks can be used
to bounce each incoming scatter gather I/O buffer addresses into this pool
of shared memory before being passed on to the host. All virtio devices on
secure guest platform need to use generic SWIOTLB DMA API. Utilize the new
virtio core platform hook platform_override_dma_ops() to achieve this.
Signed-off-by: Anshuman Khandual
Signed-off-by: Thiago Jung Bauermann
---
arch/powerpc/platforms/pseries/iommu.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c
b/arch/powerpc/platforms/pseries/iommu.c
index 5773bc7eb4bd..56b894d65dba 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -39,6 +39,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -51,6 +52,7 @@
#include
#include
#include
+#include
#include "pseries.h"
@@ -1400,5 +1402,7 @@ machine_subsys_initcall_sync(pseries,
tce_iommu_bus_notifier_init);
void platform_override_dma_ops(struct virtio_device *vdev)
{
- /* Override vdev->parent.dma_ops if required */
+ if (is_svm_platform() &&
+ !virtio_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM))
+ set_dma_ops(vdev->dev.parent, _dma_ops);
}
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 05/11] powerpc/svm: Don't release SWIOTLB buffers on secure guests
From: Anshuman Khandual Even though SWIOTLB slab gets allocated and initialized on each powerpc platform with swiotlb_init(), it gets released away again on all server platforms because ppc_swiotlb_enable variable never gets set. Secure guests would require SWIOTLB DMA API support for virtio bounce buffering purpose. Hence retain the allocated buffer by setting ppc_swiotlb_enable variable for secure guests on Ultravisor platforms. Signed-off-by: Anshuman Khandual Signed-off-by: Thiago Jung Bauermann --- arch/powerpc/kernel/svm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/kernel/svm.c b/arch/powerpc/kernel/svm.c index eab2a64d8643..891db2de8c04 100644 --- a/arch/powerpc/kernel/svm.c +++ b/arch/powerpc/kernel/svm.c @@ -16,6 +16,7 @@ static int __init init_svm(void) if (!is_svm_platform()) return 0; + ppc_swiotlb_enable = 1; swiotlb_update_mem_attributes(); return 0; ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 03/11] powerpc/svm: Add memory conversion (shared/secure) helper functions
From: Anshuman Khandual
Add the following helper functions to convert PAGE_SIZE aligned memory
buffers as shared or secure (i.e., accessible to the hypervisor or not) via
Ultravisor calls.
1. mem_convert_shared(unsigned long pfn, unsigned long npages)
2. mem_convert_secure(unsigned long pfn, unsigned long npages)
Signed-off-by: Anshuman Khandual
Signed-off-by: Thiago Jung Bauermann
---
arch/powerpc/include/asm/svm.h | 3 +++
arch/powerpc/kernel/Makefile | 1 +
arch/powerpc/kernel/svm.c | 33 +
3 files changed, 37 insertions(+)
diff --git a/arch/powerpc/include/asm/svm.h b/arch/powerpc/include/asm/svm.h
index 6f89e5d6d37f..95d69e472e52 100644
--- a/arch/powerpc/include/asm/svm.h
+++ b/arch/powerpc/include/asm/svm.h
@@ -13,6 +13,9 @@ static bool is_svm_platform(void)
{
return mfmsr() & MSR_S;
}
+
+extern void mem_convert_shared(unsigned long pfn, unsigned long npages);
+extern void mem_convert_secure(unsigned long pfn, unsigned long npages);
#else
static inline bool is_svm_platform(void)
{
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 2b4c40b255e4..98780b4e924c 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -113,6 +113,7 @@ ifeq ($(CONFIG_HAVE_IMA_KEXEC)$(CONFIG_IMA),yy)
obj-y += ima_kexec.o
endif
+obj-$(CONFIG_PPC_SVM) += svm.o
obj-$(CONFIG_AUDIT)+= audit.o
obj64-$(CONFIG_AUDIT) += compat_audit.o
diff --git a/arch/powerpc/kernel/svm.c b/arch/powerpc/kernel/svm.c
new file mode 100644
index ..37437cf92df5
--- /dev/null
+++ b/arch/powerpc/kernel/svm.c
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Secure VM platform
+ *
+ * Copyright 2018 IBM Corporation
+ * Author: Anshuman Khandual
+ */
+
+#include
+
+void mem_convert_shared(unsigned long pfn, unsigned long npages)
+{
+ if (!pfn_valid(pfn))
+ return;
+
+ /*
+* FIXME: Insert real UV call when ready
+*
+* ucall_convert_shared(paddr, size)
+*/
+}
+
+void mem_convert_secure(unsigned long pfn, unsigned long npages)
+{
+ if (!pfn_valid(pfn))
+ return;
+
+ /*
+* FIXME: Insert real UV call when ready
+*
+* ucall_convert_secure(paddr, size)
+*/
+}
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 04/11] powerpc/svm: Convert SWIOTLB buffers to shared memory
From: Anshuman Khandual
Hook the shared memory conversion functions into the ARCH_HAS_MEM_ENCRYPT
framework and call swiotlb_update_mem_attributes() to convert SWIOTLB's
buffers to shared memory.
Signed-off-by: Anshuman Khandual
Signed-off-by: Thiago Jung Bauermann
---
arch/powerpc/Kconfig | 4
arch/powerpc/include/asm/mem_encrypt.h | 19 +++
arch/powerpc/kernel/svm.c | 34 ++
3 files changed, 57 insertions(+)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 90f73d15f58a..1466d1234723 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -440,12 +440,16 @@ config MATH_EMULATION_HW_UNIMPLEMENTED
endchoice
+config ARCH_HAS_MEM_ENCRYPT
+ def_bool n
+
config PPC_SVM
bool "Secure virtual machine (SVM) support for POWERPC"
default n
depends on PPC_PSERIES
select DMA_DIRECT_OPS
select SWIOTLB
+ select ARCH_HAS_MEM_ENCRYPT
help
Support secure guests on POWERPC. There are certain POWER platforms
which support secure guests with the help of an Ultravisor executing
diff --git a/arch/powerpc/include/asm/mem_encrypt.h
b/arch/powerpc/include/asm/mem_encrypt.h
new file mode 100644
index ..2b6e37ea446c
--- /dev/null
+++ b/arch/powerpc/include/asm/mem_encrypt.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * SVM helper functions
+ *
+ * Copyright 2018 IBM Corporation
+ */
+
+#ifndef _ASM_POWERPC_MEM_ENCRYPT_H
+#define _ASM_POWERPC_MEM_ENCRYPT_H
+
+#define sme_me_mask0ULL
+
+static inline bool sme_active(void) { return false; }
+static inline bool sev_active(void) { return false; }
+
+int set_memory_encrypted(unsigned long addr, int numpages);
+int set_memory_decrypted(unsigned long addr, int numpages);
+
+#endif /* _ASM_POWERPC_MEM_ENCRYPT_H */
diff --git a/arch/powerpc/kernel/svm.c b/arch/powerpc/kernel/svm.c
index 37437cf92df5..eab2a64d8643 100644
--- a/arch/powerpc/kernel/svm.c
+++ b/arch/powerpc/kernel/svm.c
@@ -7,6 +7,20 @@
*/
#include
+#include
+#include
+#include
+
+static int __init init_svm(void)
+{
+ if (!is_svm_platform())
+ return 0;
+
+ swiotlb_update_mem_attributes();
+
+ return 0;
+}
+machine_early_initcall(pseries, init_svm);
void mem_convert_shared(unsigned long pfn, unsigned long npages)
{
@@ -31,3 +45,23 @@ void mem_convert_secure(unsigned long pfn, unsigned long
npages)
* ucall_convert_secure(paddr, size)
*/
}
+
+int set_memory_encrypted(unsigned long addr, int numpages)
+{
+ if (!PAGE_ALIGNED(addr))
+ return -EINVAL;
+
+ mem_convert_secure(PHYS_PFN(__pa(addr)), numpages);
+
+ return 0;
+}
+
+int set_memory_decrypted(unsigned long addr, int numpages)
+{
+ if (!PAGE_ALIGNED(addr))
+ return -EINVAL;
+
+ mem_convert_shared(PHYS_PFN(__pa(addr)), numpages);
+
+ return 0;
+}
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 02/11] powerpc/svm: Select CONFIG_DMA_DIRECT_OPS and CONFIG_SWIOTLB
From: Anshuman Khandual Ultravisor based secure guest platforms will use generic SWIOTLB DMA API (i.e swiotlb_dma_ops structure) available under the CONFIG_DMA_DIRECT_OPS config. Also base CONFIG_SWIOTLB needs to be explicitly selected as well. Select both these config options on powerpc server platforms with secure guest support. Signed-off-by: Anshuman Khandual Signed-off-by: Thiago Jung Bauermann --- arch/powerpc/Kconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index f786c962edf0..90f73d15f58a 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -444,6 +444,8 @@ config PPC_SVM bool "Secure virtual machine (SVM) support for POWERPC" default n depends on PPC_PSERIES + select DMA_DIRECT_OPS + select SWIOTLB help Support secure guests on POWERPC. There are certain POWER platforms which support secure guests with the help of an Ultravisor executing ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 00/11] Secure Virtual Machine Enablement
[ Some people didn't receive all the patches in this series, even though the linuxppc-dev list did so trying to send again. This is exactly the same series I posted yesterday. Sorry for the clutter. ] This series contains preliminary work to enable Secure Virtual Machines (SVM) on powerpc. SVMs request to be migrated to secure memory very early in the boot process (in prom_init()), so by default all of their memory is inaccessible to the hypervisor. There is an ultravisor call that the VM can use to request certain pages to be made accessible (aka shared). The objective of these patches is to have the guest perform this request for buffers that need to be shared with the hypervisor, such as the LPPACAs, the SWIOTLB buffer and the Debug Trace Log. This work is incomplete: there are surely other memory regions that need to be made accessible, but I'm posting it early to get comments on whether the approach being taken is appropriate. It should be applied on top of the generic virtio DMA API rework series posted earlier, which adds a platform hook to override any arch based DMA API operations for any virtio device: https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-July/175994.html I'm aware that changes need to be made to the patch series above, but IIUC it depends on upcoming virtio cleanup from Christoph Hellwig so for now the patch series above will be used as a stepping stone for this series. This code has been tested with virtio block, net and scsi devices with and without VIRTIO_F_IOMMU_PLATFORM flag. Please let me know what you think. For now I am testing on a regular guest with a couple of patches on top forcing is_svm_platform() to always return true and adding debug messages to confirm that mem_convert_shared() is being called in the expected places. These are the commands I'm using to start up the guest: Without VIRTIO_F_IOMMU_PLATFORM: qemu-system-ppc64 \ -enable-kvm \ -kernel /home/bauermann/src/linux/arch/powerpc/boot/zImage \ -append "root=PARTUUID=e550ad6f-05 ro" \ -machine pseries-2.6 \ -m 8G \ -smp 2 \ -serial mon:stdio \ -nographic \ -nodefaults \ -device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x4 \ -drive file=/home/bauermann/VMs/svm.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-0 \ -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \ -drive file=/home/bauermann/VMs/svm-blk.qcow2,format=qcow2,if=none,id=drive-virtio-disk0 \ -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,id=virtio-disk0 \ -device virtio-net,netdev=hostnet0,id=net0,mac=52:54:00:96:70:1f \ -netdev user,id=hostnet0 \ -set netdev.hostnet0.hostfwd=tcp::42022-:22 With VIRTIO_F_IOMMU_PLATFORM. Same as above plus some -global options so that the virtio devices use the modern interface rather than the transitional one: qemu-system-ppc64 \ -enable-kvm \ -kernel /home/bauermann/src/linux/arch/powerpc/boot/zImage \ -append "root=PARTUUID=e550ad6f-05 ro" \ -machine pseries-2.6 \ -m 8G \ -smp 2 \ -serial mon:stdio \ -nographic \ -nodefaults \ -device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x4 \ -drive file=/home/bauermann/VMs/svm.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-0 \ -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \ -drive file=/home/bauermann/VMs/svm-blk.qcow2,format=qcow2,if=none,id=drive-virtio-disk0 \ -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,id=virtio-disk0 \ -device virtio-net,netdev=hostnet0,id=net0,mac=52:54:00:96:70:1f \ -netdev user,id=hostnet0 \ -set netdev.hostnet0.hostfwd=tcp::42022-:22 \ -global virtio-blk-pci.iommu_platform=true \ -global virtio-blk-pci.disable-legacy=on \ -global virtio-blk-pci.disable-modern=off \ -global virtio-net-pci.iommu_platform=true \ -global virtio-net-pci.disable-legacy=on \ -global virtio-net-pci.disable-modern=off \ -global virtio-scsi-pci.iommu_platform=true \ -global virtio-scsi-pci.disable-legacy=on \ -global virtio-scsi-pci.disable-modern=off The code was tested with a couple of other permutations where one virtio device has the flag VIRTIO_F_IOMMU_PLATFORM and others don't. Please suggest some other scenarios which need to be tested as well. Anshuman Khandual (10): powerpc/svm: Detect Secure Virtual Machine (SVM) platform powerpc/svm: Select CONFIG_DMA_DIRECT_OPS and CONFIG_SWIOTLB powerpc/svm: Add memory conversion (shared/secure) helper functions powerpc/svm: Convert SWIOTLB buffers to shared memory powerpc/svm: Don't release SWIOTLB buffers on secure guests powerpc/svm: Use SWIOTLB DMA API for all virtio devices powerpc/svm: Use shared memory for Debug Trace Log (DTL) powerpc/svm: Use shared memory for LPPACA structures powerpc/svm: Force the use of bounce buffers powerpc/svm: Increase SWIOTLB buffer size Thiago Jung Bauermann (1): powerpc: Add and use LPPACA_
[RFC PATCH 01/11] powerpc/svm: Detect Secure Virtual Machine (SVM) platform
From: Anshuman Khandual
A guest requests to be moved to secure memory early at the kernel
startup (in prom_init). Define a flag that can be easily checked by other
parts of the kernel so that they can set things up accordingly. This is
done by checking the MSR(S) bit, which is always set for secure VMs.
Also add a new config option CONFIG_PPC_SVM to wrap all these code to
prevent it from being executed from non subscribing platforms. This SVM
platform detection is applicable only to guest kernels that will run under
an Ultravisor as a secure guest.
Signed-off-by: Anshuman Khandual
Signed-off-by: Ram Pai
Signed-off-by: Sukadev Bhattiprolu
Signed-off-by: Thiago Jung Bauermann
---
arch/powerpc/Kconfig | 11 +++
arch/powerpc/include/asm/reg.h | 3 +++
arch/powerpc/include/asm/svm.h | 22 ++
3 files changed, 36 insertions(+)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 9f2b75fe2c2d..f786c962edf0 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -440,6 +440,17 @@ config MATH_EMULATION_HW_UNIMPLEMENTED
endchoice
+config PPC_SVM
+ bool "Secure virtual machine (SVM) support for POWERPC"
+ default n
+ depends on PPC_PSERIES
+ help
+ Support secure guests on POWERPC. There are certain POWER platforms
+which support secure guests with the help of an Ultravisor executing
+below the hypervisor layer. This enables the support for those guests.
+
+If unsure, say "N".
+
config PPC_TRANSACTIONAL_MEM
bool "Transactional Memory support for POWERPC"
depends on PPC_BOOK3S_64
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 562568414cf4..fcf7b79356d0 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -36,6 +36,7 @@
#define MSR_TM_LG 32 /* Trans Mem Available */
#define MSR_VEC_LG 25 /* Enable AltiVec */
#define MSR_VSX_LG 23 /* Enable VSX */
+#define MSR_S_LG 22 /* Secure VM bit */
#define MSR_POW_LG 18 /* Enable Power Management */
#define MSR_WE_LG 18 /* Wait State Enable */
#define MSR_TGPR_LG17 /* TLB Update registers in use */
@@ -69,11 +70,13 @@
#define MSR_SF __MASK(MSR_SF_LG) /* Enable 64 bit mode */
#define MSR_ISF__MASK(MSR_ISF_LG) /* Interrupt 64b mode
valid on 630 */
#define MSR_HV __MASK(MSR_HV_LG) /* Hypervisor state */
+#define MSR_S __MASK(MSR_S_LG)/* Secure state */
#else
/* so tests for these bits fail on 32-bit */
#define MSR_SF 0
#define MSR_ISF0
#define MSR_HV 0
+#define MSR_S 0
#endif
/*
diff --git a/arch/powerpc/include/asm/svm.h b/arch/powerpc/include/asm/svm.h
new file mode 100644
index ..6f89e5d6d37f
--- /dev/null
+++ b/arch/powerpc/include/asm/svm.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * SVM helper functions
+ *
+ * Copyright 2018 Anshuman Khandual, IBM Corporation.
+ */
+
+#ifndef _ASM_POWERPC_SVM_H
+#define _ASM_POWERPC_SVM_H
+
+#ifdef CONFIG_PPC_SVM
+static bool is_svm_platform(void)
+{
+ return mfmsr() & MSR_S;
+}
+#else
+static inline bool is_svm_platform(void)
+{
+ return false;
+}
+#endif
+#endif /* _ASM_POWERPC_SVM_H */
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 01/11] powerpc/svm: Detect Secure Virtual Machine (SVM) platform
From: Anshuman Khandual
A guest requests to be moved to secure memory early at the kernel
startup (in prom_init). Define a flag that can be easily checked by other
parts of the kernel so that they can set things up accordingly. This is
done by checking the MSR(S) bit, which is always set for secure VMs.
Also add a new config option CONFIG_PPC_SVM to wrap all these code to
prevent it from being executed from non subscribing platforms. This SVM
platform detection is applicable only to guest kernels that will run under
an Ultravisor as a secure guest.
Signed-off-by: Anshuman Khandual
Signed-off-by: Ram Pai
Signed-off-by: Sukadev Bhattiprolu
Signed-off-by: Thiago Jung Bauermann
---
arch/powerpc/Kconfig | 11 +++
arch/powerpc/include/asm/reg.h | 3 +++
arch/powerpc/include/asm/svm.h | 22 ++
3 files changed, 36 insertions(+)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 9f2b75fe2c2d..f786c962edf0 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -440,6 +440,17 @@ config MATH_EMULATION_HW_UNIMPLEMENTED
endchoice
+config PPC_SVM
+ bool "Secure virtual machine (SVM) support for POWERPC"
+ default n
+ depends on PPC_PSERIES
+ help
+ Support secure guests on POWERPC. There are certain POWER platforms
+which support secure guests with the help of an Ultravisor executing
+below the hypervisor layer. This enables the support for those guests.
+
+If unsure, say "N".
+
config PPC_TRANSACTIONAL_MEM
bool "Transactional Memory support for POWERPC"
depends on PPC_BOOK3S_64
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 562568414cf4..fcf7b79356d0 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -36,6 +36,7 @@
#define MSR_TM_LG 32 /* Trans Mem Available */
#define MSR_VEC_LG 25 /* Enable AltiVec */
#define MSR_VSX_LG 23 /* Enable VSX */
+#define MSR_S_LG 22 /* Secure VM bit */
#define MSR_POW_LG 18 /* Enable Power Management */
#define MSR_WE_LG 18 /* Wait State Enable */
#define MSR_TGPR_LG17 /* TLB Update registers in use */
@@ -69,11 +70,13 @@
#define MSR_SF __MASK(MSR_SF_LG) /* Enable 64 bit mode */
#define MSR_ISF__MASK(MSR_ISF_LG) /* Interrupt 64b mode
valid on 630 */
#define MSR_HV __MASK(MSR_HV_LG) /* Hypervisor state */
+#define MSR_S __MASK(MSR_S_LG)/* Secure state */
#else
/* so tests for these bits fail on 32-bit */
#define MSR_SF 0
#define MSR_ISF0
#define MSR_HV 0
+#define MSR_S 0
#endif
/*
diff --git a/arch/powerpc/include/asm/svm.h b/arch/powerpc/include/asm/svm.h
new file mode 100644
index ..6f89e5d6d37f
--- /dev/null
+++ b/arch/powerpc/include/asm/svm.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * SVM helper functions
+ *
+ * Copyright 2018 Anshuman Khandual, IBM Corporation.
+ */
+
+#ifndef _ASM_POWERPC_SVM_H
+#define _ASM_POWERPC_SVM_H
+
+#ifdef CONFIG_PPC_SVM
+static bool is_svm_platform(void)
+{
+ return mfmsr() & MSR_S;
+}
+#else
+static inline bool is_svm_platform(void)
+{
+ return false;
+}
+#endif
+#endif /* _ASM_POWERPC_SVM_H */
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 06/11] powerpc/svm: Use SWIOTLB DMA API for all virtio devices
From: Anshuman Khandual
Secure guest memory (GPA range) is isolated and inaccessible from the host.
But virtio ring transactions require the back end drivers to process
incoming scatter gather buffers which require their access in host. Hence a
portion of guest memory can be converted to shared memory and all buffers
need to be bounced into the pool before being passed on to the host.
SWIOTLB buffers can be pre-allocated and converted as shared memory during
early boot. Generic SWIOTLB DMA API (swiotlb_dma_ops) callbacks can be used
to bounce each incoming scatter gather I/O buffer addresses into this pool
of shared memory before being passed on to the host. All virtio devices on
secure guest platform need to use generic SWIOTLB DMA API. Utilize the new
virtio core platform hook platform_override_dma_ops() to achieve this.
Signed-off-by: Anshuman Khandual
Signed-off-by: Thiago Jung Bauermann
---
arch/powerpc/platforms/pseries/iommu.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c
b/arch/powerpc/platforms/pseries/iommu.c
index 5773bc7eb4bd..56b894d65dba 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -39,6 +39,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -51,6 +52,7 @@
#include
#include
#include
+#include
#include "pseries.h"
@@ -1400,5 +1402,7 @@ machine_subsys_initcall_sync(pseries,
tce_iommu_bus_notifier_init);
void platform_override_dma_ops(struct virtio_device *vdev)
{
- /* Override vdev->parent.dma_ops if required */
+ if (is_svm_platform() &&
+ !virtio_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM))
+ set_dma_ops(vdev->dev.parent, _dma_ops);
}
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 11/11] powerpc/svm: Increase SWIOTLB buffer size
From: Anshuman Khandual
SWIOTLB buffer default size (64MB) is not enough for large sequential write
operations which eventually leads to kernel crash like here.
virtio-pci :00:05.0: swiotlb buffer is full (sz: 327680 bytes)
virtio-pci :00:05.0: DMA: Out of SW-IOMMU space for 327680 bytes
Kernel panic - not syncing: DMA: Random memory could be DMA read
CPU: 12 PID: 3985 Comm: mkfs.ext4 Not tainted 4.18.0-rc4+ #285
Call Trace:
[c007d2a27020] [c0cfdffc] dump_stack+0xb0/0xf4 (unreliable)
[c007d2a27060] [c0112a98] panic+0x140/0x328
[c007d2a270f0] [c01b4f88] swiotlb_full+0x108/0x130
[c007d2a27180] [c01b5f6c] swiotlb_map_page+0x25c/0x2c0
[c007d2a271e0] [c07bfaf8] vring_map_one_sg.isra.0+0x58/0x70
[c007d2a27200] [c07c08dc] virtqueue_add_sgs+0x1bc/0x690
[c007d2a272f0] [d42a1280] virtio_queue_rq+0x358/0x4a0 [virtio_blk]
[c007d2a273d0] [c06b5d68] blk_mq_dispatch_rq_list+0x1f8/0x6d0
..
Increase the SWIOTLB size to 1GB on Ultravisor based secure guests.
Signed-off-by: Anshuman Khandual
Signed-off-by: Thiago Jung Bauermann
---
arch/powerpc/Kconfig | 5 +
kernel/dma/swiotlb.c | 5 +
2 files changed, 10 insertions(+)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 1466d1234723..fee7194ce9e4 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -457,6 +457,11 @@ config PPC_SVM
If unsure, say "N".
+config SWIOTLB_DEFAULT_SIZE
+ int "Size of Software I/O TLB buffer (in MiB)"
+ default "1024"
+ depends on PPC_SVM
+
config PPC_TRANSACTIONAL_MEM
bool "Transactional Memory support for POWERPC"
depends on PPC_BOOK3S_64
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 04b68d9dffac..32dc67422d8a 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -146,8 +146,13 @@ void swiotlb_set_max_segment(unsigned int val)
max_segment = rounddown(val, PAGE_SIZE);
}
+#ifdef CONFIG_SWIOTLB_DEFAULT_SIZE
+#define IO_TLB_DEFAULT_SIZE ((unsigned long) CONFIG_SWIOTLB_DEFAULT_SIZE << 20)
+#else
/* default to 64MB */
#define IO_TLB_DEFAULT_SIZE (64UL<<20)
+#endif
+
unsigned long swiotlb_size_or_default(void)
{
unsigned long size;
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 00/11] Secure Virtual Machine Enablement
This series contains preliminary work to enable Secure Virtual Machines (SVM) on powerpc. SVMs request to be migrated to secure memory very early in the boot process (in prom_init()), so by default all of their memory is inaccessible to the hypervisor. There is an ultravisor call that the VM can use to request certain pages to be made accessible (aka shared). The objective of these patches is to have the guest perform this request for buffers that need to be shared with the hypervisor, such as the LPPACAs, the SWIOTLB buffer and the Debug Trace Log. This work is incomplete: there are surely other memory regions that need to be made accessible, but I'm posting it early to get comments on whether the approach being taken is appropriate. It should be applied on top of the generic virtio DMA API rework series posted earlier, which adds a platform hook to override any arch based DMA API operations for any virtio device: https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-July/175994.html I'm aware that changes need to be made to the patch series above, but IIUC it depends on upcoming virtio cleanup from Christoph Hellwig so for now the patch series above will be used as a stepping stone for this series. This code has been tested with virtio block, net and scsi devices with and without VIRTIO_F_IOMMU_PLATFORM flag. Please let me know what you think. For now I am testing on a regular guest with a couple of patches on top forcing is_svm_platform() to always return true and adding debug messages to confirm that mem_convert_shared() is being called in the expected places. These are the commands I'm using to start up the guest: Without VIRTIO_F_IOMMU_PLATFORM: qemu-system-ppc64 \ -enable-kvm \ -kernel /home/bauermann/src/linux/arch/powerpc/boot/zImage \ -append "root=PARTUUID=e550ad6f-05 ro" \ -machine pseries-2.6 \ -m 8G \ -smp 2 \ -serial mon:stdio \ -nographic \ -nodefaults \ -device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x4 \ -drive file=/home/bauermann/VMs/svm.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-0 \ -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \ -drive file=/home/bauermann/VMs/svm-blk.qcow2,format=qcow2,if=none,id=drive-virtio-disk0 \ -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,id=virtio-disk0 \ -device virtio-net,netdev=hostnet0,id=net0,mac=52:54:00:96:70:1f \ -netdev user,id=hostnet0 \ -set netdev.hostnet0.hostfwd=tcp::42022-:22 With VIRTIO_F_IOMMU_PLATFORM. Same as above plus some -global options so that the virtio devices use the modern interface rather than the transitional one: qemu-system-ppc64 \ -enable-kvm \ -kernel /home/bauermann/src/linux/arch/powerpc/boot/zImage \ -append "root=PARTUUID=e550ad6f-05 ro" \ -machine pseries-2.6 \ -m 8G \ -smp 2 \ -serial mon:stdio \ -nographic \ -nodefaults \ -device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x4 \ -drive file=/home/bauermann/VMs/svm.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-0 \ -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \ -drive file=/home/bauermann/VMs/svm-blk.qcow2,format=qcow2,if=none,id=drive-virtio-disk0 \ -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,id=virtio-disk0 \ -device virtio-net,netdev=hostnet0,id=net0,mac=52:54:00:96:70:1f \ -netdev user,id=hostnet0 \ -set netdev.hostnet0.hostfwd=tcp::42022-:22 \ -global virtio-blk-pci.iommu_platform=true \ -global virtio-blk-pci.disable-legacy=on \ -global virtio-blk-pci.disable-modern=off \ -global virtio-net-pci.iommu_platform=true \ -global virtio-net-pci.disable-legacy=on \ -global virtio-net-pci.disable-modern=off \ -global virtio-scsi-pci.iommu_platform=true \ -global virtio-scsi-pci.disable-legacy=on \ -global virtio-scsi-pci.disable-modern=off The code was tested with a couple of other permutations where one virtio device has the flag VIRTIO_F_IOMMU_PLATFORM and others don't. Please suggest some other scenarios which need to be tested as well. Anshuman Khandual (10): powerpc/svm: Detect Secure Virtual Machine (SVM) platform powerpc/svm: Select CONFIG_DMA_DIRECT_OPS and CONFIG_SWIOTLB powerpc/svm: Add memory conversion (shared/secure) helper functions powerpc/svm: Convert SWIOTLB buffers to shared memory powerpc/svm: Don't release SWIOTLB buffers on secure guests powerpc/svm: Use SWIOTLB DMA API for all virtio devices powerpc/svm: Use shared memory for Debug Trace Log (DTL) powerpc/svm: Use shared memory for LPPACA structures powerpc/svm: Force the use of bounce buffers powerpc/svm: Increase SWIOTLB buffer size Thiago Jung Bauermann (1): powerpc: Add and use LPPACA_SIZE constant arch/powerpc/Kconfig | 22 arch/powerpc/include/asm/mem_encrypt.h | 19 +++ arch/powerpc/include/asm/reg.h | 3 ++ arch/powerpc/include/as
[RFC PATCH 04/11] powerpc/svm: Convert SWIOTLB buffers to shared memory
From: Anshuman Khandual
Hook the shared memory conversion functions into the ARCH_HAS_MEM_ENCRYPT
framework and call swiotlb_update_mem_attributes() to convert SWIOTLB's
buffers to shared memory.
Signed-off-by: Anshuman Khandual
Signed-off-by: Thiago Jung Bauermann
---
arch/powerpc/Kconfig | 4
arch/powerpc/include/asm/mem_encrypt.h | 19 +++
arch/powerpc/kernel/svm.c | 34 ++
3 files changed, 57 insertions(+)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 90f73d15f58a..1466d1234723 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -440,12 +440,16 @@ config MATH_EMULATION_HW_UNIMPLEMENTED
endchoice
+config ARCH_HAS_MEM_ENCRYPT
+ def_bool n
+
config PPC_SVM
bool "Secure virtual machine (SVM) support for POWERPC"
default n
depends on PPC_PSERIES
select DMA_DIRECT_OPS
select SWIOTLB
+ select ARCH_HAS_MEM_ENCRYPT
help
Support secure guests on POWERPC. There are certain POWER platforms
which support secure guests with the help of an Ultravisor executing
diff --git a/arch/powerpc/include/asm/mem_encrypt.h
b/arch/powerpc/include/asm/mem_encrypt.h
new file mode 100644
index ..2b6e37ea446c
--- /dev/null
+++ b/arch/powerpc/include/asm/mem_encrypt.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * SVM helper functions
+ *
+ * Copyright 2018 IBM Corporation
+ */
+
+#ifndef _ASM_POWERPC_MEM_ENCRYPT_H
+#define _ASM_POWERPC_MEM_ENCRYPT_H
+
+#define sme_me_mask0ULL
+
+static inline bool sme_active(void) { return false; }
+static inline bool sev_active(void) { return false; }
+
+int set_memory_encrypted(unsigned long addr, int numpages);
+int set_memory_decrypted(unsigned long addr, int numpages);
+
+#endif /* _ASM_POWERPC_MEM_ENCRYPT_H */
diff --git a/arch/powerpc/kernel/svm.c b/arch/powerpc/kernel/svm.c
index 37437cf92df5..eab2a64d8643 100644
--- a/arch/powerpc/kernel/svm.c
+++ b/arch/powerpc/kernel/svm.c
@@ -7,6 +7,20 @@
*/
#include
+#include
+#include
+#include
+
+static int __init init_svm(void)
+{
+ if (!is_svm_platform())
+ return 0;
+
+ swiotlb_update_mem_attributes();
+
+ return 0;
+}
+machine_early_initcall(pseries, init_svm);
void mem_convert_shared(unsigned long pfn, unsigned long npages)
{
@@ -31,3 +45,23 @@ void mem_convert_secure(unsigned long pfn, unsigned long
npages)
* ucall_convert_secure(paddr, size)
*/
}
+
+int set_memory_encrypted(unsigned long addr, int numpages)
+{
+ if (!PAGE_ALIGNED(addr))
+ return -EINVAL;
+
+ mem_convert_secure(PHYS_PFN(__pa(addr)), numpages);
+
+ return 0;
+}
+
+int set_memory_decrypted(unsigned long addr, int numpages)
+{
+ if (!PAGE_ALIGNED(addr))
+ return -EINVAL;
+
+ mem_convert_shared(PHYS_PFN(__pa(addr)), numpages);
+
+ return 0;
+}
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 08/11] powerpc: Add and use LPPACA_SIZE constant
Helps document what the hard-coded number means.
Suggested-by: Alexey Kardashevskiy
Signed-off-by: Thiago Jung Bauermann
---
arch/powerpc/kernel/paca.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/paca.c b/arch/powerpc/kernel/paca.c
index 0ee3e6d50f28..1edf8695019d 100644
--- a/arch/powerpc/kernel/paca.c
+++ b/arch/powerpc/kernel/paca.c
@@ -24,6 +24,8 @@
#define boot_cpuid 0
#endif
+#define LPPACA_SIZE 0x400
+
static void *__init alloc_paca_data(unsigned long size, unsigned long align,
unsigned long limit, int cpu)
{
@@ -70,7 +72,7 @@ static inline void init_lppaca(struct lppaca *lppaca)
*lppaca = (struct lppaca) {
.desc = cpu_to_be32(0xd397d781),/* "LpPa" */
- .size = cpu_to_be16(0x400),
+ .size = cpu_to_be16(LPPACA_SIZE),
.fpregs_in_use = 1,
.slb_count = cpu_to_be16(64),
.vmxregs_in_use = 0,
@@ -80,14 +82,13 @@ static inline void init_lppaca(struct lppaca *lppaca)
static struct lppaca * __init new_lppaca(int cpu, unsigned long limit)
{
struct lppaca *lp;
- size_t size = 0x400;
- BUILD_BUG_ON(size < sizeof(struct lppaca));
+ BUILD_BUG_ON(LPPACA_SIZE < sizeof(struct lppaca));
if (early_cpu_has_feature(CPU_FTR_HVMODE))
return NULL;
- lp = alloc_paca_data(size, 0x400, limit, cpu);
+ lp = alloc_paca_data(LPPACA_SIZE, 0x400, limit, cpu);
init_lppaca(lp);
return lp;
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 07/11] powerpc/svm: Use shared memory for Debug Trace Log (DTL)
From: Anshuman Khandual
On Ultravisor platform kmem_cache for DTL buffers must use a constructor
function which converts the underlying buddy allocated SLUB cache pages
into shared memory so that they are accessible to the hypervisor.
Signed-off-by: Anshuman Khandual
Signed-off-by: Thiago Jung Bauermann
---
arch/powerpc/include/asm/svm.h | 1 +
arch/powerpc/kernel/svm.c | 30 ++
arch/powerpc/platforms/pseries/setup.c | 5 -
3 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/svm.h b/arch/powerpc/include/asm/svm.h
index 95d69e472e52..e00688761704 100644
--- a/arch/powerpc/include/asm/svm.h
+++ b/arch/powerpc/include/asm/svm.h
@@ -16,6 +16,7 @@ static bool is_svm_platform(void)
extern void mem_convert_shared(unsigned long pfn, unsigned long npages);
extern void mem_convert_secure(unsigned long pfn, unsigned long npages);
+extern void dtl_cache_ctor(void *addr);
#else
static inline bool is_svm_platform(void)
{
diff --git a/arch/powerpc/kernel/svm.c b/arch/powerpc/kernel/svm.c
index 891db2de8c04..1af5caa955f5 100644
--- a/arch/powerpc/kernel/svm.c
+++ b/arch/powerpc/kernel/svm.c
@@ -66,3 +66,33 @@ int set_memory_decrypted(unsigned long addr, int numpages)
return 0;
}
+
+/* There's one dispatch log per CPU. */
+#define NR_DTL_PAGE (DISPATCH_LOG_BYTES * CONFIG_NR_CPUS / PAGE_SIZE)
+
+static struct page *dtl_page_store[NR_DTL_PAGE];
+static long dtl_nr_pages;
+
+static bool is_dtl_page_shared(struct page *page)
+{
+ long i;
+
+ for (i = 0; i < dtl_nr_pages; i++)
+ if (dtl_page_store[i] == page)
+ return true;
+
+ return false;
+}
+
+void dtl_cache_ctor(void *addr)
+{
+ unsigned long pfn = PHYS_PFN(__pa(addr));
+ struct page *page = pfn_to_page(pfn);
+
+ if (!is_dtl_page_shared(page)) {
+ dtl_page_store[dtl_nr_pages] = page;
+ dtl_nr_pages++;
+ WARN_ON(dtl_nr_pages >= NR_DTL_PAGE);
+ mem_convert_shared(pfn, PAGE_SIZE);
+ }
+}
diff --git a/arch/powerpc/platforms/pseries/setup.c
b/arch/powerpc/platforms/pseries/setup.c
index 139f0af6c3d9..50ba77c802d2 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -69,6 +69,7 @@
#include
#include
#include
+#include
#include "pseries.h"
@@ -288,8 +289,10 @@ static inline int alloc_dispatch_logs(void)
static int alloc_dispatch_log_kmem_cache(void)
{
+ void (*ctor)(void *) = is_svm_platform() ? dtl_cache_ctor : NULL;
+
dtl_cache = kmem_cache_create("dtl", DISPATCH_LOG_BYTES,
- DISPATCH_LOG_BYTES, 0, NULL);
+ DISPATCH_LOG_BYTES, 0, ctor);
if (!dtl_cache) {
pr_warn("Failed to create dispatch trace log buffer cache\n");
pr_warn("Stolen time statistics will be unreliable\n");
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 03/11] powerpc/svm: Add memory conversion (shared/secure) helper functions
From: Anshuman Khandual
Add the following helper functions to convert PAGE_SIZE aligned memory
buffers as shared or secure (i.e., accessible to the hypervisor or not) via
Ultravisor calls.
1. mem_convert_shared(unsigned long pfn, unsigned long npages)
2. mem_convert_secure(unsigned long pfn, unsigned long npages)
Signed-off-by: Anshuman Khandual
Signed-off-by: Thiago Jung Bauermann
---
arch/powerpc/include/asm/svm.h | 3 +++
arch/powerpc/kernel/Makefile | 1 +
arch/powerpc/kernel/svm.c | 33 +
3 files changed, 37 insertions(+)
diff --git a/arch/powerpc/include/asm/svm.h b/arch/powerpc/include/asm/svm.h
index 6f89e5d6d37f..95d69e472e52 100644
--- a/arch/powerpc/include/asm/svm.h
+++ b/arch/powerpc/include/asm/svm.h
@@ -13,6 +13,9 @@ static bool is_svm_platform(void)
{
return mfmsr() & MSR_S;
}
+
+extern void mem_convert_shared(unsigned long pfn, unsigned long npages);
+extern void mem_convert_secure(unsigned long pfn, unsigned long npages);
#else
static inline bool is_svm_platform(void)
{
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 2b4c40b255e4..98780b4e924c 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -113,6 +113,7 @@ ifeq ($(CONFIG_HAVE_IMA_KEXEC)$(CONFIG_IMA),yy)
obj-y += ima_kexec.o
endif
+obj-$(CONFIG_PPC_SVM) += svm.o
obj-$(CONFIG_AUDIT)+= audit.o
obj64-$(CONFIG_AUDIT) += compat_audit.o
diff --git a/arch/powerpc/kernel/svm.c b/arch/powerpc/kernel/svm.c
new file mode 100644
index ..37437cf92df5
--- /dev/null
+++ b/arch/powerpc/kernel/svm.c
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Secure VM platform
+ *
+ * Copyright 2018 IBM Corporation
+ * Author: Anshuman Khandual
+ */
+
+#include
+
+void mem_convert_shared(unsigned long pfn, unsigned long npages)
+{
+ if (!pfn_valid(pfn))
+ return;
+
+ /*
+* FIXME: Insert real UV call when ready
+*
+* ucall_convert_shared(paddr, size)
+*/
+}
+
+void mem_convert_secure(unsigned long pfn, unsigned long npages)
+{
+ if (!pfn_valid(pfn))
+ return;
+
+ /*
+* FIXME: Insert real UV call when ready
+*
+* ucall_convert_secure(paddr, size)
+*/
+}
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 10/11] powerpc/svm: Force the use of bounce buffers
From: Anshuman Khandual SWIOTLB checks range of incoming CPU addresses to be bounced and see if the device can access it through it's DMA window without requiring bouncing. In such cases it just chooses to skip bouncing. But for cases like secure guests on powerpc platform all addresses need to be bounced into the shared pool of memory because the host cannot access it otherwise. Hence the need to do the bouncing is not related to device's DMA window. Hence force the use of bouncing by setting the swiotlb_force variable on secure guests. Signed-off-by: Anshuman Khandual Signed-off-by: Thiago Jung Bauermann --- arch/powerpc/kernel/svm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/kernel/svm.c b/arch/powerpc/kernel/svm.c index 1af5caa955f5..f0576ad65cd0 100644 --- a/arch/powerpc/kernel/svm.c +++ b/arch/powerpc/kernel/svm.c @@ -17,6 +17,7 @@ static int __init init_svm(void) return 0; ppc_swiotlb_enable = 1; + swiotlb_force = SWIOTLB_FORCE; swiotlb_update_mem_attributes(); return 0; ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 05/11] powerpc/svm: Don't release SWIOTLB buffers on secure guests
From: Anshuman Khandual Even though SWIOTLB slab gets allocated and initialized on each powerpc platform with swiotlb_init(), it gets released away again on all server platforms because ppc_swiotlb_enable variable never gets set. Secure guests would require SWIOTLB DMA API support for virtio bounce buffering purpose. Hence retain the allocated buffer by setting ppc_swiotlb_enable variable for secure guests on Ultravisor platforms. Signed-off-by: Anshuman Khandual Signed-off-by: Thiago Jung Bauermann --- arch/powerpc/kernel/svm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/kernel/svm.c b/arch/powerpc/kernel/svm.c index eab2a64d8643..891db2de8c04 100644 --- a/arch/powerpc/kernel/svm.c +++ b/arch/powerpc/kernel/svm.c @@ -16,6 +16,7 @@ static int __init init_svm(void) if (!is_svm_platform()) return 0; + ppc_swiotlb_enable = 1; swiotlb_update_mem_attributes(); return 0; ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 02/11] powerpc/svm: Select CONFIG_DMA_DIRECT_OPS and CONFIG_SWIOTLB
From: Anshuman Khandual Ultravisor based secure guest platforms will use generic SWIOTLB DMA API (i.e swiotlb_dma_ops structure) available under the CONFIG_DMA_DIRECT_OPS config. Also base CONFIG_SWIOTLB needs to be explicitly selected as well. Select both these config options on powerpc server platforms with secure guest support. Signed-off-by: Anshuman Khandual Signed-off-by: Thiago Jung Bauermann --- arch/powerpc/Kconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index f786c962edf0..90f73d15f58a 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -444,6 +444,8 @@ config PPC_SVM bool "Secure virtual machine (SVM) support for POWERPC" default n depends on PPC_PSERIES + select DMA_DIRECT_OPS + select SWIOTLB help Support secure guests on POWERPC. There are certain POWER platforms which support secure guests with the help of an Ultravisor executing ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 09/11] powerpc/svm: Use shared memory for LPPACA structures
From: Anshuman Khandual
LPPACA structures need to be shared with the host. Hence they need to be on
shared memory. Instead of allocating individual chunks of memory for given
structure from memblock, a contiguous chunk of memory is allocated and then
converted into shared memory. Subsequent allocation requests will come from
the contiguous chunk which will be always shared memory for all structures.
While we were able to use a kmem_cache constructor for the Debug Trace Log,
LPPACAs are allocated very early in the boot process (before SLUB is
available) so we need to use a simpler scheme here.
Signed-off-by: Anshuman Khandual
Signed-off-by: Thiago Jung Bauermann
---
arch/powerpc/kernel/paca.c | 34 +-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/paca.c b/arch/powerpc/kernel/paca.c
index 1edf8695019d..3e2aca150ad2 100644
--- a/arch/powerpc/kernel/paca.c
+++ b/arch/powerpc/kernel/paca.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include "setup.h"
@@ -25,6 +26,33 @@
#endif
#define LPPACA_SIZE 0x400
+#define SHARED_LPPACA_SIZE PAGE_ALIGN(LPPACA_SIZE * CONFIG_NR_CPUS)
+
+static phys_addr_t shared_lppaca_pa;
+static unsigned long shared_lppaca_size;
+
+static void *__init alloc_shared_lppaca(unsigned long size, unsigned long
align,
+ unsigned long limit, int cpu)
+{
+ unsigned long pa;
+
+ if (!shared_lppaca_pa) {
+ memblock_set_bottom_up(true);
+ shared_lppaca_pa = memblock_alloc_base_nid(SHARED_LPPACA_SIZE,
+ PAGE_SIZE, limit, -1, MEMBLOCK_NONE);
+ if (!shared_lppaca_pa)
+ panic("cannot allocate shared data");
+ memblock_set_bottom_up(false);
+ mem_convert_shared(PHYS_PFN(shared_lppaca_pa),
+ SHARED_LPPACA_SIZE / PAGE_SIZE);
+ }
+
+ pa = shared_lppaca_pa + shared_lppaca_size;
+ shared_lppaca_size += size;
+
+ BUG_ON(shared_lppaca_size >= SHARED_LPPACA_SIZE);
+ return __va(pa);
+}
static void *__init alloc_paca_data(unsigned long size, unsigned long align,
unsigned long limit, int cpu)
@@ -88,7 +116,11 @@ static struct lppaca * __init new_lppaca(int cpu, unsigned
long limit)
if (early_cpu_has_feature(CPU_FTR_HVMODE))
return NULL;
- lp = alloc_paca_data(LPPACA_SIZE, 0x400, limit, cpu);
+ if (is_svm_platform())
+ lp = alloc_shared_lppaca(LPPACA_SIZE, 0x400, limit, cpu);
+ else
+ lp = alloc_paca_data(LPPACA_SIZE, 0x400, limit, cpu);
+
init_lppaca(lp);
return lp;
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
