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 X-Spam-Level: X-Spam-Status: No, score=-17.2 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,MSGID_FROM_MTA_HEADER,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9C7B5C4363F for ; Thu, 1 Apr 2021 17:42:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 82CCE61005 for ; Thu, 1 Apr 2021 17:42:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235196AbhDARmX (ORCPT ); Thu, 1 Apr 2021 13:42:23 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57086 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234238AbhDARgs (ORCPT ); Thu, 1 Apr 2021 13:36:48 -0400 Received: from NAM10-MW2-obe.outbound.protection.outlook.com (mail-mw2nam10on2062f.outbound.protection.outlook.com [IPv6:2a01:111:f400:7e89::62f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E9FF1C0A88B5; Thu, 1 Apr 2021 07:11:39 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=QS+OcDL0Oda4noJaKxIJNgBBIw4J1jtPDPJ5sDwRLXzK9zWwkcPwVLsmE2UZsDcnWqtzRYBua7jT+NaZWpY0nMjpWjP1386Jb/y9CxSEgKTtXFCe1Q58xyUDzZKi7+9TvcPmRuoaP82O4zaMrMNpl5IGmOXyljOPE+dthjI51rs9J0yUCZkQFu6nv0yUO/kxyCM7/MvOdx231tv72jaaevjWWe9XjxvZlrT8CVR1TPrtnDt/W7N8bTTYQ6KCgUbD+fAljU6DD5P7W3ZwloWiKUPzE4H0S6s1wjq/ggmw2ErhnTfH+1VtwO7l2ZXKTjR7TgtBzcsHBj2+wtkRxIbR5g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=wBnsRYwnKQuGB6nfHHNMPLfT71wRQFvmGk9jsTohYlg=; b=Ox1CxyGCP1/+6DXN4PkXhQmnlWj0TLbnZQ4v7yoUtLtFSdIGW61hcDrFPfec6sQQx+TYI2DBBjomFBcPGBeQ1iPHrHRwCDIRDWoey4TMJL++o6qNl/urYky4t8KtzkHYVRbqKmcg9zf7aIFCsej4O+CW6/h96PXuGeMzVKilDm89FmmtoOq+SnL3PCR7rcGHcr1zFFNtVM+u4Uiegu2ogdYItquuXknUl8Hh0Jpq6LVaImZcAOZDos+2qbjOENcY/uqt6LBNrj0SoNd6Bd3+FIIHV79qN4DhVVeU9liomK0N8VDumETHuLU3TYpCsUMnaJ8Jf2YFCiLPp6uYUU742A== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=wBnsRYwnKQuGB6nfHHNMPLfT71wRQFvmGk9jsTohYlg=; b=IXx7ue05cEZ+SLCJp7ewaNuQfqdWmLOHX1GVp9NRtHf00Ilt0o552p0PKQFCFxzlsD5OYYgY88ymAyw28gHAHnMIDT5742iTfFeME2dySl67g5cD60+2WZDyxKqnsFG7B86UrKtZEs6MQVFpF9Fk6OnakaXZIBUQR6GKMr2rgJ8= Authentication-Results: google.com; dkim=none (message not signed) header.d=none;google.com; dmarc=none action=none header.from=amd.com; Received: from SN6PR12MB2718.namprd12.prod.outlook.com (2603:10b6:805:6f::22) by SA0PR12MB4574.namprd12.prod.outlook.com (2603:10b6:806:94::22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3999.28; Thu, 1 Apr 2021 14:11:37 +0000 Received: from SN6PR12MB2718.namprd12.prod.outlook.com ([fe80::30fb:2d6c:a0bf:2f1d]) by SN6PR12MB2718.namprd12.prod.outlook.com ([fe80::30fb:2d6c:a0bf:2f1d%3]) with mapi id 15.20.3977.033; Thu, 1 Apr 2021 14:11:37 +0000 Cc: brijesh.singh@amd.com, linux-kernel@vger.kernel.org, x86@kernel.org, kvm@vger.kernel.org, ak@linux.intel.com, Thomas Gleixner , Ingo Molnar , Joerg Roedel , "H. Peter Anvin" , Tony Luck , Dave Hansen , "Peter Zijlstra (Intel)" , Paolo Bonzini , Tom Lendacky , David Rientjes , Sean Christopherson Subject: Re: [RFC Part1 PATCH 04/13] x86/sev-snp: define page state change VMGEXIT structure To: Borislav Petkov References: <20210324164424.28124-1-brijesh.singh@amd.com> <20210324164424.28124-5-brijesh.singh@amd.com> <20210401103208.GA28954@zn.tnic> From: Brijesh Singh Message-ID: <894e7732-8ed8-cc17-1cd1-769b7d2745d1@amd.com> Date: Thu, 1 Apr 2021 09:11:34 -0500 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.9.0 In-Reply-To: <20210401103208.GA28954@zn.tnic> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Content-Language: en-US X-Originating-IP: [70.112.153.56] X-ClientProxiedBy: SN4PR0401CA0018.namprd04.prod.outlook.com (2603:10b6:803:21::28) To SN6PR12MB2718.namprd12.prod.outlook.com (2603:10b6:805:6f::22) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from Brijeshs-MacBook-Pro.local (70.112.153.56) by SN4PR0401CA0018.namprd04.prod.outlook.com (2603:10b6:803:21::28) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3999.27 via Frontend Transport; Thu, 1 Apr 2021 14:11:36 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 8aa409cc-8fbd-4156-5fbb-08d8f518102f X-MS-TrafficTypeDiagnostic: SA0PR12MB4574: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:8882; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: rRF0KY7tk6Hgz2/u9ivwKXEt1iN24J5mvoeWim+C4Y5BhDH6gYcGhfqCZDZ6QWIJSXVGyRof5+4rtCj0E6hoivfGjAB07q83SvQVZAV/CPf6zg1Ffw93Jw7JSd7HGM5e3+ozBY3mxXWaDusGe5wQ3ecWyy9g8iq4vu9E4W07wG7U5Oo2hnLtztwWJvtmqJ0S1uND0TJ5hExg+ESjWZlO5tBdNx8svk1Kenu6yjBmAreNQrXQNJtEYffK8No5J+X9Zg+FzQuI5UCcYB612nIviB/2nMHjNmY4kSGh3gxgu0TUrb5WD2kD13yo35tCn7PqPdBuF8uTSjfL8KJdH1u6vUAlbt8UsbVSkEBmv3ad+OswwSjVY2W8hCNtRYSXF00P/TdoEH/sLoX+sndO3ohWxLJEK+xt8gQbuLTINLh45d3MYtakEXBAKZlqUn3LHnKPMGDdIIypewwj0Cs8Nuyw98Oq7pE52FS5EGR1rDiFVXJi1bxdqGdT8OVoNz9/33Bs8GdnAXqNl4ebAUMvuLVLBMwtqZQnCN46dUXD3Eeh1yu2NIw1RRdiDzpuxn0fHbCwey8Osd9C/DwqBxEP64LUwyDX/DSg0F+etZXhX6p6uxYzF0V/Syxbf2ZADEWSmQyhVa9WRKu8oXyQY4wqYBGCYauboQCXoWi1wA2xR+JgjNJWPjYsQQT3D/FPC+zBYNH1NdzKYucrxNqPGtHmLiuwKY9G8p4r5WtkcwWzlY5YcbM= X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:SN6PR12MB2718.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(376002)(366004)(39860400002)(346002)(136003)(396003)(8676002)(31696002)(38100700001)(316002)(54906003)(36756003)(86362001)(66946007)(83380400001)(186003)(2906002)(956004)(16526019)(44832011)(2616005)(26005)(7416002)(6916009)(8936002)(478600001)(66556008)(66476007)(4326008)(31686004)(5660300002)(52116002)(6512007)(6506007)(6486002)(53546011)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: =?utf-8?B?YWtPTzhWK1RUdGxhdEN1ZFhpZU5TZW1yckV1cm9PMUNBdFo1OHd6OEJ1ZzVj?= =?utf-8?B?SzNKVGpwVzNLNHVDalJiVDE0cERlaG8yNnE4c0w2Vno4eDVKa0R4Tyt5a0hh?= =?utf-8?B?UnFXZW11ZXg0OFlReHdYT0NOUGw4YWI1MTFTYWtYbkljdGpBbW95a0F1MmFK?= =?utf-8?B?UHBPVGpjeDJnSzJyVGwrYU5Kc1FSeXQ3RGQxWm1wTlhLVHFyZ1hSdDJzSFlQ?= =?utf-8?B?K0JJNmd4QjliYlZ4cnM3Q1BPaWZCK1dJUnF3Vm53c3ZreW9VZG9TNmJzSzJv?= =?utf-8?B?YWpSak5DbVY0ZGpuSkZGdlBQQTRxMGdGYzBxbUpISWFGK082a09hMVBVaytD?= =?utf-8?B?VjRiZVNMc3dxU0xtaVZQV2FHbWZVcjhsRlY1Q1MzWm9LL0dsbzhXeVJxVTlr?= =?utf-8?B?VTlYam41V2RTQTNuVjhKaTBJQXo0akFWa0FGd2RvdUZIdnhKSGhIQkV0bkZI?= =?utf-8?B?U3lmWWtBdnZ5M2h6TncveWVBQkNESzhsWjJpZTEwOG01aDIyQnRaaFNRMWhH?= =?utf-8?B?NzY0ZlhoR0IxclQ0SGlBcTArcWRHWTlLVVhxMGU1WHh2Sk02Z0VPNXlTZ2pY?= =?utf-8?B?WGFGdTlLY0IwamhBbjdTUXdMRmVQQ2piS3BTK1V3RmFERVRxY2lWUkFKNitt?= =?utf-8?B?QStnK2I4UHcxUzEycFVXVktmM05obnNESXpqL2VLV2xkUURFVk9JbjB5eXFL?= =?utf-8?B?Q1U1RlcwV0RWRWVnRFlTZTVzT3EzOW8rc0k0ZFVGOWlMTVh1N0lKK3V4WG5s?= =?utf-8?B?d3Jwa0JvMkNSM1A3Z0JRbHpPNUFnQUMzYUVBM2tKR01hRU05WCt3QUF6Z2lE?= =?utf-8?B?S1ZmY0w0R3V4Y01jUkEvR0p4d0s5aFdic2oreUNjR2FvREFuK2RPOStNdWtz?= =?utf-8?B?ZTA3VkFuYXhyd2tZNDJYS3hDVGhRTkFpUmloeWdKbnc1cXN5allMaTdDVnlS?= =?utf-8?B?cUNvZ3p6YkJsZk9PdEl2engrdEFJbG44UC9RbkV3VjNUNHMwTTEwOEpXSU1v?= =?utf-8?B?TTRZSGZUc0M0NmFFSGwyRmEyaWs0alc1TllmRlF3MHZncEVxamhOS0JFZE9l?= =?utf-8?B?b0xzYkNQd0VvNU5zOEtnTDBqNjdlczU5U3ZNNStRZHUvemxFUUJpeW9yWUZn?= =?utf-8?B?SVo2Z1djMTFUMG56MGJXc2ZDTVFmRisySXZPUzRHeWMrbHpwanVRSURxZEw3?= =?utf-8?B?M1Mra0xpUW1sK3g3bkJTTklvYWhvcHM2TFBJdGM5TlBtb0tDNnYyRHBWVzlj?= =?utf-8?B?bG1vNDhQWlNmNzg0akhqNXI1TXNJYWhXakQvUU1vWXJHMCt1a1FoV3h3bjFn?= =?utf-8?B?WXhwbkZ5d0I1S0Z3ZDdqSFJkaVkzM1dJaXlOdm8vQmtNR3pETzJXUTFUcWJ4?= =?utf-8?B?SXBENTEwK25lb1V2RUJCdGR3NDRmbk1hT0lObVNYcEZNWjJaZERKVVk5L0xv?= =?utf-8?B?cWJ1b2YvSGdpZFBYWG5QejU3aXBzc2RLZkEvYWcyR1dWT0xuRUU4NExQd2R3?= =?utf-8?B?L1dmUStqZ0N5YkZ5WmI1RjJhOXIzdC9XOXFhMXUrc3hRZVIrVGlnTE5UNGFs?= =?utf-8?B?cllzNnpSZUd1MkpGeGE1clFta2UyMGNDUXVFNGk3d2UwVHh3Mm0vcm5WRmJB?= =?utf-8?B?WDMxVXk0bUluR05qRzllYm5XV3RFUU9mNllETGRzVDM1WnlYUXNaODVPTEJF?= =?utf-8?B?a2lJZTZCZGtnRnlKKzhjbTFPUnRYUXZsd20vWUhpUVRvWmpvK25melN2RDhO?= =?utf-8?Q?zkY27+FtScvdl2vpekHOJpM+MNG1jykFh0lIo7S?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 8aa409cc-8fbd-4156-5fbb-08d8f518102f X-MS-Exchange-CrossTenant-AuthSource: SN6PR12MB2718.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Apr 2021 14:11:37.4714 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: GFgHGAQE8JcARK3bIHsX6Ig2LBXH7q+1SP3Q+NuFrL79SoAOnyapLNbcLbm91KNVH1FMMEgPuLOu6kdCArvIfw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA0PR12MB4574 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 4/1/21 5:32 AM, Borislav Petkov wrote: > On Wed, Mar 24, 2021 at 11:44:15AM -0500, Brijesh Singh wrote: >> An SNP-active guest will use the page state change VNAE MGEXIT defined in > I guess this was supposed to mean "NAE VMGEXIT" but pls write "NAE" out > at least once so that reader can find its way around the spec. Noted. I will fix in next rev. >> the GHCB specification section 4.1.6 to ask the hypervisor to make the >> guest page private or shared in the RMP table. In addition to the >> private/shared, the guest can also ask the hypervisor to split or >> combine multiple 4K validated pages as a single 2M page or vice versa. >> >> Cc: Thomas Gleixner >> Cc: Ingo Molnar >> Cc: Borislav Petkov >> Cc: Joerg Roedel >> Cc: "H. Peter Anvin" >> Cc: Tony Luck >> Cc: Dave Hansen >> Cc: "Peter Zijlstra (Intel)" >> Cc: Paolo Bonzini >> Cc: Tom Lendacky >> Cc: David Rientjes >> Cc: Sean Christopherson >> Cc: x86@kernel.org >> Cc: kvm@vger.kernel.org >> Signed-off-by: Brijesh Singh >> --- >> arch/x86/include/asm/sev-snp.h | 34 +++++++++++++++++++++++++++++++++ >> arch/x86/include/uapi/asm/svm.h | 1 + >> 2 files changed, 35 insertions(+) >> >> diff --git a/arch/x86/include/asm/sev-snp.h b/arch/x86/include/asm/sev-snp.h >> index 5a6d1367cab7..f514dad276f2 100644 >> --- a/arch/x86/include/asm/sev-snp.h >> +++ b/arch/x86/include/asm/sev-snp.h >> @@ -22,6 +22,40 @@ >> #define RMP_PG_SIZE_2M 1 >> #define RMP_PG_SIZE_4K 0 >> >> +/* Page State Change MSR Protocol */ >> +#define GHCB_SNP_PAGE_STATE_CHANGE_REQ 0x0014 >> +#define GHCB_SNP_PAGE_STATE_REQ_GFN(v, o) (GHCB_SNP_PAGE_STATE_CHANGE_REQ | \ >> + ((unsigned long)((o) & 0xf) << 52) | \ >> + (((v) << 12) & 0xffffffffffffff)) > This macro needs to be more readable and I'm not sure the masking is > correct. IOW, something like this perhaps: > > #define GHCB_SNP_PAGE_STATE_REQ_GFN(va, operation) \ > ((((operation) & 0xf) << 52) | ((va) & GENMASK_ULL(51, 12)) | GHCB_SNP_PAGE_STATE_CHANGE_REQ) I guess I was trying to keep it in consistent with sev-es.h macro definitions in which the command is used before the fields. In next version, I will use the msb to lsb ordering. > > where you have each GHCBData element at the proper place, msb to lsb. > Now, GHCB spec says: > > "GHCBData[51:12] – Guest physical frame number" > > and I'm not clear as to what this macro takes: a virtual address or a > pfn. If it is a pfn, then you need to do: > > (((pfn) << 12) & GENMASK_ULL(51, 0)) > > but if it is a virtual address you need to do what I have above. Since > you do "<< 12" then it must be a pfn already but then you should call > the argument "pfn" so that it is clear what it takes. Yes, the macro takes the pfn. > Your mask above covers [55:0] but [55:52] is the page operation so the > high bit in that mask needs to be 51. Ack. I will limit the mask to 51 so that we don't go outside the pfn field width. thank you for pointing it. > AFAICT, ofc. > >> +#define SNP_PAGE_STATE_PRIVATE 1 >> +#define SNP_PAGE_STATE_SHARED 2 >> +#define SNP_PAGE_STATE_PSMASH 3 >> +#define SNP_PAGE_STATE_UNSMASH 4 >> + >> +#define GHCB_SNP_PAGE_STATE_CHANGE_RESP 0x0015 >> +#define GHCB_SNP_PAGE_STATE_RESP_VAL(val) ((val) >> 32) > ^^^^^^^^^^^^ > > Some stray tabs here and above, pls pay attention to vertical alignment too. I noticed that sev-es.h uses tab when defining the macro to build command. Another example where I tried to keep a bit consistentency with sev-es.h. I will drop it in next rev. > >> + >> +/* Page State Change NAE event */ >> +#define SNP_PAGE_STATE_CHANGE_MAX_ENTRY 253 >> +struct __packed snp_page_state_header { >> + uint16_t cur_entry; >> + uint16_t end_entry; >> + uint32_t reserved; >> +}; >> + >> +struct __packed snp_page_state_entry { >> + uint64_t cur_page:12; >> + uint64_t gfn:40; >> + uint64_t operation:4; >> + uint64_t pagesize:1; >> + uint64_t reserved:7; >> +}; > Any particular reason for the uint_t types or can you use our > shorter u types? IIRC, the spec structure has uint_t, so I used it as-is. No strong reason for using it. I will switch to u64 type in the next version. > >> + >> +struct __packed snp_page_state_change { >> + struct snp_page_state_header header; >> + struct snp_page_state_entry entry[SNP_PAGE_STATE_CHANGE_MAX_ENTRY]; > Also, looking further into the patchset, I wonder if it would make sense > to do: > > s/PAGE_STATE_CHANGE/PSC/g > > to avoid breaking lines of huge statements: > > + if ((GHCB_SEV_GHCB_RESP_CODE(val) != GHCB_SNP_PAGE_STATE_CHANGE_RESP) || > + (GHCB_SNP_PAGE_STATE_RESP_VAL(val) != 0)) > + sev_es_terminate(GHCB_SEV_ES_REASON_GENERAL_REQUEST); > > and turn them into something more readable > > + if ((GHCB_SEV_GHCB_RESP_CODE(val) != GHCB_SNP_PSC_RESP) || > + (GHCB_SNP_PSC_RESP_VAL(val))) > + sev_es_terminate(GHCB_SEV_ES_REASON_GENERAL_REQUEST); > > Also, you have GHCB_SEV and GHCB_SNP prefixes and I wonder whether we > even need them and have everything be prefixed simply GHCB_ because that > is the protocol, after all. > > Which will then turn the above into: > > + if ((GHCB_RESP_CODE(val) != GHCB_PSC_RESP) || (GHCB_PSC_RESP_VAL(val))) > + sev_es_terminate(GHCB_REASON_GENERAL_REQUEST); > > Oh yeah baby! :-) It looks much better :-). I am good with dropping GHCB_{SEV,SNP} prefix, and also rename the PAGE_STATE_CHANGE to PSC. Thanks. > > Thx. >