Yan Zhao yan.y.zhao@intel.com writes:
On Tue, Dec 12, 2023 at 12:46:22PM -0800, Sagi Shahar wrote:
From: Ackerley Tng ackerleytng@google.com
If guest memory is backed by restricted memfd
- UPM is being used, hence encrypted memory region has to be registered
- Can avoid making a copy of guest memory before getting TDX to initialize the memory region
Signed-off-by: Ackerley Tng ackerleytng@google.com Signed-off-by: Ryan Afranji afranji@google.com Signed-off-by: Sagi Shahar sagis@google.com
.../selftests/kvm/lib/x86_64/tdx/tdx_util.c | 41 +++++++++++++++---- 1 file changed, 32 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx_util.c b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx_util.c index 6b995c3f6153..063ff486fb86 100644 --- a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx_util.c +++ b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx_util.c @@ -192,6 +192,21 @@ static void tdx_td_finalizemr(struct kvm_vm *vm) tdx_ioctl(vm->fd, KVM_TDX_FINALIZE_VM, 0, NULL); } +/*
- Other ioctls
- */
+/**
- Register a memory region that may contain encrypted data in KVM.
- */
+static void register_encrypted_memory_region(
- struct kvm_vm *vm, struct userspace_mem_region *region)
+{
- vm_set_memory_attributes(vm, region->region.guest_phys_addr,
region->region.memory_size,KVM_MEMORY_ATTRIBUTE_PRIVATE);+}
/*
- TD creation/setup/finalization
*/ @@ -376,30 +391,38 @@ static void load_td_memory_region(struct kvm_vm *vm, if (!sparsebit_any_set(pages)) return;
- if (region->region.guest_memfd != -1)
register_encrypted_memory_region(vm, region);- sparsebit_for_each_set_range(pages, i, j) { const uint64_t size_to_load = (j - i + 1) * vm->page_size; const uint64_t offset = (i - lowest_page_in_region) * vm->page_size; const uint64_t hva = hva_base + offset; const uint64_t gpa = gpa_base + offset;
void *source_addr;
void *source_addr = (void *)hva;/* * KVM_TDX_INIT_MEM_REGION ioctl cannot encrypt memory in place, * hence we have to make a copy if there's only one backing * memory source */
source_addr = mmap(NULL, size_to_load, PROT_READ | PROT_WRITE,MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);TEST_ASSERT(source_addr,"Could not allocate memory for loading memory region");memcpy(source_addr, (void *)hva, size_to_load);
if (region->region.guest_memfd == -1) {source_addr = mmap(NULL, size_to_load, PROT_READ | PROT_WRITE,MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);TEST_ASSERT(source_addr,"Could not allocate memory for loading memory region");memcpy(source_addr, (void *)hva, size_to_load);memset((void *)hva, 0, size_to_load);}tdx_init_mem_region(vm, source_addr, gpa, size_to_load);
munmap(source_addr, size_to_load);
if (region->region.guest_memfd == -1)munmap(source_addr, size_to_load);For memslot 0, 1, 2, when guest_memfd != -1, is it possible to also munmap(mmap_start, mmap_size) after finish loading?
Thank you for your review!
Did you mean "possible" as in whether it is "correct" to do munmap() for the rest of the earlier memslots containing non-test-code?
It is correct because the munmap() just deallocates memory that was recently allocated in mmap() in this same change. The memory set up for the VM is not affected.
Hope that answers your question, and here's some further detail, hope this helps too:
load_td_memory_region() loads a memory region into a TD by calling the KVM_TDX_INIT_MEM_REGION ioctl, which copies (and encrypts) the data in an existing memory region into TD private memory for the TD to use.
In these selftests, we use the KVM selftest framework to set up a TD's memory as if it were a regular VM, and then use this function to copy+encrypt the memory that is already set up for the TD. This lets us re-use most of the code from the KVM selftest framework for TDs, which is extremely useful for setting up page tables, exception handlers, etc for the TD. These key pieces of memory setup are in the memslots with smaller indices, as you pointed out.
KVM_TDX_INIT_MEM_REGION ioctl uses the provided gpa and struct kvm to look up the destination address, and uses source_addr as the source address for the copying.
If we are not using guest_memfd (region->region.guest_memfd == -1), then we need to make the source and destination address different by copying the contents at the source address somewhere else for the call to tdx_init_mem_region(). That is what the mmap() is doing. This temporary buffer then needs to be freed, hence the munmap(). Without this copying, the destination address for the ioctl's copy would be the same as the source address, since those very same pages are provided in the memslot for this memory region.
If we are using guest_memfd, then the destination address for the ioctl's copy will be taken from the guest_memfd, which is already different from the source address, hence we can skip the copying.
} -- 2.43.0.472.g3155946c3a-goog