From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM04-MW2-obe.outbound.protection.outlook.com (NAM04-MW2-obe.outbound.protection.outlook.com [40.107.101.45]) by mx.groups.io with SMTP id smtpd.web09.878.1664808765432385538 for ; Mon, 03 Oct 2022 07:52:45 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@amd.com header.s=selector1 header.b=Zp4+yRC4; 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.101.45, mailfrom: thomas.lendacky@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=fbsDH3SnSYwrG1jopGzw05q+5QveFnyHz5stsaFKXnQxyoJnK/FpGamLcPTv/2K4ZLbhymttc/XVLObzqACEjBOAC3axTWBii0I6xS8hngb86OPDNeQKcdUFh4YPFGTATIG4Zddehcp8xAcNmHoyqCkMRsvXKSIgaG/dRTO0s2m/yDfOo9w6YxkZ2S6wgtU7WR6q8jyMXYE9uqTpwc9DgQxzSekcxbXHhBBWhvNhPn7w7xp41gF1hKSn/vCUAgAL5p69tfgrvlaMfYSbJKJCSLi6lA/4MBEOJq7DNmPs4NsyxfzeFhmve4ZmX5ahS4trC6pmfWFEBObHTYDI/h4V/g== 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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=mjxTiVhOtSm4AcJjCg00GriSUZNew9GaW0qMj/l5xdI=; b=X+vr6hb4vbaCtsKajxknnxV/CkdTQa8X3b0RAN4gGL1S3Vi1+clVr4aEJDpaUYd7q0sR6hYM0jzofQXOlgXbzALgwGIPXqUytQKzBE6PqKOEGlWmik7/dBvJ8h6CWsNXd1OWg3V9fHgWmkHMNIhzGD96MmWwl2M3ybHpnOsdcunpRu/y5rGeXpNy74cw6Ynulc2qlsZBKAgNC0Q6MHNehEzVJLEjfX+4Tk5t/UUsCEwHO1tG0hMNI2sJBCAGhwClNrzP4jy1cjIgKrGhykGXlF6W3SgPSfULICGHBtAxvieVHWhUyeA9xvFLAwakgAJouiHR2autPP0oWCQa6nqjgg== 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=mjxTiVhOtSm4AcJjCg00GriSUZNew9GaW0qMj/l5xdI=; b=Zp4+yRC4sJahKw/7p66s2vzE3NcAbzDCTs7COh5dXDbZkCML8eDDnbGh/9O7nnOGqBeVTpCdq1Brl2HCgZElCaZw0/++1iX1o8XNqSHmEV67aWXtsJnYgAS82ae/p47EFpJaX2WGimuRGJYLSZmFtvLIuUuvULbrW0UeTwhpFiM= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; Received: from DM4PR12MB5229.namprd12.prod.outlook.com (2603:10b6:5:398::12) by MW4PR12MB7334.namprd12.prod.outlook.com (2603:10b6:303:219::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5676.24; Mon, 3 Oct 2022 14:52:43 +0000 Received: from DM4PR12MB5229.namprd12.prod.outlook.com ([fe80::c175:4c:c0d:1396]) by DM4PR12MB5229.namprd12.prod.outlook.com ([fe80::c175:4c:c0d:1396%4]) with mapi id 15.20.5676.030; Mon, 3 Oct 2022 14:52:43 +0000 Message-ID: <2ecad3cc-b3bd-5470-30f9-7290c1d9b2e6@amd.com> Date: Mon, 3 Oct 2022 09:52:41 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH v5 1/7] OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe To: Dionna Glaze , devel@edk2.groups.io Cc: Gerd Hoffmann , James Bottomley , Jiewen Yao References: <20220930230627.3371754-1-dionnaglaze@google.com> <20220930230627.3371754-2-dionnaglaze@google.com> From: "Lendacky, Thomas" In-Reply-To: <20220930230627.3371754-2-dionnaglaze@google.com> X-ClientProxiedBy: CH0PR03CA0046.namprd03.prod.outlook.com (2603:10b6:610:b3::21) To DM4PR12MB5229.namprd12.prod.outlook.com (2603:10b6:5:398::12) Return-Path: Thomas.Lendacky@amd.com MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DM4PR12MB5229:EE_|MW4PR12MB7334:EE_ X-MS-Office365-Filtering-Correlation-Id: 7b972fa9-f374-43cf-1ae7-08daa54eed32 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 0XxwCIffw2lJOHzbOrNdkUNioBm+1bZV53Qj6WO6P5lrO11Lb8GFExF1O9Hcb+e4U0kEMByehR/ky7pcTo0hAf/9IY63Wk5cCfdKnIdwp4qr0pkT48sZXUM4BlUayYEvB2FZBKjNevwy1eI/79RyXfGFJBOk8XNFvw1z1AFiiEevkKNeXPYtPaqoF7ZJe9KPoQmzvHqp8G+ZjlFZJFNspwig5hLrnITN75ntvI0jLTir0IuOjBTDBKNgIQw3N8Qr/2SEVCfSjbFg9Nl/LhadUGNBcjxuTYK9UjPiJy1BltFH13UmbScEG0CmZqTXC1tqdG0/e48jVSh9Wh6mLE6mdREjRQ77YpZIt8cTfn+o2t51QAFC2SzbrPsgD/UZ4cVD5MoV4WAGFvx2AMt9YnIIdXXLveN/DXAzDUeNCd+rYFncmo8y+fmpdo3TJgmHWh1Z8/33V0gXqaeNuWsaCPRZdquOMVarRWLXVGLvZRikWPijCqTCgmCZroHPlV4wq/+zH0LSoF6TDvs8Q6+7DRxmPkBDWnR9TSCPRmhBekKcYVMmSuC9B8+iV8AZP86l70NUlY027dsMehDKHw6U+fhx/YIG3o50tPBQL92fCFJg0rf9xS9PL1QqsWvwk/7R/0/H18kJp+NdzhtHwuIgjVGpt3I+wqjHs5Wx7qRoegb3XTyZK1riYdyQxtw9LjCLO1mtFfSuStZ2AwghvxqaMllv14ZLkFyK4AWz+m66ZWuLwGS2ekQdC6Q7waBsYjPOVCzddcFanO0wJtuQIa5IYIzUdCklrmeJg5SN8RIedHccwNY= X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DM4PR12MB5229.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230022)(4636009)(376002)(39860400002)(346002)(396003)(136003)(366004)(451199015)(66556008)(8676002)(4326008)(66946007)(66476007)(53546011)(54906003)(6512007)(26005)(6506007)(2616005)(186003)(19627235002)(36756003)(316002)(5660300002)(8936002)(2906002)(38100700002)(31696002)(86362001)(41300700001)(83380400001)(6486002)(31686004)(478600001)(43740500002)(45980500001);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dGR6a3c3K0x4VGdIQ3lSSmJoQUxSUEExNFRmY0FyV1phNTArRzdJT3dHd01u?= =?utf-8?B?T29HVlRMeTZQdDVMZThrdS81LzNybXFzODJuaXh0SVJpS0hWTEp0SnNMZkpp?= =?utf-8?B?bkR3UUY0UEdHb3RvZ1NyRGZiT1gxaVRtem42L2FscmhOaGVNRnRleHZJSFA4?= =?utf-8?B?YXZiM21qalo0NGFOSm1VQ2NnazJHMnBQczZGcURKZ1NvVXlZcUZUa0toYzFI?= =?utf-8?B?bjdyWlhqOGN0V0FEbHk5UnhVRHdoODVscjJTaXQwd29HMWhkdENicDdvU29s?= =?utf-8?B?b3lDRVpEbTRxUmlQTEM0UE5aR3FGOXArZWluemNRYzdKVlY1aUM5eUdCb3VY?= =?utf-8?B?dEVESm1XbTRZck5ONlVNMkV5TkR4Vit2MXRYQk5FalkxUXlkanNXR3Fha294?= =?utf-8?B?QjliTnNrSC96bit0K3hVUkhuSnlSeGJnbFNBS1JjSkxCcDdMQkpmNzVPUndm?= =?utf-8?B?RE5tZVJDc3lkZkNyOHo1TWdzQ0lHcDlxbWg5UmRCM21xeFBUQUEyT3dEQ05k?= =?utf-8?B?Qlo4bjFDNEV1T2dleWh3T0wvY3NIQmwyUFp1U0xTMEExRVVQeUhrTncyZU5i?= =?utf-8?B?ZFlCRFNyZmdTYnVUYzBKeHlxdGR0VnBPemdTeWhDMm1MdmFCd2NDTXJoTE1r?= =?utf-8?B?NjkwT0pmT29aZmNqWUcrVEllemxUU0labFRCbDV1ZEFnNUdRQndhekxSNU9I?= =?utf-8?B?aWhmUzFqa1ptZlVFTjZTazZQaTVmSDRaRHZOWjIyZFJuWExaVlFQMWNjbGkz?= =?utf-8?B?VHYzdDZTb0JZVDFBcldOZURaSzZPV2RKR05XMmhMYXVtUFpWb0h5M2xGKzEy?= =?utf-8?B?UjlPeXlRRGJ0dGdTQTZ1cnZQWVBFUzRlemZkQnVtSGt5Y2ZBVW8zbHl6OWRh?= =?utf-8?B?dmZ4WUVOeDlXUCtlSlU5V3dKWGh5S2JoWkY0eUlLRGRGQVNZc2J1clpGaG9U?= =?utf-8?B?NWRiNU5GbUVJbnB0VmdGT3lwVmJCWXhpczZHWm9sVGpRL2c0ZzRyaWJxcWVZ?= =?utf-8?B?a3c1T0hlRlVVWm04MDNLYVl0T2RSdXZPdmZOWjZKS3NOMnNVUEdFOFNBbzl2?= =?utf-8?B?TGNQRjdsL3JmT1hCelMxSEJaSE50dTNWcWV6Z2lWeU5pcG1XOEI5anVCSTJW?= =?utf-8?B?K3lzdGlnMXQ5UEY2UE9xdjVWalU4L0wvdXNXQnVsN0ZWYlBoMDZMS0x1a2Nl?= =?utf-8?B?OFpGRXpNbVRUeUpuTS9kcnErOE1uc1NaU3FIblU5VjgzRG1aVnJDbFduampv?= =?utf-8?B?bjFpeWJXMTBPQXlYd0pGYW01WUhYMGJnYVpjU3paRmZDNzU4d0ZYM01ZMm5k?= =?utf-8?B?QWV1VnJjVEw5K2FOL3lNQnlqalFrcm9aYWNkRVZnL2VHb2p1UlcrS21zK3Yr?= =?utf-8?B?VEtSZ0xFRGx0a3p0VkNSdFZMYWJ6Rkphd1I3Tkd2MGMwVmlBKzN5VDlSZXlD?= =?utf-8?B?UklOSU1xZDlZWTNHODM5Nk9OZGpEbDNTRlF4WXZXeVJWTTAybkhSNzg3OC8w?= =?utf-8?B?Y2pvKyttUU1tUUhQQVl6T2tadzhscnNGQlBLSEZMVTVuRWRmcUx6QTh4SFBM?= =?utf-8?B?UU1ocWY4bzZhV3ZoRlRZWFVrcGk3VkF5RU5kYWJiWERNNzZ3YlFtYjdmQkZh?= =?utf-8?B?azVUZzN5by9iQW5IeXhvZGZySDFVUUpiU0g1djNlQjBYYW1qdmJiZDlsbTdN?= =?utf-8?B?V08vVGdxT050U1R3WUo5MGNOZmM4VHNSTDBmVTFJZ3kvQzV2TVplL1ZSRUZ6?= =?utf-8?B?UXNQYUE4eHNRQlJhM2hWNmd0N0IrbU9vY3hkWVY4YjFNVEd5dXpRand4L1Qx?= =?utf-8?B?WmZkQUwyUStGOStEREJFVjlmZ3psQUN5THJnZDdDY2NsaXhSY3Y4MlJmckti?= =?utf-8?B?dWJOdFR6bEVJcGo4bUd0SG9ZN3Nmb2JGWGZXTENuZWp6MHBzei9RZXRTZi9L?= =?utf-8?B?SzViMTNlbnRoeWlUWnU5eTBCdlIyQ3ZZN3FGTHJkd1NVRzRnVUNjRUZGZDhC?= =?utf-8?B?VFVUM0R1c3RoUEthYVV2YkxaeWdhd3l5d0lDVStaYTdleEpGWElLc1FDdWR2?= =?utf-8?B?L3liWUU5L2dnc3UrNk5JMGIvd2ZublA5aXc2SGVDWXFiMUw3alp5K2hXeEFV?= =?utf-8?Q?Fc3qdGYEP1cIfJ26UKtRmKxR2?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 7b972fa9-f374-43cf-1ae7-08daa54eed32 X-MS-Exchange-CrossTenant-AuthSource: DM4PR12MB5229.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Oct 2022 14:52:43.3402 (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: St73SDESamkn1+cMJfA9pdOarEdMU/8HPhq2v6j11DnAAH5pkAtGv/YyT/Zkku/z31N0SOlTv3xxKAS7cAYkvA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR12MB7334 Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 9/30/22 18:06, Dionna Glaze wrote: > From: Sophia Wolf > > When a guest OS does not support unaccepted memory, the unaccepted > memory must be accepted before returning a memory map to the caller. > > EfiMemoryAcceptProtocol is defined in MdePkg and is implemented / > Installed in AmdSevDxe for AMD SEV-SNP memory acceptance. > > Cc: Gerd Hoffmann > Cc: James Bottomley > Cc: Jiewen Yao > Cc: Tom Lendacky > Signed-off-by: Dionna Glaze Just some formatting suggestions and one area of cleanup from previous version of the patch below. Assuming you take care of those: Reviewed-by: Tom Lendacky > --- > OvmfPkg/AmdSevDxe/AmdSevDxe.c | 57 ++++++++++++++++++-- > OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 3 ++ > OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c | 24 +++++++-- > 3 files changed, 76 insertions(+), 8 deletions(-) > > diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c > index 662d3c4ccb..77d3caa833 100644 > --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c > +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c > @@ -20,6 +20,7 @@ > #include > #include > #include > +#include > > STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION mSnpBootDxeTable = { > SIGNATURE_32 ('A', 'M', 'D', 'E'), > @@ -31,6 +32,38 @@ STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION mSnpBootDxeTable = { > FixedPcdGet32 (PcdOvmfCpuidSize), > }; > > +STATIC EFI_HANDLE mAmdSevDxeHandle = NULL; > + > +STATIC > +EFI_STATUS > +EFIAPI > +AmdSevMemoryAccept ( > + IN EFI_MEMORY_ACCEPT_PROTOCOL *This, > + IN EFI_PHYSICAL_ADDRESS StartAddress, > + IN UINTN Size > +) > +{ > + // > + // The StartAddress must be page-aligned, and the Size must be a positive > + // multiple of SIZE_4KB. Use an assert instead of returning an erros since > + // this is an EDK2-internal protocol. > + // > + ASSERT (((StartAddress & ~(SIZE_4KB - 1)) == 0) && > + ((Size & ~(SIZE_4KB - 1)) == 0) && > + (Size != 0)); Create a generic alignment check macro? #define IS_ALIGNED(x, y) (((x) & ((y) - 1)) == 0) Maybe keep the ASSERTs separate so they better identify which condition caused the assert, e.g.: ASSERT (IS_ALIGNED (StartAddress, SIZE_4KB)); ASSERT (IS_ALIGNED (Size, SIZE_4KB)); ASSERT (Size != 0); ? Not sure if those are worth it or not, though. > + > + MemEncryptSevSnpPreValidateSystemRam ( > + StartAddress, > + EFI_SIZE_TO_PAGES (Size) > + ); > + > + return EFI_SUCCESS; > +} > + > +STATIC EFI_MEMORY_ACCEPT_PROTOCOL mMemoryAcceptProtocol = { > + AmdSevMemoryAccept > +}; > + > EFI_STATUS > EFIAPI > AmdSevDxeEntryPoint ( > @@ -147,11 +180,27 @@ AmdSevDxeEntryPoint ( > } > } > > - // > - // If its SEV-SNP active guest then install the CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB. > - // It contains the location for both the Secrets and CPUID page. > - // > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "Install EfiMemoryAcceptProtocol failed.\n")); > + } Looks like this shouldn't be here. > + > if (MemEncryptSevSnpIsEnabled ()) { > + // > + // Memory acceptance began being required in SEV-SNP, so install the > + // memory accept protocol implementation for a SEV-SNP active guest. > + // > + Status = gBS->InstallProtocolInterface ( > + &mAmdSevDxeHandle, > + &gEfiMemoryAcceptProtocolGuid, > + EFI_NATIVE_INTERFACE, > + &mMemoryAcceptProtocol > + ); Need to indent these two more spaces to align with the "s" in Install. Thanks, Tom > + ASSERT_EFI_ERROR (Status); > + > + // > + // If its SEV-SNP active guest then install the CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB. > + // It contains the location for both the Secrets and CPUID page. > + // > return gBS->InstallConfigurationTable ( > &gConfidentialComputingSevSnpBlobGuid, > &mSnpBootDxeTable > diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf > index 9acf860cf2..5ddddabc32 100644 > --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf > +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf > @@ -47,6 +47,9 @@ > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize > > +[Protocols] > + gEfiMemoryAcceptProtocolGuid > + > [Guids] > gConfidentialComputingSevSnpBlobGuid > > diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c > index d3a95e4913..ee3710f7b3 100644 > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c > @@ -14,6 +14,7 @@ > #include > > #include "SnpPageStateChange.h" > +#include "VirtualMemory.h" > > /** > Pre-validate the system RAM when SEV-SNP is enabled in the guest VM. > @@ -29,12 +30,27 @@ MemEncryptSevSnpPreValidateSystemRam ( > IN UINTN NumPages > ) > { > + EFI_STATUS Status; > + > if (!MemEncryptSevSnpIsEnabled ()) { > return; > } > > - // > - // All the pre-validation must be completed in the PEI phase. > - // > - ASSERT (FALSE); > + // DXE pre-validation may happen with the memory accept protocol. > + // The protocol should only be called outside the prevalidated ranges > + // that the PEI stage code explicitly skips. Specifically, only memory > + // ranges that are classified as unaccepted. > + if (BaseAddress >= SIZE_4GB) { > + Status = InternalMemEncryptSevCreateIdentityMap1G ( > + 0, > + BaseAddress, > + EFI_PAGES_TO_SIZE (NumPages) > + ); > + if (EFI_ERROR (Status)) { > + ASSERT (FALSE); > + CpuDeadLoop (); > + } > + } > + > + InternalSetPageState (BaseAddress, NumPages, SevSnpPagePrivate, TRUE); > }