Re: [RFC PATCH 03/20] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs

[Chao Gao <chao.gao@intel.com>]

>Regarding question #3 from the cover letter:
>
>  3. Two helpers, cpu_vmcs_load() and cpu_vmcs_store(), are added in patch 3
>     to save and restore the current VMCS. KVM has a variant of cpu_vmcs_load(),
>     i.e., vmcs_load(). Extracting KVM's version would cause a lot of code
>     churn, and I don't think that can be justified for reducing ~16 LoC
>     duplication. Please let me know if you disagree.
>
>I'm fine with the SEAMLDR code having its own code, because I agree it's not worth
>extracting KVM's macro maze just to get at VMPTRLD.  But I'm not fine with creating
>a new, inferior framework.  So if we elect to leave KVM alone for the time being,
>I would prefer to simply open code VMPTRST and VMPTRLD in seamldr.c, e.g.

Agreed. And the code below makes perfect sense to me, so I will incorporate it
into my next version.

Thanks for your prompt feedback.

>
>static inline int seamldr_call(u64 fn, struct tdx_module_args *args)
>{
>	u64 vmcs;
>	int ret;
>
>	if (!is_seamldr_call(fn))
>		return -EINVAL;
>
>	/*
>	 * SEAMRET from P-SEAMLDR invalidates the current VMCS.  Save/restore
>	 * the VMCS across P-SEAMLDR SEAMCALLs to avoid clobbering KVM state.
>	 * Disable interrupts as KVM is allowed to do VMREAD/VMWRITE in IRQ
>	 * context (but not NMI context).
>	 */
>	guard(irqsave)();
>
>	asm goto("1: vmptrst %0\n\t"
>		 _ASM_EXTABLE(1b, %l[error])
>		 : "=m" (&vmcs) : "cc" : error);
>
>	ret = seamldr_prerr(fn, args);
>
>	/*
>	 * Restore the current VMCS pointer.  VMPTSTR "returns" all ones if the
>	 * current VMCS is invalid.
>	 */
>	if (vmcs != -1ULL) {
>		asm goto("1: vmptrld %0\n\t"
>			 "jna %l[error]\n\t"
>			 _ASM_EXTABLE(1b, %l[error])
>			 : : "m" (&vmcs) : "cc" : error);
>	}
>
>	return ret;
>
>error:
>	WARN_ONCE(1, "Failed to save/restore the current VMCS");
>	return -EIO;
>}