From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM10-MW2-obe.outbound.protection.outlook.com (NAM10-MW2-obe.outbound.protection.outlook.com [40.107.94.79]) by mx.groups.io with SMTP id smtpd.web12.53043.1606229874476772043 for ; Tue, 24 Nov 2020 06:57:54 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@amdcloud.onmicrosoft.com header.s=selector2-amdcloud-onmicrosoft-com header.b=H9jt5mF2; spf=permerror, err=parse error for token &{10 18 %{i}._ip.%{h}._ehlo.%{d}._spf.vali.email}: invalid domain name (domain: amd.com, ip: 40.107.94.79, mailfrom: thomas.lendacky@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=LF+evLNAsxJ/t4D8enPCMHuYJI9NYVknG1TNjlkZh9mn6Ms8X4/j3BXYw356mmU7A/8PJHJALyClkc1qzEHlkHXxe3MwKLqgHhbfXzRuJ7cixvjFV6m3yDvk5a4l5Tzn1rTvfrDQBSP49a09OOzIxeOk/wBsrQvlRxwP17RHFctyVp6bPF4ZK6hAChRNFTbsH2iLHp/321HsPgfy12nit/YesurLRZkF6uROB22Pfnv0jwxWm9Y5Sr2/7FgnQ+InQe6opl1MX14acD/7KG56Z49QO6P1uB8TT5MbQpfFlozx6qU8oG79LPu2B654+ornNipf0oz8k8dxqIZM9YspFQ== 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=kHgom6U8HhrEp5mOo4Y2MLVPeWxMycIuFwO1EbEXE0c=; b=UEipl2/GtcxsJqAHDRN9mZkhSn4V1bqqay8iuWyUQaOrkvoeXiWZhkN8U46lMRzIOQ4+DwHfaxBoDpRidCX9hsiqK4dRWgf0lby2MkoZTk7kBkmUbNyE1KnUrbXq5p5erj7jpP7NWMTku9sUcZ+bi4LN3Vy5chtOKnlAiexNqm6My+YwTpDSdl2weY6ITxh5l0nfYUb4GCBuGmU4ma0piiOcTWbzyoWiFEZyS0ZoERCMom36uiqwj43ivtg6a+em/P7KSnTCPkP98dV7+GvBIBe0rQP1OhH7S1TVkLu0Hg4e42LbS5DiF4UseSJWq9GO6y9XdLlDo5ZU3noGuY915Q== 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=amdcloud.onmicrosoft.com; s=selector2-amdcloud-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=kHgom6U8HhrEp5mOo4Y2MLVPeWxMycIuFwO1EbEXE0c=; b=H9jt5mF2hvJAM5nq+CbOMCC+4CFwQ71CwYgsl0dGS57fIJYAz+wbDCof2Zc3ZvUTsShiAwk5y6Ap68S7KFOqXR0cHr8DNckEJRfIcpdpqVILekXkXPcyDokCGCZGFKBN/RPK128Cq3m2jRTQZG/j6TkeT9WgtGuIzusjWk8KQL8= Authentication-Results: redhat.com; dkim=none (message not signed) header.d=none;redhat.com; dmarc=none action=none header.from=amd.com; Received: from DM5PR12MB1355.namprd12.prod.outlook.com (2603:10b6:3:6e::7) by DM6PR12MB2987.namprd12.prod.outlook.com (2603:10b6:5:3b::25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3541.23; Tue, 24 Nov 2020 14:57:50 +0000 Received: from DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::d95e:b9d:1d6a:e845]) by DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::d95e:b9d:1d6a:e845%12]) with mapi id 15.20.3589.030; Tue, 24 Nov 2020 14:57:50 +0000 Subject: Re: [PATCH v2 3/6] OvmfPkg: convert ES Reset Block structure to be guided To: Laszlo Ersek , James Bottomley , devel@edk2.groups.io Cc: dovmurik@linux.vnet.ibm.com, Dov.Murik1@il.ibm.com, ashish.kalra@amd.com, brijesh.singh@amd.com, tobin@ibm.com, david.kaplan@amd.com, jon.grimm@amd.com, frankeh@us.ibm.com, "Dr . David Alan Gilbert" References: <20201120184521.19437-1-jejb@linux.ibm.com> <20201120184521.19437-4-jejb@linux.ibm.com> From: "Lendacky, Thomas" Message-ID: Date: Tue, 24 Nov 2020 08:57:48 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 In-Reply-To: X-Originating-IP: [67.79.209.213] X-ClientProxiedBy: SN6PR16CA0066.namprd16.prod.outlook.com (2603:10b6:805:ca::43) To DM5PR12MB1355.namprd12.prod.outlook.com (2603:10b6:3:6e::7) Return-Path: thomas.lendacky@amd.com MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from office-linux.texastahm.com (67.79.209.213) by SN6PR16CA0066.namprd16.prod.outlook.com (2603:10b6:805:ca::43) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3589.20 via Frontend Transport; Tue, 24 Nov 2020 14:57:49 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: c0d446cd-cc22-430f-2a0e-08d890895024 X-MS-TrafficTypeDiagnostic: DM6PR12MB2987: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:10000; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 6ZzGABnhL6HEUi2AkRI0bm6vnoa4G4X6xXEP6oS86VJd91mPvHHwhRrc5bmBbU6AWGSDVtFyKAwgfg2Jo8R1oKkh2oWTlIFHMn5M9XLGhalLvgJVDES0Etr1+fmjmGxiiwGKa9I1Ej8NtQqUPti1ogExIsVG7A9I/Ll5CB6eH78gjoloBbMYyNzNFlHP/LUNLtIw0ur8Mbci2kWguZLDgKA07HHgl9E3h21JzB/NGwekYDlVqKZ6SSafR3lMEFPSRmCSaUURNZjuv7MRKLEFu42f9QdK4Ns+AHz3Psye+zfoTsNVKh2dDiDpMfFTGkgzWHQ6fKOXYOhqvz8HG9aqKIdA24W00Vrl5pLM62APNFuGmRN6mFq6gd4LZ8IVx8natjraCxwMWB9v5Fd6JaT4i88lQmjz2TEAZD2Bm3ANPbzQMdpcG65KQJJPfwUEl5e9hDn6JDO2p18h6CmNFcpy2A== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DM5PR12MB1355.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(376002)(366004)(396003)(39860400002)(346002)(136003)(19627235002)(66946007)(31686004)(6506007)(966005)(53546011)(45080400002)(66476007)(52116002)(86362001)(2906002)(478600001)(66556008)(5660300002)(36756003)(6486002)(8936002)(16526019)(83380400001)(2616005)(26005)(8676002)(186003)(4326008)(31696002)(316002)(110136005)(6512007)(956004)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: HkYx2phYk4WqBqLQAIexTT9q0KcfLsWmaRQ1yHDOV3IhiaUPbV2hX4KG6jYAB7VbM4E2dP57ipfwSBycJDF7Aq+O6Hr51v38ph2seAwkxNc3X2HT5oSBuni5T44bIfzR12chpm0uLTIXJrh3Kwq3gjT4/2NepaXu5Kgvu7M/wb/AjdIUoxpw1ZEcQ4h5MQxmCk85qOA+8TAb0i9U7z4qIIQjpPMTrCeh7wQrsGevt2KU8eMRN56QFmtVhAwhBm6DxFp/HwiVDnBmu+1z73sSUAFo5Z706vuwYYuHpGHLUaqVFGaTgY16HY0/INYjJ50p+oNp9JmT4huL7f2iHT/iurTj6fReJ3zjWOh8AM0FV1mba6UnE6O9pZiaFWPnV3xuzU1gPoBAI5e89DErV+6AHWAwvq+OM7bOw+VK5OQulR3mD0pj6HK7tkUYmltiwSFS3UCo+5IBQe2Nd9JgahDpsnniZaXhOma+uWF8lm/FQtHMruA1mSFz2pMddtxtkDTUTcdTpzoAns0Dy6joHn7Jp8nwr6PZLQGYL1gC9aKeOdTC4gnIuUw1ZGgGlvBjS6I59XmM0C/zujfRnFtgxoHXoK6ESjyJ6VlNHF54X7fTnw+f1ol0HsKkSnMYGWp2PIyOobjcVPrEAB0w1taXs6KBExVCnFq6yYmjOL7vC7VdOBeEcOziCLj/GwtpSgEm4/rs43HPcaZVYbA7Roa6oP78JveZIeqC/ySIy+340hxlCUUM1lIcJ/ESoMx6DVgnZEv9UjAZGySbUtDONhQpXqdfhQp/7PIRJ+gXeJ/Vp6Njt4habAcGpS4cKZTKRPI7i1q+Tg9iF7y0JAlOWSVUZ/vBJzM9PodefThQCDfu0ThUUDwulavVlq1qzrNNYnwPKKq1w4dHrH/aPZOARpuGoMCJlA== X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: c0d446cd-cc22-430f-2a0e-08d890895024 X-MS-Exchange-CrossTenant-AuthSource: DM5PR12MB1355.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Nov 2020 14:57:50.4340 (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: 0J/CxDq1IRI+mvUIMaW25edKVMtwa3weALo9iY0DIuoVVr6FIlNNX3b2n+ivV1hEME5xwcbed48QnVPHh7mhkA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB2987 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 11/23/20 4:16 PM, Laszlo Ersek wrote: > On 11/20/20 19:45, James Bottomley wrote: >> Convert the current ES reset block structure to an extensible guid >> based structure by appending a header and length, which allow for >> multiple guid based data packets to be inserted. I was wondering if this patch should be submitted outside of this series? I'm not sure it makes any difference, other than being able to be merged (possibly) quicker and independent of the series. Then this series simply takes advantage of it. Thanks, Tom >> >> Ref: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3077&data=04%7C01%7Cthomas.lendacky%40amd.com%7Cd9de2a47d7d74b94e04208d88ffd6f03%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637417665948466692%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=x%2F%2Fbn5J9dNcXZqSs8Ui0GgABg6XDbmx%2BUh285EqwLcA%3D&reserved=0 >> Signed-off-by: James Bottomley >> >> --- >> >> v2: added >> --- >> OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 49 +++++++++++++++----- >> 1 file changed, 38 insertions(+), 11 deletions(-) > > (1) Please update the subject line to: > > OvmfPkg/ResetVector: convert SEV-ES Reset Block structure to be GUIDed > > - edk2 prefers including module names too in the patch subjects > - "ES" is harder to understand than "SEV-ES" > - "GUIDed" is harder to misread as "guided" > - subject length is still OK (70 chars) > > >> >> diff --git a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm >> index 980e0138e7fe..baf9d09f3625 100644 >> --- a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm >> +++ b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm >> @@ -25,21 +25,40 @@ ALIGN 16 >> TIMES (0x1000 - ($ - EndOfPageTables) - 0x20) DB 0 >> %endif >> >> +; >> +; Padding to ensure first guid starts at 0xffffffd0 >> +; >> +TIMES (32 - ((guidedStructureEnd - guidedStructureStart) % 32)) DB 0 > > (2) This will insert 32 zero bytes if the size is already aligned to 32 > bytes (because 32-0 = 32). In other words, the above produces 1 to 32 > zero bytes, dependent on table size. > > The variant I proposed in point (5) at > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F67621&data=04%7C01%7Cthomas.lendacky%40amd.com%7Cd9de2a47d7d74b94e04208d88ffd6f03%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637417665948466692%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=nLOZvHpzFMRLV6uEeQvETY1SqI4AaSRc92WYbz8r9cA%3D&reserved=0 > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.redhat.com%2Farchives%2Fedk2-devel-archive%2F2020-November%2Fmsg00781.html&data=04%7C01%7Cthomas.lendacky%40amd.com%7Cd9de2a47d7d74b94e04208d88ffd6f03%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637417665948466692%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=yMYv%2BKKFtCs9G7qgdheAVmS2iLexxJWjbgSTTFcCetM%3D&reserved=0 > > takes this into account, and only prepends 0 to 31 bytes (inclusive): > > TIMES (31 - (guidedStructureEnd - guidedStructureStart + 31) % 32) DB 0 > > - This variant subtracts 1 inside the remainder operation (which is > expressed as adding 31). > > - For compensation, it adds 1 just outside of the remainder operation. > This addition in effect increases the subtrahend for the leftmost 32. > Therefore this (-1) addend is ultimately folded into the leftmost 32, > yielding 31 on the leftmost side. > > TIMES (32 - ((guidedStructureEnd - guidedStructureStart - 1) % 32 + 1)) DB 0 > ^^^ ^^^ > | | > | compensate > | in the > | remainder > | > slide down residue > class modulo 32 > > > TIMES (32 - ((guidedStructureEnd - guidedStructureStart + 31) % 32) - 1) DB 0 > ^^^^ ^^^ > | | > | unnest > | increment > | from > | subtrahend > | > express modular > subtraction as > addition, to avoid > using % on a negative > integer (in case size > were 0) > > TIMES (31 - ((guidedStructureEnd - guidedStructureStart + 31) % 32)) DB 0 > ^^ > | > fold previous (-1) addend into leftmost constant > > - This juggling of 1 results in no changes for residue classes 1 through > 31, but wraps the outermost result (the padding size) for residue > class 0, from 32 to 0. > > >> + >> +; Guided structure. To traverse this you should first verify the >> +; presence of the table header guid > > (3) suggest "table footer GUID" (the GUID follows the data, in address > order) > >> +; (96b582de-1fb2-45f7-baea-a366c55a082d) at 0xffffffd0. If that >> +; is found, the two bytes at 0xffffffce are the entire table length. > > (4) can we make the whole table size field 32-bit? I don't have a > particular use case in mind, it just looks more extensible than 16-bit. > We can still keep the individual structs we have in mind 16-bit sized. > >> +; >> +; The table is composed of structures with the form: >> +; >> +; Data (arbitrary bytes identified by guid) >> +; length from start of guid to end of data (2 bytes) > > (5) This is hard to interpret, as "data" precedes "guid" in address > space (guid is a footer, not a header). > > I suggest "length from start of data to end of GUID" > > >> +; guid (16 bytes) >> +; >> +; so work back from the header using the length to traverse until you > > (6) suggest "from the footer" > > >> +; either find the guid you're looking for or run off the end of the >> +; table. > > (7) suggest "run off the beginning of the table" > > ... I realize "start" and "end" can be interpreted temporally and > spatially. In a forward traversal they are the same, but now they > aren't. I suggest we use the spatial (address space order) > interpretation. > >> +; >> +guidedStructureStart: >> + >> ; >> ; SEV-ES Processor Reset support >> ; >> ; sevEsResetBlock: >> ; For the initial boot of an AP under SEV-ES, the "reset" RIP must be >> -; programmed to the RAM area defined by SEV_ES_AP_RESET_IP. A known offset >> -; and GUID will be used to locate this block in the firmware and extract >> -; the build time RIP value. The GUID must always be 48 bytes from the >> -; end of the firmware. >> +; programmed to the RAM area defined by SEV_ES_AP_RESET_IP. The data >> +; format is >> ; >> -; 0xffffffca (-0x36) - IP value >> -; 0xffffffcc (-0x34) - CS segment base [31:16] >> -; 0xffffffce (-0x32) - Size of the SEV-ES reset block >> -; 0xffffffd0 (-0x30) - SEV-ES reset block GUID >> -; (00f771de-1a7e-4fcb-890e-68c77e2fb44e) >> +; IP value [0:15] >> +; CS segment base [31:16] >> +; >> +; SEV-ES reset block GUID: 00f771de-1a7e-4fcb-890e-68c77e2fb44e > > (8) Did I understand from the v1 discussion that the corresponding QEMU > parser is not upstream yet? (Or at least not released?) > > (9) The 16-bit size field of the SEV-ES reset block structure is not > documented. > > >> ; >> ; A hypervisor reads the CS segement base and IP value. The CS segment base >> ; value represents the high order 16-bits of the CS segment base, so the >> @@ -48,8 +67,6 @@ ALIGN 16 >> ; program the EIP register with the IP value as read. >> ; >> >> -TIMES (32 - (sevEsResetBlockEnd - sevEsResetBlockStart)) DB 0 >> - >> sevEsResetBlockStart: >> DD SEV_ES_AP_RESET_IP >> DW sevEsResetBlockEnd - sevEsResetBlockStart >> @@ -57,6 +74,16 @@ sevEsResetBlockStart: >> DB 0x89, 0x0E, 0x68, 0xC7, 0x7E, 0x2F, 0xB4, 0x4E >> sevEsResetBlockEnd: >> >> +; >> +; Table header: length of whole table followed by table header > > (10) I suggest "table footer" (twice) > > >> +; guid: 96b582de-1fb2-45f7-baea-a366c55a082d >> +; >> + DW guidedStructureEnd - guidedStructureStart >> + DB 0xDE, 0x82, 0xB5, 0x96, 0xB2, 0x1F, 0xF7, 0x45 >> + DB 0xBA, 0xEA, 0xA3, 0x66, 0xC5, 0x5A, 0x08, 0x2D >> + >> +guidedStructureEnd: >> + >> ALIGN 16 >> >> applicationProcessorEntryPoint: >> > > Thanks! > Laszlo >