Xiaoyao Li <[email protected]> writes:
>
> [...snip...]
>
>> +static void tdx_load_private_memory(struct kvm_vm *vm)
>> +{
>> + struct userspace_mem_region *region;
>> + int ctr;
>> +
>> + hash_for_each(vm->regions.slot_hash, ctr, region, slot_node) {
>> + const struct sparsebit *protected_pages =
>> region->protected_phy_pages;
>> + const gpa_t gpa_base = region->region.guest_phys_addr;
>> + const u64 hva_base = region->region.userspace_addr;
>> + const sparsebit_idx_t lowest_page_in_region = gpa_base >>
>> vm->page_shift;
>> + void *source_pages = NULL;
>> + sparsebit_idx_t i, j;
>> +
>> + if (!sparsebit_any_set(protected_pages))
>
> sparebit_any_set() doens't check if the input is NULL. So we need to
> check it here.
>
>> + continue;
>> +
>> + TEST_ASSERT(region->region.guest_memfd != -1,
>> + "TD private memory must be backed by guest_memfd");
>> +
>> + sparsebit_for_each_set_range(protected_pages, i, j) {
>> + const u64 size_to_load = (j - i + 1) * vm->page_size;
>> + const u64 offset =
>> + (i - lowest_page_in_region) * vm->page_size;
>> + const u64 hva = hva_base + offset;
>> + const u64 gpa = gpa_base + offset;
>> +
>> + if (!kvm_has_gmem_attributes)
>> + source_pages = (void *)hva;
>> +
>
>> + vm_mem_set_private(vm, gpa, size_to_load);
>
> So vm_mem_set_private() has to be called at this late stage when run
> with in-place gmem. But for non in-place gmem, we can actually call
> vm_mem_set_private() in __vm_phy_pages_alloc().
>
> Calling vm_mem_set_private() here instead of in __vm_phy_pages_alloc()
> looks like a trick to me.
I thought this is fine because __vm_phy_pages_alloc() seems to be a
rather low-level function, where the responsibility of the function is
just to allocate (for find some physical pages). Calling
vm_mem_set_private() in there seems to be doing too much.
> That is, we cannot set the page as private
> when allocating a guest physical page as protected because if doing so,
> we cannot write the initial content to it.
>
I think calling it here isn't a trick, it's a good way to reuse all the
existing code that builds up the guest image in place. In-place
conversion allows you to set stuff up in shared memory and then convert
everything when you're done and also populate the memory.
> This is the topic about how to implement the infras for in-place gmem,
> not the issue of this series. Let me go read the selftest patches of
> gmem in-place series and we can discuss there.
>
>> + tdx_init_mem_region(vm, source_pages, gpa,
>> size_to_load);
>> + }
>> + }
>> +}
>> +
>> +void tdx_vm_finalize(struct kvm_vm *vm)
>> +{
>> + tdx_load_private_memory(vm);
>> + tdx_vm_ioctl(vm, KVM_TDX_FINALIZE_VM, 0, NULL);
>> +}
>>
