Received-SPF: pass (google.com: domain of linux-kernel+bounces-180448-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <9f315b56-7da8-428d-bc19-224e19f557e0@intel.com>
Date: Thu, 16 May 2024 10:34:43 +1200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 04/16] KVM: x86/mmu: Add address conversion functions for
 TDX shared bit of GPA
To: Rick Edgecombe <rick.p.edgecombe@intel.com>, <pbonzini@redhat.com>,
	<seanjc@google.com>, <kvm@vger.kernel.org>
CC: <linux-kernel@vger.kernel.org>, <isaku.yamahata@gmail.com>,
	<erdemaktas@google.com>, <sagis@google.com>, <yan.y.zhao@intel.com>,
	<dmatlack@google.com>
References: <20240515005952.3410568-1-rick.p.edgecombe@intel.com>
 <20240515005952.3410568-5-rick.p.edgecombe@intel.com>
Content-Language: en-US
From: "Huang, Kai" <kai.huang@intel.com>
In-Reply-To: <20240515005952.3410568-5-rick.p.edgecombe@intel.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?cUp4L3p5MFhEM3lQYXdUd2ZBL2kxdHIvd0xNL2lFMzk1MldaUExaTnVkbGNF?=
 =?utf-8?B?R0g5Wit5M1F4SUVJT01wVmRDSnAxUGZwZXdFd2VYNEJTRUxYbjJhM3hDZ3Bh?=
 =?utf-8?B?a1d6VFcrTHRFSEtSTTMzUFNCcU5xVGdtekxqWVFtYVpRMmtMTlBKVDJEc3dq?=
 =?utf-8?B?K3JFYmR5NUdDUUtCV0JVZkRTeHJ4R3VMS3dxeklRY3pvd2xYdTRsbUNYTFBo?=
 =?utf-8?B?ckJJM3p1Nm0va2xlVmhrYk9NNDEzR3RZLzJ6eUw5aVZUOE1LRUgzWmVZUGZQ?=
 =?utf-8?B?SWFMTCtCYkMrUjBIMjBlMzZ1cGxoQWh1WmNOcHhDNFN4WmZpdllnYXRDSzhQ?=
 =?utf-8?B?cWFIdk9ocWtoT04yTFh4RXExeDFWZGs0eFJVc25TckIzWmIxRzB1Q0NmK2lo?=
 =?utf-8?B?enhaZEs3dHBXR24ycnJyY0g3OFBGRi9DUFdUZ3VpSURyMWQvWXNLbjBCVlFB?=
 =?utf-8?B?UndJcUFhRjZqTEx1NHVCWVZySGhoNlJIa3Q2c1N2a011VUZKTENraGNNT1JH?=
 =?utf-8?B?QXZ1S3NiU2lKeXBHeGp3WjNhZXZUTG5pTFMzYXRweDlxWURyVFBMK2FXQVNU?=
 =?utf-8?B?NUVnYnhJbFU0OHU2VElsNExsVmk0N204UXV1RkFqQllWSXQ4YStKUzJVa3NV?=
 =?utf-8?B?Rm9lc3JmZDF0ekx6Y243QmlwZEU2dVJwQ1VOeEVqN3BCZU9naWVpcmlNa0F2?=
 =?utf-8?B?bUJMWGdNVEozUEJNbGhGYzc4aXAyUGh6QXJwTVZBSGtPSTJYTis0TWtuelJj?=
 =?utf-8?B?VFh0U3dDcUc1NzdUZ3VqR3JXdURvc0pEU2c1c0RPNEI4MmtBNnJnUlBNc09l?=
 =?utf-8?B?MGJoQzcvaFljaWVBc3kySTQ1TVNMcXYwTjllaU9RYmxNa3llZkFnSG14d3FE?=
 =?utf-8?B?Nnh2MSt6N2JCVExkTVUwSVZSNjBGOGFWK3o5T1lxak8rL3VxM2JQb2dKOHpK?=
 =?utf-8?B?SEZ5TUdibGJKUWJUV2RJN0NOSVg3eWE5Q0JKb0NXc3g4RlJ2Y011WVphSmt6?=
 =?utf-8?B?VUMzVU1TNFMvNVFCT2c1aUJ6bFRRQU5rR2JPcnBaQnFhdmhJdTFQS0dzS29v?=
 =?utf-8?B?czNXZTNLaDdDU0RNZStzbnFYcTFZY2tUdmNMUytESXM1SVZLZUttNmZRT05G?=
 =?utf-8?B?WTNDYU5XcWpUV1BVb3hINC9tOWtrUGNvbndVZER4RGNjOHhVaTBpR2NCQ2t4?=
 =?utf-8?B?VlcvSzhHN2VxWXlOeE1UdGhWckJrcGNZSWRGOHlXM3FmRWUxQ1crRkJvYmh2?=
 =?utf-8?B?cU5TMmsxd0xCODhiK1M0djZLNUtweU5KRExlMFByUVpHKzhPMTZtOWlDNzdC?=
 =?utf-8?B?NlBaNUpQbzh6c0lLSjEwUFErVDI3VjRoOE93VHlKeVVEUDJuQng3cG1hck5R?=
 =?utf-8?B?Yy93azBYZnpibEJCZDJGdHkvQmhlUUQ4ZThrdmpYT0luT1N0Uy9zN0EzMTBa?=
 =?utf-8?B?c0JTbmd5WGlkaGRKVmIyU25iYlhoQTZRZnBkeWR1RDRFWkZNR043R3JrRGM0?=
 =?utf-8?B?ZzRJOTZydjFsUkdPd2FsQjg4ZlRkZnNQTWNtMEUwWjMvWWtDQWVueDdIK2Rj?=
 =?utf-8?B?VCtzeXhXYWpWNG91VGFMSnpTcnQ4RjVUL2R5a0dCek12cFpYZUc0WUYvbjlF?=
 =?utf-8?B?VElSQXZ6L3QzV0JtMEtTYmc5VmZzK2Q3bDd3aGNMQU51R0JxYXNMdklQeFNk?=
 =?utf-8?B?VS90Rk5QalBFQUNuOXlRQ2ZGSWJFcFdTWmVsMitNeU5pRDhITzR1NU5tNzNz?=
 =?utf-8?B?diszMGNNV2NtU0tnL0ZBNlBZRjNZVTNRY1ZnSURLM1d5VlBEWkxzZ1ZrS1Ft?=
 =?utf-8?B?Rit6NlJrMVo2TmVCOHNGN3V4aXc0ajBGcmdWZ25tU2p3cGV2UFF3dHgvQ2Vn?=
 =?utf-8?B?QWZEajVvaTZ1NHZKNFIxTWI0a0hGVGoyaXp3bEQ0SkFLUllsMzVrTXptaSt5?=
 =?utf-8?B?NktSRVcrUU40MW83bXdReGwwdEtJL2dMcVlQU0E3Z1VZYys0ZVdVMWxEejdK?=
 =?utf-8?B?ZjNRTXdGZ0o2NVB6aHhjNStyM2tna3NiUmVuRHhKMGUzSFJqRDV4ajk1dWxW?=
 =?utf-8?B?RWdtamY4UWxlWGFBcUhmS3EvS3Nuc2J4R0RHbkxTbC84cWlWNWhKQXNVaXFi?=
 =?utf-8?Q?XtmC3xZKM4kmdb/FhC2Jd4JN+?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 4985e809-e844-45ee-0f4c-08dc752f3ca7
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5978.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 May 2024 22:34:52.2751
 (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: bzLMGgkPDOnvS2Bk8BkAW51uvXw8WUalcVwDr1HWoU7iQ9DtV8xPaFivtd6tUNvZ+kFsjz8HQRbvPEjN6lynfQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA3PR11MB8046
X-OriginatorOrg: intel.com


On 15/05/2024 12:59 pm, Rick Edgecombe wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
> 
> Introduce a "gfn_shared_mask" field in the kvm_arch structure to record GPA
> shared bit and provide address conversion helpers for TDX shared bit of
> GPA.
> 
> TDX designates a specific GPA bit as the shared bit, which can be either
> bit 51 or bit 47 based on configuration.
> 
> This GPA shared bit indicates whether the corresponding physical page is
> shared (if shared bit set) or private (if shared bit cleared).
> 
> - GPAs with shared bit set will be mapped by VMM into conventional EPT,
>    which is pointed by shared EPTP in TDVMCS, resides in host VMM memory
>    and is managed by VMM.
> - GPAs with shared bit cleared will be mapped by VMM firstly into a
>    mirrored EPT, which resides in host VMM memory. Changes of the mirrored
>    EPT are then propagated into a private EPT, which resides outside of host
>    VMM memory and is managed by TDX module.
> 
> Add the "gfn_shared_mask" field to the kvm_arch structure for each VM with
> a default value of 0. It will be set to the position of the GPA shared bit
> in GFN through TD specific initialization code.
> 
> Provide helpers to utilize the gfn_shared_mask to determine whether a GPA
> is shared or private, retrieve the GPA shared bit value, and insert/strip
> shared bit to/from a GPA.

I am seriously thinking whether we should just abandon this whole 
kvm_gfn_shared_mask() thing.

We already have enough mechanisms around private memory and the mapping 
of it:

1) Xarray to query whether a given GFN is private or shared;
2) fault->is_private to indicate whether a faulting address is private 
or shared;
3) sp->is_private to indicate whether a "page table" is only for private 
mapping;

Consider this as 4) -- I also like to have a kvm->arch.has_mirrored_pt 
(or a better name) as I replied here:

https://lore.kernel.org/kvm/20240515005952.3410568-17-rick.p.edgecombe@intel.com/T/#m49b37658f03e786c6aa43719cbf748215170980d

So I believe we really already have enough mechanisms in the *COMMON* 
code for private page/mapping support.  I intend to believe the whole 
GPA shared bit thing can be hidden in TDX specific operations.  If 
there's really a need to apply/strip GPA shared bit in the common code, 
we can do via kvm_x86_ops callback (I'll review other patches to see).

And btw, I think ...

[...]

> +
> +/*
> + *			default or SEV-SNP	TDX: where S = (47 or 51) - 12
> + * gfn_shared_mask	0			S bit
> + * is_private_gpa()	always false		true if GPA has S bit clear

.. this @is_private_gpa(), and ...

> + * gfn_to_shared()	nop			set S bit
> + * gfn_to_private()	nop			clear S bit
> + *
> + * fault.is_private means that host page should be gotten from guest_memfd
> + * is_private_gpa() means that KVM MMU should invoke private MMU hooks.
> + */

.. this invoking MMU hooks based on @is_private_gpa() makes no sense, 
because clearly for SEV-SNP @is_priavate_gpa() isn't report the fact 
when the GPA is indeed private, and the MMU hooks should be invoked 
based on whether the faulting GPA is private or not, but not this 
@is_private_gpa().