Received-SPF: pass (google.com: domain of linux-kernel+bounces-94678-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <5f230626-2738-41cc-875d-ab1a7ef19f64@intel.com>
Date: Thu, 7 Mar 2024 11:49:11 +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>, "Kirill A. Shutemov"
	<kirill.shutemov@linux.intel.com>
CC: 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>
From: "Huang, Kai" <kai.huang@intel.com>
In-Reply-To: <ZejxqaEBi3q0TU_d@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?alc5ZExTUEhwaG9HR0F2Ykt1SWc4Zmo4TlZQenNYRGJSckVxRjBwR0gvK3lJ?=
 =?utf-8?B?K09lTlhuY1hKd250dDVpVlJwMjB5RmtJa2xaL0VDb2FGMTZLT2llNlh5MWNN?=
 =?utf-8?B?c2s5bGxtTENjK3FHdDN6UEFHUjlmS3RTbUdKSWl2RXQrN0RTVWh1SlNnTFVR?=
 =?utf-8?B?Y3ZvaGpOdEdXTWpVOVNtTmY1TU1wRnJMR1VGZ1FocGx0c0pMOU5ObDNiRk5X?=
 =?utf-8?B?eWlMb2JreDkraGpJZSt4QlVTd0pZajRjU2djVDFzYWpHVTVsN3J6WmM0Ym4r?=
 =?utf-8?B?UVY0d2ZCUHc4c0g2SjFEVEpEdWNvMDVpQVZuaEtpWnBvWjVMODFqcnBPdElL?=
 =?utf-8?B?elFFM0FWMFh6aVhqNkphOXdlRUtUbk4yeU9ZWWUwSndHWXdROEVTbmlocWtR?=
 =?utf-8?B?MDN5OTJ3REZvNUpXbmpYT01XaGo1S0FJRGJSa2RtbktjQk85V3htSHo5cFE1?=
 =?utf-8?B?TjBuM2pIcnlvUW41RjJDdFliRkF0K2lLZXNDMDd1a3FlRC9sWDkwNTc4akxS?=
 =?utf-8?B?T1VqZEdyOHp6Y1Z4NkxaRC9tWk5qMHAzRXZLcnI5Z2tpT0xJb1N4TEpGVzFY?=
 =?utf-8?B?ZFFGU29TZ3lkNG9sNFBZQXVRTDRkSlczRGhBcGxIU24xWUlndktoWHhLMjUy?=
 =?utf-8?B?M2dmOXIyRlR2QmhxT3RyaDVPTnFidDBmOWFQZG1XWllmWHlER2dQWG05akVx?=
 =?utf-8?B?T0tpQUUyWHN6QThpay9qaFZFRmRXdUNiRXN3RHdGaHpScG4xOHA3RXJySEJm?=
 =?utf-8?B?dkhjVnRQQ2MxM2dKbXU0L0JsVTlPOThUSE01UzVTQTQrMEFQM2JweXo5c1Az?=
 =?utf-8?B?c3B6WUZIRnNQUy9JOWt6N3VMY29maGk3QUsxdEJKM2ZJekVCc2xuVDI0WUM2?=
 =?utf-8?B?MXBsY3Vlei9iV3RnVEpsRU5rSUpka215TnVQSTFNWTRPbkQ3aW81L1JKcm1t?=
 =?utf-8?B?dU5xOFdIbmNPQ054RGpPNzByQzZ0VTRXVTMwYldmOEdqb0NtSUdET2VJMnlU?=
 =?utf-8?B?ZlZGSXQzMDFsRkZYdmVBMDlIVWkyTmlHYVdnOE1CdHgwYmc0NkdDbDQzbGtG?=
 =?utf-8?B?emJrWHFiNklvbStYTnJqVkJCN21jdHRUbml5ZitCdFQyL2N0ZE1RbHh3NDJH?=
 =?utf-8?B?SGRXSmRIOTBSR3dFQ0haS3Iwb1hWb3ZObWtBTjFWczBHUWtvOVVNaHBBbk04?=
 =?utf-8?B?dXZhMU8zMmI4QWY1SDhjUXZZRWQzYzc5c2twdGN0S1BxSXVEdzNvTURGZGZH?=
 =?utf-8?B?Rm9nOTUxV3A0WkdGZFk0dTJNdTFhZ0oyYkFLeVhNdHlzTk5ZZG1XcDF4K1l3?=
 =?utf-8?B?TlErR0NUb2FHZkFZc2o0TW5RaXEzZlhVdko1RHc0eFEyZmhiMXo1d1Rhdmlh?=
 =?utf-8?B?cDBBSnVXWCtIYkZkYTM4V0hrNTBXVUFqOFNIUnFDbSsxQm9kRDVBM1J0Z3A5?=
 =?utf-8?B?QURhZzE4MXNsV0FiQVZLa3ZuUndNQ0QvZVlPN0svZFBnWWdtUldhYkYzOVI1?=
 =?utf-8?B?K2VCaGRBbEF4NHZwcUhrT1hoSFl6QTZnS2lWOWNUam55UFVldjJIYWtnVWE5?=
 =?utf-8?B?cnhBc0dZRDBuYTZ0YXhVdkVHblNUNk9ZS2NkbWxDK2ZBNnlQdms4K3lqVVNp?=
 =?utf-8?B?VHRrQ3Y2N011Y21WeFFHWlNweitFbWNUNXkrczF1WXdoemNEbWE2aEFSUlUv?=
 =?utf-8?B?a2Rwalg3aUdSNk9Nc0ZISHhRY3psMDhFSEFpcjdsSmRaVFlQU3dkOCtuL2Mw?=
 =?utf-8?B?aTZRSUtSUGFtSVlBWHR0V2xzNCszWFJvZHVmU1VQTFBlbys1SFhsaExPdTZX?=
 =?utf-8?B?WHpBeGxWc3BVMUJnVE9xcVN4OXE1NmhxcGtOZzRXSWpkK2FLRkNqQndUWEV3?=
 =?utf-8?B?WHR5QWovM0tnZnExMHVOVjkwYldOaWlRMW1rQmJkTjdRMm5pc1BkQjRvWnJm?=
 =?utf-8?B?VzVoajA0TnhxNmM0L0tRaDcyV25Hdnh5cHVGdEJ5Y1dQMEZiajVJd09SNEZh?=
 =?utf-8?B?SEhYclFtVCtQbHNrdHdaU1BibTBZK0ovUjR2RzQ3ZDNLajBDY3RzWDZYeTh3?=
 =?utf-8?B?dktIaXpLaGNkU1FVQVNZeDMwaHY0R09DS0M0cnJvM0tLN0prQU1wMmczMXNW?=
 =?utf-8?Q?c2oMQaGVxtGgEyTrhPC1knl9v?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e1feed50-49fc-439d-55f4-08dc3e2fa965
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5978.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Mar 2024 22:49:20.7706
 (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: OjZGyRfDXnI1+oD6PgDQ4Zk7J1HiLA0+gOu1B8fA4xuKUhi0VkN0nqJcaNeLhhV4h17Q2puhPHldGVbr2PGHjw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB5982
X-OriginatorOrg: intel.com


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?