On Mon, Apr 29, 2024, Kai Huang wrote: > On Thu, 2024-04-25 at 15:43 -0700, Sean Christopherson wrote: > > > And the odd is currently the common SEAMCALL functions, a.k.a, > > > __seamcall() and seamcall() (the latter is a mocro actually), both return > > > u64, so if we want to have such CR4.VMX check code in the common code, we > > > need to invent a new error code for it. > > > > Oh, I wasn't thinking that we'd check CR4.VMXE before *every* SEAMCALL, just > > before the TDH.SYS.LP.INIT call, i.e. before the one that is most likely to fail > > due to a software bug that results in the CPU not doing VMXON before enabling > > TDX. > > > > Again, my intent is to add a simple, cheap, and targeted sanity check to help > > deal with potential failures in code that historically has been less than rock > > solid, and in function that has a big fat assumption that the caller has done > > VMXON on the CPU. > > I see. > > (To be fair, personally I don't recall that we ever had any bug due to > "cpu not in post-VMXON before SEAMCALL", but maybe it's just me. :-).) > > But if tdx_enable() doesn't call tdx_cpu_enable() internally, then we will > have two functions need to handle. Why? I assume there will be exactly one caller of TDH.SYS.LP.INIT. > For tdx_enable(), given it's still good idea to disable CPU hotplug around > it, we can still do some check for all online cpus at the beginning, like: > > on_each_cpu(check_cr4_vmx(), &err, 1); If it gets to that point, just omit the check. I really think you're making much ado about nothing. My suggestion is essentially "throw in a CR4.VMXE check before TDH.SYS.LP.INIT if it's easy". If it's not easy for some reason, then don't do it. > Btw, please also see my last reply to Chao why I don't like calling > tdx_cpu_enable() inside tdx_enable(): > > https://lore.kernel.org/lkml/1fd17c931d5c2effcf1105b63deac8e3fb1664bc.camel@xxxxxxxxx/ > > That being said, I can try to add additional patch(es) to do CR4.VMX check > if you want, but personally I found hard to have a strong justification to > do so. >
