MIME-Version: 1.0
References: <cover.1637799475.git.isaku.yamahata@intel.com>
 <4ede5c987a4ae938a37ab7fe70d5e1d561ee97d4.1637799475.git.isaku.yamahata@intel.com>
 <878rxcht3g.ffs@tglx> <20211126091913.GA11523@gao-cwp>
In-Reply-To: <20211126091913.GA11523@gao-cwp>
From:   Lai Jiangshan <jiangshanlai@gmail.com>
Date:   Mon, 29 Nov 2021 15:08:39 +0800
Message-ID: <CAJhGHyAbBUyyVKL7=Cior_uat9rij1BB4iBwX+EDCAUVs1Npgg@mail.gmail.com>
Subject: Re: [RFC PATCH v3 53/59] KVM: x86: Add a helper function to restore 4
 host MSRs on exit to user space
To:     Chao Gao <chao.gao@intel.com>
Cc:     Thomas Gleixner <tglx@linutronix.de>, isaku.yamahata@intel.com,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        "H . Peter Anvin" <hpa@zytor.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>,
        Erdem Aktas <erdemaktas@google.com>,
        Connor Kuehl <ckuehl@redhat.com>,
        Sean Christopherson <seanjc@google.com>,
        LKML <linux-kernel@vger.kernel.org>, kvm@vger.kernel.org,
        isaku.yamahata@gmail.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Nov 29, 2021 at 2:00 AM Chao Gao <chao.gao@intel.com> 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@intel.com>
> >
> >> $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.

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).

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.
But there are a lot of other ways for the host to share the values
to SEAM in zero overhead.

Could you provide more information?