Received-SPF: pass (google.com: domain of linux-kernel+bounces-184324-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <3e7413b5-482a-4243-be6c-21a0ee232cc4@intel.com>
Date: Tue, 21 May 2024 12:42:49 +1200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 2/9] KVM: nVMX: Initialize #VE info page for vmcs02 when
 proving #VE support
To: Sean Christopherson <seanjc@google.com>
CC: Paolo Bonzini <pbonzini@redhat.com>, <kvm@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
References: <20240518000430.1118488-1-seanjc@google.com>
 <20240518000430.1118488-3-seanjc@google.com>
 <78b3a0ef-54dc-4f49-863e-fe8288a980a7@intel.com>
 <ZkvbUNGEZwUHgHV9@google.com>
 <b1def408-f6e8-4ab5-ac7a-52f11f490337@intel.com>
 <ZkvpDkOTW8SwrO5g@google.com>
Content-Language: en-US
From: "Huang, Kai" <kai.huang@intel.com>
In-Reply-To: <ZkvpDkOTW8SwrO5g@google.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ME81YlZxYlc3aFl2cmdQaDA4Ry9NYjR6RmNYT1dyT1NYYWViWkF5TmRKRjZE?=
 =?utf-8?B?Sk1zcFBMK0plY0NDTzN5TVR0QkVQd2MrcEsyR0NUdEgvaEpJTithQnVzenFK?=
 =?utf-8?B?Sk03ZEVjT3l5YmVueFh2Tzhoa3VDU0JJU2crU0Y0ak9TM0Jnd3oxVnlhQVhL?=
 =?utf-8?B?WWJ0VlRta2trYXRnbHBQWElqTW0rN2NBamZTYUhibjFMQ1dJZE52Y3JnR3Ax?=
 =?utf-8?B?dXptcitCTXNvR2UyV21uK3pjWThWR0tqbTl0V1FWYTJITXNHNWFNSFBjZURp?=
 =?utf-8?B?V3dDaVZsejhVRlNGSjdwa2FHSWN4WmpMbzJkeDBvYzFQc3JhaGJDUEdyMmZS?=
 =?utf-8?B?T2hQdm83ZVJqYlI5L3hDUzk5WFB2LzRKOUxZMlRNYjVhQ0FPbEN1R3BuOUFv?=
 =?utf-8?B?bm41b05JRm9Qb1F6TmpnYkNUbzI5OWdCazFUN0FOOUdiYUNXTEhFR1dlM0NL?=
 =?utf-8?B?bkJGcURCUUk4NDRpZ2pEdHVIVHFONVl1MVZVS3lVcTY2cGY1N1lNdUhiSU0x?=
 =?utf-8?B?NEdCK1FCRDJHOTYxU2JSZWt1ejB2aXhGbEcwNmNrQ0dETDlxMlhGRXNTWDRQ?=
 =?utf-8?B?WXJjeVFwVW5jelBMaGtvdVBLRVpjbzNnOW5lSzlhUVdUUFhrQmFra0d4SkZB?=
 =?utf-8?B?RVdXNVY4c2xxamdnRzhmYnlXdUV3SDJjMlpaNmtHbGJiQnpwWFhETXZPRzZl?=
 =?utf-8?B?QzIzUTBPeVRJMlVoSUY1SlB6M3h5alp6bzVleE84ZzNScUprL25taEZzNkVa?=
 =?utf-8?B?YXN1bENPT1dRNXdBYk9JNEhkZkwxdHZTZG1Ia2Rqa2dzK01rblVsV0hIU21Q?=
 =?utf-8?B?WTg1ZFdUS1Z4Uk1tZmdock5UM1BWTk40ZW44ellsSUU5cGRBT0ExMHkwNUk2?=
 =?utf-8?B?YTR0TjUvUHdBTVc2MWlNVnhFNFpqU3RNZlZPOU9LRDBQNW11ZXpsRGoyU1Z5?=
 =?utf-8?B?Vk1rS2Fpd1JBTTRrVXBoNitVL3FDa1ZrRjI4T3JnSjZDK28wVkpQVzFOQ2h5?=
 =?utf-8?B?akZweHMzQTd2T05EdVlzVzlNUm1GSXVKY2JPaXYxQkRRWVZSQXNPRmpKNEoz?=
 =?utf-8?B?MnZRdnl1U29xcDIyM2huOWtUa2RSOW5rZnYrS1ZmQ1RET3FQblhlazhrYUEv?=
 =?utf-8?B?MytvQjlmTERLL2s5T2RXOWdYUUZmS1Nsb1pNVVRpY2pSa3pod2dOSnA5VFVW?=
 =?utf-8?B?YmtVeWU1WlZRK0lDTk9iOTNXbllQREkxczNkenZTTGRTSEFCRnN1YnkxbWI3?=
 =?utf-8?B?N2FPMFE1ak5wR3ZhY0pEZUxDaU5jSnVEVUxjUmZKMEV5WTg5QmIvdXMzQlJk?=
 =?utf-8?B?MHFOR2FUb0lqUlowU3ZjeWd6ODJtbGRuSDBOamVGZE4yeGNZVU5SSTBOVUtH?=
 =?utf-8?B?NmVtMjV5T2d3YVdQc2haSHVmVkdrNVFkZ3hTdWVtRnpKb1R6QVFnell2OHJ2?=
 =?utf-8?B?SnJuajRZNFlPQnBwR0Vkamo2eDRZbmdEOWtKSjhnbUFUcU55K2Fxd2lxNjFS?=
 =?utf-8?B?YjlDUExnaDdvbTZ5UU9FNnZlZXFETll0aHNVc2E5NC9RQ0pzUEQ5dTRBVCs1?=
 =?utf-8?B?aURUVFN1S2JpazBnVWhpUlBXZDJHc2lZdHlCUUpmdkcxWjFBNlBGR3JZQVUw?=
 =?utf-8?B?R0hmVU9xY3FkcXVXVEg3RmV5amJaaVlsdXVmbm1WL3prbkpaVE9kQm5jcXhT?=
 =?utf-8?B?cG56a3lwK2JDNjlSMFhzeDcwU0k0cGQ4VnBxZFpMOWxPRGVnd3UwZCtDNngx?=
 =?utf-8?B?MXdjVlBDQjduMlFtNkhFUUR2cFIxWTBPaGh4T1hzZGxNVWpmSkZZLzVvQSsy?=
 =?utf-8?B?Vlc3ZjFiRWVyekUzaStNc3Q0S0d5VEhoZGRaeFlaa3ZNNE1jMEhBdnM2ZGNG?=
 =?utf-8?B?V0MxUkEyR0NJc0Qrc3ZINmtMSGtjeVZYcG5FamlRczRUM1JHaXlJK3NSbnFJ?=
 =?utf-8?B?TXJXQjZ2MVVJUnRETkNmMmhNdDB3cWx4MVdJYTJ5Nkx1d0orMk9sb1NDWERy?=
 =?utf-8?B?R3NlVHZ4LzlmSjlNTzFRTnJ6eFNoZkNYenFvRW5HTUtEU24yVmtjWW5INThs?=
 =?utf-8?B?N0t3Qjg3b2RWVC94WEU0WldNWXp2MCtMMGtqWlZGVG1DNGlkNDlPakN4SEUw?=
 =?utf-8?Q?RP+pnblYwYgRxPdzuohQYH3vh?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 214a9ae7-de33-43d3-28d4-08dc792ef44f
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5978.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 May 2024 00:42:55.5842
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: JX4QZjxao0+nNGfIQzQngZ7vdTGzG7fPDHf+zxEyOFNoNFclU4fQISHtmm9moJq0khkTg2xJztmo5j2kKI6n7w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR11MB7196
X-OriginatorOrg: intel.com


On 21/05/2024 12:21 pm, Sean Christopherson wrote:
> On Tue, May 21, 2024, Kai Huang wrote:
>> On 21/05/2024 11:22 am, Sean Christopherson wrote:
>>> On Tue, May 21, 2024, Kai Huang wrote:
>>>> On 18/05/2024 12:04 pm, Sean Christopherson wrote:
>>>>> Point vmcs02.VE_INFORMATION_ADDRESS at the vCPU's #VE info page when
>>>>> initializing vmcs02, otherwise KVM will run L2 with EPT Violation #VE
>>>>> enabled and a VE info address pointing at pfn 0.
>>>>
>>>> How about we just clear EPT_VIOLATION_VE bit in 2nd_exec_control
>>>> unconditionally for vmcs02?
>>>
>>> Because then KVM wouldn't get any EPT Violation #VE coverage for L2, and as
>>> evidence by the KVM-Unit-Test failure, running L2 with EPT Violation #VEs enabled
>>> provides unique coverage.  Doing so definitely provides coverage beyond what is
>>> strictly needed for TDX, but it's just as easy to set the VE info page in vmcs02
>>> as it is so clear EPT_VIOLATION_VE, so why not.
>>>
>>>> Your next patch says:
>>>>
>>>> "
>>>> Always handle #VEs, e.g. due to prove EPT Violation #VE failures, in L0,
>>>> as KVM does not expose any #VE capabilities to L1, i.e. any and all #VEs
>>>> are KVM's responsibility.
>>>> "
>>>
>>> I don't see how that's relevant to whether or not KVM enables EPT Violation #VEs
>>> while L2 is running.  That patch simply routes all #VEs to L0, it doesn't affect
>>> whether or not it's safe to enable EPT Violation #VEs for L2.
>>
>> My logic is, if #VE exit cannot possibly happen for L2, then we don't need
>> to deal whether to route #VE exits to L1. :-)
>>
>> Well, actually I think conceptually, it kinda makes sense to route #VE exits
>> to L1:
>>
>> L1 should never enable #VE related bits so L1 is certainly not expecting to
> 
> Not "should never", "can never".  If L1 attempts to enable EPT_VIOLATION_VE, then
> VM-Enter will VM-Fail.
> 
>> see #VE from L2.  But how to act should be depending on L1's logic? E.g., it
>> can choose to ignore, or just kill the L2 etc?
> 
> No.  Architecturally, from L1's perspective, a #VE VM-Exit _cannot_ occur in L2.
> L1 can inject a #VE into L2, but a #VE cannot be generated by the CPU and thus
> cannot cause a VM-Exit.

OK.  The point is not to argue about L1 how to handle, but whether we 
should inject to L1 -- L1 can do whatever it believes legal/sane.

But I understand the purpose is to test/validate, so it's fine for L0 to 
handle, and by handle it eventually means we want to just dump that #VE 
exit.

But now L0 always handles #VE exits from L2, and AFAICT L0 will just 
kill the L1, until the patch:

	KVM: VMX: Don't kill the VM on an unexpected #VE

lands.

So looks that patch at least should be done first.  Otherwise it doesn't 
make a lot sense to kill L1 for #VE exits from L2.

> 
>> Unconditionally disable #VE in vmcs02 can avoid such issue because it's just
>> not possible for L2 to have the #VE exit.
> 
> Sure, but by that argument we could just avoid all nested VMX issues by never
> enabling anything for L2.
> 
> If there's an argument to be made for disabling EPT_VIOLATION_VE in vmcs02, it's
> that the potential maintenance cost of keeping nEPT, nVMX, and the shadow MMU
> healthy outweighs the benefits.  I.e. we don't have a use case for enabling
> EPT_VIOLATION_VE while L2 is running, so why validate it?

Yeah.  I am not sure the purpose of validating #VE exits from L2.

> 
> If whatever bug the KUT EPT found ends up being a KVM bug that specifically only
> affects nVMX, then it'd be worth revisiting whether or not it's worth enabling
> EPT_VIOLATION_VE in vmcs02.  But that's a rather big "if" at this point.

OK.