Re: [PATCH v9 05/18] x86/virt/tdx: Add SEAMCALL infrastructure
From: Huang, Kai
Date: Tue Feb 14 2023 - 17:18:05 EST
On Tue, 2023-02-14 at 09:27 -0800, Dave Hansen wrote:
> On 2/14/23 00:57, Huang, Kai wrote:
> > Consider this case:
> >
> > 1) KVM does VMXON for all online cpus (a VM created)
> > 2) Another kernel component is calling tdx_enable()
> > 3) KVM does VMXOFF for all online cpus (last VM is destroyed)
>
> Doctor, it hurts when I...
>
> Then let's just call tdx_enable() from other kernel components.
>
> Kai, I'm worried that this is, again, making things more complicated
> than they have to be.
The handling of #UD/#GP itself only takes ~10 LoC. All those complicated logic
comes from we depend on caller of TDX to ensure VMXON has been done.
AFAICT we have below options:
1) Don't support VMXON in the core-kernel, then
1.a Handle #UD/#GP in assembly as shown in this patch; Or
1.b Disable interrupt from CR4.VMXE check until SEAMCALL is done in
seamcall().
2) Let's support VMXON in the core-kernel (by moving VMXON from KVM to the core-
x86), then we get rid of all above. We explicitly do VMXON (if haven't done)
inside tdx_enable() to make sure SEAMCALL doesn't cause #UD. No #UD/#GP
handling is needed in assembly. No interrupt disable in seamcall().
(well #GP can theoretically still happen if BIOS is buggy, we can keep assembly
code change if it's better -- just ~10 LoC).
Supporting VMXON in the core-kernel also has other advantages:
1) We can get rid of the logic to always try to do LP.INIT for all online cpus.
LP.INIT can just be done: a) during module initialization; b) in TDX CPU hotplug
callback.
2) The TDX CPU hotplug callback can just do VMXON and LP.INIT. No CR4.VMXE
check is needed. And it can be put before KVM (all TDX users)' hotplug
callback.
The downside of supporting VMXON to the core-kernel:
1) Need patch(es) to change KVM, so those patches need to be reviewed by KVM
maintainers.
2) No other cons.
Logically, supporting VMXON in the core-kernel makes things simple. And long-
termly, I _think_ we will need it to support future TDX features.
The effort to support VMXON in the core-kernel would be ~300 LOC. I can already
utilize some old patches, but need to polish those patches and do some test.
What's your thinking?