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 3D633C3DA49 for ; Sun, 28 Jul 2024 11:16:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B44226B0082; Sun, 28 Jul 2024 07:16:58 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AF3B96B0083; Sun, 28 Jul 2024 07:16:58 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 993B26B0085; Sun, 28 Jul 2024 07:16:58 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 7AB296B0082 for ; Sun, 28 Jul 2024 07:16:58 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id DCE7BA0236 for ; Sun, 28 Jul 2024 11:16:57 +0000 (UTC) X-FDA: 82388909274.12.CC884E3 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.8]) by imf27.hostedemail.com (Postfix) with ESMTP id 2D5194001B for ; Sun, 28 Jul 2024 11:16:54 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=GXov0t+i; spf=none (imf27.hostedemail.com: domain of binbin.wu@linux.intel.com has no SPF policy when checking 192.198.163.8) smtp.mailfrom=binbin.wu@linux.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=1722165346; 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=Z2TgBd9yqLpeDnsgSDiQyVsLdgmCdcZy6IpAGOm/Low=; b=hn1zXk2JrCmGnTtS1aXyDEeI92ymb8JtsukptzVcwIMvLU/V7TZcHEPiIAz7rjr+6Onvfi VUUs5NMxTjzP3RxrtLURYiIqRrDH2ksGE6AKakNSdIp5uk1uiJ6B/hfxHJSoBsh3eQhe3l OSs8cd3zbg9O02kT0ldvpRGurmWh64U= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=GXov0t+i; spf=none (imf27.hostedemail.com: domain of binbin.wu@linux.intel.com has no SPF policy when checking 192.198.163.8) smtp.mailfrom=binbin.wu@linux.intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722165346; a=rsa-sha256; cv=none; b=sfT4rzrc12TN7xIcA1yhtyhdy46/7yqfSQIu9aUSVR/YLgqSH/CMQUn0mnjHCRrch0l/x/ 4fupBX1lXvcFBlavh/HJx8Z8PrKp0PaKau4rWwqxj0LKxCUnozHLZHc+biP1WbsOQ+nXCr Gwgk5JVeX68yeJecndpUBGBiHaiSi4I= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1722165415; x=1753701415; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=1JKGz+l/9tFKb3tLci1KBK86LYVt9Ga7B8yR/DX71fU=; b=GXov0t+in6qjku69Vw2utmD5aan03f50pRB7DSbqrnN+eVvrEC1AGPNH /dwxMIvfuhN8N9fuFur+rOvaqzQbif9DILOox+SBGwhPxv6CeddLb9Fy7 J/Eev1/wDyZnEdXaN/UKCztJQighW20ZAhjxQPkf9vXuEN1+XYapQ8Bw4 071qxfOM4+B30yeeE0L5IffGJDnthEd+4dFSJtsBU80X7HYI0e0ne67Kn /isBkL7jGN3ncSNpYPpsR82eCtad0z9IamIf9XyeA/lnLCabJO+Qp4pNZ 0e641bcrzxZ9YAqvIvt1iItiAy1zJstf1wcHRB85BKJvHM9wNaJQq1coy A==; X-CSE-ConnectionGUID: OUV1Byu7RwWTDsLPir0m+A== X-CSE-MsgGUID: DwrIK8gCQN+NXK/0wF0P5g== X-IronPort-AV: E=McAfee;i="6700,10204,11146"; a="37416187" X-IronPort-AV: E=Sophos;i="6.09,243,1716274800"; d="scan'208";a="37416187" Received: from orviesa005.jf.intel.com ([10.64.159.145]) by fmvoesa102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Jul 2024 04:16:53 -0700 X-CSE-ConnectionGUID: oOIPNndLQUKUQA0YxAVaVQ== X-CSE-MsgGUID: 4O7e87TSTiqCnO1mXOsPWQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.09,243,1716274800"; d="scan'208";a="58476523" Received: from binbinwu-mobl.ccr.corp.intel.com (HELO [10.124.241.187]) ([10.124.241.187]) by orviesa005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Jul 2024 04:16:47 -0700 Message-ID: <18729cf6-bf3a-4a11-a9fc-a35792cd1736@linux.intel.com> Date: Sun, 28 Jul 2024 19:16:44 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH v5 09/29] KVM: selftests: TDX: Add report_fatal_error test To: Sean Christopherson , Yan Zhao Cc: Ackerley Tng , sagis@google.com, linux-kselftest@vger.kernel.org, afranji@google.com, erdemaktas@google.com, isaku.yamahata@intel.com, pbonzini@redhat.com, shuah@kernel.org, pgonda@google.com, haibo1.xu@intel.com, chao.p.peng@linux.intel.com, vannapurve@google.com, runanwang@google.com, vipinsh@google.com, jmattson@google.com, dmatlack@google.com, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, linux-mm@kvack.org References: Content-Language: en-US From: Binbin Wu In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 2D5194001B X-Stat-Signature: 3ibcmjdoc8jnmh1z3ot66qbpj1w65qdg X-Rspam-User: X-HE-Tag: 1722165414-689964 X-HE-Meta: U2FsdGVkX18CJEzW2CVKp0eJt0dCCUqFFbS+k2f0nR+I1iFvcFOUOTz4rEZJ+L5f2y1oU2wLlvqrcOg4MnK11yGm+tq9hKJ2uB0nDCwzB82VlnjaNh1Irpod4lpPxn2GPbRnTtX5VMKJ1ZueAep9/zxfZrl4GWn0XNUMvbSVlBpJ/82azLcQXRJi7sNX0KH6654ntLcYNA1PMRXxPBp3bArc9GIf7AKE4iMoJdPPfP74dc5cK4hwP3GQU4RxsUHtFQPb5bYVfc+Z1Rv2CW+6BYHK4NeRc1ba14ujDrInIMxXT/NOWwH771JxUlvGo5LfyVUJ4PGT4b6Q95Bz2E/63L9yw+mhbkCdKnZEvz1M/r1NPUpeBl/USgOACzncUuyePpNIwv3ki/MU0yR4e+diLldzufTwBUWPvBQO0bWPoxkFfw/TWd6lTXZCey6yLb2TF1n3twHUjaB1MpNwjkK5BLmE6LqXjhQKTa2uB2b/npFSHI2WNqpWujriOrcApZGnuSrkTnmlR/T/9TLBAc731l6jdOMYp1IL6D3wA0Pk+E4XMBTqid3Y0mxd8tNZkRwYBXsm8jPTXJJBHH0DP2iQP5Fj7hd6L2+QJ+DngN5r3A/Sb7DfFuK42kIwPejMDORxi4N58nWwCOrjuGKJUFZG/XcVyYCH6GSFXpFG8CH5QvtOYdq9wpaO4BvBLGs5DdPl7emKBAwUDU+96fZYDTkOr8inMlsEivUp+ztJygk/1AkZPuAdOTiUc1Rs6nJNs9oDShyJVjDzkt2Oj730OKiEVPhlUeeD8BrHX3vlQrFg/CxoKQKIrbrPI2CjoGhBVUcarHTgRDhRB4ZBLoomFVfQ/bQcdeTC+lRkR/wM3C4SP6nT3/gm3dsFCSwVkKVwI/JM7k6FtgA2395jVmY8eMi4hRethtZT3cVBdGWiCV3nUWP+5VS+Kmnbw05hk9kipHj4mUdtLOHRVwdaLjy8IG0 +cUdguaJ z2Xir5a3LSFARkt5j34LahBCoqDkbyR/Fg6XqhHIqO4z4B4XLfzJeV5VRgyxoD80N7SS3l5IRcZ1TELm46qUObT+sYPMPpAoQamz/eCsEdoQx6l1NcvsxykvHv2cZdTJVZGizCpJFiDJrtuxmgAYenzGE4YfPWBr1WDPHuJV3Zp4feJyIqOk71zs749j9WEjtzDPJ8QXZIlsJ10lARvrGOP+S3ySe7Ipq7UqXXfXRgUyGkVXUATKG2JzQW/QSgC1oX3IrR7C81wf5V1OfSda/M2K8Wja8Ix/5Nb1AiuJtjX3dhfl55usp1c2mXmnRUGqahh4H0ZN027k40QBAbIfjK3ELaLLZebPNqj+E 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: On 4/23/2024 5:23 AM, Sean Christopherson wrote: > On Thu, Apr 18, 2024, Yan Zhao wrote: >> On Tue, Apr 16, 2024 at 11:50:19AM -0700, Sean Christopherson wrote: >>> On Mon, Apr 15, 2024, Yan Zhao wrote: >>>> On Mon, Apr 15, 2024 at 08:05:49AM +0000, Ackerley Tng wrote: >>>>>>> The Intel GHCI Spec says in R12, bit 63 is set if the GPA is valid. As a >>>>>> But above "__LINE__" is obviously not a valid GPA. >>>>>> >>>>>> Do you think it's better to check "data_gpa" is with shared bit on and >>>>>> aligned in 4K before setting bit 63? >>>>>> >>>>> I read "valid" in the spec to mean that the value in R13 "should be >>>>> considered as useful" or "should be passed on to the host VMM via the >>>>> TDX module", and not so much as in "validated". >>>>> >>>>> We could validate the data_gpa as you suggested to check alignment and >>>>> shared bit, but perhaps that could be a higher-level function that calls >>>>> tdg_vp_vmcall_report_fatal_error()? >>>>> >>>>> If it helps, shall we rename "data_gpa" to "data" for this lower-level, >>>>> generic helper function and remove these two lines >>>>> >>>>> if (data_gpa) >>>>> error_code |= 0x8000000000000000; >>>>> >>>>> A higher-level function could perhaps do the validation as you suggested >>>>> and then set bit 63. >>>> This could be all right. But I'm not sure if it would be a burden for >>>> higher-level function to set bit 63 which is of GHCI details. >>>> >>>> What about adding another "data_gpa_valid" parameter and then test >>>> "data_gpa_valid" rather than test "data_gpa" to set bit 63? >>> Who cares what the GHCI says about validity? The GHCI is a spec for getting >>> random guests to play nice with random hosts. Selftests own both, and the goal >>> of selftests is to test that KVM (and KVM's dependencies) adhere to their relevant >>> specs. And more importantly, KVM is NOT inheriting the GHCI ABI verbatim[*]. >>> >>> So except for the bits and bobs that *KVM* (or the TDX module) gets involved in, >>> just ignore the GHCI (or even deliberately abuse it). To put it differently, use >>> selftests verify *KVM's* ABI and functionality. >>> >>> As it pertains to this thread, while I haven't looked at any of this in detail, >>> I'm guessing that whether or not bit 63 is set is a complete "don't care", i.e. >>> KVM and the TDX Module should pass it through as-is. >>> >>> [*] https://lore.kernel.org/all/Zg18ul8Q4PGQMWam@google.com >> Ok. It makes sense to KVM_EXIT_TDX. >> But what if the TDVMCALL is handled in TDX specific code in kernel in future? >> (not possible?) > KVM will "handle" ReportFatalError, and will do so before this code lands[*], but > I *highly* doubt KVM will ever do anything but forward the information to userspace, > e.g. as KVM_SYSTEM_EVENT_CRASH with data[] filled in with the raw register information. > >> Should guest set bits correctly according to GHCI? > No. Selftests exist first and foremost to verify KVM behavior, not to verify > firmware behavior. We can and should use selftests to verify that *KVM* doesn't > *violate* the GHCI, but that doesn't mean that selftests themselves can't ignore > and/or abuse the GCHI, especially since the GHCI definition for ReportFatalError > is frankly awful. > > E.g. the GHCI prescibes actual behavior for R13, but then doesn't say *anything* > about what's in the data page. Why!?!?! If the format in the data page is > completely undefined, what's the point of restricting R13 to only be allowed to > hold a GPA? The description of R13 in GHCI:   4KB-aligned GPA where additional error data is shared by the TD. The   VMM must validate that this GPA has the Shared bit set. In other words,   that a shared-mapping is used, and that this is a valid mapping for the   TD. This shared memory region is expected to hold a zero-terminated   string. IIUC, according the GHCI, R13 is a 4K aligned shared buffer provided by the TDX guest to pass additional error message to VMM, i.e., it needs to be a shared GPA.  And the content in the buffer is expected to hold a zero-terminated string. Do you think "a zero-terminated string" describes the format in the data page? > > And the wording is just as awful: > > The VMM must validate that this GPA has the Shared bit set. In other words, > that a shared-mapping is used, and that this is a valid mapping for the TD. > > I'm pretty sure it's just saying that the TDX module isn't going to verify the > operate, i.e. that the VMM needs to protect itself, but it would be so much > better to simply state "The TDX Module does not verify this GPA", because saying > the VMM "must" do something leads to pointless discussions like this one, where > we're debating over whether or *our* VMM should inject an error into *our* guest. > > Anyways, we should do what makes sense for selftests and ignore the stupidity of > the GHCI when doing so yields better code. If that means abusing R13, go for it. > If it's a sticking point for anyone, just use one of the "optional" registers. > > Whatever we do, bury the host and guest side of selftests behind #defines or helpers > so that there are at most two pieces of code that care which register holds which > piece of information. > > [*] https://lore.kernel.org/all/20240404230247.GU2444378@ls.amr.corp.intel.com >