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 57A1AC25B7C for ; Mon, 27 May 2024 12:26:17 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B79D96B0089; Mon, 27 May 2024 08:26:16 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B29746B008A; Mon, 27 May 2024 08:26:16 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9F1036B008C; Mon, 27 May 2024 08:26:16 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 810E66B0089 for ; Mon, 27 May 2024 08:26:16 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 065B0A2D34 for ; Mon, 27 May 2024 12:26:16 +0000 (UTC) X-FDA: 82164098352.22.CFBE563 Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.9]) by imf04.hostedemail.com (Postfix) with ESMTP id 7C11F4000C for ; Mon, 27 May 2024 12:26:12 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=FurH6WBc; dmarc=pass (policy=none) header.from=intel.com; spf=none (imf04.hostedemail.com: domain of binbin.wu@linux.intel.com has no SPF policy when checking 198.175.65.9) smtp.mailfrom=binbin.wu@linux.intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1716812773; 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=Hz/BropN5PTtTO7tDpujJEj+6Q0C8Hro1h7OpSMeaNQ=; b=VOQ71Lq06LdjV9OsEQdt0ivsd+KCZWnoHpa9Fht1qXvWe8Iur2QXLkJVo0Q4LysDoKWYDa JKYCSLs3DcuVgV7INeph2p2GN+Z1SA9M5HphiS9MrZzP5OwnnlnUGLdKJ3thCbBSOrRbJ/ RYn/qhxWQMZoVvGY+GBiIKjmh9V9V9g= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=FurH6WBc; dmarc=pass (policy=none) header.from=intel.com; spf=none (imf04.hostedemail.com: domain of binbin.wu@linux.intel.com has no SPF policy when checking 198.175.65.9) smtp.mailfrom=binbin.wu@linux.intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1716812773; a=rsa-sha256; cv=none; b=jiImsggnpgXRt8gmqAcW9+WzZ1rVIWEyfAi0MxmAu/iy7FDlN+eYHCqCXk22fi4IIYiPfe k0saMUJVGCxSdACpoHrP06H1asWfqMoqZ7Q8JrQ9MQIof7XwzBrQMkon7/TDffU4g53nSK 0lEXvB3ZEWCW1b23lQ9thAXXZ2NPX38= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1716812773; x=1748348773; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=/c0n1b92ma2tktOqUNb41OqeN2cWnA5+c1eV0kosDcw=; b=FurH6WBcBKVIbgoLvhskhmjYnU6IgjlggqECixqd7COR9skgG27pSJjK G2NrCJjbgeuM75kP4HN29usQnk6DvHYP8l4OCMsRQXau7T0fwuuB11XeN VtD+yRAo+T/gn4Y/JxwiK3BkZqZAxRbcuxqzO4YRPFpN9D86ZfqDGwnP9 k2qygOQV9o4InhpgncxncYrLQ2gDqwsZQxM7qsfl9x0oWoAtmMtUCcpdu CetiZwp2wIAGgmW/Yre9WIzxu5D4q75xVWm736FLxXg2KOqQRjc/5hDap MhAluBhm5GGkI6Zn+2oN32hmase16LXfyHgFSJfnRyuHJpp74U1QmCub3 Q==; X-CSE-ConnectionGUID: sNCnQIJ7Qpiejj39btWmvQ== X-CSE-MsgGUID: BA+/QOhwQ36DAH5lXpYKSA== X-IronPort-AV: E=McAfee;i="6600,9927,11084"; a="35647270" X-IronPort-AV: E=Sophos;i="6.08,192,1712646000"; d="scan'208";a="35647270" Received: from orviesa006.jf.intel.com ([10.64.159.146]) by orvoesa101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 May 2024 05:26:11 -0700 X-CSE-ConnectionGUID: 4RopLxNGRK+EFM85hO/PLQ== X-CSE-MsgGUID: KIV1k5CaR5mnjLatSwP3dw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.08,192,1712646000"; d="scan'208";a="35245657" Received: from binbinwu-mobl.ccr.corp.intel.com (HELO [10.124.234.76]) ([10.124.234.76]) by orviesa006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 May 2024 05:26:02 -0700 Message-ID: <7da9c4a3-8597-44aa-a7ad-cc2bd2a85024@linux.intel.com> Date: Mon, 27 May 2024 20:25:59 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v15 09/20] KVM: SEV: Add support to handle MSR based Page State Change VMGEXIT To: Michael Roth Cc: Paolo Bonzini , kvm@vger.kernel.org, linux-coco@lists.linux.dev, linux-mm@kvack.org, linux-crypto@vger.kernel.org, x86@kernel.org, linux-kernel@vger.kernel.org, tglx@linutronix.de, mingo@redhat.com, jroedel@suse.de, thomas.lendacky@amd.com, hpa@zytor.com, ardb@kernel.org, seanjc@google.com, vkuznets@redhat.com, jmattson@google.com, luto@kernel.org, dave.hansen@linux.intel.com, slp@redhat.com, pgonda@google.com, peterz@infradead.org, srinivas.pandruvada@linux.intel.com, rientjes@google.com, dovmurik@linux.ibm.com, tobin@ibm.com, bp@alien8.de, vbabka@suse.cz, kirill@shutemov.name, ak@linux.intel.com, tony.luck@intel.com, sathyanarayanan.kuppuswamy@linux.intel.com, alpergun@google.com, jarkko@kernel.org, ashish.kalra@amd.com, nikunj.dadhania@amd.com, pankaj.gupta@amd.com, liam.merwick@oracle.com, Brijesh Singh , "Yamahata, Isaku" References: <20240501085210.2213060-1-michael.roth@amd.com> <20240501085210.2213060-10-michael.roth@amd.com> <84e8460d-f8e7-46d7-a274-90ea7aec2203@linux.intel.com> <7d6a4320-89f5-48ce-95ff-54b00e7e9597@linux.intel.com> Content-Language: en-US From: Binbin Wu In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 7C11F4000C X-Stat-Signature: 8g9imonkkdkupuy3e4m95ye3hyhipnh3 X-HE-Tag: 1716812772-363637 X-HE-Meta: U2FsdGVkX1//JlgRX5cvDjd17iBHZBW8EnufcM0TYTzGBpc6ogc5u1OsV/IryB4luCiTXZZgbpqklQCr7c+2TX06NvA7Ub1bcdDoz76oVJtOCYTx90dYSHcw8XOsUgkJaHXhdGyGpIVrt6gbAnlAOwv+KsVL7Xvcc8bN7E4yww84SD/EUXB0v2z4UDtveZeegFxMWvOlayJDh0gnLI49tyR61giOU8cjwG7efxwMCHlba53NRoU+cT2sOUrFpGGo2KvDTzRwt7LT+Dipd+law3CpHl/TtVmDxDgnkgEoYUHzFDskxYfOR7OGemyCxumwjZhHZ5wP+GMmnb+FBazSn2vpst+hfgFeDzwd4eEQ3Iq2dE1oLtxIcBnclYcM2Jh0kDrCTxZqem5QSZacE8Jn40HPdWopDvvxXKVEYPSxncD2nuD3jSMcHj3VeSsMQbbUs/a99CERb95lo/8MgEQhxfX1pHZc4mZVUXIFDJ5NAp031yOBPvkF56XflDG9tSdy+YH88wN+e0gtTZ418Wu/Fet/iAvqic899JI/HkOpjDCCHWx/yf+btk7QFzsbsGKK2nOnVfokCU8qZRZE0wCWejs6332/vcDlCrSPHDmcu+pYm/M3q31BrVcq9qmCP9DVS/GoDvPhc9Fom3E3+dUTqoHGhrS6P3lCqTbyuy99hzhPO8/eRg1fcHwCq26LlAJGWF//wY5OrX0CwTV6NuvXK8Jt2bd+jPaH5kkKjcC7vDfFZoh07VxqCJ9CPRqLiteFnKNGuBmLH5AL6T+f4jas5ClG+FSPh0eMQ9na4dlfQDwIYA5zQ9vJFs/tjAr/EV3pVoyKPWl5MUbVs1Pjfx1tdWXWWop5gpb4rSttrhZYic9aHJvStw8lSjksnyQ+eyKCqfjtox0fzu7EinW+enpKyTohp0AIsB12QztRHGTLdiDjYe50qqAIw4WtKnFzp5hImc68SMP1F/X25uUIEoy +wt1qsZm G0C94piJ3tcXjat4DINpimSqyM2Txf5TbSgHBLaaF6yi1UsCeahLBCBKVSbOWux8xujvZhFG12ykS7xY12ShxCZKfLYajVW+lOVK++DoH2RLd3iTM06RRMro0B2uWqDNUWfS7g6LuWoBXUgvGx79wXayPEje1ZqOclAPOWdjYHIncGIxc2plRNM7vDU7NvZPfP5KOdMlkHP8FiGsAu/8ONd73gZahBYwg94poqoBnVs79vb8zwYNVB9ZtFM3jvSh3i3WERQnAw+nhv7nf9cJ4DhJ+rg== 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 5/22/2024 5:49 AM, Michael Roth wrote: > On Tue, May 21, 2024 at 08:49:59AM +0800, Binbin Wu wrote: >> >> On 5/17/2024 1:23 AM, Paolo Bonzini wrote: >>> On Thu, May 16, 2024 at 10:29 AM Binbin Wu wrote: >>>> >>>> On 5/1/2024 4:51 PM, Michael Roth wrote: >>>>> SEV-SNP VMs can ask the hypervisor to change the page state in the RMP >>>>> table to be private or shared using the Page State Change MSR protocol >>>>> as defined in the GHCB specification. >>>>> >>>>> When using gmem, private/shared memory is allocated through separate >>>>> pools, and KVM relies on userspace issuing a KVM_SET_MEMORY_ATTRIBUTES >>>>> KVM ioctl to tell the KVM MMU whether or not a particular GFN should be >>>>> backed by private memory or not. >>>>> >>>>> Forward these page state change requests to userspace so that it can >>>>> issue the expected KVM ioctls. The KVM MMU will handle updating the RMP >>>>> entries when it is ready to map a private page into a guest. >>>>> >>>>> Use the existing KVM_HC_MAP_GPA_RANGE hypercall format to deliver these >>>>> requests to userspace via KVM_EXIT_HYPERCALL. >>>>> >>>>> Signed-off-by: Michael Roth >>>>> Co-developed-by: Brijesh Singh >>>>> Signed-off-by: Brijesh Singh >>>>> Signed-off-by: Ashish Kalra >>>>> --- >>>>> arch/x86/include/asm/sev-common.h | 6 ++++ >>>>> arch/x86/kvm/svm/sev.c | 48 +++++++++++++++++++++++++++++++ >>>>> 2 files changed, 54 insertions(+) >>>>> >>>>> diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h >>>>> index 1006bfffe07a..6d68db812de1 100644 >>>>> --- a/arch/x86/include/asm/sev-common.h >>>>> +++ b/arch/x86/include/asm/sev-common.h >>>>> @@ -101,11 +101,17 @@ enum psc_op { >>>>> /* GHCBData[11:0] */ \ >>>>> GHCB_MSR_PSC_REQ) >>>>> >>>>> +#define GHCB_MSR_PSC_REQ_TO_GFN(msr) (((msr) & GENMASK_ULL(51, 12)) >> 12) >>>>> +#define GHCB_MSR_PSC_REQ_TO_OP(msr) (((msr) & GENMASK_ULL(55, 52)) >> 52) >>>>> + >>>>> #define GHCB_MSR_PSC_RESP 0x015 >>>>> #define GHCB_MSR_PSC_RESP_VAL(val) \ >>>>> /* GHCBData[63:32] */ \ >>>>> (((u64)(val) & GENMASK_ULL(63, 32)) >> 32) >>>>> >>>>> +/* Set highest bit as a generic error response */ >>>>> +#define GHCB_MSR_PSC_RESP_ERROR (BIT_ULL(63) | GHCB_MSR_PSC_RESP) >>>>> + >>>>> /* GHCB Hypervisor Feature Request/Response */ >>>>> #define GHCB_MSR_HV_FT_REQ 0x080 >>>>> #define GHCB_MSR_HV_FT_RESP 0x081 >>>>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c >>>>> index e1ac5af4cb74..720775c9d0b8 100644 >>>>> --- a/arch/x86/kvm/svm/sev.c >>>>> +++ b/arch/x86/kvm/svm/sev.c >>>>> @@ -3461,6 +3461,48 @@ static void set_ghcb_msr(struct vcpu_svm *svm, u64 value) >>>>> svm->vmcb->control.ghcb_gpa = value; >>>>> } >>>>> >>>>> +static int snp_complete_psc_msr(struct kvm_vcpu *vcpu) >>>>> +{ >>>>> + struct vcpu_svm *svm = to_svm(vcpu); >>>>> + >>>>> + if (vcpu->run->hypercall.ret) >>>> Do we have definition of ret? I didn't find clear documentation about it. >>>> According to the code, 0 means succssful. Is there any other error codes >>>> need to or can be interpreted? >>> They are defined in include/uapi/linux/kvm_para.h >>> >>> #define KVM_ENOSYS 1000 >>> #define KVM_EFAULT EFAULT /* 14 */ >>> #define KVM_EINVAL EINVAL /* 22 */ >>> #define KVM_E2BIG E2BIG /* 7 */ >>> #define KVM_EPERM EPERM /* 1*/ >>> #define KVM_EOPNOTSUPP 95 >>> >>> Linux however does not expect the hypercall to fail for SEV/SEV-ES; and >>> it will terminate the guest if the PSC operation fails for SEV-SNP. So >>> it's best for userspace if the hypercall always succeeds. :) >> Thanks for the info. >> >> For TDX, it wants to restrict the size of memory range for conversion in one >> hypercall to avoid a too long latency. >> Previously, in TDX QEMU patchset v5, the limitation is in userspace and  if >> the size is too big, the status_code will set to TDG_VP_VMCALL_RETRY and the >> failed GPA for guest to retry is updated. >> https://lore.kernel.org/all/20240229063726.610065-51-xiaoyao.li@intel.com/ >> >> When TDX converts TDVMCALL_MAP_GPA to KVM_HC_MAP_GPA_RANGE, do you think >> which is more reasonable to set the restriction? In KVM (TDX specific code) >> or userspace? >> If userspace is preferred, then the interface needs to  be extended to >> support it. > With SNP we might get a batch of requests in a single GHCB request, and > potentially each of those requests need to get set out to userspace as > a single KVM_HC_MAP_GPA_RANGE. The subsequent patch here handles that in > a loop by issuing a new KVM_HC_MAP_GPA_RANGE via the completion handler. > So we also sort of need to split large requests into multiple userspace > requests in some cases. > > It seems like TDX should be able to do something similar by limiting the > size of each KVM_HC_MAP_GPA_RANGE to TDX_MAP_GPA_MAX_LEN, and then > returning TDG_VP_VMCALL_RETRY to guest if the original size was greater > than TDX_MAP_GPA_MAX_LEN. But at that point you're effectively done with > the entire request and can return to guest, so it actually seems a little > more straightforward than the SNP case above. E.g. TDX has a 1:1 mapping > between TDG_VP_VMCALL_MAP_GPA and KVM_HC_MAP_GPA_RANGE events. (And even > similar names :)) > > So doesn't seem like there's a good reason to expose any of these > throttling details to userspace, The reasons I want to put the throttling in userspace are: 1. Hardcode the TDX_MAP_GPA_MAX_LEN in kernel may not be preferred. 2. The throttling thing doesn't need to be TDX specific, it can be generic in userspace. I think we can set a reasonable value in userspace, so that for SNP, it doesn't trigger the throttling since the large request will be split to multiple userspace requests. > in which case existing > KVM_HC_MAP_GPA_RANGE interface seems like it should be sufficient. > > -Mike > >> >>>> For TDX, it may also want to use KVM_HC_MAP_GPA_RANGE hypercall to >>>> userspace via KVM_EXIT_HYPERCALL. >>> Yes, definitely. >>> >>> Paolo >>>