Re: [PATCH] x86/tdx: Handle load_unaligned_zeropad() page-cross to a shared page

From: Dave Hansen <dave.hansen@intel.com>
To: Sean Christopherson <seanjc@google.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
	luto@kernel.org, peterz@infradead.org,
	sathyanarayanan.kuppuswamy@linux.intel.com, ak@linux.intel.com,
	dan.j.williams@intel.com, david@redhat.com, hpa@zytor.com,
	thomas.lendacky@amd.com, x86@kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] x86/tdx: Handle load_unaligned_zeropad() page-cross to a shared page
Date: Tue, 17 May 2022 15:52:03 -0700	[thread overview]
Message-ID: <4ee65514-2b08-9ca8-84e8-92a6ab1d9652@intel.com> (raw)
In-Reply-To: <YoQkTtrMiU2bff9i@google.com>

On 5/17/22 15:40, Sean Christopherson wrote:
> On Tue, May 17, 2022, Dave Hansen wrote:
>> The SDM makes it sound like we should be more judicious about using
>> 've->instr_len' though.  "All VM exits other than those listed in the
>> above items leave this field undefined."  Looking over
>> virt_exception_kernel(), we've got five cases from CPU instructions that
>> cause unconditional VMEXITs:
> 
> None of the below exit unconditionally.
...
>   For fault-like VM exits due to attempts to execute one of the following
>   instructions that cause VM exits unconditionally or based on the settings of
>   VM-execution controls.

Ahh, got it, thanks.  I bailed on reading that sentence before I got to
the "VM-execution controls" bit.

>> Then handle_mmio() can say:
>>
>> 	/*
>> 	 * VM-exit instruction length is not provided for the EPT
>> 	 * violations that MMIO causes.  Use the insn_decode() length:
> 
> This is inaccurate.  The instruction length _is_ provided on EPT Violation VM-Exits
> (it's also provided by all Intel CPUs on EPT Misconfigs even though the SDM doesn't
> say so).
> 
> The instruction length is wrong in the TDX case because there is no EPT Violation
> VM-Exit.  The EPT Violation is morphed to a #VE by the CPU, and the instruction
> length isn't one of the fields that's saved into the #VE info struct by the CPU.
> When the TDX Module gets control on the TDCALL, VMCS.INSTRUCTION_LENGTH will hold
> the length of the TDCALL, not the instruction that caused the #VE, i.e. the TDX
> Module can't provide the correct length.
> 
> For all other #VE cases in TDX, the #VE is injected by software (TDX module) after
> the instruction-based VM-Exit.  Before injecting the #VE, the TDX module grabs the
> length from the VMCS and manually records it in the #VE info struct.

That's horribly entertaining background. :)

But, it doesn't get us much closer to deciding when we can consume the
instruction length out of the ve_info.  It seems like magic that jut
happens to work at the moment.