Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 31 Mar 2021 22:06:17 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Dave Hansen <dave.hansen@intel.com>
Cc:     Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@linux.intel.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Andy Lutomirski <luto@kernel.org>,
        Andi Kleen <ak@linux.intel.com>,
        Kirill Shutemov <kirill.shutemov@linux.intel.com>,
        Kuppuswamy Sathyanarayanan <knsathya@kernel.org>,
        Dan Williams <dan.j.williams@intel.com>,
        Raj Ashok <ashok.raj@intel.com>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 1/1] x86/tdx: Handle MWAIT, MONITOR and WBINVD
Message-ID: <YGTyWUQbxVZeeko+@google.com>
References: <2FE32855-EA5D-44E4-AACC-25E9B1476547@amacapital.net>
 <e62cfd0ae90de435e6819979d9027f76d835a22a.1617224710.git.sathyanarayanan.kuppuswamy@linux.intel.com>
 <YGTvSvr2T2v3t3XA@google.com>
 <5d961c25-3dee-4a5d-4bba-a97d157a5a49@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <5d961c25-3dee-4a5d-4bba-a97d157a5a49@intel.com>
Precedence: bulk

On Wed, Mar 31, 2021, Dave Hansen wrote:
> On 3/31/21 2:53 PM, Sean Christopherson wrote:
> > On Wed, Mar 31, 2021, Kuppuswamy Sathyanarayanan wrote:
> >> Changes since v3:
> >>  * WARN user if SEAM does not disable MONITOR/MWAIT instruction.
> > Why bother?  There are a whole pile of features that are dictated by the TDX
> > module spec.  MONITOR/MWAIT is about as uninteresting as it gets, e.g. absolute
> > worst case scenario is the guest kernel crashes, whereas a lot of spec violations
> > would compromise the security of the guest.
> 
> So, what should we do?  In the #VE handler:
> 
> 	switch (exit_reason) {
> 	case SOMETHING_WE_HANDLE:
> 		blah();
> 		break;
> 		...
> 	default:
> 		pr_err("unhadled #VE, exit reason: %d\n", exit_reason);
> 		BUG_ON(1);
> 	}
> 
> ?
> 
> Is this the *ONLY* one of these, or are we going to have another twenty?
> 
> If this is the only one, we might as well give a nice string error
> message.  If there are twenty more, let's just dump the exit reason,
> BUG() and move on with our lives.

I've no objection to a nice message in the #VE handler.  What I'm objecting to
is sanity checking the CPUID model provided by the TDX module.  If we don't
trust the TDX module to honor the spec, then there are a huge pile of things
that are far higher priority than MONITOR/MWAIT.