With SEV memory encryption and in some cases also with SME memory encryption, coherent memory is unencrypted. In those cases, TTM doesn't set up the correct page protection. Fix this by having the TTM coherent page allocator call into the platform code to determine whether coherent memory is encrypted or not, and modify the page protection if it is not.
v2: - Use force_dma_unencrypted() rather than sev_active() to catch also the special SME encryption cases.
From: Thomas Hellstrom thellstrom@vmware.com
The force_dma_unencrypted symbol is needed by TTM to set up the correct page protection when memory encryption is active. Export it.
Cc: Dave Hansen dave.hansen@linux.intel.com Cc: Andy Lutomirski luto@kernel.org Cc: Peter Zijlstra peterz@infradead.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Ingo Molnar mingo@redhat.com Cc: Borislav Petkov bp@alien8.de Cc: "H. Peter Anvin" hpa@zytor.com Cc: Heiko Carstens heiko.carstens@de.ibm.com Cc: Christian Borntraeger borntraeger@de.ibm.com Cc: Tom Lendacky thomas.lendacky@amd.com Cc: Christian König christian.koenig@amd.com Signed-off-by: Thomas Hellstrom thellstrom@vmware.com --- arch/x86/mm/mem_encrypt.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c index fece30ca8b0c..bbfe8802d63a 100644 --- a/arch/x86/mm/mem_encrypt.c +++ b/arch/x86/mm/mem_encrypt.c @@ -377,6 +377,7 @@ bool force_dma_unencrypted(struct device *dev)
return false; } +EXPORT_SYMBOL(force_dma_unencrypted);
/* Architecture __weak replacement functions */ void __init mem_encrypt_free_decrypted_mem(void)
On 9/3/19 6:15 AM, Thomas Hellström (VMware) wrote:
The force_dma_unencrypted symbol is needed by TTM to set up the correct page protection when memory encryption is active. Export it.
It would be great if this had enough background that I didn't have to look at patch 4 to figure out what TTM might be.
Why is TTM special? How many other drivers would have to be modified in a one-off fashion if we go this way? What's the logic behind this being a non-GPL export?
On 9/3/19 5:14 PM, Dave Hansen wrote:
On 9/3/19 6:15 AM, Thomas Hellström (VMware) wrote:
The force_dma_unencrypted symbol is needed by TTM to set up the correct page protection when memory encryption is active. Export it.
It would be great if this had enough background that I didn't have to look at patch 4 to figure out what TTM might be.
Why is TTM special? How many other drivers would have to be modified in a one-off fashion if we go this way? What's the logic behind this being a non-GPL export?
TTM tries to abstract mapping of graphics buffer objects regardless where they live. Be it in pci memory or system memory. As such it needs to figure out the proper page protection. For example if a buffer object is moved from pci memory to system memory transparently to a user-space application, all user-space mappings need to be killed and then reinstated pointing to the new location, sometimes with a new page protection.
I try to keep away as much as possible from the non-GPL vs GPL export discussions. I have no strong opinion on the subject. Although since sev_active() is a non-GPL export, I decided to mimic that.
Thanks Thomas
From: Thomas Hellstrom thellstrom@vmware.com
The force_dma_unencrypted symbol is needed by TTM to set up the correct page protection when memory encryption is active. Export it.
Cc: Dave Hansen dave.hansen@linux.intel.com Cc: Andy Lutomirski luto@kernel.org Cc: Peter Zijlstra peterz@infradead.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Ingo Molnar mingo@redhat.com Cc: Borislav Petkov bp@alien8.de Cc: "H. Peter Anvin" hpa@zytor.com Cc: Heiko Carstens heiko.carstens@de.ibm.com Cc: Christian Borntraeger borntraeger@de.ibm.com Cc: Tom Lendacky thomas.lendacky@amd.com Cc: Christian König christian.koenig@amd.com Signed-off-by: Thomas Hellstrom thellstrom@vmware.com --- arch/s390/mm/init.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c index 20340a03ad90..eec7cc303a31 100644 --- a/arch/s390/mm/init.c +++ b/arch/s390/mm/init.c @@ -165,6 +165,7 @@ bool force_dma_unencrypted(struct device *dev) { return sev_active(); } +EXPORT_SYMBOL(force_dma_unencrypted);
/* protected virtualization */ static void pv_init(void)
From: Thomas Hellstrom thellstrom@vmware.com
With TTM pages allocated out of the DMA pool, use the force_dma_unencrypted function to be able to set up the correct page-protection. Previously it was unconditionally set to encrypted, which only works with SME encryption on devices with a large enough DMA mask.
Tested with vmwgfx and sev-es. Screen garbage without this patch and normal functionality with it.
Cc: Dave Hansen dave.hansen@linux.intel.com Cc: Andy Lutomirski luto@kernel.org Cc: Peter Zijlstra peterz@infradead.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Ingo Molnar mingo@redhat.com Cc: Borislav Petkov bp@alien8.de Cc: "H. Peter Anvin" hpa@zytor.com Cc: Heiko Carstens heiko.carstens@de.ibm.com Cc: Christian Borntraeger borntraeger@de.ibm.com Cc: Tom Lendacky thomas.lendacky@amd.com Cc: Christian König christian.koenig@amd.com Signed-off-by: Thomas Hellstrom thellstrom@vmware.com --- drivers/gpu/drm/ttm/ttm_bo_util.c | 17 +++++++++++++---- drivers/gpu/drm/ttm/ttm_bo_vm.c | 21 ++++++++++----------- drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 4 ++++ drivers/gpu/drm/vmwgfx/vmwgfx_blit.c | 6 ++++-- include/drm/ttm/ttm_bo_driver.h | 8 +++++--- include/drm/ttm/ttm_tt.h | 1 + 6 files changed, 37 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index fe81c565e7ef..d5ad8f03b63f 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -419,11 +419,13 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo, page = i * dir + add; if (old_iomap == NULL) { pgprot_t prot = ttm_io_prot(old_mem->placement, + ttm->page_flags, PAGE_KERNEL); ret = ttm_copy_ttm_io_page(ttm, new_iomap, page, prot); } else if (new_iomap == NULL) { pgprot_t prot = ttm_io_prot(new_mem->placement, + ttm->page_flags, PAGE_KERNEL); ret = ttm_copy_io_ttm_page(ttm, old_iomap, page, prot); @@ -526,11 +528,11 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, return 0; }
-pgprot_t ttm_io_prot(uint32_t caching_flags, pgprot_t tmp) +pgprot_t ttm_io_prot(u32 caching_flags, u32 tt_page_flags, pgprot_t tmp) { /* Cached mappings need no adjustment */ if (caching_flags & TTM_PL_FLAG_CACHED) - return tmp; + goto check_encryption;
#if defined(__i386__) || defined(__x86_64__) if (caching_flags & TTM_PL_FLAG_WC) @@ -548,6 +550,11 @@ pgprot_t ttm_io_prot(uint32_t caching_flags, pgprot_t tmp) #if defined(__sparc__) tmp = pgprot_noncached(tmp); #endif + +check_encryption: + if (tt_page_flags & TTM_PAGE_FLAG_DECRYPTED) + tmp = pgprot_decrypted(tmp); + return tmp; } EXPORT_SYMBOL(ttm_io_prot); @@ -594,7 +601,8 @@ static int ttm_bo_kmap_ttm(struct ttm_buffer_object *bo, if (ret) return ret;
- if (num_pages == 1 && (mem->placement & TTM_PL_FLAG_CACHED)) { + if (num_pages == 1 && (mem->placement & TTM_PL_FLAG_CACHED) && + !(ttm->page_flags & TTM_PAGE_FLAG_DECRYPTED)) { /* * We're mapping a single page, and the desired * page protection is consistent with the bo. @@ -608,7 +616,8 @@ static int ttm_bo_kmap_ttm(struct ttm_buffer_object *bo, * We need to use vmap to get the desired page protection * or to make the buffer object look contiguous. */ - prot = ttm_io_prot(mem->placement, PAGE_KERNEL); + prot = ttm_io_prot(mem->placement, ttm->page_flags, + PAGE_KERNEL); map->bo_kmap_type = ttm_bo_map_vmap; map->virtual = vmap(ttm->pages + start_page, num_pages, 0, prot); diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c index 76eedb963693..194d8d618d23 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c @@ -226,12 +226,7 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf) * by mmap_sem in write mode. */ cvma = *vma; - cvma.vm_page_prot = vm_get_page_prot(cvma.vm_flags); - - if (bo->mem.bus.is_iomem) { - cvma.vm_page_prot = ttm_io_prot(bo->mem.placement, - cvma.vm_page_prot); - } else { + if (!bo->mem.bus.is_iomem) { struct ttm_operation_ctx ctx = { .interruptible = false, .no_wait_gpu = false, @@ -240,14 +235,18 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf) };
ttm = bo->ttm; - cvma.vm_page_prot = ttm_io_prot(bo->mem.placement, - cvma.vm_page_prot); - - /* Allocate all page at once, most common usage */ - if (ttm_tt_populate(ttm, &ctx)) { + if (ttm_tt_populate(bo->ttm, &ctx)) { ret = VM_FAULT_OOM; goto out_io_unlock; } + cvma.vm_page_prot = ttm_io_prot(bo->mem.placement, + ttm->page_flags, + cvma.vm_page_prot); + } else { + /* Iomem should not be marked encrypted */ + cvma.vm_page_prot = ttm_io_prot(bo->mem.placement, + TTM_PAGE_FLAG_DECRYPTED, + cvma.vm_page_prot); }
/* diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c index 7d78e6deac89..9b15df8ecd49 100644 --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c @@ -48,6 +48,7 @@ #include <linux/atomic.h> #include <linux/device.h> #include <linux/kthread.h> +#include <linux/dma-direct.h> #include <drm/ttm/ttm_bo_driver.h> #include <drm/ttm/ttm_page_alloc.h> #include <drm/ttm/ttm_set_memory.h> @@ -984,6 +985,9 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev, }
ttm->state = tt_unbound; + if (force_dma_unencrypted(dev)) + ttm->page_flags |= TTM_PAGE_FLAG_DECRYPTED; + return 0; } EXPORT_SYMBOL_GPL(ttm_dma_populate); diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c index bb46ca0c458f..d3ced89a37e9 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c @@ -483,8 +483,10 @@ int vmw_bo_cpu_blit(struct ttm_buffer_object *dst, d.src_pages = src->ttm->pages; d.dst_num_pages = dst->num_pages; d.src_num_pages = src->num_pages; - d.dst_prot = ttm_io_prot(dst->mem.placement, PAGE_KERNEL); - d.src_prot = ttm_io_prot(src->mem.placement, PAGE_KERNEL); + d.dst_prot = ttm_io_prot(dst->mem.placement, dst->ttm->page_flags, + PAGE_KERNEL); + d.src_prot = ttm_io_prot(src->mem.placement, src->ttm->page_flags, + PAGE_KERNEL); d.diff = diff;
for (j = 0; j < h; ++j) { diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h index 6f536caea368..68ead1bd3042 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -893,13 +893,15 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo); /** * ttm_io_prot * - * @c_state: Caching state. + * @caching_flags: The caching flags of the map. + * @tt_page_flags: The tt_page_flags of the map, TTM_PAGE_FLAG_* * @tmp: Page protection flag for a normal, cached mapping. * * Utility function that returns the pgprot_t that should be used for - * setting up a PTE with the caching model indicated by @c_state. + * setting up a PTE with the caching model indicated by @caching_flags, + * and encryption state indicated by @tt_page_flags, */ -pgprot_t ttm_io_prot(uint32_t caching_flags, pgprot_t tmp); +pgprot_t ttm_io_prot(u32 caching_flags, u32 tt_page_flags, pgprot_t tmp);
extern const struct ttm_mem_type_manager_func ttm_bo_manager_func;
diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h index c0e928abf592..45cc26355513 100644 --- a/include/drm/ttm/ttm_tt.h +++ b/include/drm/ttm/ttm_tt.h @@ -41,6 +41,7 @@ struct ttm_operation_ctx; #define TTM_PAGE_FLAG_DMA32 (1 << 7) #define TTM_PAGE_FLAG_SG (1 << 8) #define TTM_PAGE_FLAG_NO_RETRY (1 << 9) +#define TTM_PAGE_FLAG_DECRYPTED (1 << 10)
enum ttm_caching_state { tt_uncached,
This whole thing looks like a fascinating collection of hacks. :)
ttm is taking a stack-alllocated "VMA" and handing it to vmf_insert_*() which obviously are expecting "real" VMAs that are linked into the mm. It's extracting some pgprot_t information from the real VMA, making a psuedo-temporary VMA, then passing the temporary one back into the insertion functions:
static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf) {
...
struct vm_area_struct cvma;
...
if (vma->vm_flags & VM_MIXEDMAP) ret = vmf_insert_mixed(&cvma, address, __pfn_to_pfn_t(pfn, PFN_DEV)); else ret = vmf_insert_pfn(&cvma, address, pfn);
I can totally see why this needs new exports. But, man, it doesn't seem like something we want to keep *feeding*.
The real problem here is that the encryption bits from the device VMA's "true" vma->vm_page_prot don't match the ones that actually get inserted, probably because the device ptes need the encryption bits cleared but the system memory PTEs need them set *and* they're mixed under one VMA.
The thing we need to stop is having mixed encryption rules under one VMA.
On Tue, Sep 3, 2019 at 9:38 PM Dave Hansen dave.hansen@intel.com wrote:
This whole thing looks like a fascinating collection of hacks. :)
ttm is taking a stack-alllocated "VMA" and handing it to vmf_insert_*() which obviously are expecting "real" VMAs that are linked into the mm. It's extracting some pgprot_t information from the real VMA, making a psuedo-temporary VMA, then passing the temporary one back into the insertion functions:
static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf) {
...
struct vm_area_struct cvma;...
if (vma->vm_flags & VM_MIXEDMAP) ret = vmf_insert_mixed(&cvma, address, __pfn_to_pfn_t(pfn, PFN_DEV)); else ret = vmf_insert_pfn(&cvma, address, pfn);I can totally see why this needs new exports. But, man, it doesn't seem like something we want to keep *feeding*.
The real problem here is that the encryption bits from the device VMA's "true" vma->vm_page_prot don't match the ones that actually get inserted, probably because the device ptes need the encryption bits cleared but the system memory PTEs need them set *and* they're mixed under one VMA.
The thing we need to stop is having mixed encryption rules under one VMA.
The point here is that we want this. We need to be able to move the buffer between device ptes and system memory ptes, transparently, behind userspace back, without races. And the fast path (which is "no pte exists for this vma") must be real fast, so taking mmap_sem and replacing the vma is no-go. -Daniel
On 9/3/19 12:51 PM, Daniel Vetter wrote:
The thing we need to stop is having mixed encryption rules under one VMA.
The point here is that we want this. We need to be able to move the buffer between device ptes and system memory ptes, transparently, behind userspace back, without races. And the fast path (which is "no pte exists for this vma") must be real fast, so taking mmap_sem and replacing the vma is no-go.
So, when the user asks for encryption and we say, "sure, we'll encrypt that", then we want the device driver to be able to transparently undo that encryption under the covers for device memory? That seems suboptimal.
I'd rather the device driver just say: "Nope, you can't encrypt my VMA". Because that's the truth.
On 9/3/19 9:55 PM, Dave Hansen wrote:
On 9/3/19 12:51 PM, Daniel Vetter wrote:
The thing we need to stop is having mixed encryption rules under one VMA.
The point here is that we want this. We need to be able to move the buffer between device ptes and system memory ptes, transparently, behind userspace back, without races. And the fast path (which is "no pte exists for this vma") must be real fast, so taking mmap_sem and replacing the vma is no-go.
So, when the user asks for encryption and we say, "sure, we'll encrypt that", then we want the device driver to be able to transparently undo that encryption under the covers for device memory? That seems suboptimal.
I'd rather the device driver just say: "Nope, you can't encrypt my VMA". Because that's the truth.
The thing here is that it's the underlying physical memory that define the correct encryption flags. If it's DMA memory and SEV is active or PCI memory. It's always unencrypted. User-space in a SEV vm should always, from a data protection point of view, *assume* that graphics buffers are unencrypted. (Which will of course limit the use of gpus and display controllers in a SEV vm). Platform code sets the vma encryption to on by default.
So the question here should really be, can we determine already at mmap time whether backing memory will be unencrypted and adjust the *real* vma->vm_page_prot under the mmap_sem?
Possibly, but that requires populating the buffer with memory at mmap time rather than at first fault time.
And it still requires knowledge whether the device DMA is always unencrypted (or if SEV is active).
/Thomas
On 9/3/19 1:36 PM, Thomas Hellström (VMware) wrote:
So the question here should really be, can we determine already at mmap time whether backing memory will be unencrypted and adjust the *real* vma->vm_page_prot under the mmap_sem?
Possibly, but that requires populating the buffer with memory at mmap time rather than at first fault time.
I'm not connecting the dots.
vma->vm_page_prot is used to create a VMA's PTEs regardless of if they are created at mmap() or fault time. If we establish a good vma->vm_page_prot, can't we just use it forever for demand faults?
Or, are you concerned that if an attempt is made to demand-fault page that's incompatible with vma->vm_page_prot that we have to SEGV?
And it still requires knowledge whether the device DMA is always unencrypted (or if SEV is active).
I may be getting mixed up on MKTME (the Intel memory encryption) and SEV. Is SEV supported on all memory types? Page cache, hugetlbfs, anonymous? Or just anonymous?
On 9/3/19 10:51 PM, Dave Hansen wrote:
On 9/3/19 1:36 PM, Thomas Hellström (VMware) wrote:
So the question here should really be, can we determine already at mmap time whether backing memory will be unencrypted and adjust the *real* vma->vm_page_prot under the mmap_sem?
Possibly, but that requires populating the buffer with memory at mmap time rather than at first fault time.
I'm not connecting the dots.
vma->vm_page_prot is used to create a VMA's PTEs regardless of if they are created at mmap() or fault time. If we establish a good vma->vm_page_prot, can't we just use it forever for demand faults?
With SEV I think that we could possibly establish the encryption flags at vma creation time. But thinking of it, it would actually break with SME where buffer content can be moved between encrypted system memory and unencrypted graphics card PCI memory behind user-space's back. That would imply killing all user-space encrypted PTEs and at fault time set up new ones pointing to unencrypted PCI memory..
Or, are you concerned that if an attempt is made to demand-fault page that's incompatible with vma->vm_page_prot that we have to SEGV?
And it still requires knowledge whether the device DMA is always unencrypted (or if SEV is active).
I may be getting mixed up on MKTME (the Intel memory encryption) and SEV. Is SEV supported on all memory types? Page cache, hugetlbfs, anonymous? Or just anonymous?
SEV AFAIK encrypts *all* memory except DMA memory. To do that it uses a SWIOTLB backed by unencrypted memory, and it also flips coherent DMA memory to unencrypted (which is a very slow operation and patch 4 deals with caching such memory).
/Thomas
On Tue, Sep 3, 2019 at 2:05 PM Thomas Hellström (VMware) thomas_os@shipmail.org wrote:
On 9/3/19 10:51 PM, Dave Hansen wrote:
On 9/3/19 1:36 PM, Thomas Hellström (VMware) wrote:
So the question here should really be, can we determine already at mmap time whether backing memory will be unencrypted and adjust the *real* vma->vm_page_prot under the mmap_sem?
Possibly, but that requires populating the buffer with memory at mmap time rather than at first fault time.
I'm not connecting the dots.
vma->vm_page_prot is used to create a VMA's PTEs regardless of if they are created at mmap() or fault time. If we establish a good vma->vm_page_prot, can't we just use it forever for demand faults?
With SEV I think that we could possibly establish the encryption flags at vma creation time. But thinking of it, it would actually break with SME where buffer content can be moved between encrypted system memory and unencrypted graphics card PCI memory behind user-space's back. That would imply killing all user-space encrypted PTEs and at fault time set up new ones pointing to unencrypted PCI memory..
Or, are you concerned that if an attempt is made to demand-fault page that's incompatible with vma->vm_page_prot that we have to SEGV?
And it still requires knowledge whether the device DMA is always unencrypted (or if SEV is active).
I may be getting mixed up on MKTME (the Intel memory encryption) and SEV. Is SEV supported on all memory types? Page cache, hugetlbfs, anonymous? Or just anonymous?
SEV AFAIK encrypts *all* memory except DMA memory. To do that it uses a SWIOTLB backed by unencrypted memory, and it also flips coherent DMA memory to unencrypted (which is a very slow operation and patch 4 deals with caching such memory).
I'm still lost. You have some fancy VMA where the backing pages change behind the application's back. This isn't particularly novel -- plain old anonymous memory and plain old mapped files do this too. Can't you all the insert_pfn APIs and call it a day? What's so special that you need all this magic? ISTM you should be able to allocate memory that's addressable by the device (dma_alloc_coherent() or whatever) and then map it into user memory just like you'd map any other page.
I feel like I'm missing something here.
On 9/3/19 11:46 PM, Andy Lutomirski wrote:
On Tue, Sep 3, 2019 at 2:05 PM Thomas Hellström (VMware) thomas_os@shipmail.org wrote:
On 9/3/19 10:51 PM, Dave Hansen wrote:
On 9/3/19 1:36 PM, Thomas Hellström (VMware) wrote:
So the question here should really be, can we determine already at mmap time whether backing memory will be unencrypted and adjust the *real* vma->vm_page_prot under the mmap_sem?
Possibly, but that requires populating the buffer with memory at mmap time rather than at first fault time.
I'm not connecting the dots.
vma->vm_page_prot is used to create a VMA's PTEs regardless of if they are created at mmap() or fault time. If we establish a good vma->vm_page_prot, can't we just use it forever for demand faults?
With SEV I think that we could possibly establish the encryption flags at vma creation time. But thinking of it, it would actually break with SME where buffer content can be moved between encrypted system memory and unencrypted graphics card PCI memory behind user-space's back. That would imply killing all user-space encrypted PTEs and at fault time set up new ones pointing to unencrypted PCI memory..
Or, are you concerned that if an attempt is made to demand-fault page that's incompatible with vma->vm_page_prot that we have to SEGV?
And it still requires knowledge whether the device DMA is always unencrypted (or if SEV is active).
I may be getting mixed up on MKTME (the Intel memory encryption) and SEV. Is SEV supported on all memory types? Page cache, hugetlbfs, anonymous? Or just anonymous?
SEV AFAIK encrypts *all* memory except DMA memory. To do that it uses a SWIOTLB backed by unencrypted memory, and it also flips coherent DMA memory to unencrypted (which is a very slow operation and patch 4 deals with caching such memory).
I'm still lost. You have some fancy VMA where the backing pages change behind the application's back. This isn't particularly novel -- plain old anonymous memory and plain old mapped files do this too. Can't you all the insert_pfn APIs and call it a day? What's so special that you need all this magic? ISTM you should be able to allocate memory that's addressable by the device (dma_alloc_coherent() or whatever) and then map it into user memory just like you'd map any other page.
I feel like I'm missing something here.
Yes, so in this case we use dma_alloc_coherent().
With SEV, that gives us unencrypted pages. (Pages whose linear kernel map is marked unencrypted). With SME that (typcially) gives us encrypted pages. In both these cases, vm_get_page_prot() returns an encrypted page protection, which lands in vma->vm_page_prot.
In the SEV case, we therefore need to modify the page protection to unencrypted. Hence we need to know whether we're running under SEV and therefore need to modify the protection. If not, the user-space PTE would incorrectly have the encryption flag set.
/Thomas
On 9/4/19 12:08 AM, Thomas Hellström (VMware) wrote:
On 9/3/19 11:46 PM, Andy Lutomirski wrote:
On Tue, Sep 3, 2019 at 2:05 PM Thomas Hellström (VMware) thomas_os@shipmail.org wrote:
On 9/3/19 10:51 PM, Dave Hansen wrote:
On 9/3/19 1:36 PM, Thomas Hellström (VMware) wrote:
So the question here should really be, can we determine already at mmap time whether backing memory will be unencrypted and adjust the *real* vma->vm_page_prot under the mmap_sem?
Possibly, but that requires populating the buffer with memory at mmap time rather than at first fault time.
I'm not connecting the dots.
vma->vm_page_prot is used to create a VMA's PTEs regardless of if they are created at mmap() or fault time. If we establish a good vma->vm_page_prot, can't we just use it forever for demand faults?
With SEV I think that we could possibly establish the encryption flags at vma creation time. But thinking of it, it would actually break with SME where buffer content can be moved between encrypted system memory and unencrypted graphics card PCI memory behind user-space's back. That would imply killing all user-space encrypted PTEs and at fault time set up new ones pointing to unencrypted PCI memory..
Or, are you concerned that if an attempt is made to demand-fault page that's incompatible with vma->vm_page_prot that we have to SEGV?
And it still requires knowledge whether the device DMA is always unencrypted (or if SEV is active).
I may be getting mixed up on MKTME (the Intel memory encryption) and SEV. Is SEV supported on all memory types? Page cache, hugetlbfs, anonymous? Or just anonymous?
SEV AFAIK encrypts *all* memory except DMA memory. To do that it uses a SWIOTLB backed by unencrypted memory, and it also flips coherent DMA memory to unencrypted (which is a very slow operation and patch 4 deals with caching such memory).
I'm still lost. You have some fancy VMA where the backing pages change behind the application's back. This isn't particularly novel -- plain old anonymous memory and plain old mapped files do this too. Can't you all the insert_pfn APIs and call it a day? What's so special that you need all this magic? ISTM you should be able to allocate memory that's addressable by the device (dma_alloc_coherent() or whatever) and then map it into user memory just like you'd map any other page.
I feel like I'm missing something here.
Yes, so in this case we use dma_alloc_coherent().
With SEV, that gives us unencrypted pages. (Pages whose linear kernel map is marked unencrypted). With SME that (typcially) gives us encrypted pages. In both these cases, vm_get_page_prot() returns an encrypted page protection, which lands in vma->vm_page_prot.
In the SEV case, we therefore need to modify the page protection to unencrypted. Hence we need to know whether we're running under SEV and therefore need to modify the protection. If not, the user-space PTE would incorrectly have the encryption flag set.
/Thomas
And, of course, had we not been "fancy", we could have used dma_mmap_coherent(), which in theory should set up the correct user-space page protection. But now we're moving stuff around so we can't.
/Thomas
Thomas, this series has garnered a nak and a whole pile of thoroughly confused reviewers.
Could you take another stab at this along with a more ample changelog explaining the context of the problem? I suspect that's a better place to start than having us all piece together the disparate parts of the thread.
Hi, Dave,
On 9/4/19 1:10 AM, Dave Hansen wrote:
Thomas, this series has garnered a nak and a whole pile of thoroughly confused reviewers.
Could you take another stab at this along with a more ample changelog explaining the context of the problem? I suspect that's a better place to start than having us all piece together the disparate parts of the thread.
Sure.
I was just trying to follow up on the emails to get a better understanding what got people confused in the first place.
Thanks,
Thomas
On Sep 3, 2019, at 3:15 PM, Thomas Hellström (VMware) thomas_os@shipmail.org wrote:
On 9/4/19 12:08 AM, Thomas Hellström (VMware) wrote:
On 9/3/19 11:46 PM, Andy Lutomirski wrote: On Tue, Sep 3, 2019 at 2:05 PM Thomas Hellström (VMware) thomas_os@shipmail.org wrote:
On 9/3/19 10:51 PM, Dave Hansen wrote:
On 9/3/19 1:36 PM, Thomas Hellström (VMware) wrote: So the question here should really be, can we determine already at mmap time whether backing memory will be unencrypted and adjust the *real* vma->vm_page_prot under the mmap_sem?
Possibly, but that requires populating the buffer with memory at mmap time rather than at first fault time.
I'm not connecting the dots.
vma->vm_page_prot is used to create a VMA's PTEs regardless of if they are created at mmap() or fault time. If we establish a good vma->vm_page_prot, can't we just use it forever for demand faults?
With SEV I think that we could possibly establish the encryption flags at vma creation time. But thinking of it, it would actually break with SME where buffer content can be moved between encrypted system memory and unencrypted graphics card PCI memory behind user-space's back. That would imply killing all user-space encrypted PTEs and at fault time set up new ones pointing to unencrypted PCI memory..
Or, are you concerned that if an attempt is made to demand-fault page that's incompatible with vma->vm_page_prot that we have to SEGV?
And it still requires knowledge whether the device DMA is always unencrypted (or if SEV is active).
I may be getting mixed up on MKTME (the Intel memory encryption) and SEV. Is SEV supported on all memory types? Page cache, hugetlbfs, anonymous? Or just anonymous?
SEV AFAIK encrypts *all* memory except DMA memory. To do that it uses a SWIOTLB backed by unencrypted memory, and it also flips coherent DMA memory to unencrypted (which is a very slow operation and patch 4 deals with caching such memory).
I'm still lost. You have some fancy VMA where the backing pages change behind the application's back. This isn't particularly novel -- plain old anonymous memory and plain old mapped files do this too. Can't you all the insert_pfn APIs and call it a day? What's so special that you need all this magic? ISTM you should be able to allocate memory that's addressable by the device (dma_alloc_coherent() or whatever) and then map it into user memory just like you'd map any other page.
I feel like I'm missing something here.
Yes, so in this case we use dma_alloc_coherent().
With SEV, that gives us unencrypted pages. (Pages whose linear kernel map is marked unencrypted). With SME that (typcially) gives us encrypted pages. In both these cases, vm_get_page_prot() returns an encrypted page protection, which lands in vma->vm_page_prot.
In the SEV case, we therefore need to modify the page protection to unencrypted. Hence we need to know whether we're running under SEV and therefore need to modify the protection. If not, the user-space PTE would incorrectly have the encryption flag set.
I’m still confused. You got unencrypted pages with an unencrypted PFN. Why do you need to fiddle? You have a PFN, and you’re inserting it with vmf_insert_pfn(). This should just work, no? There doesn’t seem to be any real funny business in dma_mmap_attrs() or dma_common_mmap().
But, reading this, I have more questions:
Can’t you get rid of cvma by using vmf_insert_pfn_prot()?
Would it make sense to add a vmf_insert_dma_page() to directly do exactly what you’re trying to do?
And a broader question just because I’m still confused: why isn’t the encryption bit in the PFN? The whole SEV/SME system seems like it’s trying a bit to hard to be fully invisible to the kernel.
On 9/4/19 1:15 AM, Andy Lutomirski wrote:
On Sep 3, 2019, at 3:15 PM, Thomas Hellström (VMware) thomas_os@shipmail.org wrote:
On 9/4/19 12:08 AM, Thomas Hellström (VMware) wrote:
On 9/3/19 11:46 PM, Andy Lutomirski wrote: On Tue, Sep 3, 2019 at 2:05 PM Thomas Hellström (VMware) thomas_os@shipmail.org wrote:
On 9/3/19 10:51 PM, Dave Hansen wrote:
> On 9/3/19 1:36 PM, Thomas Hellström (VMware) wrote: > So the question here should really be, can we determine already at mmap > time whether backing memory will be unencrypted and adjust the *real* > vma->vm_page_prot under the mmap_sem? > > Possibly, but that requires populating the buffer with memory at mmap > time rather than at first fault time. I'm not connecting the dots.
vma->vm_page_prot is used to create a VMA's PTEs regardless of if they are created at mmap() or fault time. If we establish a good vma->vm_page_prot, can't we just use it forever for demand faults?
With SEV I think that we could possibly establish the encryption flags at vma creation time. But thinking of it, it would actually break with SME where buffer content can be moved between encrypted system memory and unencrypted graphics card PCI memory behind user-space's back. That would imply killing all user-space encrypted PTEs and at fault time set up new ones pointing to unencrypted PCI memory..
Or, are you concerned that if an attempt is made to demand-fault page that's incompatible with vma->vm_page_prot that we have to SEGV?
> And it still requires knowledge whether the device DMA is always > unencrypted (or if SEV is active). I may be getting mixed up on MKTME (the Intel memory encryption) and SEV. Is SEV supported on all memory types? Page cache, hugetlbfs, anonymous? Or just anonymous?
SEV AFAIK encrypts *all* memory except DMA memory. To do that it uses a SWIOTLB backed by unencrypted memory, and it also flips coherent DMA memory to unencrypted (which is a very slow operation and patch 4 deals with caching such memory).
I'm still lost. You have some fancy VMA where the backing pages change behind the application's back. This isn't particularly novel -- plain old anonymous memory and plain old mapped files do this too. Can't you all the insert_pfn APIs and call it a day? What's so special that you need all this magic? ISTM you should be able to allocate memory that's addressable by the device (dma_alloc_coherent() or whatever) and then map it into user memory just like you'd map any other page.
I feel like I'm missing something here.
Yes, so in this case we use dma_alloc_coherent().
With SEV, that gives us unencrypted pages. (Pages whose linear kernel map is marked unencrypted). With SME that (typcially) gives us encrypted pages. In both these cases, vm_get_page_prot() returns an encrypted page protection, which lands in vma->vm_page_prot.
In the SEV case, we therefore need to modify the page protection to unencrypted. Hence we need to know whether we're running under SEV and therefore need to modify the protection. If not, the user-space PTE would incorrectly have the encryption flag set.
I’m still confused. You got unencrypted pages with an unencrypted PFN. Why do you need to fiddle? You have a PFN, and you’re inserting it with vmf_insert_pfn(). This should just work, no?
OK now I see what causes the confusion.
With SEV, the encryption state is, while *physically* encoded in an address bit, from what I can tell, not *logically* encoded in the pfn, but in the page_prot for cpu mapping purposes. That is, page_to_pfn() returns the same pfn whether the page is encrypted or unencrypted. Hence nobody can't tell from the pfn whether the page is unencrypted or encrypted.
For device DMA address purposes, the encryption status is encoded in the dma address by the dma layer in phys_to_dma().
There doesn’t seem to be any real funny business in dma_mmap_attrs() or dma_common_mmap().
No, from what I can tell the call in these functions to dma_pgprot() generates an incorrect page protection since it doesn't take unencrypted coherent memory into account. I don't think anybody has used these functions yet with SEV.
But, reading this, I have more questions:
Can’t you get rid of cvma by using vmf_insert_pfn_prot()?
It looks like that, although there are comments in the code about serious performance problems using VM_PFNMAP / vmf_insert_pfn() with write-combining and PAT, so that would require some serious testing with hardware I don't have. But I guess there is definitely room for improvement here. Ideally we'd like to be able to change the vma->vm_page_prot within fault(). But we can
Would it make sense to add a vmf_insert_dma_page() to directly do exactly what you’re trying to do?
Yes, but as a longer term solution I would prefer a general dma_pgprot() exported, so that we could, in a dma-compliant way, use coherent pages with other apis, like kmap_atomic_prot() and vmap(). That is, basically split coherent page allocation in two steps: Allocation and mapping.
And a broader question just because I’m still confused: why isn’t the encryption bit in the PFN? The whole SEV/SME system seems like it’s trying a bit to hard to be fully invisible to the kernel.
I guess you'd have to ask AMD about that. But my understanding is that encoding it in an address bit does make it trivial to do decryption / encryption on the fly to DMA devices that are not otherwise aware of it, just by handing them a special physical address. For cpu mapping purposes it might become awkward to encode it in the pfn since pfn_to_page and friends would need knowledge about this. Personally I think it would have made sense to track it like PAT in track_pfn_insert().
Thanks,
Thomas
On Wed, Sep 4, 2019 at 8:49 AM Thomas Hellström (VMware) thomas_os@shipmail.org wrote:
On 9/4/19 1:15 AM, Andy Lutomirski wrote:
But, reading this, I have more questions:
Can’t you get rid of cvma by using vmf_insert_pfn_prot()?
It looks like that, although there are comments in the code about serious performance problems using VM_PFNMAP / vmf_insert_pfn() with write-combining and PAT, so that would require some serious testing with hardware I don't have. But I guess there is definitely room for improvement here. Ideally we'd like to be able to change the vma->vm_page_prot within fault(). But we can
Just a quick comment on this: It's the repeated (per-pfn/pte) lookup of the PAT tables, which are dead slow. If you have a struct io_mapping then that can be done once, and then just blindly inserted. See remap_io_mapping in i915. -Daniel
On 9/4/19 9:53 AM, Daniel Vetter wrote:
On Wed, Sep 4, 2019 at 8:49 AM Thomas Hellström (VMware) thomas_os@shipmail.org wrote:
On 9/4/19 1:15 AM, Andy Lutomirski wrote:
But, reading this, I have more questions:
Can’t you get rid of cvma by using vmf_insert_pfn_prot()?
It looks like that, although there are comments in the code about serious performance problems using VM_PFNMAP / vmf_insert_pfn() with write-combining and PAT, so that would require some serious testing with hardware I don't have. But I guess there is definitely room for improvement here. Ideally we'd like to be able to change the vma->vm_page_prot within fault(). But we can
Just a quick comment on this: It's the repeated (per-pfn/pte) lookup of the PAT tables, which are dead slow. If you have a struct io_mapping then that can be done once, and then just blindly inserted. See remap_io_mapping in i915. -Daniel
Thanks, Daniel.
Indeed looks a lot like remap_pfn_range(), but usable at fault time?
/Thomas
On Wed, Sep 4, 2019 at 12:38 PM Thomas Hellström (VMware) thomas_os@shipmail.org wrote:
On 9/4/19 9:53 AM, Daniel Vetter wrote:
On Wed, Sep 4, 2019 at 8:49 AM Thomas Hellström (VMware) thomas_os@shipmail.org wrote:
On 9/4/19 1:15 AM, Andy Lutomirski wrote:
But, reading this, I have more questions:
Can’t you get rid of cvma by using vmf_insert_pfn_prot()?
It looks like that, although there are comments in the code about serious performance problems using VM_PFNMAP / vmf_insert_pfn() with write-combining and PAT, so that would require some serious testing with hardware I don't have. But I guess there is definitely room for improvement here. Ideally we'd like to be able to change the vma->vm_page_prot within fault(). But we can
Just a quick comment on this: It's the repeated (per-pfn/pte) lookup of the PAT tables, which are dead slow. If you have a struct io_mapping then that can be done once, and then just blindly inserted. See remap_io_mapping in i915. -Daniel
Thanks, Daniel.
Indeed looks a lot like remap_pfn_range(), but usable at fault time?
Yeah we call it from our fault handler. It's essentially vm_insert_pfn except the pat track isn't there, but instead relies on the pat tracking io_mapping has done already. -Daniel
Am 03.09.19 um 23:05 schrieb Thomas Hellström (VMware):
On 9/3/19 10:51 PM, Dave Hansen wrote:
On 9/3/19 1:36 PM, Thomas Hellström (VMware) wrote:
So the question here should really be, can we determine already at mmap time whether backing memory will be unencrypted and adjust the *real* vma->vm_page_prot under the mmap_sem?
Possibly, but that requires populating the buffer with memory at mmap time rather than at first fault time.
I'm not connecting the dots.
vma->vm_page_prot is used to create a VMA's PTEs regardless of if they are created at mmap() or fault time. If we establish a good vma->vm_page_prot, can't we just use it forever for demand faults?
With SEV I think that we could possibly establish the encryption flags at vma creation time. But thinking of it, it would actually break with SME where buffer content can be moved between encrypted system memory and unencrypted graphics card PCI memory behind user-space's back. That would imply killing all user-space encrypted PTEs and at fault time set up new ones pointing to unencrypted PCI memory..
Well my problem is where do you see encrypted system memory here?
At least for AMD GPUs all memory accessed must be unencrypted and that counts for both system as well as PCI memory.
So I don't get why we can't assume always unencrypted and keep it like that.
Regards, Christian.
Hi, Christian,
On 9/4/19 9:33 AM, Koenig, Christian wrote:
Am 03.09.19 um 23:05 schrieb Thomas Hellström (VMware):
On 9/3/19 10:51 PM, Dave Hansen wrote:
On 9/3/19 1:36 PM, Thomas Hellström (VMware) wrote:
So the question here should really be, can we determine already at mmap time whether backing memory will be unencrypted and adjust the *real* vma->vm_page_prot under the mmap_sem?
Possibly, but that requires populating the buffer with memory at mmap time rather than at first fault time.
I'm not connecting the dots.
vma->vm_page_prot is used to create a VMA's PTEs regardless of if they are created at mmap() or fault time. If we establish a good vma->vm_page_prot, can't we just use it forever for demand faults?
With SEV I think that we could possibly establish the encryption flags at vma creation time. But thinking of it, it would actually break with SME where buffer content can be moved between encrypted system memory and unencrypted graphics card PCI memory behind user-space's back. That would imply killing all user-space encrypted PTEs and at fault time set up new ones pointing to unencrypted PCI memory..
Well my problem is where do you see encrypted system memory here?
At least for AMD GPUs all memory accessed must be unencrypted and that counts for both system as well as PCI memory.
We're talking SME now right?
The current SME setup is that if a device's DMA mask says it's capable of addressing the encryption bit, coherent memory will be encrypted. The memory controllers will decrypt for the device on the fly. Otherwise coherent memory will be decrypted.
So I don't get why we can't assume always unencrypted and keep it like that.
I see two reasons. First, it would break with a real device that signals it's capable of addressing the encryption bit.
Second I can imagine unaccelerated setups (something like vkms using prime feeding a VNC connection) where we actually want the TTM buffers encrypted to protect data.
But at least the latter reason is way far out in the future.
So for me I'm ok with that if that works for you?
/Thomas
Regards, Christian.
On 9/4/19 10:19 AM, Thomas Hellström (VMware) wrote:
Hi, Christian,
On 9/4/19 9:33 AM, Koenig, Christian wrote:
Am 03.09.19 um 23:05 schrieb Thomas Hellström (VMware):
On 9/3/19 10:51 PM, Dave Hansen wrote:
On 9/3/19 1:36 PM, Thomas Hellström (VMware) wrote:
So the question here should really be, can we determine already at mmap time whether backing memory will be unencrypted and adjust the *real* vma->vm_page_prot under the mmap_sem?
Possibly, but that requires populating the buffer with memory at mmap time rather than at first fault time.
I'm not connecting the dots.
vma->vm_page_prot is used to create a VMA's PTEs regardless of if they are created at mmap() or fault time. If we establish a good vma->vm_page_prot, can't we just use it forever for demand faults?
With SEV I think that we could possibly establish the encryption flags at vma creation time. But thinking of it, it would actually break with SME where buffer content can be moved between encrypted system memory and unencrypted graphics card PCI memory behind user-space's back. That would imply killing all user-space encrypted PTEs and at fault time set up new ones pointing to unencrypted PCI memory..
Well my problem is where do you see encrypted system memory here?
At least for AMD GPUs all memory accessed must be unencrypted and that counts for both system as well as PCI memory.
We're talking SME now right?
The current SME setup is that if a device's DMA mask says it's capable of addressing the encryption bit, coherent memory will be encrypted. The memory controllers will decrypt for the device on the fly. Otherwise coherent memory will be decrypted.
So I don't get why we can't assume always unencrypted and keep it like that.
I see two reasons. First, it would break with a real device that signals it's capable of addressing the encryption bit.
Second I can imagine unaccelerated setups (something like vkms using prime feeding a VNC connection) where we actually want the TTM buffers encrypted to protect data.
But at least the latter reason is way far out in the future.
So for me I'm ok with that if that works for you?
Hmm, BTW,
Are you sure the AMD GPUs use unencrypted system memory rather than relying on the memory controllers to decrypt?
In that case it seems strange that they get away with encrypted TTM PTEs, whereas vmwgfx don't...
/Thomas
/Thomas
Regards, Christian.
Am 04.09.19 um 10:19 schrieb Thomas Hellström (VMware):
Hi, Christian,
On 9/4/19 9:33 AM, Koenig, Christian wrote:
Am 03.09.19 um 23:05 schrieb Thomas Hellström (VMware):
On 9/3/19 10:51 PM, Dave Hansen wrote:
On 9/3/19 1:36 PM, Thomas Hellström (VMware) wrote:
So the question here should really be, can we determine already at mmap time whether backing memory will be unencrypted and adjust the *real* vma->vm_page_prot under the mmap_sem?
Possibly, but that requires populating the buffer with memory at mmap time rather than at first fault time.
I'm not connecting the dots.
vma->vm_page_prot is used to create a VMA's PTEs regardless of if they are created at mmap() or fault time. If we establish a good vma->vm_page_prot, can't we just use it forever for demand faults?
With SEV I think that we could possibly establish the encryption flags at vma creation time. But thinking of it, it would actually break with SME where buffer content can be moved between encrypted system memory and unencrypted graphics card PCI memory behind user-space's back. That would imply killing all user-space encrypted PTEs and at fault time set up new ones pointing to unencrypted PCI memory..
Well my problem is where do you see encrypted system memory here?
At least for AMD GPUs all memory accessed must be unencrypted and that counts for both system as well as PCI memory.
We're talking SME now right?
The current SME setup is that if a device's DMA mask says it's capable of addressing the encryption bit, coherent memory will be encrypted. The memory controllers will decrypt for the device on the fly. Otherwise coherent memory will be decrypted.
So I don't get why we can't assume always unencrypted and keep it like that.
I see two reasons. First, it would break with a real device that signals it's capable of addressing the encryption bit.
Why? Because we don't use dma_mmap_coherent()?
I've already talked with Christoph that we probably want to switch TTM over to using that instead to also get rid of the ttm_io_prot() hack.
Regards, Christian.
Second I can imagine unaccelerated setups (something like vkms using prime feeding a VNC connection) where we actually want the TTM buffers encrypted to protect data.
But at least the latter reason is way far out in the future.
So for me I'm ok with that if that works for you?
/Thomas
Regards, Christian.
On 9/4/19 1:10 PM, Koenig, Christian wrote:
Am 04.09.19 um 10:19 schrieb Thomas Hellström (VMware):
Hi, Christian,
On 9/4/19 9:33 AM, Koenig, Christian wrote:
Am 03.09.19 um 23:05 schrieb Thomas Hellström (VMware):
On 9/3/19 10:51 PM, Dave Hansen wrote:
On 9/3/19 1:36 PM, Thomas Hellström (VMware) wrote:
So the question here should really be, can we determine already at mmap time whether backing memory will be unencrypted and adjust the *real* vma->vm_page_prot under the mmap_sem?
Possibly, but that requires populating the buffer with memory at mmap time rather than at first fault time.
I'm not connecting the dots.
vma->vm_page_prot is used to create a VMA's PTEs regardless of if they are created at mmap() or fault time. If we establish a good vma->vm_page_prot, can't we just use it forever for demand faults?
With SEV I think that we could possibly establish the encryption flags at vma creation time. But thinking of it, it would actually break with SME where buffer content can be moved between encrypted system memory and unencrypted graphics card PCI memory behind user-space's back. That would imply killing all user-space encrypted PTEs and at fault time set up new ones pointing to unencrypted PCI memory..
Well my problem is where do you see encrypted system memory here?
At least for AMD GPUs all memory accessed must be unencrypted and that counts for both system as well as PCI memory.
We're talking SME now right?
The current SME setup is that if a device's DMA mask says it's capable of addressing the encryption bit, coherent memory will be encrypted. The memory controllers will decrypt for the device on the fly. Otherwise coherent memory will be decrypted.
So I don't get why we can't assume always unencrypted and keep it like that.
I see two reasons. First, it would break with a real device that signals it's capable of addressing the encryption bit.
Why? Because we don't use dma_mmap_coherent()?
Well, assuming always unencrypted would obviously break on a real device with encrypted coherent memory?
dma_mmap_coherent() would work from the encryption point of view (although I think it's currently buggy and will send out an RFC for what I believe is a fix for that).
I've already talked with Christoph that we probably want to switch TTM over to using that instead to also get rid of the ttm_io_prot() hack.
OK, would that mean us ditching other memory modes completely? And on-the-fly caching transitions? or is it just for the special case of cached coherent memory? Do we need to cache the coherent kernel mappings in TTM as well, for ttm_bo_kmap()?
/Thomas
Regards, Christian.
Second I can imagine unaccelerated setups (something like vkms using prime feeding a VNC connection) where we actually want the TTM buffers encrypted to protect data.
But at least the latter reason is way far out in the future.
So for me I'm ok with that if that works for you?
/Thomas
Regards, Christian.
On 9/4/19 2:35 PM, Thomas Hellström (VMware) wrote:
I've already talked with Christoph that we probably want to switch TTM over to using that instead to also get rid of the ttm_io_prot() hack.
OK, would that mean us ditching other memory modes completely? And on-the-fly caching transitions? or is it just for the special case of cached coherent memory? Do we need to cache the coherent kernel mappings in TTM as well, for ttm_bo_kmap()?
Reading this again, I wanted to point out that I'm not against this. Just curious.
/Thomas
From: Thomas Hellstrom thellstrom@vmware.com
The TTM dma pool allocates coherent pages for use with TTM. When forcing unencrypted DMA, such allocations become very expensive since the linear kernel map has to be changed to mark the pages decrypted. To avoid too many such allocations and frees, cache the decrypted pages even if they are in the normal cpu caching state, where otherwise the pool frees them immediately when unused.
Tested with vmwgfx on SEV-ES.
Cc: Dave Hansen dave.hansen@linux.intel.com Cc: Andy Lutomirski luto@kernel.org Cc: Peter Zijlstra peterz@infradead.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Ingo Molnar mingo@redhat.com Cc: Borislav Petkov bp@alien8.de Cc: "H. Peter Anvin" hpa@zytor.com Cc: Heiko Carstens heiko.carstens@de.ibm.com Cc: Christian Borntraeger borntraeger@de.ibm.com Cc: Tom Lendacky thomas.lendacky@amd.com Cc: Christian König christian.koenig@amd.com Signed-off-by: Thomas Hellstrom thellstrom@vmware.com --- drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c index 9b15df8ecd49..a3247f24e106 100644 --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c @@ -1000,7 +1000,7 @@ void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma, struct device *dev) struct dma_pool *pool; struct dma_page *d_page, *next; enum pool_type type; - bool is_cached = false; + bool immediate_free = false; unsigned count, i, npages = 0; unsigned long irq_flags;
@@ -1035,8 +1035,17 @@ void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma, struct device *dev) if (!pool) return;
- is_cached = (ttm_dma_find_pool(pool->dev, - ttm_to_type(ttm->page_flags, tt_cached)) == pool); + /* + * If memory is cached and sev encryption is not active, allocating + * and freeing coherent memory is relatively cheap, so we can free + * it immediately. If sev encryption is active, allocating coherent + * memory involves a call to set_memory_decrypted() which is very + * expensive, so cache coherent pages is sev is active. + */ + immediate_free = (ttm_dma_find_pool + (pool->dev, + ttm_to_type(ttm->page_flags, tt_cached)) == pool && + !force_dma_unencrypted(dev));
/* make sure pages array match list and count number of pages */ count = 0; @@ -1051,13 +1060,13 @@ void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma, struct device *dev) d_page->vaddr &= ~VADDR_FLAG_UPDATED_COUNT; }
- if (is_cached) + if (immediate_free) ttm_dma_page_put(pool, d_page); }
spin_lock_irqsave(&pool->lock, irq_flags); pool->npages_in_use -= count; - if (is_cached) { + if (immediate_free) { pool->nfrees += count; } else { pool->npages_free += count;
On Tue, Sep 03, 2019 at 03:15:00PM +0200, Thomas Hellström (VMware) wrote:
With SEV memory encryption and in some cases also with SME memory encryption, coherent memory is unencrypted. In those cases, TTM doesn't set up the correct page protection. Fix this by having the TTM coherent page allocator call into the platform code to determine whether coherent memory is encrypted or not, and modify the page protection if it is not.
v2:
- Use force_dma_unencrypted() rather than sev_active() to catch also the special SME encryption cases.
We should probably cc Christoph Hellwig on this ... better to hear his screams before merging than afterwards. As much as I don't support screaming maintainers, that seems the least bad option here. -Daniel
On 9/3/19 3:15 PM, Thomas Hellström (VMware) wrote:
With SEV memory encryption and in some cases also with SME memory encryption, coherent memory is unencrypted. In those cases, TTM doesn't set up the correct page protection. Fix this by having the TTM coherent page allocator call into the platform code to determine whether coherent memory is encrypted or not, and modify the page protection if it is not.
v2:
- Use force_dma_unencrypted() rather than sev_active() to catch also the special SME encryption cases.
So, this patchset is obviously withdrawn since
a) We shouldn't have TTM shortcut the dma API in this way. b) To reviewers it was pretty unclear why this was needed in the first place, and became even more unclear in the context of the TTM fault handler.
I've just send out an RFC patchset that basically does the same but in the context of dma_mmap_coherent() I hope this clears things up and we should hopefully be able to use a new dma API function from within the TTM fault handler.
Thanks,
Thomas
dri-devel@lists.freedesktop.org
-
Andy Lutomirski -
Andy Lutomirski -
Daniel Vetter -
Dave Hansen -
Koenig, Christian -
Thomas Hellström (VMware)