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.81]) by mx.groups.io with SMTP id smtpd.web09.175.1620837321057321008 for ; Wed, 12 May 2021 09:35:21 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@amd.com header.s=selector1 header.b=RuAF7la2; 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.81, mailfrom: brijesh.singh@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ZDPSpFTRExUqv42yUC/EyBQdzR9arxlWERV52pkwi0+8UJKisoCQxHPKNhDhCwXytesGObiyyU/6oXHuMT2DsP23r8rW6ZJsL9xly5LA/OWRw6YLvaGr197dLNbFDv8djapPxA5HnqGzW2CFo6zuCptNIm7lDxvWHMrNCGLdoj52NOwJyArKga3liNAfAZr34bPS2gFA6VKfuVQj0AQiC7ye6hmtKnk3eFIIGB9Y6YeoFx490a4yCzi9ffRVTKKJwygSNsqWgKBmVBI0DI00c/YBlAvDji4euLnHeGIV52v15++4xP6aW8Kp5VgmpXQ84qGtOI9VEga8eTfk/lYJog== 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=uXQpo9Iv01h9z70UZYKRPizb3/bnk1wjTVwn5kIywFI=; b=eRp/12kyPiLtL/CX4M4XpcNmpAeW0CYzRmMN/hYsBox8yUvZhSh7+7UsWahwOBTTxjPulOIt/b2Sa7cgGZiKmdS7A+mdkOsgt/kz+VhmCr0FCwLzn3Tsxot4gLU3QPL8z/flz/7WlaYn9/jN5FEb9gLSE+0Ct+VV4Y4kHL9FpWwaGAibAw0q3UYo6E10tER9dzsL7WFeGd7Ru2SK+rxhDIh80gRMInGnk2a1JvkgFsLISgJ54YqeELD6kjRiuQffDadOD0Yc7ZvRSOMIDsnTVHJJfrmSzcQzufy2imEuoFJZWfNLdDCi6bV/B2sTrMZsQyBo7J/1fKJ/JDv7LhAXQg== 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=uXQpo9Iv01h9z70UZYKRPizb3/bnk1wjTVwn5kIywFI=; b=RuAF7la2vPrPygtDy6sxvV2oCRr/vNICc6xHti58FhzD1UBoZWkFt2ZAtaYOHwwALIfEjowsK0k03syfl5Pzv2yW+8eDwKA3JhqLu5ffgc5ox42cnrJV2TX8sqwag9nDk1EzJF5fhrB6JgmZnl14y2KvLIFJLBwXn+YxB6gdxkM= 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 SN6PR12MB2638.namprd12.prod.outlook.com (2603:10b6:805:6f::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4108.27; Wed, 12 May 2021 16:35:18 +0000 Received: from SN6PR12MB2718.namprd12.prod.outlook.com ([fe80::9898:5b48:a062:db94]) by SN6PR12MB2718.namprd12.prod.outlook.com ([fe80::9898:5b48:a062:db94%6]) with mapi id 15.20.4129.025; Wed, 12 May 2021 16:35:18 +0000 CC: brijesh.singh@amd.com, James Bottomley , Min Xu , Jiewen Yao , Tom Lendacky , Jordan Justen , Ard Biesheuvel , Erdem Aktas Subject: Re: [edk2-devel] [PATCH 13/13] OvmfPkg/BaseMemEncryptSevLib: remove Flush parameter To: Laszlo Ersek , devel@edk2.groups.io References: <20210507203838.23706-1-brijesh.singh@amd.com> <20210507203838.23706-14-brijesh.singh@amd.com> <3c85698e-0b5e-38b0-f752-30204cb78cad@redhat.com> <41c63c63-554d-1f7d-5d27-bd649261e897@amd.com> From: "Brijesh Singh" Message-ID: <2833c64b-3782-dcc2-5c09-aeeb6bb061f5@amd.com> Date: Wed, 12 May 2021 11:35:16 -0500 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.10.0 In-Reply-To: <41c63c63-554d-1f7d-5d27-bd649261e897@amd.com> X-Originating-IP: [70.112.153.56] X-ClientProxiedBy: SN6PR05CA0013.namprd05.prod.outlook.com (2603:10b6:805:de::26) To SN6PR12MB2718.namprd12.prod.outlook.com (2603:10b6:805:6f::22) Return-Path: brijesh.singh@amd.com MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from Brijeshs-MacBook-Pro.local (70.112.153.56) by SN6PR05CA0013.namprd05.prod.outlook.com (2603:10b6:805:de::26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4150.11 via Frontend Transport; Wed, 12 May 2021 16:35:17 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 1ead1f19-c994-4252-43f1-08d91563ed81 X-MS-TrafficTypeDiagnostic: SN6PR12MB2638: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:849; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: +RDYv99OmljN9K5Xr5U+FIPwj+qh5gqw1uIAugduv8WLk6rzEYf/WkT1GFua96FXIxsH5cHQmKVo64fftZ/qCCx4jKDij7jKT1W4RggLE7sUkMxRgV483YhyE0v0nigl4Aej54P5Z+LYCowiLE1WtyHQiaJKd+aQ2A7KVnlJ/iAmDE4AbQzY0cD4flQGvi30JHYcbsQATvOdiQduGUlmMcGr5gsSzLaDKCJh57irFGgwxg9Bs91MEAVmDq6LdFmniVxCsmUnpzMy7L8HBF2/F2CN0//zZsjoyhSti9Gl8XSTdJvtuaNgfDo6XpOD2lG7fS5c3PuPZiThh8mzNeQaRl6hx3VF6XCeenfJx6qYeqO/jLc8z0kfW39xW8IAQF8a9SiqLfDD/gHzlt/lvOt7aORg4zZ+Ik/cnCg8NyBTK9LPy9ncddi/SdWRnAttBh2/5V5g+EIT0Jd26sWgExzytx4repmFXDwUuH5hghuDAHNFaDBgtq1Olphkc5OaRMyw+ZT3KbXjDkKAnHaBw5NuJdpSk72JsYCtC45dc05H57Z70FCHRtXrxdNaw79lZ2sn3MmNmr2ubQwrw8rH8Si6/mmNyqqvCcv6irRXLiHTLAJhZo15CfrXW+oSAfHK6ma7TbunQ6cLfRyByfr0lwyw4Q4Ah18v11fZoz7qDqpBpsMv98MuuBUaiOTH1kf+qQhQ57fTn19n8K9aiws4Qz71Wx7jLeEU4u4kLMCtjEmsaFp6EWPoVNbv/myeNcwaSx6WpZM4AspccMM3PtZJ3ZlbAwufjFK3Wsg9tQuZTZJ61JGzsTgBezmywmKYwL+dJAqRnvMLm48qZaqqxDR9Mm1vog== 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)(366004)(39860400002)(396003)(346002)(136003)(376002)(38350700002)(316002)(966005)(36756003)(6486002)(86362001)(8676002)(6512007)(30864003)(2616005)(44832011)(6506007)(31686004)(38100700002)(26005)(956004)(83380400001)(2906002)(53546011)(66476007)(52116002)(186003)(66556008)(66946007)(8936002)(5660300002)(31696002)(54906003)(45080400002)(478600001)(19627235002)(4326008)(16526019)(213903007)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: =?us-ascii?Q?LfIgoTBRnd8QZvTnVSFauokkqJyivvOmziTvbhjNcmHX1ZAqh4XlwpaQiH+n?= =?us-ascii?Q?PA8EhtK8t4w2FTSi2Ec+7M+tDKmebhveDuOaJnv4/5Hb8UrL4leXBQqVbQdO?= =?us-ascii?Q?5s7iELPCQ1Wtz2pqjqrr6K8bIWvG3mcjhHql6wGRVgXSzE5EiXEPr8tUBL/d?= =?us-ascii?Q?CNTdz16OmwfTFuzYNlvRjYbyQBT2RiYAC/ni5NtnLP5KxVctxxgiwFH8XGFY?= =?us-ascii?Q?lVZHQue/5jigWji/314x/quZ7zvII9/zpcDyxqT2UO2gnnyPbnFFDmDqHzpO?= =?us-ascii?Q?iCF5whnF9BOVgQ/M1S8UOdMmfFtvTzQdbhfufRM+emxgLF1wBcbScBuxcKMV?= =?us-ascii?Q?v0M0md8bIpmTj22132F3ILx8DZEYuf92HqecPbweqMDfM+neiwnm/1PgO6uG?= =?us-ascii?Q?F61aVOnK136gw47wvjwRUndifwHWcM9nefZV8p/9OdsIRjtHMZtk9USakc6f?= =?us-ascii?Q?/PJVkRZ4NzrOcKiluuhXnQXbXpRmAsGcol21hgsUjRkeH0N8S8kC7uO1pRWh?= =?us-ascii?Q?jlWrWpOTdcmN1bH513MrvxharLENsU7CX4U3za3Ag3L/tSRnwZx3HL0lcEd/?= =?us-ascii?Q?2coOs2PamnUs5ow6DCQjMUhBRmhiksnV3YtfRGLEaUpTiCLy55D0iUY7yp53?= =?us-ascii?Q?2Nepc/D6DTliPgygBwiYHRUla8EM/g7RGorx+Yg3cvE0J8Gt2vBe9xMf+U/R?= =?us-ascii?Q?slzhMP13pWpvVfnUNy29nEZ6rWFNC0QWuvWqU60qs4IQz+Uz+AVjP3YHyKQo?= =?us-ascii?Q?O3qufXflrnGuHgmvhrXIiEZqbVOFh8zNG+oXBOxOTEA4bAbM4nsLBeRowQlQ?= =?us-ascii?Q?1eG3BI+dqYiqRIXNLkUJbKCX1LoCr4D6CJqaUbF1uocY58wwm9l8IiHm3WWX?= =?us-ascii?Q?f3DmBq8lvTuPqWvDbAUWg/vThE1Zo7bdsXC3IMUNjIdXum2T1Xn+zGauiELx?= =?us-ascii?Q?qRJCh9DTao+M9caq/6XO0W8W9DQnep9sQkWH+ZxUDokEIY8ZjKFJKYhy8K7d?= =?us-ascii?Q?NUFYmtyacdYdfN+O1AwQ9u0NdI9N1hUEkQmphHaX9PvQwI/su40SUYIgKW8T?= =?us-ascii?Q?tvJm/IVTkqwZ4LqeBO5pzcqK6DyFMP9hgAK9KzEx1+perl60aFcHejJm1nKS?= =?us-ascii?Q?ZYcw7rb6Y+UjrqCne5uaiK+QZ5JHSg+xJeqrZ1sBPNar+bCqfHQVG5NIIBHc?= =?us-ascii?Q?QWTGvMp64eZfo+lsvx4ZXe+xEQiczW3K4EugpWu0zbgds212QqSlyW+MiqRe?= =?us-ascii?Q?ugkYSih7MZmHYgbZACARDl75YzNJqyQmrQhSNDXCKa38qvkwlw+ngtzWy4b/?= =?us-ascii?Q?LLBydb7ZhHjos4sQVQAzwtqh?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 1ead1f19-c994-4252-43f1-08d91563ed81 X-MS-Exchange-CrossTenant-AuthSource: SN6PR12MB2718.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 May 2021 16:35:18.1862 (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: icjQEVVdJQlZv79HdetAbWLOaALufCmEZE3/owmVrhIgI9IckVPvbzzhqqesQNYjS2V7rLep3+X3adjI+KD97g== X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR12MB2638 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Content-Language: en-US On 5/11/21 12:45 PM, Brijesh Singh wrote: > On 5/11/21 6:55 AM, Laszlo Ersek wrote: >> I don't fully understand the updates in this patch: >> >> On 05/07/21 22:38, Brijesh Singh wrote: >>> BZ: https://nam11.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2= Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3275&data=3D04%7C01%7Cbri= jesh.singh%40amd.com%7Cc383d8fdc1264644760508d91473b003%7C3dd8961fe4884e608= e11a82d994e183d%7C0%7C0%7C637563309382960811%7CUnknown%7CTWFpbGZsb3d8eyJWIj= oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sda= ta=3DM3xHM2yU0m3VtPn1xGe1k5Wq0d6Vbdf9gMqDX1NxpgA%3D&reserved=3D0 >>> >>> The Flush parameter is used to provide a hint whether the specified ran= ge >>> is Mmio address. Now that we have a dedicated helper to clear the >>> memory encryption mask for the Mmio address range, its safe to remove t= he >>> Flush parameter from MemEncryptSev{Set,Clear}PageEncMask(). >> This looks good; it matches my request (1) from: >> >> https://nam11.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Flist= man.redhat.com%2Farchives%2Fedk2-devel-archive%2F2021-May%2Fmsg00109.html&a= mp;data=3D04%7C01%7Cbrijesh.singh%40amd.com%7Cc383d8fdc1264644760508d91473b= 003%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637563309382960811%7CUnkno= wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVC= I6Mn0%3D%7C1000&sdata=3D3TQrliMABDaa%2FtCN%2FvKewTmLfVRKIou2La5yRGGyzJY= %3D&reserved=3D0 >> >>> Cc: James Bottomley >>> Cc: Min Xu >>> Cc: Jiewen Yao >>> Cc: Tom Lendacky >>> Cc: Jordan Justen >>> Cc: Ard Biesheuvel >>> Cc: Laszlo Ersek >>> Cc: Erdem Aktas >>> Signed-off-by: Brijesh Singh >>> --- >>> OvmfPkg/Include/Library/MemEncryptSevLib.h | 10 ++---- >>> .../BaseMemEncryptSevLib/X64/VirtualMemory.h | 10 ++---- >>> OvmfPkg/AmdSevDxe/AmdSevDxe.c | 3 +- >>> OvmfPkg/IoMmuDxe/AmdSevIoMmu.c | 6 ++-- >>> .../Ia32/MemEncryptSevLib.c | 10 ++---- >>> .../X64/MemEncryptSevLib.c | 16 +++------- >>> .../X64/PeiDxeVirtualMemory.c | 32 +++++++++++-------- >>> .../X64/SecVirtualMemory.c | 8 ++--- >>> .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 3 +- >>> OvmfPkg/PlatformPei/AmdSev.c | 3 +- >>> 10 files changed, 35 insertions(+), 66 deletions(-) >>> >>> diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h b/OvmfPkg/Inclu= de/Library/MemEncryptSevLib.h >>> index b91490d5d44d..76d06c206c8b 100644 >>> --- a/OvmfPkg/Include/Library/MemEncryptSevLib.h >>> +++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h >>> @@ -100,8 +100,6 @@ MemEncryptSevIsEnabled ( >>> address of a memory region. >>> @param[in] NumPages The number of pages from start m= emory >>> region. >>> - @param[in] Flush Flush the caches before clearing= the bit >>> - (mostly TRUE except MMIO address= es) >>> =20 >>> @retval RETURN_SUCCESS The attributes were cleared for = the >>> memory region. >>> @@ -114,8 +112,7 @@ EFIAPI >>> MemEncryptSevClearPageEncMask ( >>> IN PHYSICAL_ADDRESS Cr3BaseAddress, >>> IN PHYSICAL_ADDRESS BaseAddress, >>> - IN UINTN NumPages, >>> - IN BOOLEAN Flush >>> + IN UINTN NumPages >>> ); >>> =20 >>> /** >>> @@ -128,8 +125,6 @@ MemEncryptSevClearPageEncMask ( >>> address of a memory region. >>> @param[in] NumPages The number of pages from start m= emory >>> region. >>> - @param[in] Flush Flush the caches before setting = the bit >>> - (mostly TRUE except MMIO address= es) >>> =20 >>> @retval RETURN_SUCCESS The attributes were set for the = memory >>> region. >>> @@ -142,8 +137,7 @@ EFIAPI >>> MemEncryptSevSetPageEncMask ( >>> IN PHYSICAL_ADDRESS Cr3BaseAddress, >>> IN PHYSICAL_ADDRESS BaseAddress, >>> - IN UINTN NumPages, >>> - IN BOOLEAN Flush >>> + IN UINTN NumPages >>> ); >>> =20 >>> =20 >>> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h b= /OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h >>> index 8dc39e647b90..21bbbd1c4f9c 100644 >>> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h >>> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h >>> @@ -58,8 +58,6 @@ InternalGetMemEncryptionAddressMask ( >>> @param[in] PhysicalAddress The physical address that is the= start >>> address of a memory region. >>> @param[in] Length The length of memory region >>> - @param[in] Flush Flush the caches before applying= the >>> - encryption mask >>> =20 >>> @retval RETURN_SUCCESS The attributes were cleared for = the >>> memory region. >>> @@ -72,8 +70,7 @@ EFIAPI >>> InternalMemEncryptSevSetMemoryDecrypted ( >>> IN PHYSICAL_ADDRESS Cr3BaseAddress, >>> IN PHYSICAL_ADDRESS PhysicalAddress, >>> - IN UINTN Length, >>> - IN BOOLEAN Flush >>> + IN UINTN Length >>> ); >>> =20 >>> /** >>> @@ -85,8 +82,6 @@ InternalMemEncryptSevSetMemoryDecrypted ( >>> @param[in] PhysicalAddress The physical address that is the= start >>> address of a memory region. >>> @param[in] Length The length of memory region >>> - @param[in] Flush Flush the caches before applying= the >>> - encryption mask >>> =20 >>> @retval RETURN_SUCCESS The attributes were set for the = memory >>> region. >>> @@ -99,8 +94,7 @@ EFIAPI >>> InternalMemEncryptSevSetMemoryEncrypted ( >>> IN PHYSICAL_ADDRESS Cr3BaseAddress, >>> IN PHYSICAL_ADDRESS PhysicalAddress, >>> - IN UINTN Length, >>> - IN BOOLEAN Flush >>> + IN UINTN Length >>> ); >>> =20 >>> /** >>> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDx= e.c >>> index 80831b81facf..41e4b291d070 100644 >>> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c >>> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c >>> @@ -120,8 +120,7 @@ AmdSevDxeEntryPoint ( >>> Status =3D MemEncryptSevClearPageEncMask ( >>> 0, // Cr3BaseAddress -- use current CR3 >>> MapPagesBase, // BaseAddress >>> - MapPagesCount, // NumPages >>> - TRUE // Flush >>> + MapPagesCount // NumPages >>> ); >>> if (EFI_ERROR (Status)) { >>> DEBUG ((DEBUG_ERROR, "%a: MemEncryptSevClearPageEncMask(): %r\n"= , >> (1) You missed my comment (2) in >> . > Ah, I will fix in rev2. > > >>> diff --git a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c b/OvmfPkg/IoMmuDxe/AmdSevIo= Mmu.c >>> index 49ffa2448811..b30628078f73 100644 >>> --- a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c >>> +++ b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c >>> @@ -252,8 +252,7 @@ IoMmuMap ( >>> Status =3D MemEncryptSevClearPageEncMask ( >>> 0, >>> MapInfo->PlainTextAddress, >>> - MapInfo->NumberOfPages, >>> - TRUE >>> + MapInfo->NumberOfPages >>> ); >>> ASSERT_EFI_ERROR (Status); >>> if (EFI_ERROR (Status)) { >>> @@ -407,8 +406,7 @@ IoMmuUnmapWorker ( >>> Status =3D MemEncryptSevSetPageEncMask ( >>> 0, >>> MapInfo->PlainTextAddress, >>> - MapInfo->NumberOfPages, >>> - TRUE >>> + MapInfo->NumberOfPages >>> ); >>> ASSERT_EFI_ERROR (Status); >>> if (EFI_ERROR (Status)) { >>> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib= .c b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c >>> index 169d3118e44f..be260e0d1014 100644 >>> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c >>> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c >>> @@ -25,8 +25,6 @@ >>> address of a memory region. >>> @param[in] NumPages The number of pages from start m= emory >>> region. >>> - @param[in] Flush Flush the caches before clearing= the bit >>> - (mostly TRUE except MMIO address= es) >>> =20 >>> @retval RETURN_SUCCESS The attributes were cleared for = the >>> memory region. >>> @@ -39,8 +37,7 @@ EFIAPI >>> MemEncryptSevClearPageEncMask ( >>> IN PHYSICAL_ADDRESS Cr3BaseAddress, >>> IN PHYSICAL_ADDRESS BaseAddress, >>> - IN UINTN NumPages, >>> - IN BOOLEAN Flush >>> + IN UINTN NumPages >>> ) >>> { >>> // >>> @@ -59,8 +56,6 @@ MemEncryptSevClearPageEncMask ( >>> address of a memory region. >>> @param[in] NumPages The number of pages from start m= emory >>> region. >>> - @param[in] Flush Flush the caches before setting = the bit >>> - (mostly TRUE except MMIO address= es) >>> =20 >>> @retval RETURN_SUCCESS The attributes were set for the = memory >>> region. >>> @@ -73,8 +68,7 @@ EFIAPI >>> MemEncryptSevSetPageEncMask ( >>> IN PHYSICAL_ADDRESS Cr3BaseAddress, >>> IN PHYSICAL_ADDRESS BaseAddress, >>> - IN UINTN NumPages, >>> - IN BOOLEAN Flush >>> + IN UINTN NumPages >>> ) >>> { >>> // >>> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.= c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c >>> index a2bf698bcde7..a57e8fd37fa7 100644 >>> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c >>> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c >>> @@ -27,8 +27,6 @@ >>> address of a memory region. >>> @param[in] NumPages The number of pages from start m= emory >>> region. >>> - @param[in] Flush Flush the caches before clearing= the bit >>> - (mostly TRUE except MMIO address= es) >>> =20 >>> @retval RETURN_SUCCESS The attributes were cleared for = the >>> memory region. >>> @@ -41,15 +39,13 @@ EFIAPI >>> MemEncryptSevClearPageEncMask ( >>> IN PHYSICAL_ADDRESS Cr3BaseAddress, >>> IN PHYSICAL_ADDRESS BaseAddress, >>> - IN UINTN NumPages, >>> - IN BOOLEAN Flush >>> + IN UINTN NumPages >>> ) >>> { >>> return InternalMemEncryptSevSetMemoryDecrypted ( >>> Cr3BaseAddress, >>> BaseAddress, >>> - EFI_PAGES_TO_SIZE (NumPages), >>> - Flush >>> + EFI_PAGES_TO_SIZE (NumPages) >>> ); >>> } >>> =20 >>> @@ -63,8 +59,6 @@ MemEncryptSevClearPageEncMask ( >>> address of a memory region. >>> @param[in] NumPages The number of pages from start m= emory >>> region. >>> - @param[in] Flush Flush the caches before setting = the bit >>> - (mostly TRUE except MMIO address= es) >>> =20 >>> @retval RETURN_SUCCESS The attributes were set for the = memory >>> region. >>> @@ -77,15 +71,13 @@ EFIAPI >>> MemEncryptSevSetPageEncMask ( >>> IN PHYSICAL_ADDRESS Cr3BaseAddress, >>> IN PHYSICAL_ADDRESS BaseAddress, >>> - IN UINTN NumPages, >>> - IN BOOLEAN Flush >>> + IN UINTN NumPages >>> ) >>> { >>> return InternalMemEncryptSevSetMemoryEncrypted ( >>> Cr3BaseAddress, >>> BaseAddress, >>> - EFI_PAGES_TO_SIZE (NumPages), >>> - Flush >>> + EFI_PAGES_TO_SIZE (NumPages) >>> ); >>> } >>> =20 >>> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemo= ry.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c >>> index a18d336a8789..ad1021bd3e43 100644 >>> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c >>> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c >>> @@ -555,8 +555,7 @@ EnableReadOnlyPageWriteProtect ( >>> address of a memory region. >>> @param[in] Length The length of memory region >>> @param[in] Mode Set or Clear mode >>> - @param[in] CacheFlush Flush the caches before applying= the >>> - encryption mask >>> + @param[in] Mmio The physical address range is Mm= io. >>> =20 >>> @retval RETURN_SUCCESS The attributes were cleared for = the >>> memory region. >>> @@ -572,7 +571,7 @@ SetMemoryEncDec ( >>> IN PHYSICAL_ADDRESS PhysicalAddress, >>> IN UINTN Length, >>> IN MAP_RANGE_MODE Mode, >>> - IN BOOLEAN CacheFlush >>> + IN BOOLEAN Mmio >>> ) >>> { >>> PAGE_MAP_AND_DIRECTORY_POINTER *PageMapLevel4Entry; >>> @@ -585,12 +584,23 @@ SetMemoryEncDec ( >>> UINT64 AddressEncMask; >>> BOOLEAN IsWpEnabled; >>> RETURN_STATUS Status; >>> + BOOLEAN CacheFlush; >>> =20 >>> // >>> // Set PageMapLevel4Entry to suppress incorrect compiler/analyzer wa= rnings. >>> // >>> PageMapLevel4Entry =3D NULL; >>> =20 >>> + // >>> + // The cache need to flushed for the non-Mmio address range. >>> + // >>> + if (Mmio =3D=3D TRUE) { >>> + CacheFlush =3D FALSE; >>> + } else { >>> + CacheFlush =3D TRUE; >>> + } >>> + >>> + // >>> DEBUG (( >>> DEBUG_VERBOSE, >>> "%a:%a: Cr3Base=3D0x%Lx Physical=3D0x%Lx Length=3D0x%Lx Mode=3D%a = CacheFlush=3D%u\n", >> (2) The calculation of "CacheFlush" from "Mmio" is awkward. First, we >> don't compare BOOLEANs against TRUE or FALSE, BOOLEANs just stand alone >> in controlling expression (or otherwise "logical") context. Second, why >> not just write: >> >> CacheFlush =3D !Mmio; >> >> But even so... >> >> (3) ... The introduction of the "Mmio" parameter is inexplicable to me. >> It apparently replaces CacheFlush (with inverse meaning), but neither >> the commit message, nor the (RFCv2 -> PATCH) changelog, explain why this >> replacement makes sense. > The=C2=A0 internal function is used for clearing the mask for both system= RAM > as well as Mmio, so we need a way to tell the internal function that > call is for the Mmio range. I thought making all the changes in a single > file makes sense but I see it can get harder for the review. I guess I > could split the work in two patches > > 1) Drop the cache flush param from high level > MemEncryptSev{Set,Clear}PageEncMask and don't touch anything in the > SetMemoryEncDec() > > 2) Rename the Flush parameter to Mmio in the SetMemoryEncDec() > > Does it makes sense to you ? I am going to drop the Mmio hunk from this patch to make it sane. I was thinking ahead for the SEV-SNP support; For SEV and ES support, the SetMemoryEncDec() does not need to know whether its called for Mmio or RAM address. In SNP, the SetMemoryEncDec() will need to know this information so that it can skip the page state change. -Brijesh