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 E1060C7EE23 for ; Thu, 8 Jun 2023 14:07:39 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3179E6B0072; Thu, 8 Jun 2023 10:07:39 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2C7C06B0074; Thu, 8 Jun 2023 10:07:39 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1B6D36B0075; Thu, 8 Jun 2023 10:07:39 -0400 (EDT) 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 0C0F06B0072 for ; Thu, 8 Jun 2023 10:07:39 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id AA2471401D1 for ; Thu, 8 Jun 2023 14:07:38 +0000 (UTC) X-FDA: 80879758596.13.588F67C Received: from mga06.intel.com (mga06b.intel.com [134.134.136.31]) by imf06.hostedemail.com (Postfix) with ESMTP id 855C118014C for ; Thu, 8 Jun 2023 14:06:29 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=CHKyf5gX; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf06.hostedemail.com: domain of dave.hansen@intel.com designates 134.134.136.31 as permitted sender) smtp.mailfrom=dave.hansen@intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1686233190; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=wZGkjZnWNn3OX2y2Ra2UYeMx4AYNUY6V4Y5Gk2CQlqQ=; b=SagNEyAPm0wtPAe/Jq9+gO69nuKbFAaDdQH5SS/4jjWiGhqrCjmsXk/KDZWNLU7W/23DMX YAXV6nUKBTYn90IarzZPuD2BLWM/f+VSmwmWXftoMe9iRoazMhPAnEmJCtc2p+BBtBOuHZ i2nNjezv1jDZOT5GBoIt98JqCM47F6k= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=CHKyf5gX; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf06.hostedemail.com: domain of dave.hansen@intel.com designates 134.134.136.31 as permitted sender) smtp.mailfrom=dave.hansen@intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1686233190; a=rsa-sha256; cv=none; b=WW5DD2Mode30FrU18PjzIozIRnnhboJXDAzkwcrZCoqXtwIawHxXRzVy/PijzxwLAbrhIs cDOA3dDrz5S2OhUlq5o7nWCX7oXJDX0wjDzRVMJBPPDk3rnaQMH0y+2di6yGvC9X95VRNX Nwrr6SvgfenEji1TapJiH2HCYI+fuJw= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1686233189; x=1717769189; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=TmBDcLd5Xfp9VvdnTTxpDHAil2WXF25OsQyhgty2URs=; b=CHKyf5gX+YXq+MtXMakUER1eLeB7p7yfBSDTrk4bj/H7S6+AQGlG5QPq uV+5xhfXMtN/e2pIVY8dEiVtEHaRiatze9CaFCZKOmBTFvTF6bZXuerSu BoQQ3FU0v4kqP3B7yxjlasUZc9ncLjfajFaFsQ865mrh7ocQgz6OgvlCo /lDfxWCwtSLa4bxNo9s/+2pDqYWrokYBAuaYlczH7MtJp8WGyYV2BhgX/ qpltgaH9urVLNL6lJ0zQUu45ukh9ErB2UK/vrpZ0V2vm2AScOehTWBC5l tMIrA6ygyHbix3CUCxXRJlY4387hZiKqua03QVW4EFBgBvYtBZfdMMF9K w==; X-IronPort-AV: E=McAfee;i="6600,9927,10734"; a="420887666" X-IronPort-AV: E=Sophos;i="6.00,226,1681196400"; d="scan'208";a="420887666" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Jun 2023 06:43:31 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10734"; a="854354478" X-IronPort-AV: E=Sophos;i="6.00,226,1681196400"; d="scan'208";a="854354478" Received: from swalker-mobl1.amr.corp.intel.com (HELO [10.209.22.184]) ([10.209.22.184]) by fmsmga001-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Jun 2023 06:43:30 -0700 Message-ID: Date: Thu, 8 Jun 2023 06:43:29 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: Re: [PATCH v11 07/20] x86/virt/tdx: Add skeleton to enable TDX on demand Content-Language: en-US To: "Huang, Kai" , "kvm@vger.kernel.org" , "linux-kernel@vger.kernel.org" Cc: "Luck, Tony" , "david@redhat.com" , "bagasdotme@gmail.com" , "ak@linux.intel.com" , "Wysocki, Rafael J" , "kirill.shutemov@linux.intel.com" , "Chatre, Reinette" , "Christopherson,, Sean" , "pbonzini@redhat.com" , "tglx@linutronix.de" , "Yamahata, Isaku" , "linux-mm@kvack.org" , "peterz@infradead.org" , "Shahar, Sagi" , "imammedo@redhat.com" , "Gao, Chao" , "Brown, Len" , "sathyanarayanan.kuppuswamy@linux.intel.com" , "Huang, Ying" , "Williams, Dan J" References: <21b3a45cb73b4e1917c1eba75b7769781a15aa14.1685887183.git.kai.huang@intel.com> <963deb7f6bcadbf2848ef54540ba1758b43731d6.camel@intel.com> From: Dave Hansen In-Reply-To: <963deb7f6bcadbf2848ef54540ba1758b43731d6.camel@intel.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 855C118014C X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: ppkzadx46h31ftqu4w4hgkuarh3hxs8y X-HE-Tag: 1686233189-804240 X-HE-Meta: U2FsdGVkX191q/zYnFPNDkjxvPqgynF1tZYS2QsfLksbPRq7zlvGWscip4bgvKVyk+Amy+vz1+oCroSh3QU99O5SFyToHneFvgRc+IFsG8Ts4IA9WpX7EDNQc2iAOjzqpDMHMEje90ntB+Bj4qEeWMEa5DWXAH/WWIaCFBmYs42yNx3VFNnHIlX73XhlTNKMeIOa4OM81tL4boZNFROGBRYMqur4f+0FITckaqrR8Gv39Cd7QXa8B0wbUi3h3MIAEvuLSqncpRX6qWOiZ6SUgUyC5JwHLR6GCXKB5lJ9/5aYe57xuuicHA0w/YvxfwREhv1c+tnC/FR7KTFipdysbn9qgBRqPtRUCvMQyroWibCYz3bmf/0UXvsn5PSLm+cyUB8ubZpuZaXov5aEnkniGw+yerAiqs8FP907iwO0PwZZTuvJ6mG6uffeaSnc+rtCfqx0ifXYtt7pKbDj1J7D/yXArwXbF4wKEUnOn1EMQe6eMrLJ8FhZeZYAbiUGwIeDEqpilJhlkT1BEgOq7FOvBJjPfasKRe1dMiWODwI0hJ7ElNEe09VHuPlQZn/CCyW6l3geyR3Tdox0XJMgGboCNMQpsfUk1DGiY1VoEhg+HBXWORUiyyiuJ/a/SQO2EVSw+H7TgtcsuzrSLC332+1wxJJW8mrURLwWJZnUxBOtMetwDDod1Vj3r3rp3E6/zWzLmjgGDQd2BmHQVwEML+pz0RJj5/PaDQiyg3Ft2gvWuTq12nCf2I8ewhtsZAzstx/NaUp1vjPHNjVIbFcMv1JKp1Ai7wtukqiJem8MRI+uluMybgBJ+hvm7LFx5CvhehXiLVpl2iU3XwCSEhq0UnheYEurkWA+mfSvYKt4FUHY9of0plu0/VUJ55GWygJ50am/DN0kEO/jT3as9Vx0ItCWK8aeYaAreGFY8TC4GCc9lPqo+3xkEBeWdGyve+sGoWTPzpEDWT6v+/PnEDVy/Bq kWVX5mgY NsP7vrDO5gplM6TLk5BM0Xc4BSWgqBeGrxD+cW+Vu7ipobWS3Fxzfj4iM0bH9CaZR2J7BZMfO9VYyBvh6wJmtXZRailyshvEBYqk90VgsDrSuUwMvSLSkToxL3x8hgPzzxmoiX7bHXuxi6Y5yWKfEtRltefuLhc6MwzOHCWMWqql2J5fgbDbd0h3dJYW1S+G4sS9h 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: On 6/7/23 19:10, Huang, Kai wrote: > On Wed, 2023-06-07 at 08:22 -0700, Dave Hansen wrote: >> On 6/4/23 07:27, Kai Huang wrote: >> ... >>> +static int try_init_module_global(void) >>> +{ >>> + unsigned long flags; >>> + int ret; >>> + >>> + /* >>> + * The TDX module global initialization only needs to be done >>> + * once on any cpu. >>> + */ >>> + raw_spin_lock_irqsave(&tdx_global_init_lock, flags); >> >> Why is this "raw_"? >> >> There's zero mention of it anywhere. > > Isaku pointed out the normal spinlock_t is converted to sleeping lock for > PREEMPT_RT kernel. KVM calls this with IRQ disabled, thus requires a non- > sleeping lock. > > How about adding below comment here? > > /* > * Normal spinlock_t is converted to sleeping lock in PREEMPT_RT > * kernel. Use raw_spinlock_t instead so this function can be called > * even when IRQ is disabled in any kernel configuration. > */ Go look at *EVERY* *OTHER* raw_spinlock_t in the kernel. Do any of them say this? Comment the function, say that it's always called with interrupts and preempt disabled. Leaves it at that. *Maybe* add on that it needs raw spinlocks because of it. But don't (try to) explain the background of the lock type. >>> +int tdx_cpu_enable(void) >>> +{ >>> + unsigned int lp_status; >>> + int ret; >>> + >>> + if (!platform_tdx_enabled()) >>> + return -EINVAL; >>> + >>> + lp_status = __this_cpu_read(tdx_lp_init_status); >>> + >>> + /* Already done */ >>> + if (lp_status & TDX_LP_INIT_DONE) >>> + return lp_status & TDX_LP_INIT_FAILED ? -EINVAL : 0; >>> + >>> + /* >>> + * The TDX module global initialization is the very first step >>> + * to enable TDX. Need to do it first (if hasn't been done) >>> + * before doing the per-cpu initialization. >>> + */ >>> + ret = try_init_module_global(); >>> + >>> + /* >>> + * If the module global initialization failed, there's no point >>> + * to do the per-cpu initialization. Just mark it as done but >>> + * failed. >>> + */ >>> + if (ret) >>> + goto update_status; >>> + >>> + /* All '0's are just unused parameters */ >>> + ret = seamcall(TDH_SYS_LP_INIT, 0, 0, 0, 0, NULL, NULL); >>> + >>> +update_status: >>> + lp_status = TDX_LP_INIT_DONE; >>> + if (ret) >>> + lp_status |= TDX_LP_INIT_FAILED; >>> + >>> + this_cpu_write(tdx_lp_init_status, lp_status); >>> + >>> + return ret; >>> +} >>> +EXPORT_SYMBOL_GPL(tdx_cpu_enable); >> >> You danced around it in the changelog, but the reason for the exports is >> not clear. > > I'll add one sentence to the changelog to explain: > > Export both tdx_cpu_enable() and tdx_enable() as KVM will be the kernel > component to use TDX. Intel doesn't pay me by the word. Do you get paid that way? If not, please just say: Export both tdx_cpu_enable() and tdx_enable() for KVM use.