On Thu, 9 Apr 2020, Tom Lendacky wrote: > > When a device required unencrypted memory and the context does not allow > > required => requires >
Fixed, thanks. > > blocking, memory must be returned from the atomic coherent pools. > > > > This avoids the remap when CONFIG_DMA_DIRECT_REMAP is not enabled and the > > config only requires CONFIG_DMA_COHERENT_POOL. This will be used for > > CONFIG_AMD_MEM_ENCRYPT in a subsequent patch. > > > > Keep all memory in these pools unencrypted. > > > > Signed-off-by: David Rientjes <rient...@google.com> > > --- > > kernel/dma/direct.c | 16 ++++++++++++++++ > > kernel/dma/pool.c | 15 +++++++++++++-- > > 2 files changed, 29 insertions(+), 2 deletions(-) > > > > diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c > > index 70800ca64f13..44165263c185 100644 > > --- a/kernel/dma/direct.c > > +++ b/kernel/dma/direct.c > > @@ -124,6 +124,18 @@ void *dma_direct_alloc_pages(struct device *dev, size_t > > size, > > struct page *page; > > void *ret; > > + /* > > + * Unencrypted memory must come directly from DMA atomic pools if > > + * blocking is not allowed. > > + */ > > + if (IS_ENABLED(CONFIG_DMA_COHERENT_POOL) && > > + force_dma_unencrypted(dev) && !gfpflags_allow_blocking(gfp)) { > > + ret = dma_alloc_from_pool(dev, PAGE_ALIGN(size), &page, gfp); > > + if (!ret) > > + return NULL; > > + goto done; > > + } > > + > > if (IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) && > > dma_alloc_need_uncached(dev, attrs) && > > !gfpflags_allow_blocking(gfp)) { > > @@ -203,6 +215,10 @@ void dma_direct_free_pages(struct device *dev, size_t > > size, void *cpu_addr, > > { > > unsigned int page_order = get_order(size); > > + if (IS_ENABLED(CONFIG_DMA_COHERENT_POOL) && > > + dma_free_from_pool(dev, cpu_addr, PAGE_ALIGN(size))) > > + return; > > + > > if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) && > > !force_dma_unencrypted(dev)) { > > /* cpu_addr is a struct page cookie, not a kernel address */ > > diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c > > index e14c5a2da734..6685ab89cfa7 100644 > > --- a/kernel/dma/pool.c > > +++ b/kernel/dma/pool.c > > @@ -9,6 +9,7 @@ > > #include <linux/dma-contiguous.h> > > #include <linux/init.h> > > #include <linux/genalloc.h> > > +#include <linux/set_memory.h> > > #include <linux/slab.h> > > #include <linux/vmalloc.h> > > #include <linux/workqueue.h> > > @@ -55,12 +56,20 @@ static int atomic_pool_expand(struct gen_pool *pool, > > size_t pool_size, > > arch_dma_prep_coherent(page, pool_size); > > +#ifdef CONFIG_DMA_DIRECT_REMAP > > addr = dma_common_contiguous_remap(page, pool_size, > > pgprot_dmacoherent(PAGE_KERNEL), > > __builtin_return_address(0)); > > if (!addr) > > goto free_page; > > - > > +#else > > + addr = page_to_virt(page); > > +#endif > > + /* > > + * Memory in the atomic DMA pools must be unencrypted, the pools do > > not > > + * shrink so no re-encryption occurs in dma_direct_free_pages(). > > + */ > > + set_memory_decrypted((unsigned long)page_to_virt(page), 1 << order); > > ret = gen_pool_add_virt(pool, (unsigned long)addr, page_to_phys(page), > > pool_size, NUMA_NO_NODE); > > if (ret) > > @@ -69,8 +78,10 @@ static int atomic_pool_expand(struct gen_pool *pool, > > size_t pool_size, > > return 0; > > remove_mapping: > > +#ifdef CONFIG_DMA_DIRECT_REMAP > > dma_common_free_remap(addr, pool_size); > > You're about to free the memory, but you've called set_memory_decrypted() > against it, so you need to do a set_memory_encrypted() to bring it back to a > state ready for allocation again. > Ah, good catch, thanks. I notice that I should also be checking the return value of set_memory_decrypted() because pages added to the coherent pools *must* be unencrypted. If it fails, we fail the expansion. And do the same thing for set_memory_encrypted(), which would be a bizarre situation (decrypt succeeded, encrypt failed), by simply leaking the page. diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c --- a/kernel/dma/pool.c +++ b/kernel/dma/pool.c @@ -7,6 +7,7 @@ #include <linux/dma-contiguous.h> #include <linux/init.h> #include <linux/genalloc.h> +#include <linux/set_memory.h> #include <linux/slab.h> #include <linux/vmalloc.h> #include <linux/workqueue.h> @@ -53,22 +54,42 @@ static int atomic_pool_expand(struct gen_pool *pool, size_t pool_size, arch_dma_prep_coherent(page, pool_size); +#ifdef CONFIG_DMA_DIRECT_REMAP addr = dma_common_contiguous_remap(page, pool_size, pgprot_dmacoherent(PAGE_KERNEL), __builtin_return_address(0)); if (!addr) goto free_page; - +#else + addr = page_to_virt(page); +#endif + /* + * Memory in the atomic DMA pools must be unencrypted, the pools do not + * shrink so no re-encryption occurs in dma_direct_free_pages(). + */ + ret = set_memory_decrypted((unsigned long)page_to_virt(page), + 1 << order); + if (ret) + goto remove_mapping; ret = gen_pool_add_virt(pool, (unsigned long)addr, page_to_phys(page), pool_size, NUMA_NO_NODE); if (ret) - goto remove_mapping; + goto encrypt_mapping; return 0; +encrypt_mapping: + ret = set_memory_encrypted((unsigned long)page_to_virt(page), + 1 << order); + if (WARN_ON_ONCE(ret)) { + /* Decrypt succeeded but encrypt failed, purposely leak */ + goto out; + } remove_mapping: +#ifdef CONFIG_DMA_DIRECT_REMAP dma_common_free_remap(addr, pool_size); -free_page: +#endif +free_page: __maybe_unused if (!dma_release_from_contiguous(NULL, page, 1 << order)) __free_pages(page, order); out: _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu