Re: [RFC PATCH v3 53/59] KVM: x86: Add a helper function to restore 4 host MSRs on exit to user space

[Chao Gao]

On Mon, Nov 29, 2021 at 03:08:39PM +0800, Lai Jiangshan wrote:
>On Mon, Nov 29, 2021 at 2:00 AM Chao Gao <chao.gao@xxxxxxxxx> wrote:
>>
>> On Thu, Nov 25, 2021 at 09:34:59PM +0100, Thomas Gleixner wrote:
>> >On Wed, Nov 24 2021 at 16:20, isaku yamahata wrote:
>> >> From: Chao Gao <chao.gao@xxxxxxxxx>
>> >
>> >> $Subject: KVM: x86: Add a helper function to restore 4 host MSRs on exit to user space
>> >
>> >Which user space are you talking about? This subject line is misleading
>>
>> Host Ring3.
>>
>> >at best. The unconditional reset is happening when a TDX VM exits
>> >because the SEAM firmware enforces this to prevent unformation leaks.
>>
>> Yes.
>>
>> >
>> >It also does not matter whether this are four or ten MSR.
>>
>> Indeed, the number of MSRs doesn't matter.
>>
>> >Fact is that
>> >the SEAM firmware is buggy because it does not save/restore those MSRs.
>>
>> It is done deliberately. It gives host a chance to do "lazy" restoration.
>> "lazy" means don't save/restore them on each TD entry/exit but defer
>> restoration to when it is neccesary e.g., when vCPU is scheduled out or
>> when kernel is about to return to Ring3.
>>
>> The TDX module unconditionally reset 4 host MSRs (MSR_SYSCALL_MASK,
>> MSR_START, MSR_LSTAR, MSR_TSC_AUX) to architectural INIT state on exit from
>> TDX VM to KVM.
>
>I did not find the information in intel-tdx-module-1eas.pdf nor
>intel-tdx-cpu-architectural-specification.pdf.
>
>Maybe the version I downloaded is outdated.

Hi Jiangshan,

Please refer to Table 22.162 MSRs that may be Modified by TDH.VP.ENTER,
in section 22.2.40 TDH.VP.ENTER leaf.

>
>I guess that the "lazy" restoration mode is not a valid optimization.
>The SEAM module should restore it to the original value when it tries
>to reset it to architectural INIT state on exit from TDX VM to KVM
>since the SEAM module also does it via wrmsr (correct me if not).

Correct.

>
>If the SEAM module doesn't know "the original value" of the these
>MSRs, it would be mere an optimization to save an rdmsr in SEAM.

Yes. Just a rdmsr is saved in TDX module at the cost of host's
restoring a MSR. If restoration (wrmsr) can be done in a lazy fashion
or even the MSR isn't used by host, some CPU cycles can be saved.

>But there are a lot of other ways for the host to share the values
>to SEAM in zero overhead.

I am not sure. Looks it requests a new interface between host and TDX
module. I guess one problem is how/when to verify host's inputs in case
they are invalid.

Thanks
Chao

>
>Could you provide more information?