Date:   Mon, 29 Nov 2021 18:05:48 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     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>, erdemaktas@google.com,
        Connor Kuehl <ckuehl@redhat.com>, linux-kernel@vger.kernel.org,
        kvm@vger.kernel.org, isaku.yamahata@gmail.com,
        Sean Christopherson <sean.j.christopherson@intel.com>,
        Kai Huang <kai.huang@linux.intel.com>
Subject: Re: [RFC PATCH v3 18/59] KVM: x86: Add flag to mark TSC as immutable
 (for TDX)
Message-ID: <YaUWfFpYP/7lcpmZ@google.com>
References: <cover.1637799475.git.isaku.yamahata@intel.com>
 <00772535f09b2bf98e6bc7008e81c6ffb381ed84.1637799475.git.isaku.yamahata@intel.com>
 <87ilwgja6x.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87ilwgja6x.ffs@tglx>
Precedence: bulk

On Thu, Nov 25, 2021, Thomas Gleixner wrote:
> On Wed, Nov 24 2021 at 16:20, isaku yamahata wrote:
> > From: Sean Christopherson <sean.j.christopherson@intel.com>
> >
> > The TSC for TDX1 guests is fixed at TD creation time.  Add tsc_immutable
> 
> What's a TDX1 guest?

The "revision 1.0" version of TDX.  Some of these patches use "TDX1" to identify
behaviors that may not hold true in future iterations of TDX, and also to highlight
things that are dictated by the spec, e.g. some of the guest TSC frequency values.
For this patch in particular, there's probably no need to differentiate TDX1 vs. TDX,
the qualification was more for cases where KVM needs to define magic values to adhere
to the spec, e.g. to make it clear that the magic values aren't made up by KVM.

> > to reflect that the TSC of the guest cannot be changed in any way, and
> > use it to short circuit all paths that lead to one of the myriad TSC
> > adjustment flows.
> 
> I can kinda see the reason for this being valuable on it's own, but in
> general why does TDX need a gazillion flags to disable tons of different
> things if _ALL_ these flags are going to be set by for TDX guests
> anyway?
> 
> Seperate flags make only sense when they have a value on their own,
> i.e. are useful for things outside of TDX. If not they are just useless
> ballast.

SEV-SNP and TDX have different, but overlapping, restrictions.  And SEV-ES also
shares most SEV-SNP's restrictions.  TDX guests that can be debugged and/or profiled
also have different restrictions, though I forget if any of these flags would be
affected.

The goal with individual flags is to avoid seemingly arbitrary is_snp_guest() and
is_tdx_guest() checks throughout common x86 code, e.g. to avoid confusion over why
KVM does X for TDX but Y for SNP.  And I personally find it easer to audit KVM
behavior with respect to the SNP/TDX specs if the non-obvious restrictions are
explicitly set when the VM is created.

For some of the flags, there's also hope that future iterations of TDX will remove
some of the restrictions, though that's more of a bonus than a direct justification
for adding individual flags.