Received-SPF: pass (google.com: domain of linux-kernel+bounces-152701-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Mon, 22 Apr 2024 11:34:18 +0800
From: Yan Zhao <yan.y.zhao@intel.com>
To: <isaku.yamahata@intel.com>
CC: <kvm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<isaku.yamahata@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>,
	<erdemaktas@google.com>, Sean Christopherson <seanjc@google.com>, Sagi Shahar
	<sagis@google.com>, Kai Huang <kai.huang@intel.com>, <chen.bo@intel.com>,
	<hang.yuan@intel.com>, <tina.zhang@intel.com>, Binbin Wu
	<binbin.wu@linux.intel.com>
Subject: Re: [PATCH v19 058/130] KVM: x86/mmu: Add a private pointer to
 struct kvm_mmu_page
Message-ID: <ZiXautOkEweWfUL0@yzhao56-desk.sh.intel.com>
Reply-To: Yan Zhao <yan.y.zhao@intel.com>
References: <cover.1708933498.git.isaku.yamahata@intel.com>
 <9d86b5a2787d20ffb5a58f86e43601a660521f16.1708933498.git.isaku.yamahata@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <9d86b5a2787d20ffb5a58f86e43601a660521f16.1708933498.git.isaku.yamahata@intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?SL9PeiRrLaI8vDvP5J7qhWXqiHl191a4Y4LZdf92OURL2xxjBwcmrwOQpbp2?=
 =?us-ascii?Q?btv53LsGUlGWYeLiIIFFKJXEiOiDWromO6OqYxjYIeVC46/dbcC2iMJd4Ywa?=
 =?us-ascii?Q?bdy7xFTTpQo3MbHah8Xkclij62BaXxvctk7tfPI0AiJO59G7qGUxT2BMQraP?=
 =?us-ascii?Q?tdJvmGzggsMol1X9vJhmn8vk9fdmzorrkEe3ljSvzbbytJHEOZFxxXL7bemF?=
 =?us-ascii?Q?3SXvMswGXDMCcIbj9u5qT1vT3ywg4fW3RX4eJdWmBpHbhnaLr+d+H8APuCAS?=
 =?us-ascii?Q?D6irEVZw2ttMx0RsOOfM781suPU7GOcghSvs5pXuVP2BXJKYc4eO0TV38scT?=
 =?us-ascii?Q?+kCI2a/si8oMO/u4FalGP0uQOA25w7+bfYo5MF8c3EuunENxhtIQUDXKZX6Z?=
 =?us-ascii?Q?Zeut0IADH7Yrea0UI1zbRpfkPUVCB2QK8XNurQeJuyliLAjqR5v8tQ3ZQD+Y?=
 =?us-ascii?Q?oHYFOUyKjM0i20JCpjESu17wkhWXzxTEcZVrSUU1IVbVTNl57WCYxgumAcWP?=
 =?us-ascii?Q?YHBo7XOzuni3C9KmUPsecAVkO9JZb34axmZ628l1XX6IVpfRl+oAiWuyBiVR?=
 =?us-ascii?Q?kmnhJyH6dZrzFSGKE+er4745IoAfkeatEgealc7TwSQwEPstxc4PptFQnrkg?=
 =?us-ascii?Q?fjFqi5b9YHl5i4qrJoqsvWZHg96lD9xA+zHGkwa0Eo1iGMYy3OfuwemKvRkT?=
 =?us-ascii?Q?mkrCbkLMCUK1d2G0IBaTx/SQ2Ivkaw5gVS20lkFG6XCnu4ckeq7rBSEf3QKv?=
 =?us-ascii?Q?6c1TJPr0g++Ay9ZbovgrnVeyImQPC//60dIl5aEWy4k/KVI6CpAhIjRio4O+?=
 =?us-ascii?Q?HL1MAHoVNUSGr9rapT1vhTOOyodgW2mslhVEomqXlPbrL/DeyiQdJsS3nGjQ?=
 =?us-ascii?Q?z5MwvQvAx5Z6oXOTIkqvonKlYKRq7lSCNSXMXkXWDsFQ7h6QeN7WojWpoi8/?=
 =?us-ascii?Q?sok3VEHSv38KxrE0TFeUky3Hmu3dCCB/rYLIZ2sjPixElKnrtWOn55dmlPZp?=
 =?us-ascii?Q?LnxPiwoVCjNfBcsqmXXnI8SSC4tY8sBK1ojeacmaTGegbYuw8+weASobpeMf?=
 =?us-ascii?Q?Ih77RRYJqx5wWfwdXSm7YR12BNXn0w+gGJD8ahgx+03oYzvcGSyCpjXy1f6u?=
 =?us-ascii?Q?PR99Y3ijTvCKOFtOvAL1uJ6bj0SY5I1ZUkPpXOnFvL8vasxSdq7TPPJJTP7g?=
 =?us-ascii?Q?SNoLwZEEA2yQb6jxfsZUTPKNrJaRaTxYIhlD9UXqchDtSehYCYgUL3IANz3k?=
 =?us-ascii?Q?crLLqtpc7qJEoDBcRgt1FzC6yuUizzWiEnzR1m0NI/hGnSbHsLn5C7OngRAE?=
 =?us-ascii?Q?LjwVXc7z6eRxNmmt4KnecxGqotTYZUmr0ZroX4SbLuS2ZOLQ5ERMvPC7XWJ+?=
 =?us-ascii?Q?9ulBZ0/G2dhCbVQSSI8O1neMrbcAWqA/+CZNl6CBs2ZseVVSFDrSowoLgdXW?=
 =?us-ascii?Q?y/5cxQOhryYswcWSfIHy08amFdD+1uzhOtf5UXHMc57Ghhpf+k7YN28a0QzS?=
 =?us-ascii?Q?p2/9qKJHbNfX77fAy2uGy/bvC6o3PsBucF302r0lzNYIvduqFvALWbjPgAsl?=
 =?us-ascii?Q?N7OcNTK03ug4Cu1tfRR05h4ae7ze+eJKltzoM0/6?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 9281dbfc-6294-4ced-1a26-08dc627d2b9b
X-MS-Exchange-CrossTenant-AuthSource: DS7PR11MB5966.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Apr 2024 03:34:52.3191
 (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: 50UMemuLAzPc7Hf6/TNM30//fJo01rfm9NOxeMyUMiaVLv0kSik+CNNBlt02ajQQz5McjB5eRwCikvyiQ3fOFw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR11MB4929
X-OriginatorOrg: intel.com

On Mon, Feb 26, 2024 at 12:26:00AM -0800, isaku.yamahata@intel.com wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
> +static inline void *kvm_mmu_private_spt(struct kvm_mmu_page *sp)
> +{
> +	return sp->private_spt;
> +}
> +
> +static inline void kvm_mmu_init_private_spt(struct kvm_mmu_page *sp, void *private_spt)
> +{
> +	sp->private_spt = private_spt;
> +}
This function is actually not used for initialization.
Instead, it's only called after failure of free_private_spt() in order to
intentionally leak the page to prevent kernel from accessing the encrypted page.

So to avoid confusion, how about renaming it to kvm_mmu_leak_private_spt() and
always resetting the pointer to NULL?

static inline void kvm_mmu_leak_private_spt(struct kvm_mmu_page *sp)
{
	sp->private_spt = NULL;
}

> +
> +static inline void kvm_mmu_alloc_private_spt(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp)
> +{
> +	bool is_root = vcpu->arch.root_mmu.root_role.level == sp->role.level;
> +
> +	KVM_BUG_ON(!kvm_mmu_page_role_is_private(sp->role), vcpu->kvm);
> +	if (is_root)
> +		/*
> +		 * Because TDX module assigns root Secure-EPT page and set it to
> +		 * Secure-EPTP when TD vcpu is created, secure page table for
> +		 * root isn't needed.
> +		 */
> +		sp->private_spt = NULL;
> +	else {
> +		/*
> +		 * Because the TDX module doesn't trust VMM and initializes
> +		 * the pages itself, KVM doesn't initialize them.  Allocate
> +		 * pages with garbage and give them to the TDX module.
> +		 */
> +		sp->private_spt = kvm_mmu_memory_cache_alloc(&vcpu->arch.mmu_private_spt_cache);
> +		/*
> +		 * Because mmu_private_spt_cache is topped up before starting
> +		 * kvm page fault resolving, the allocation above shouldn't
> +		 * fail.
> +		 */
> +		WARN_ON_ONCE(!sp->private_spt);
> +	}
> +}