Received-SPF: pass (google.com: domain of linux-kernel+bounces-94704-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <db22392e-fb88-44f6-8592-7429c606cf10@intel.com>
Date: Thu, 7 Mar 2024 12:20:10 +1300
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 11/16] KVM: x86/mmu: Explicitly disallow private accesses
 to emulated MMIO
Content-Language: en-US
To: Sean Christopherson <seanjc@google.com>
CC: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>, Paolo Bonzini
	<pbonzini@redhat.com>, <kvm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	Yan Zhao <yan.y.zhao@intel.com>, Isaku Yamahata <isaku.yamahata@intel.com>,
	Michael Roth <michael.roth@amd.com>, Yu Zhang <yu.c.zhang@linux.intel.com>,
	Chao Peng <chao.p.peng@linux.intel.com>, Fuad Tabba <tabba@google.com>,
	"David Matlack" <dmatlack@google.com>
References: <20240228024147.41573-1-seanjc@google.com>
 <20240228024147.41573-12-seanjc@google.com>
 <0a05d5ec-5352-4bab-96ae-2fa35235477c@intel.com>
 <ZejxqaEBi3q0TU_d@google.com>
 <5f230626-2738-41cc-875d-ab1a7ef19f64@intel.com>
 <Zej1sBTPLXIhfemS@google.com>
From: "Huang, Kai" <kai.huang@intel.com>
In-Reply-To: <Zej1sBTPLXIhfemS@google.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?a3FDejNkUWZibnJ4eWsvZGcxd1ZtK0tVUU0rNlBRellyTkREMWd4eXVOOTJ2?=
 =?utf-8?B?ajhzY0p0NVZvSGt6UjJLL09BQkdacitxK3NxdHhuclVWQzRsbWFYMFBWMXVE?=
 =?utf-8?B?V0Iyd2wxTnZZUWtEQ0lQZjBnbTNia1R2aFB3WXVlQVh6aFY0YmFacHZmb2p4?=
 =?utf-8?B?c0k4TUpNbHNYRUhnSCs5VW5GdWdqNHgwS05PRTlLKzFpNEhVeUdPTzZkL2tq?=
 =?utf-8?B?VUlFdFZWYW9kTkxTNXZGRUdTZmV3TW5Rem1xYk1YMGVaekl5Mk9PR0lUWm8v?=
 =?utf-8?B?R0cvNGNuNDdpWnptNDBQaHlpZldDcDlyaVVJQ29LRkdjNk9jUHZnNzg4Tnhh?=
 =?utf-8?B?ckJKVmtmbU5rSCt0Sm9HVjI0QVdWNjNxMWViaUJPcllRbmlMTjJYWjI0ODNj?=
 =?utf-8?B?QlJoSjBvU3k5ZVpaS25OOExQZHhRV05xZXp0SlVSSGpQSzBJdkZqeFRIZWpX?=
 =?utf-8?B?Nit5QXpnYW1Gdy9kWXN5WnJzR2E5N1hlRmhwNGUyQWFPbDdMOFQvUlUrL3Vs?=
 =?utf-8?B?N1dxZDBXRE5KdnZ2MHMxNnk3WE9NQ1hHQnpHOXdNNEU0ZHZ6N21RejZiZDBa?=
 =?utf-8?B?L2xBK0JaZUZuV0krUHFUemtjNmx3cCt0RXZzaGpiVWVTYmVzYy9LYUxRQVBs?=
 =?utf-8?B?NEw2eTNpQmEzRktyQnVIUXZjb1NTRkFzUi9HWElkYzkzV1ladjRUNncrWkY0?=
 =?utf-8?B?V1kvZHFIL1hISWxIT2dWbW5qaHR6OFI1eDY0Si9KK2pza2hWSFFLVUpvaFo1?=
 =?utf-8?B?ajNHUElNOGZGWUpjaHlUVVJsODZJbzFwWW83bVZnaVFhNFJyOU1xZkdSTmxL?=
 =?utf-8?B?TFpQV2dnVGo2a3JPVG5PcEI3YmJNZWFHaSthQ2p1V3p4d0F5Mzhjcis0Q0pr?=
 =?utf-8?B?NXl3S3IwY3plK1VQc0plc1M4dUk1b0tTWnhQdWFtQktmVTN6a2JRMVo0SjNr?=
 =?utf-8?B?VExwZmcxWmM0eHFvU3BlVGl5bWdDamJERzJVSGJ0TGJYTEFYb2szcHltZ2pV?=
 =?utf-8?B?NEpLWTlXMEE2cnoxVnVoZ1dCeHBpQXRPeURvc1NlSnhPNW5yd1JqUzJSa1Bp?=
 =?utf-8?B?eWpQNlE5dlV5dGdiaUc3RjIrbFcwZTFCZTdocjZHRmxSK1FNZXhrN1BuWVU2?=
 =?utf-8?B?RDdteGhZM1JsZWlGVWNhaS9CODdjMGVab0hRc0lLaUpUSXY0Y1hlSTlZK28x?=
 =?utf-8?B?b2xCQ0J3T1I2NjBFT0ZJSHQ2MlFjb2hBeDM2MG5FcVdBM1YrNmt0a1hPbTgv?=
 =?utf-8?B?dlZNZmFsYkVGUFFIZjhtWnZzMXFPMWd6ZTZwQUlXSUJ3MEQ4aTNWUTRFQUZC?=
 =?utf-8?B?d3BTL2NSUC93c1BJbkdhZ2NjcCtrMzduMnIxVFlHbE5QK1BYUk5iVkh0TDBQ?=
 =?utf-8?B?TmxwQmxCUG96YVFFOTdXRExXUUJKQ1Y4cWZLanAwSi9sUnNLZGliR2c1Z3Br?=
 =?utf-8?B?bWMxc2RWTmlDMHhNUUNDRlUreFFYVFU0cThpNk9HcU1yNmdHdnI4MjBrNVJY?=
 =?utf-8?B?eXpDMUYrS0drZ3FFOEtKNWMzdXluOHp4OHczVU9iOHk2SjdRb0Z3cFJWVzNH?=
 =?utf-8?B?Ykd6U3kzUDhJL3dob0NWZjZnR25zeERlRWZPa3BCVmZlZkNOTGtjVnR6M1J1?=
 =?utf-8?B?ajZwaklMNjBuVlhKWmZDM0lkaUN5OEREdERqZVh4RGRTd01ZUVlxd2dJOHE0?=
 =?utf-8?B?Z2tzMmxJNjhDdldMeXBzV3pqQzJsQzNLODhmNng0ZXh0b0R3VTh2ZS95Zm43?=
 =?utf-8?B?alBITzhHVXdNeGYyNWZFdVh3ZC9wK1JCS2ppQm1JMjl5MXQyNFl2MDA3Z1R4?=
 =?utf-8?B?SVpmbTB2d2ZiWE1EM1p1TEVwYUxIWXlsRUNNVGFNT0FXWkEyNmVLTjBGcDZQ?=
 =?utf-8?B?TENSREJVLzhvMGRRb0pkQk1qOHZydlF3YUZ3R2Y3NjlXYWprbWVMU1lvWS92?=
 =?utf-8?B?SXZDbkJFd0FRTDE4MFR4cjFTYllqMzN2Y3RVY1pzaW5zQ2JEOFYxdlhGYW01?=
 =?utf-8?B?MG5lRm5YdEJ3QS9vZTNtSDJmbEpaS3JIemZoK09tSHpNZzR3YURmSnFiZXJR?=
 =?utf-8?B?akEwa0VNVytHdlJtM1puOTQ3MzJpYjhCU1RCWnF5MU5CbGRnd2ZRUGVJaEhn?=
 =?utf-8?Q?Zmm6lFAYKs0sBx9qlMsy9vpci?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 5a55696f-0774-4820-0683-08dc3e33fd49
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5978.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Mar 2024 23:20:19.5368
 (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: QXEAS1Tx3X9DVRdncaWunTGrFLjLjHn4dUe6YFU/dIE8r30pl99LOpBwBpDSQMy6iUTaj1Dr+UDbkXUEunoB5w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV3PR11MB8728
X-OriginatorOrg: intel.com


On 7/03/2024 12:01 pm, Sean Christopherson wrote:
> On Thu, Mar 07, 2024, Kai Huang wrote:
>>
>>
>> On 7/03/2024 11:43 am, Sean Christopherson wrote:
>>> On Thu, Mar 07, 2024, Kai Huang wrote:
>>>>
>>>>
>>>> On 28/02/2024 3:41 pm, Sean Christopherson wrote:
>>>>> Explicitly detect and disallow private accesses to emulated MMIO in
>>>>> kvm_handle_noslot_fault() instead of relying on kvm_faultin_pfn_private()
>>>>> to perform the check.  This will allow the page fault path to go straight
>>>>> to kvm_handle_noslot_fault() without bouncing through __kvm_faultin_pfn().
>>>>>
>>>>> Signed-off-by: Sean Christopherson <seanjc@google.com>
>>>>> ---
>>>>>     arch/x86/kvm/mmu/mmu.c | 5 +++++
>>>>>     1 file changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
>>>>> index 5c8caab64ba2..ebdb3fcce3dc 100644
>>>>> --- a/arch/x86/kvm/mmu/mmu.c
>>>>> +++ b/arch/x86/kvm/mmu/mmu.c
>>>>> @@ -3314,6 +3314,11 @@ static int kvm_handle_noslot_fault(struct kvm_vcpu *vcpu,
>>>>>     {
>>>>>     	gva_t gva = fault->is_tdp ? 0 : fault->addr;
>>>>> +	if (fault->is_private) {
>>>>> +		kvm_mmu_prepare_memory_fault_exit(vcpu, fault);
>>>>> +		return -EFAULT;
>>>>> +	}
>>>>> +
>>>>
>>>> As mentioned in another reply in this series, unless I am mistaken, for TDX
>>>> guest the _first_ MMIO access would still cause EPT violation with MMIO GFN
>>>> being private.
>>>>
>>>> Returning to userspace cannot really help here because the MMIO mapping is
>>>> inside the guest.
>>>
>>> That's a guest bug.  The guest *knows* it's a TDX VM, it *has* to know.  Accessing
>>> emulated MMIO and thus taking a #VE before enabling paging is nonsensical.  Either
>>> enable paging and setup MMIO regions as shared, or go straight to TDCALL.
>>
>> +Kirill,
>>
>> I kinda forgot the detail, but what I am afraid is there might be bunch of
>> existing TDX guests (since TDX guest code is upstream-ed) using unmodified
>> drivers, which doesn't map MMIO regions as shared I suppose.
>>
>> Kirill,
>>
>> Could you clarify whether TDX guest code maps MMIO regions as shared since
>> beginning?
> 
> Y'all get the same answer we gave the SNP folks: KVM does not yet support TDX,
> so as far is KVM is concerned, there is no existing functionality to support.
> 
> s/firmware/Linux if this is a Linux kernel problem.
> 
>    On Thu, Feb 08, 2024, Paolo Bonzini wrote:
>    > On Thu, Feb 8, 2024 at 6:27 PM Sean Christopherson <seanjc@google.com> wrote:
>    > > No.  KVM does not yet support SNP, so as far as KVM's ABI goes, there are no
>    > > existing guests.  Yes, I realize that I am burying my head in the sand to some
>    > > extent, but it is simply not sustainable for KVM to keep trying to pick up the
>    > > pieces of poorly defined hardware specs and broken guest firmware.
>    >
>    > 101% agreed. There are cases in which we have to and should bend
>    > together backwards for guests (e.g. older Linux kernels), but not for
>    > code that---according to current practices---is chosen by the host
>    > admin.
>    >
>    > (I am of the opinion that "bring your own firmware" is the only sane
>    > way to handle attestation/measurement, but that's not how things are
>    > done currently).

Fair enough, and good to know. :-)

(Still better to hear from Kirill, though.)