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> <CAJhGHyAbBUyyVKL7=Cior_uat9rij1BB4iBwX+EDCAUVs1Npgg@mail.gmail.com>
 <20211129092605.GA30191@gao-cwp>
In-Reply-To: <20211129092605.GA30191@gao-cwp>
From:   Lai Jiangshan <jiangshanlai@gmail.com>
Date:   Tue, 30 Nov 2021 12:58:47 +0800
Message-ID: <CAJhGHyCiZn8ZwBbVepU+tfmTV6gcDhXxzvS39BwpgUj+6LCZ0g@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 5:16 PM Chao Gao <chao.gao@intel.com> wrote:

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

No file in this link:
https://www.intel.com/content/www/us/en/developer/articles/technical/intel-trust-domain-extensions.html
has chapter 22.

>
> >
> >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 it adds overall overhead because the wrmsr in TDX module
can't be skipped while the unneeded potential overhead of
wrmsr is added in user return path.

If TDX module restores the original MSR value, the host hypervisor
doesn't need to step in.

I think I'm reviewing the code without the code.  It is definitely
wrong design to (ab)use the host's user-return-msr mechanism.

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

If the requirement of "lazy restoration" is being added (not seen in the
published document yet), you are changing the ABI between the host and
the TDX module.