On Mon, Jun 15, 2026, Ackerley Tng wrote:
> Sean Christopherson <[email protected]> writes:
>
> > On Fri, Jun 05, 2026, Ackerley Tng wrote:
> >> Sean Christopherson <[email protected]> writes:
> >> Many tests use vm_userspace_mem_region_add(), CoCo tests that require
> >> finalizing shouldn't be disallowed that option.
> >
> > What does that have to do with finalizing the VM?
>
> I could add more memslots after finalizing the VM, but then I'd have to
> make the guest accept more memory. Hence, I'd rather set up all the
> memslots and then finalize the VM.
Why? It's not like the performance of this code matters terribly.
Regardless, nothing *requires* a test to go down that route. As I said before,
my goal with the selftests infrastructure is to effectively optimize the entire
experience, i.e. to provide default behavior that works well for the majority of
tests. Attempting to provide default behavior that perfectly fits *every* test
is simply impossible.
> sev_smoke_test currently has a separate vm_sev_launch(), which is the
> equivalent of tdx_init_mem_region(), and that's called in the tests
> themselves.
>
> sev_smoke_test also uses vcpu_args_set() after creating the VM with
> vm_create_shape_with_one_vcpu(). Would that work if vm_sev_launch() got
> moved into kvm_arch_vm_finalize_vcpus()?
No, because vCPU state would be encrypted at that point. Moving vm_sev_launch()
would also break test_sev_dbg(), which sets a global buffer to all 0xaa prior to
encrypting that data.
> >> >> It's also possible to have some kvm_vm_finalize() call that can be
> >> >> explicitly and manually invoked from selftests just for CoCo selftests.
> >> >
> >> > Why bother? It's obviously possible to all kvm_arch_vm_finalize_vcpus()
> >> > directly.
> >>
> >> Works for me to call directly. Do you mean kvm_arch_vm_finalize_vcpus()
> >> is the right function where the TD is finalized?
> >>
> >> For tests that need to do more setup after creating a vm, is the only
> >> way out to call __vm_create() then vm_vcpu_add() to avoid premature
> >> finalization in __vm_create_with_vcpus() when
> >> kvm_arch_vm_finalize_vcpus() is called?
> >
> > Depends on what you're doing. Sometimes, the answer will be yes. That's
> > why
> > there are "low level" APIs, so that some tests can do fancy things, while
> > most
> > tests can leave the details to the infrastructure.
> >
> > If there's a recurring problem, or we anticipate one, then we can and should
> > figure out how to minimize the pain so that tests don't have to deal with
> > the
> > same boilerplate issues over and over. Hence the __shared idea.
>
> I still think kvm_arch_vm_finalize_vcpus() is an odd place to be
> finalizing the VM.
That's literally why the function exists though. The one and only existing
implementation (on arm64) uses it to initialize the vGIC.
void kvm_arch_vm_finalize_vcpus(struct kvm_vm *vm)
{
if (vm->arch.has_gic)
__vgic_v3_init(vm->arch.gic_fd);
}
That's *very* similar to the proposed TDX usage, where some per-VM asset(s) can
be initialized/frozen only after all vCPUs have been added. In other words, the
name is describing where in the VM creation/setup flow the hook is called, and
perhaps more importantly, the impact of the call: vCPUs are finalized (obviously
with a different definition of "finalized" based on the VM properties).
> I would prefer to not have to explicitly call some function like
> kvm_arch_vm_finalize() (no vcpu in the name), but a common arch function
> calling vm_sev_launch() and tdx_vm_finalize() is what I can think of
> for test setup flexibility, without too much magic.
We can't have our cake and eat it too. Either we launch/finalize SEV/TDX VMs as
part of the common VM creation flows (as proposed for TDX), or we force tests to
manually launch/finalize the VM after additional setup. The only way to have it
both ways is to utilize crazy macro shenanigans to execute arbitrary code
between
creating the VM and launching/finalizing the VM, and even I would balk at that.
I honestly don't see any reason to try to figure out which of the two approaches
is optimal at this time. Whatever decision we make isn't set in stone, and in
fact should be relative easy to change. And without being able to see all the
TDX/SEV tests that are waiting in the wings, it's more or less impossible to
make
an informed decision.
I definitely want to have SEV and TDX use the same core approach in the end, but
I don't think we should force the issue right now, because the cost of reworking
the SEV and/or TDX infrastructure when there are only a bare handful of tests is
so low. It will cost more to try to predict the future of a 50/50 outcome than
it will to make a completely wild guess between the two options and rework
things
if we guess wrong.
> For now, I can't think of many uses of __shared. ucall shared memory is
> allocated dynamically, and we can also make it shared cleanly within
> ucall code.
Uh, every selftest that uses global variables to communicate between guest and
host?
> The global variables (sync_global_to_guest()) will appear in the guest
> as long as sync_global_to_guest() is called before
> kvm_arch_vm_finalize(), which I think makes sense to people writing
> tests for CoCo.
Yes, but that's completely orthogonal to all of this.
> So 2 questions to push this along:
>
> 1. What do you think of a kvm_arch_vm_finalize() that calls
> vm_sev_launch() and tdx_vm_finalize()? My key issue is that
> kvm_arch_vm_finalize_*vcpus*() seems to be for finalizing vCPUs
> rather than the whole VM.
Key word "seems". I'm pretty sure Oliver picked kvm_arch_vm_finalize_vcpus() as
the name in commit 8911c7dbc607 ("KVM: arm64: selftests: Create a VGICv3 for
'default' VMs") for the same reasons I think it's a good fit for coco VMs: like
finalizing TDX VMs, initializing the vGIC effectively finalizes vCPUs.
We could rename it to kvm_arch_vm_finalize(), but that won't change the fact
that
we'll need to decide between automagic vs. manual finalization, and it
definitely
should be a separate discussion.
> 3. Would you like __shared implemented together with this series, as a
> prerequisite, or later?
Only if __shared is a hard requirement for base TDX support, which I assume is
not the case.
