On 11/26/21 00:26, Thomas Gleixner wrote:
Paolo,
On Thu, Nov 25 2021 at 23:13, Paolo Bonzini wrote:
On 11/25/21 22:05, Thomas Gleixner wrote:
If there are some patches that are actually independent, go ahead and
submit them early. But more practically, for the bulk of the changes
what you need to do is:
1) incorporate into patch 55 a version of tdx.c that essentially does
KVM_BUG_ON or WARN_ON for each function. Temporarily keep the same huge
patch that adds the remaining 2000 lines of tdx.c
There is absolutely no reason to populate anything upfront at all.
Why?
Simply because that whole muck cannot work until all pieces are in place.
It can, sort of. It cannot run a complete guest, but it could in
principle run a toy guest with a custom userspace, like the ones that
make up tools/testing/selftests/kvm. (Note that KVM_BUG_ON marks the VM
as bugged but doesn't hang the whole machine).
AMD was working on infrastructure to do this for SEV and SEV-ES.
So why would you provide:
handle_A(...) { BUG(); }
..
handle_Z(...) { BUG(); }
with all the invocations in the common emulation dispatcher:
handle_stuff(reason)
{
switch(reason) {
case A: return handle_A();
...
case Z: return handle_Z();
default: BUG();
}
};
If it's a switch statement that's good, but the common case is more
similar to this:
vmx_handle_A(...) { ... }
+tdx_handle_A(...) { ... }
+
+vt_handle_A(...) {
+ if (is_tdx(vcpu->kvm))
+ tdx_handle_A(...);
+ else
+ vmx_handle_A(...);
+}
...
- .handle_A = vmx_handle_A,
+ .handle_A = vt_handle_A,
And you could indeed do it in a single patch, without adding the stub
tdx_handle_A upfront. But you would have code that is broken and who
knows what the effects would be of calling vmx_handle_A on a TDX virtual
machine. It could be an error, or it could be memory corruption.
In both scenarious you cannot boot a TDX guest until you reached $Z, but
in the gradual one you and the reviewers have the pleasure of looking at
one thing at a time.
I think both of them are gradual. Not having the stubs might be a
little more gradual, but it is a very minor issue for the reviewability
of the whole thing.
Paolo
