Re: [PATCH 1/1] drm/ttm: Set memory as decrypted for ttm framebuffer mappings

From: Jiandi An
Date: Wed Aug 22 2018 - 17:12:58 EST



On 08/22/2018 02:09 PM, Christian KÃnig wrote:
> Am 22.08.2018 um 20:57 schrieb Jiandi An:
>> Framebuffer memory needs to be accessed decrypted. Ensure the
>> memory encryption mask is not set for the ttm framebuffer mappings.
>
> NAK, the memory not only needs to be decrypted while CPU accessed but all the time.
>
> ttm_page_alloc.c and ttm_page_alloc_dma.c should already take care of that while mapping the pages.
>
> Regards,
> Christian.
>

The path where the issue comes in is booting guest with AMD SEV enabled while using virtio-vga device.
The ttm->pages doesn't go through ttm_populate_and_map_pages().

In the kernel path, the virtio_gpu driver calls virtio_gpu_alloc_object() and goes down to ttm to
allocate pages in ttm_pool_populate(). Driver in guest then does virtio_gpu_vmap_fb() which goes
down to ttm_bo_kmap_ttm() and does a vmap() for GVA to GPA for those pages. If SEV is enabled,
decryption should be set in this GVA to GPA mapping. Guest then sends object attach command to host
via virtio_gpu_object_attach() which stuffs the pages' physical addresses (guest physical address)
in a scatter list and send them to host QEMU. Host QEMU maps those guest pages GPA to HPA and access
via HVA while guest write stuff in those pages via GVA.

virtio_gpu_alloc_object()
virtio_gpu_object_create()
sturct virtio_gpu_object bo kzalloc()
ttm_bo_init()
...
ttm_tt_create()
bo->ttm = bdev->driver->ttm_tt_create()
virtio_gpu_ttm_tt_create()
...
ttm_tt_populate()
ttm_pool_populate()
ttm_get_pages(ttm->pages, ttm->num_pages)

virtio_gpu_vmap_fb()
virtio_gpu_object_kmap(obj, NULL)
ttm_bo_kmap
ttm_mem_io_reserve()
ttm_bo_kmap_ttm()
vmap()
struct virtio_gpu_object bo->vmap = ttm_kmap_obj_virtual(&bo->kmap, &is_iomem);


virtio_gpu_object_attach() <<== Sending attach backing command
virtio_gpu_object_get_sg_table()
if (bo->tbo.ttm->state == tt_unpopulated)
virtio_gpu_object bo.ttm->bdev->driver->ttm_tt_populate(bo->tbo.ttm, &ctx);
bo->pages = kmalloc(sizeof(struct sg_table), GFP_KERNEL);
sg_alloc_table_from_pages(bo->tbo.ttm->pages) //Getfrom ttm->pages and put in sg
__sg_alloc_table_from_pages()


There is a separate userspace path for example when displace manager kicks in,
virtio_gpu_alloc_object() followed by virtio_gpu_object_attach() are called through
the ioctl virtio_gpu_resource_create_ioctl(). The mapping of the pages created in this
page are handled in the do_page_fault() path in ttm_bo_vm_ops's ttm_bo_vm_fault() handler.

do_page_fault()
handle_mm_fault()
__do_page_fault()
ttm_bo_vm_fault()
ttm_bo_reserve()
__ttm_bo_reserve()

ttm_mem_io_reserve_vm()
ttm_mem_io_reserve()
bdev->driver->io_mem_reserve()
virtio_gpu_ttm_io_mem_reserve()
mem->bus.is_iomem = false
mem->mem_type == TTM_PL_TT


I originally handled setting pages decrypted for the kernel path for GVA to GPA mapping in guest
in virtio_gpu_vmap_fb() at the virtio_gpu driver layer. But for the user space path
virtio_gpu_vmap_fb() is not called. Mapping is handled in the page_fault handler. So I moved it
to the ttm layer.

What layer do you recommend as more appropriate to handle setting memory decrypted for GVA to GPA
mapping for thos ttm->pages?

Thanks
Jiandi

>>
>> Signed-off-by: Jiandi An <jiandi.an@xxxxxxx>
>> ---
>> Â drivers/gpu/drm/ttm/ttm_bo_util.c | 12 +++++++++++-
>> Â drivers/gpu/drm/ttm/ttm_bo_vm.cÂÂ |Â 6 ++++--
>> Â 2 files changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
>> index 046a6dda690a..b3f5d26f571e 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
>> @@ -29,6 +29,7 @@
>> ÂÂ * Authors: Thomas Hellstrom <thellstrom-at-vmware-dot-com>
>> ÂÂ */
>> Â +#include <asm/set_memory.h>
>> Â #include <drm/ttm/ttm_bo_driver.h>
>> Â #include <drm/ttm/ttm_placement.h>
>> Â #include <drm/drm_vma_manager.h>
>> @@ -639,7 +640,11 @@ int ttm_bo_kmap(struct ttm_buffer_object *bo,
>> ÂÂÂÂÂ if (ret)
>> ÂÂÂÂÂÂÂÂÂ return ret;
>> ÂÂÂÂÂ if (!bo->mem.bus.is_iomem) {
>> -ÂÂÂÂÂÂÂ return ttm_bo_kmap_ttm(bo, start_page, num_pages, map);
>> +ÂÂÂÂÂÂÂ ret = ttm_bo_kmap_ttm(bo, start_page, num_pages, map);
>> +ÂÂÂÂÂÂÂ if (!ret && sev_active())
>> +ÂÂÂÂÂÂÂÂÂÂÂ set_memory_decrypted((unsigned long) map->virtual,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ num_pages);
>> +ÂÂÂÂÂÂÂ return ret;
>> ÂÂÂÂÂ } else {
>> ÂÂÂÂÂÂÂÂÂ offset = start_page << PAGE_SHIFT;
>> ÂÂÂÂÂÂÂÂÂ size = num_pages << PAGE_SHIFT;
>> @@ -661,9 +666,14 @@ void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map)
>> ÂÂÂÂÂÂÂÂÂ iounmap(map->virtual);
>> ÂÂÂÂÂÂÂÂÂ break;
>> ÂÂÂÂÂ case ttm_bo_map_vmap:
>> +ÂÂÂÂÂÂÂ if (sev_active())
>> +ÂÂÂÂÂÂÂÂÂÂÂ set_memory_encrypted((unsigned long) map->virtual,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ bo->num_pages);
>> ÂÂÂÂÂÂÂÂÂ vunmap(map->virtual);
>> ÂÂÂÂÂÂÂÂÂ break;
>> ÂÂÂÂÂ case ttm_bo_map_kmap:
>> +ÂÂÂÂÂÂÂ if (sev_active())
>> +ÂÂÂÂÂÂÂÂÂÂÂ set_memory_encrypted((unsigned long) map->virtual, 1);
>> ÂÂÂÂÂÂÂÂÂ kunmap(map->page);
>> ÂÂÂÂÂÂÂÂÂ break;
>> ÂÂÂÂÂ case ttm_bo_map_premapped:
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
>> index 6fe91c1b692d..211d3549fd9f 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
>> @@ -249,10 +249,12 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf)
>> ÂÂÂÂÂÂ * Speculatively prefault a number of pages. Only error on
>> ÂÂÂÂÂÂ * first page.
>> ÂÂÂÂÂÂ */
>> +
>> +ÂÂÂ /* Mark framebuffer pages decrypted */
>> +ÂÂÂ cvma.vm_page_prot = pgprot_decrypted(cvma.vm_page_prot);
>> +
>> ÂÂÂÂÂ for (i = 0; i < TTM_BO_VM_NUM_PREFAULT; ++i) {
>> ÂÂÂÂÂÂÂÂÂ if (bo->mem.bus.is_iomem) {
>> -ÂÂÂÂÂÂÂÂÂÂÂ /* Iomem should not be marked encrypted */
>> -ÂÂÂÂÂÂÂÂÂÂÂ cvma.vm_page_prot = pgprot_decrypted(cvma.vm_page_prot);
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂ pfn = ttm_bo_io_mem_pfn(bo, page_offset);
>> ÂÂÂÂÂÂÂÂÂ } else {
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂ page = ttm->pages[page_offset];
>