From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id B0F4AD29FBB for ; Wed, 6 Nov 2024 08:12:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CAC986B0085; Wed, 6 Nov 2024 03:12:53 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id C5ADB6B0088; Wed, 6 Nov 2024 03:12:53 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B22C16B0089; Wed, 6 Nov 2024 03:12:53 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 94B856B0085 for ; Wed, 6 Nov 2024 03:12:53 -0500 (EST) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 24073AD378 for ; Wed, 6 Nov 2024 08:12:53 +0000 (UTC) X-FDA: 82754952504.13.1CDCF16 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.8]) by imf15.hostedemail.com (Postfix) with ESMTP id 229D7A0006 for ; Wed, 6 Nov 2024 08:12:14 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=mNyFfZHr; spf=pass (imf15.hostedemail.com: domain of ying.huang@intel.com designates 192.198.163.8 as permitted sender) smtp.mailfrom=ying.huang@intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1730880634; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=kiEzh0avh+qXkdHbdesqr8hHRaTRlRT5fkr4cDcYilo=; b=205CQhZAuwGZjnAcxxGOqD3isRECsQmIIpubmEm9YG8bk7zdqxGOhLp+Rnqmm9MFubb7YC KeAK9mBsJIlNHM1ehwTc2EN7lNmnSjSLRSAQ+qSUC3rtLV5S1X1/QCCYd72jYlW756JhjH Teb3rxNmiqFuq22q9hePIuYhhmS7qak= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1730880634; a=rsa-sha256; cv=none; b=fLn+/zTW8sx6wACFm4aWVu1AS/nqQSSXls3D8mnPYrlvwmsVfapmMa/CAWyHWMs0fpQvFZ BiGX7kwPgJ58aOHdEXEgaZ2F7ggBGsaiLXhIPsAEjbEf7dYyY/lHz72MhN5mFh8kRh6wEA UVciZ2idjeh9HXwYiOgHhhGiLFDweJ4= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=mNyFfZHr; spf=pass (imf15.hostedemail.com: domain of ying.huang@intel.com designates 192.198.163.8 as permitted sender) smtp.mailfrom=ying.huang@intel.com; dmarc=pass (policy=none) header.from=intel.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1730880770; x=1762416770; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version; bh=LAjBJIrf+QHNvH66aqJrh4mDtpWYTGpgIATIK51lkbk=; b=mNyFfZHrT/gOBfxZads1tHw729BomzL1y41xHdfgw79qoMj4N0AQKtJP +O1AR5VTmsxXklu/Uze8IKiY196iI30nG/3LdMXN4nHwtvmI0Qy35Pheu kf3Bi2mBp3Xrw7R7jrpscJy3WTjfKC4B3djzZ2m/9U75ikSdpwdI6ITPx DJlunUAl7rPmVAVPzwELoFE9p8ZT9wHMQluBSH5lTBk2NuCWWQuF4khh7 ov52dRrHfDf1s7+U3L7M+IBiNQr1THtkvaRJAJThNMojmvHosaXu4O+JU 16mqk/++kNyn2l+tgQA5FVg9/K3+SG88OnWOnk45l6mxngNmyDNZdx24f w==; X-CSE-ConnectionGUID: JRdqPScYRyCLLzVOQdNLQA== X-CSE-MsgGUID: qVzuViNlSS6qwUsTwwxxjg== X-IronPort-AV: E=McAfee;i="6700,10204,11247"; a="48181095" X-IronPort-AV: E=Sophos;i="6.11,262,1725346800"; d="scan'208";a="48181095" Received: from orviesa007.jf.intel.com ([10.64.159.147]) by fmvoesa102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Nov 2024 00:12:48 -0800 X-CSE-ConnectionGUID: uwapckj2T5ejfV4+VNGkKA== X-CSE-MsgGUID: udN9Jek8SPSeLJvI8gkZqw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.11,262,1725346800"; d="scan'208";a="84801241" Received: from yhuang6-desk2.sh.intel.com (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.238.208.55]) by orviesa007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Nov 2024 00:12:45 -0800 From: "Huang, Ying" To: Dave Hansen Cc: Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , "Kirill A . Shutemov" , , , , , Dan Williams , "Kai Huang" , David Hildenbrand , "Oscar Salvador" , "H. Peter Anvin" , "Andy Lutomirski" Subject: Re: [PATCH -V3 RESEND] x86, tdx, memory hotplug: Check whole hot-adding memory range for TDX In-Reply-To: <702d9dbd-fb7d-44d0-a352-e78adf92254e@intel.com> (Dave Hansen's message of "Tue, 5 Nov 2024 10:34:59 -0800") References: <20241031085151.186111-1-ying.huang@intel.com> <702d9dbd-fb7d-44d0-a352-e78adf92254e@intel.com> Date: Wed, 06 Nov 2024 16:09:11 +0800 Message-ID: <8734k47pi0.fsf@yhuang6-desk2.ccr.corp.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 229D7A0006 X-Stat-Signature: emezkgsxzkewx4pouji9jo8mqkzy63bw X-HE-Tag: 1730880734-394714 X-HE-Meta: U2FsdGVkX1/QXqZD/fgLDJ+/Xi6gckE2hC1g1XGQ9Ds9Qsuw6JWcBx7nK2fuVPB7G0etrWnl4iXlSFFIDYuBGOzvtm14fscEsP8uYxEdxgHU0NV8VGH/UZZnQjQFpC8V7Iqm/JdizAlK51spKKQ1EoLLv0MvBVepiP4o7PW4qDKceHr36bWorEnJwTB8ffImtOJZfZBatRcODRgFEBq/V67hkprNx6f1CoDhhjxVBhWMNy9ympI5H8dhf335qWBpUVvrtspd5zYYGd6lAdUfxoOetH8m6iwU1nnFUKMcFxU22fUKcdl6NY4sQqvuN0jKYgN96N3yJvVQnwL3TWndolKctVHndKVwPN0d04NQiyIJz5w7ffcuCeTJAMm2jVkFjonIgkGpI1fzHZblmCRTbMgQgRNJGhd5YFYws6ZKwzTZ/TSSfJaYtSkTEMm0ghrbmHHvwX9SZj/GLE3wHHdVTBayVrVz1HwOLp2FqDIn4+HheiwbJKSgUqGVeDbBeeriz8bhAIhWavrnKftEjjhq2SvnWhpfdgDXAnJ/DHdNFPbFUnE+UxaycLxPJgtcPD3+ClPIUhPSmhR7bUWQ5PQogIJht1CcTqKeuMAQw7YmupeOEIWe+0hp3trkA6GllPgfz7/t98Hub3SCDxl4wX7sXAyjBA3Ll38WI20pIj81GgkgryOPeJEFFJJC1mNWvrjE5TAX5DOdUwJMvOoj7NKk6gyJTuDurFK+Dk3tOIBEvD7B/c0mhcmwv8Puhxxr7WISC+d4Mp8cT3xHg4Q1d9e3O3llH8qrSTHeb3rPKyHhkb/JZC8M2ypf/8hfuDv1iBHCG9mYBxdRHKE6DxBvATbhAcuZiTvE66IVRmjBdMTPl932r8a1EXb2e6PyKFbV/uPDAWCxFrt/roMqjgOLhYDkSn+NyXf8kEeXdWthdL/07njv2cV+DPq69QS/kLXabdpHNBJogBffIMaTso+Jv2I 4E/eyEhI JZrPiO0wGSdh0Pj3h2bDaz11yChgiJzqvSaT587StINkVZOln3RJ68RsaGIF+2y8yB0o9lPmdXCH23ryNzalJTb7PosIvb4eqg9wRC1rd1sV5R//un/bbV7rMuKyDpAB7DKH1hUjgYEuxpF/xrU4SB+41Hr/scYG8gR/63u77GHhbRhLl7wx8lTa/rvSslfLnU5BNZtWyIk4af2L5T3G1eN4IloXS/T7FZbtj X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: Hi, Dave, Thanks a lot for your detailed review! Dave Hansen writes: > First and foremost, this touches x86 and core mm code, but it seem to > solidly lean on being an x86 thing. If anyone thinks this isn't x86 > tree material, please speak up. > > On 10/31/24 01:51, Huang Ying wrote: >> Therefore, this patch checks the TDX compatibility of the whole > > Please zap the "this patch" nomenclature. It showed up in a couple of > places. ChatGPT is actually pretty good at this kind of stuff and using > imperative voice. Sure. Will do that. >> hot-adding memory range through a newly added architecture specific >> function (arch_check_hotplug_memory_range()). If this patch rejects >> the memory hot-adding for TDX compatibility, it will output a kernel >> log message like below, >> >> virt/tdx: Reject hot-adding memory range: 0xXXXXXXXX-0xXXXXXXXX for TDX compatibility. > > I think this is more clear and much more succinct: > > virt/tdx: Rejecting incompatible memory range: 0xXXXXXXXX-0xXXXXXXXX Yes. This looks better, will use this in the next version. > >> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h >> index eba178996d84..6db5da34e4ba 100644 >> --- a/arch/x86/include/asm/tdx.h >> +++ b/arch/x86/include/asm/tdx.h >> @@ -116,11 +116,13 @@ static inline u64 sc_retry(sc_func_t func, u64 fn, >> int tdx_cpu_enable(void); >> int tdx_enable(void); >> const char *tdx_dump_mce_info(struct mce *m); >> +int tdx_check_hotplug_memory_range(u64 start, u64 size); >> #else >> static inline void tdx_init(void) { } >> static inline int tdx_cpu_enable(void) { return -ENODEV; } >> static inline int tdx_enable(void) { return -ENODEV; } >> static inline const char *tdx_dump_mce_info(struct mce *m) { return NULL; } >> +static inline int tdx_check_hotplug_memory_range(u64 start, u64 size) { return 0; } >> #endif /* CONFIG_INTEL_TDX_HOST */ >> >> #endif /* !__ASSEMBLY__ */ >> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c >> index ff253648706f..30a4ad4272ce 100644 >> --- a/arch/x86/mm/init_64.c >> +++ b/arch/x86/mm/init_64.c >> @@ -55,6 +55,7 @@ >> #include >> #include >> #include >> +#include >> >> #include "mm_internal.h" >> >> @@ -974,6 +975,11 @@ int add_pages(int nid, unsigned long start_pfn, unsigned long nr_pages, >> return ret; >> } >> >> +int arch_check_hotplug_memory_range(u64 start, u64 size) >> +{ >> + return tdx_check_hotplug_memory_range(start, size); >> +} >> + >> int arch_add_memory(int nid, u64 start, u64 size, >> struct mhp_params *params) >> { >> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c >> index 4e2b2e2ac9f9..f70b4ebe7cc5 100644 >> --- a/arch/x86/virt/vmx/tdx/tdx.c >> +++ b/arch/x86/virt/vmx/tdx/tdx.c >> @@ -1388,36 +1388,37 @@ static bool is_tdx_memory(unsigned long start_pfn, unsigned long end_pfn) >> return false; >> } >> >> -static int tdx_memory_notifier(struct notifier_block *nb, unsigned long action, >> - void *v) >> +/* >> + * We don't allow mixture of TDX and !TDX memory in the buddy so we >> + * won't run into trouble when launching encrypted VMs that really >> + * need TDX-capable memory. >> + */ > > No "we's" please. > > I'd probably explain it like this: > > /* > * By convention, all RAM in the buddy must be TDX compatible whenever > * TDX is enabled. This avoids having to do extra work later to find > * TDX compatible memory to run VMs. Enforce that convention and reject > * attempted hot-adds of any TDX-incompatible ranges. > * > * Returns 0 to pass the checks and allow the hot-add > * Returns -ERRNO to fail the checks and reject the hot-add > */ This looks better, Thanks! Will use it in the next version. >> +int tdx_check_hotplug_memory_range(u64 start, u64 size) >> { >> - struct memory_notify *mn = v; >> - >> - if (action != MEM_GOING_ONLINE) >> - return NOTIFY_OK; >> + u64 start_pfn = PHYS_PFN(start); >> + u64 end_pfn = PHYS_PFN(start + size); > > Nit: ^ please vertically align those Sure. Will do that in the next version. >> /* >> * Empty list means TDX isn't enabled. Allow any memory >> - * to go online. >> + * to be hot-added. >> */ >> if (list_empty(&tdx_memlist)) >> - return NOTIFY_OK; >> + return 0; > > The changelog also needs _some_ discussion of why the locking context is > the same between the old and new uses of this function and why this > doesn't need any locking _here_. Sure. Will do that in the next version. >> /* >> * The TDX memory configuration is static and can not be >> - * changed. Reject onlining any memory which is outside of >> + * changed. Reject hot-adding any memory which is outside of >> * the static configuration whether it supports TDX or not. >> */ >> - if (is_tdx_memory(mn->start_pfn, mn->start_pfn + mn->nr_pages)) >> - return NOTIFY_OK; >> + if (is_tdx_memory(start_pfn, end_pfn)) >> + return 0; >> >> - return NOTIFY_BAD; >> + pr_info("Reject hot-adding memory range: %#llx-%#llx for TDX compatibility.\n", >> + start, start + size); >> + >> + return -EINVAL; >> } -- Best Regards, Huang, Ying