Re: [RFC PATCH v3 54/59] KVM: X86: Introduce initial_tsc_khz in struct kvm_arch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux