Received-SPF: pass (google.com: domain of linux-kernel+bounces-96376-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <d0390205-e083-44e7-8046-f5754dd3bf06@intel.com>
Date: Fri, 8 Mar 2024 13:09:22 +1300
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 11/16] KVM: x86/mmu: Explicitly disallow private accesses
 to emulated MMIO
To: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
CC: Sean Christopherson <seanjc@google.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>
 <3acw6nkfyre4t46i5gmd4lzxxlveiaksp55hunidfhi6lr6brh@7oqn63pu3flb>
Content-Language: en-US
From: "Huang, Kai" <kai.huang@intel.com>
In-Reply-To: <3acw6nkfyre4t46i5gmd4lzxxlveiaksp55hunidfhi6lr6brh@7oqn63pu3flb>
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?M2kzMU1ZMjFXV0x6bXFVQm95SURhQkQrUVZMQUJvRVQ1RFd0TkNwSnRZRG9E?=
 =?utf-8?B?RWdXeiticnV5VldxY3lSQ0F4eXVLVlh0Zi9SSmpBaDdXQ00vdVJBUHhFTXMw?=
 =?utf-8?B?SmRuQXpPTXdNZVdWOUZRTmswd1Z2bzhxNFBZMGhXL09NZ1lPQzFlUFRxakYx?=
 =?utf-8?B?dFl2STVpeGY1cHNsTzc4d3I2T3I1TUErM3Q2em9uTDFvYjBQYWVnMDFlamRL?=
 =?utf-8?B?WWJEOUpjS0lkUGZYa2ZCWEhNUEhQbGRPTlo3NDFEdksxcGNKNm5RRURyS1FM?=
 =?utf-8?B?TlNzSXdlZllaUStMeitoRG1WZzd5d3MwNEtZK002cURmVGNtMkx4UFFzbkNz?=
 =?utf-8?B?NHdoZFJ1Tjc5QTE2Z0krcFBUOW04aUJKZHVqUDRtdW1JbnExcXg5OUlTYjJ0?=
 =?utf-8?B?UXZTWHBLcGdPSktjNnZxQmRnemF5Y3lFemhHZXFjbjN4cDh2N2tkQzBGSXFn?=
 =?utf-8?B?WTlxb2I2TUhteExIYTE1amNXUndnQmhoUzhPdTkybXFKMG5MZm9vQktlQ29m?=
 =?utf-8?B?OTZqMi9LbUtrcWw4S2NhWEhsb05CVHJuZ3d2WDJ1cDlBMUp0ZGF2VGtzWmJZ?=
 =?utf-8?B?dExpbWlGQ2I4UllMZ3RGNXE4eGZSVHR0QWs4SnFDYUNaTVRGREptMHVKNThp?=
 =?utf-8?B?N0VQRkswRmRxTHIxZ0VPWVlhQndibUZPRjJTekxkMzRxdFRrTHZZcVE5Sk4x?=
 =?utf-8?B?bFNHLzJKWTB2dG9SZExYajdzVGhVQlR2akZWcVBIMVZoMXJxZDZ0Q211OVJ5?=
 =?utf-8?B?V3I3U3cyTnRNUzBXa1lTNmZJd005ZktyWE5JajJMSlNKcFVsa0EwdzFZY2Jq?=
 =?utf-8?B?MXFoNlphNVUzVnpBbFkxQlZTMjNVOFNzNXIrZnh0aitMY3VqZk9PRG5udXFT?=
 =?utf-8?B?U3JkUkpaKzVtdGJjUmdSZlZ1Z2RwTlJDMFM0TmZ3YmUrQ3N3YmdYSEhRYzht?=
 =?utf-8?B?TVhCOWZ6WVdsbFJudy9XMmlNQjR6dklqSFBqOFVrb0JHeW9YcFEySDVETVFV?=
 =?utf-8?B?N3RTSUQrNnhibVZVZHJxakdkMit5MDdsdnp5dVRYS0FoNDk5U0RaendmUnBi?=
 =?utf-8?B?bDhROHVnNk9QRnVpa3BRc3kzWXhmV2o2eHpidlRVYlMrbTJIZXRvajBvSzYr?=
 =?utf-8?B?MzZNeHF0WUlPdDcwZE5MZThIdjdoYjhpbDlPMnJFQy9NajJpYkhGc1ZNOGFo?=
 =?utf-8?B?eEZHa0V0NzZZTUs5alB6SVVzSjlzSDBZSjI5cEI0M1UvSlRDOTlZNEpVb2RY?=
 =?utf-8?B?eGE5SHNYNi9FU3RjUzQ4LzRpWWhTZXh4eHhIN0xpWDNYQ3F6ZEVVemxpNkFF?=
 =?utf-8?B?YXJFVzFWQjMwVUJPSnBiSGlobUtQcnNGS3NESUc2aWdRc0ZEWk96bVp4OXpW?=
 =?utf-8?B?ZVVGUWdOeU9yeWdwbDZCc3NjajhhOHhyaVM1NE80b2lKS2MvUC9yMmpmOGlN?=
 =?utf-8?B?RUlrYmpCenl6dFR6MmxJQVZ6TDdBRExhbStzTlRlbFJxalhIalFNWFROUGhy?=
 =?utf-8?B?QUhuSWw3N2dtUGNuN2NMTGZLcnNOVGdRdUlsTE5zVVlIOWRVT1ZQeGQwdEpI?=
 =?utf-8?B?VUVZOTNYS3pEZUFLblZEcXlqUFphUWF5V1BKWjRUa3o2UmVwNDFPbzhDZ0Vz?=
 =?utf-8?B?aEZLYnVTNW1yakRjd3BOTlRGZkdhcERMQ2QySzUvYSs0Ym5nOWJ1eVBIVTU1?=
 =?utf-8?B?Q01XUktKVHM3S0F1WEx6dzlnN0sxQnkyWjRtSWZLb0dUZWRaTDRETGpsTWF6?=
 =?utf-8?B?WU5wbXBOM09tTGdMeUlTdTI4S3QyaTV1RE1TMzFOckhvZ1RNVnJvWWpFK1Vs?=
 =?utf-8?B?Y2tmMWZBeUI0dFNqRE9RZVJJeHlWQURVdlpTamluMm1yOVJmSUVyNm5sTUNT?=
 =?utf-8?B?dnQrR2VjYzAxd2poN01GdVNaSHM3eE1GRHo0eU9GTm5HMWg4OW5USWNYR0xH?=
 =?utf-8?B?NWJPd3dlZDRkRStCUWJBcDBrUXpCZG9mSDUrWFBsVlExSVRkS3YrQWJHa1VQ?=
 =?utf-8?B?STlYQTZOcjZjLzRIMHBjSUtvSCticktEelZ1QjNDSmIySnI4WGlUNGRoS2xT?=
 =?utf-8?B?K2pMcG1LcXpEdlRrV2JEUE42Z3l3Q3JVMnFabGtUS1BDeEovZFk1R2ZyM1Y5?=
 =?utf-8?Q?KTYIJXlbtsb1lmjxjGrRBxuNx?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 5cc8f467-4ada-492a-c9aa-08dc3f040799
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5978.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Mar 2024 00:09:32.1531
 (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: CqDFVMyhTEJwJdWBiXclVl9KYY6MZUoToirklfbNdlJwYeyd9Wsbjd59vuoZuF42tSdhkSX7pe/U2ULAybJEyw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB5084
X-OriginatorOrg: intel.com


On 8/03/2024 6:10 am, Kirill A. Shutemov wrote:
> On Thu, Mar 07, 2024 at 11:49:11AM +1300, Huang, Kai 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.
> 
> Unmodified drivers gets their MMIO regions mapped with ioremap() that sets
> shared bit, unless asked explicitly to make it private (encrypted).
> 

Thanks for clarification.  Obviously I had bad memory.