From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM02-DM3-obe.outbound.protection.outlook.com (NAM02-DM3-obe.outbound.protection.outlook.com [40.107.95.62]) by mx.groups.io with SMTP id smtpd.web11.37496.1681910360500400420 for ; Wed, 19 Apr 2023 06:19:20 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@amd.com header.s=selector1 header.b=GhXmoGvD; 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.95.62, mailfrom: thomas.lendacky@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=UUwtgdK1Wz+evKAAmHzTUzMujDWnCf5QL9E1Mo+7DfigK63RYyAlZ3W6xmHcKyVGuorlYRPGGCcfFCm4Y09SdIhDU3UMfpHeqKGPR30kj6CR5GDz5auM5Ns0d0D5KFaG6jXsxCNT05NTbY8rcMudUBF31XykJ01NklQoxB994R213XW2E2O7PMAZcK9+pFmOfdBF31AM00ZG7M1BINb7DVdCoHNBZKwCAI6Dz5lXSNNIvwCpy3i21tmOF1JfJSnpn9nqJv5HOnOlgN/Bu/+4b0fFSOysWIl4jLTEhCUAMENsnC9atj3EKa8O9GtReO2AYAeXcmBzCKj5i7AtqylTOQ== 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=tpdogSMlCDLPfv8q0eAg0+gB6j12jR9MlEmO3N1mWBY=; b=GPyn7pU3/mxkDvkIki+PAo/LmA9Se9PKtU/WAJOYuyrrwCr3WXvP4QIpmPtzn5Nz6LjWb0N4af9im4z6d+poAU/ngA8esbFF10jQ+OSIcO5DY+Iq/ybix6oT9QRjsXI4LNBpG/u2nIstwieEAdbOJCymnbKC3r6JL30YdHGC6kbUD/vrGiQ3D4mU+llUFUcSPXhBcvmp4nS/7BE3pPAaOoU1z7ooAwOMRSRXnYoIXDt4Kx+O8UB8s0idg7yEsR/5tI0UrwkrzCTY+mU1vSQWHLqDOSwKlkuRAKKVAakLzdCo0IuqESFD6zXi6Rw3gSIuHk0rDNAKhKWbyZ+vwIenOA== 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=tpdogSMlCDLPfv8q0eAg0+gB6j12jR9MlEmO3N1mWBY=; b=GhXmoGvDCSMyzz02yK85jyvEIYlNq/+FnWpOq5Vb7oxzaM9bFPSoB0djngdVFCv7LpIs/fjeNOq5uQ8erXfOEB4MpPUbwU/9PpPyLh4nEnVlE3OkjWQO6DPo5Iy0bhx52rSomUe6F7XSW21yBs1d6d0wmnPmRcouUKyfCDhBiYs= 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 CY8PR12MB7434.namprd12.prod.outlook.com (2603:10b6:930:52::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6319.21; Wed, 19 Apr 2023 13:19:18 +0000 Received: from DM4PR12MB5229.namprd12.prod.outlook.com ([fe80::ea32:baf8:cc85:9648]) by DM4PR12MB5229.namprd12.prod.outlook.com ([fe80::ea32:baf8:cc85:9648%6]) with mapi id 15.20.6298.045; Wed, 19 Apr 2023 13:19:17 +0000 Message-ID: <2ff87b88-38c0-5213-64e9-8f26f1603d0c@amd.com> Date: Wed, 19 Apr 2023 08:19:15 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 Subject: Re: [edk2-devel] [Patch V2 0/8] Use CpuPageTableLib to create and update smm page table To: devel@edk2.groups.io, dun.tan@intel.com, "Ni, Ray" References: <1755241E6695EAE7.1885@groups.io> <2301e275-1f69-e5c0-997d-d967264aa590@amd.com> <4c5afff9-39b6-6f3a-ce05-0aadb8b004b9@amd.com> From: "Lendacky, Thomas" In-Reply-To: X-ClientProxiedBy: CH0PR04CA0009.namprd04.prod.outlook.com (2603:10b6:610:76::14) 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_|CY8PR12MB7434:EE_ X-MS-Office365-Filtering-Correlation-Id: 8974381a-7338-4608-9031-08db40d8adca X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: D+JsLW2U2w6sA6BosM4JfRJoFb/qtpmoOBJXtpq9pC9DeLlb02DX7YRSm3zoqI0XdiJLFLhFemtvMed70gK2/KQpFCWGQ6LUFQZftzD3yrIovCvZpSfmoDiDwFiQ3Rv2MatlKkPBBIX+hYK+JhUH6iiJWcUIAkC3FzZoQJAB74D5o6w1aIoiU63GicSrI4v5LQ1d0mzqUg2X2P6kQAB08HJ7UulfwO89cPqaHRZAd6laSZekoPSiQd2mhlQI+VWUz6v6L5D+TmnN4Sbf9uOkP2QyzPx1GyJ1etRAc5WxtnBVPR17BFgcow7vnkpZxCxg6V2Gd5++48CnfeO5Nap+zIqcCkPaxWYsDV+/m5Hiia4Jd8aSloc6GbEwi+LgJTYYTjcTTiXV0iMAHonTt3hyZRNh6nHdyO0dEjzHeRzxqaa7E60h0isxO3D1SU8hxEpbauw7LnPeQhct34UnrDrZgHbYTiJzlCTEGheehXOS3bscRvuJIv3q0JRPAQ1OPeDKL7w6F3p/kHp9tlV/7y6cWFxaf+RXLcjC7DmtiRkd1AL/sd3qZeiYngNHLqJAgWsfkxpx/41U93wEkwPyBCWVLnBhmFFw32E1LLoJwE5rZoV7Ihg1ULAbI+uR6ahwLnO79IJ0LmnuJ/NaccTPAMMqV1tAP2V9CliMTFyVcwl00UNUc12aXt8ASD/y5VzFjIa0Sct/eQmirbjMUL6fdmcmXw== 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:(13230028)(4636009)(346002)(396003)(136003)(39860400002)(366004)(376002)(451199021)(8936002)(8676002)(2616005)(41300700001)(316002)(6486002)(186003)(66946007)(6916009)(66476007)(66556008)(2906002)(83380400001)(15650500001)(30864003)(31686004)(5660300002)(478600001)(26005)(6512007)(6506007)(53546011)(38100700002)(86362001)(19627235002)(31696002)(66899021)(36756003)(966005)(45980500001)(43740500002)(44824005);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?NzJldTNjWVhWbFJDblZ0eklEaHRobzhSNE5UTk1nYjgvRkQ4NnlCVlhUMjlI?= =?utf-8?B?NEJEUWFZcy82Wit6OGJ2NDFiV0NvZ2ZiRHpUL3NkZXN1L1RYZ0FQbjRlVzRM?= =?utf-8?B?RUF5Uk05ZHJEY1Y4NlhnV0FmbnJSVHVnZThnOUxLVEFwN0VQdG1RdTMrL0VH?= =?utf-8?B?cEg0Wm8zQXY3elVZb09ESVpHcHY5NFFSRGIzU1Vhd3FWOEN4QlFxc1BCZE1s?= =?utf-8?B?RHRiQkc3b1JsNEo2ZTQxb3RpeDUvaVdPZTBMZ3plNWF6QVRHZkdhSEZMWjFQ?= =?utf-8?B?L2s3Z1JKTjNUbUdVcHMweGJPT1JCV1ptWEcwaHNtSVM1RGFncDhXbFpvaUdl?= =?utf-8?B?d0pwTzVSWGg2MnhLMTAweFUzMmZUb29XZDYyckFSajhCa0F2NkZFQ3NoRVFH?= =?utf-8?B?U2RWdHhyYmUrL1NDcEQzc2JiS1JkdnlzTk4yZG53T296aVV4bzM1dmpVUktZ?= =?utf-8?B?QVNrdldFUWdPNGY4b0xtNVFLVHVRQ3VVOXFSa2l4RkltMHY3Y21lUWhHb2RQ?= =?utf-8?B?SkxNT1JXcityOHhGSnAyNnhlZWdkTU93bWhmTTIwUysvdmpNK2U2NHoxTzFX?= =?utf-8?B?L3NhZUtQVG5CNHhnTktiOGJpZ3NSWG1heWVkbm9XRy9xdU52cXZHTHFqNUIz?= =?utf-8?B?THVQejRlY01ydkVNWTFIQndwS3RLSVg1dVhNUllRdnpOZGg3TG51STVMNXBl?= =?utf-8?B?ZkZvNjhqNWJJNGFHQkxNb080a3QxZDJEemwrY3ZpUmMvUU5OYldtYWtOT1ZC?= =?utf-8?B?UGVJYzMybzR1cGpLOUhLT0xjc2p5ZmcrRTJ0TkliVmtVL3J3QStCRTJFUzlp?= =?utf-8?B?YUpySTdsMmFuSTM2NlJkNmJuYWRqazhRUjlNSHpKTDhHYVVMWUhWRFgyejc0?= =?utf-8?B?TysxVnFBdVFpdTd1QkphVFI2NEgvWmp4YnhLOThnV1pUcy9yQ1RESkFNL1RN?= =?utf-8?B?L1psTlhRNnh6YURVclhadkVETHc2U1JBNDFnNkc2SHU3WDJLODRzTkx5OXRp?= =?utf-8?B?R2JuSFpCcUFpYXVmUDlmU2JhK2s2SGFNRjdiVW9uQWxVZTBXS29CdUw5UXgw?= =?utf-8?B?dzJEcE9nNmtoSllKWVNkcVVtenp4LzlHc1E5RmluL09XN1doUjZkSzBFMDdz?= =?utf-8?B?VWZVNENzaFRUWk5lRkh4N2R4QkcyRE82bHppbmNPWkx0QW1DUWZrZEdwcDV4?= =?utf-8?B?NXlud0ZXOSs5bVRnQXdBaHBZVEZjcE56SnJ1YWZlL2VnUmNJOWJoNGIxdlNu?= =?utf-8?B?c3E2VGFUVjJhUUNYTWZxTm8rRXF6WTlxN01jK1Q5SlBUdTQwZktZT3h2TDdU?= =?utf-8?B?eDAzYnAvU0ppT21mdnM4dEhabjEyb3pqbDlFWjVDV29PY2NLSDdkRG5LczJN?= =?utf-8?B?aUpJemljeExzaVFKZ1N3VHFFRi9jRFlmcmlCM1BzdHBXSUhBY1FXUUd5ZFg3?= =?utf-8?B?Ym9KanVrSWVueG9DQzJITlU5UXorT0dSVm1iTTBwVGtwRUc2TVhhNHNOaEx0?= =?utf-8?B?cXFmTnZtRDdDcms2YVFLU2tSYWVsUFJuWHd0a3BIRzJyNWl2VVFQMXdrT2Mv?= =?utf-8?B?RWhObHdCcEpUa3ZsV1dpTCs3MDFicU1XYXBtYkpaVkpNejByVjkrZWs3eUhR?= =?utf-8?B?bE5QcldPSythK0lkWmtnSlRLK1ZqWlVRcGJXc0JLMmNDcDJmYU02YXdhRGRV?= =?utf-8?B?S0lMRTdoWGZmVjlxUmtNTHdSMFRSWWRLdEk3aGhHZzBYQXdDTUwzMU5USnNO?= =?utf-8?B?aDM3V1dWMkkzeHNYTk5pcnVvM0h3b1kyb2dBRTJPUkh0ditKUkVtTXhsWE9i?= =?utf-8?B?Z3Exb3NhbDZVRk9GekVuTGg5QmNMR1E1cWY0V01GcjRpeWp5Y2lnOVk3Q2Nx?= =?utf-8?B?VWVVazBCc2VQM2srK0x2ZjkrMk1NNVF1bjRSZnM2RkFwSk43dEVJUERkV2Fy?= =?utf-8?B?UHJKRk43S3JmT01XalR4bnVCRDVGajA3cjExU0Qwb0dPQVAwcjcxMU9mS1VT?= =?utf-8?B?ZHpXYlQrWG9SMk1Za09XM1ZIeHpLSDFPQ0E4WFVSbWdwOGJSWUhOZWI4QWZq?= =?utf-8?B?T2MySzR6RGZ4azdCK2RyeC9YbjBqQlhqNmhyYlVTYjlzNHN4ZnFhb0ptN1pX?= =?utf-8?Q?/v88wpUaZP42x8uMMuOt2jgJp?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 8974381a-7338-4608-9031-08db40d8adca X-MS-Exchange-CrossTenant-AuthSource: DM4PR12MB5229.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Apr 2023 13:19:17.7567 (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: 3Wj1PbLlHb4H7wnm/vab4xR3jfQJXHvG2MllZxsDwO8wKuNwscydwLthH/OfkCuAW8Uv48fHJBOJXtqvErUTUQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR12MB7434 Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 4/19/23 00:39, duntan via groups.io wrote: > Hi Tom, > > This PF happened because that CR0.WP is set and DxeMemEncryptSevLib sets a part of smm page table as RO before ReadyToLock while CpuSmm driver assumes the smm page table is not marked as RO before ReadyToLock. > The code flow to set smm page table to RO is QemuFlashBeforeProbe()-->MemEncryptSevClearMmioPageEncMask()-->SetMemoryEncDec()-->EnablePageTableProtection(). New page table memory allocated by DxeMemEncryptSevLib is marked as RO in EnablePageTableProtection(). However, In PiSmmCpuDxeSmm InitPaging() function, the RO page table marked by DxeMemEncryptSevLib may be modified again and InitPaging() function doesn't clear CR0.WP in advance since it has the assumption that smm page table memory is always RW before SetPageTableAttributes(), which causes this PF. > So Could you please help test the SEV enabled smm code without my patch set? I think this issue may also happen in current edk2 code. Upstream master branch boots without issue. SmmInstallProtocolInterface: 47B7FA8C-F4BD-4AF6-8200-333086F0D2C8 0 GetUefiMemoryMap Patch page table start ... Patch page table done! MemoryAttributesTable: Version - 0x00000001 NumberOfEntries - 0x00000035 DescriptorSize - 0x00000030 Entry (0x3FF01028) ... Thanks, Tom > > Thanks, > Dun > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Lendacky, Thomas via groups.io > Sent: Wednesday, April 19, 2023 5:06 AM > To: Tan, Dun ; devel@edk2.groups.io; Ni, Ray > Subject: Re: [edk2-devel] [Patch V2 0/8] Use CpuPageTableLib to create and update smm page table > > On 4/18/23 04:57, Tan, Dun wrote: >> Hi Tom, >> >> I added a new patch in my code branch. This new patch removes the code that may apply AddressEncMask to smm page table non-leaf entries when splitting page table. >> Could you please help test if this code branch works? >> https://github.com/td36/edk2/tree/SmmPageTable_V2 > > It got further, but it still crashed: > > SmmInstallProtocolInterface: 47B7FA8C-F4BD-4AF6-8200-333086F0D2C8 0 GetUefiMemoryMap Patch page table start ... > !!!! X64 Exception Type - 0E(#PF - Page-Fault) CPU Apic ID - 00000000 !!!! > ExceptionData - 0000000000000003 I:0 R:0 U:0 W:1 P:1 PK:0 SS:0 SGX:0 RIP - 000000003FFC7744, CS - 0000000000000038, RFLAGS - 0000000000010082 RAX - 00000000FFFFFF83, RCX - 000000003FFB5C78, RDX - 0000000000000000 RBX - 000000003FC01000, RSP - 000000003FFB4790, RBP - 000000003FFB47B0 RSI - 000000003FC01000, RDI - 0000000000000000 > R8 - 000000003FFB4840, R9 - 0000000000000002, R10 - 0000000000000000 > R11 - 0000000000000000, R12 - 000000003FFB5C78, R13 - 000000003FFB4840 > R14 - 0000000080000000, R15 - 0000000000000000 > DS - 0000000000000020, ES - 0000000000000020, FS - 0000000000000020 > GS - 0000000000000020, SS - 0000000000000020 > CR0 - 0000000080010033, CR2 - 000000003FC01000, CR3 - 000000003FF81000 > CR4 - 0000000000000668, CR8 - 0000000000000000 > DR0 - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000 > DR3 - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400 GDTR - 000000003FFAC000 000000000000004F, LDTR - 0000000000000000 > IDTR - 000000003FFAF000 00000000000001FF, TR - 0000000000000040 > FXSAVE_STATE - 000000003FFB0C60 > SMM exception at access (0x3FC01000) > It is invoked from the instruction before IP(0x3FFC7744) in module (/root/kernels/ovmf-dun-build-Ia32X64/Build/Ovmf3264/DEBUG_GCC5/X64/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSm > m/DEBUG/PiSmmCpuDxeSmm.dll) > > > Thanks, > Tom > >> >> Thanks, >> Dun >> >> -----Original Message----- >> From: devel@edk2.groups.io On Behalf Of >> Lendacky, Thomas via groups.io >> Sent: Saturday, April 15, 2023 3:05 AM >> To: devel@edk2.groups.io; Ni, Ray ; Tan, Dun >> >> Subject: Re: [edk2-devel] [Patch V2 0/8] Use CpuPageTableLib to create >> and update smm page table >> >> On 4/14/23 12:19, Ni, Ray via groups.io wrote: >>> tom, >>> if the c bit is not required for non leaf page table entries, why the trunk code sets the c bit for all entities including nonleaf ones? >> >> Because it's effectively the correct thing to do, even though it doesn't matter. >> >>> >>> i went back to read again the smm issue you met. you said the c bit is set for non leaf entries that caused a deference issue. But the pagetablelib code doesn't set c bit to non leaf entries. then who sets the c bit? >> >> I guess that's the main question, how did that get set? I haven't had the time to fully examine and follow the codepath in the pagetable library to figure out why it was set. Maybe as part of a page split? >> >> Thanks, >> Tom >> >>> >>> thanks, >>> ray >>> >>> thanks, >>> ray >>> ________________________________ >>> From: devel@edk2.groups.io on behalf of >>> Lendacky, Thomas via groups.io >>> Sent: Friday, April 14, 2023 9:43:52 PM >>> To: Ni, Ray ; Tan, Dun ; >>> devel@edk2.groups.io >>> Subject: Re: [edk2-devel] [Patch V2 0/8] Use CpuPageTableLib to >>> create and update smm page table >>> >>> On 4/14/23 00:07, Ni, Ray wrote: >>>> >>>> >>>>> -----Original Message----- >>>>> From: Tom Lendacky >>>>> Sent: Friday, April 14, 2023 12:19 AM >>>>> To: Tan, Dun ; devel@edk2.groups.io >>>>> Cc: Ni, Ray >>>>> Subject: Re: [edk2-devel] [Patch V2 0/8] Use CpuPageTableLib to >>>>> create and update smm page table >>>>> >>>>> On 4/13/23 04:14, Tan, Dun wrote: >>>>>> Hi Tom, >>>>>> >>>>>> Thank you for your help with testing. >>>>>> For the build failure, it's because that the CpuPageTableLib >>>>>> instance is >>>>> added into OvmfPkg DSC in the last pacth ' OvmfPkg: Add >>>>> CpuPageTableLib required by PiSmmCpuDxe'. I have moved this patch >>>>> to the head of the patch set. >>>>>> >>>>>> For the boot failure, I think it's because that the encrypt mask >>>>>> was not >>>>> applied to the memory used by page table in page table non-leaf entry. >>>>> Initially I thought the encrypt mask would only be applied to the >>>>> leaf entry in AMD SEV feature. So I treated the encryption process >>>>> as non 1:1 mapping, which only applies the encrypt mask to leaf >>>>> entry. I'm also curious why the DxeIpl patch set works good. All >>>>> the page table non-leaf entries are also not encrypted in the DxeIpl page table related patch set. >>>>> >>>>> Right, and that works for SEV. All non-leaf pagetable entries are >>>>> treated as encrypted regardless of the encryption bit. Since the >>>>> tables were built being mapped encrypted, the pagetable walk works >>>>> when the non-leaf entries don't have the encryption bit set. In >>>>> this case, though, the encryption bit is present in the non-leaf >>>>> entry and that is the reason why there are issues. >>>> >>>> Can you point us which doc here >>>> (https://www.amd.com/en/developer/sev.html) >>>> says the page table is encrypted regardless the KEY_ID bits value? >>>> How can the encryption engine know if a chunk of memory belongs to page table? >>> >>> It doesn't. For an SEV guest, when the hardware walks the pagetables, >>> it will always treat the memory accesses as encrypted (see section >>> 15.34.5 of the AMD APM Vol 2 at https://www.amd.com/system/files/TechDocs/24593.pdf). >>> >>> But, because the initial pagetables that are built to map everything >>> as encrypted/private to start with (see >>> OvmfPkg/ResetVector/Ia32/PageTables64.asm), only changing to shared >>> when specifically requested, any memory allocated and used will be encrypted. >>> Thus, when new pagetables are allocated/created in the >>> CpuPageTableLib library, they will be encrypted and so everything >>> works. And those new pagetables will map everything encrypted by >>> default, except for the GHCB pages. If they were mapped shared when >>> they were created, then the pagetable walk would fail. >>> >>>> >>>> My understanding to SEV is any physical address field in guest page >>>> table should have the KEY_ID bits set if the physical pages are >>>> private to guest. Only some pages for GMCB don't have KEY_ID bits set as those are shared between guest and host. >>> >>> Right, the encryption bit in the leaf entry of the pagetables will >>> determine the encryption mode. >>> >>>> >>>> I thought Dun's patch works because all guest memory is marked as >>>> shared because the KEY_ID bits in all entries are not set. Only some >>>> pages that're used by GMCB have the KEY_ID bits set. >>> >>> Just the opposite, the CpuPageTableLib library marks everything >>> encrypted and only clears the encryption bit for the GHCB pages. >>> >>> In MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c, the >>> CreateIdentityMappingPageTables() function retrieves the encryption >>> bit and saves it in AddressEncMask. AddressEncMask is then applied to >>> the mapping attribute used when calling CreateOrUpdatePageTable() to >>> build the initial pagetables. >>> >>>> >>>> >>>>> >>>>> Here is some debug after setting PagingEntry at line 436 of >>>>> UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c: >>>>> >>>>> *** DEBUG: PageTableLibMapInLevel:437 - PagingEntry = 3FF81000 >>>>> *** DEBUG: PageTableLibMapInLevel:437 - PagingEntry = 3FF80000 >>>>> *** DEBUG: PageTableLibMapInLevel:437 - PagingEntry = 3FF83000 >>>>> *** DEBUG: PageTableLibMapInLevel:437 - PagingEntry = 3FF81000 >>>>> *** DEBUG: PageTableLibMapInLevel:437 - PagingEntry = 3FF80000 >>>>> *** DEBUG: PageTableLibMapInLevel:437 - PagingEntry = 3FF83000 >>>>> *** DEBUG: PageTableLibMapInLevel:437 - PagingEntry = 3FF81000 >>>>> *** DEBUG: PageTableLibMapInLevel:437 - PagingEntry = 3FF80000 >>>>> *** DEBUG: PageTableLibMapInLevel:437 - PagingEntry = 800003FC01000 >>>> >>>> Are you testing the SME or SEV? >>>> My understanding is with SME, only the highest C bit should be set >>>> indicating the physical page is encrypted. >>> >>> I am testing SEV. There is only a single bit to indicate whether a >>> page is encrypted. The guest ASID is used to determine what key is >>> used to decrypt the page. From a pagetable leaf entry, SME and SEV >>> are equivalent, the encryption bit determines how the memory will be accessed. >>> >>> SME and SEV differ in how they deal with instruction fetches and >>> pagetable walks, with SME obeying the encryption bit and SEV always >>> performing the accesses as encrypted accesses for security. >>> >>> Thanks, >>> Tom >>> >>>> >>>> >>>> >>>>> !!!! X64 Exception Type - 0D(#GP - General Protection) CPU Apic ID >>>>> - >>>>> 00000000 !!!! >>>>> >>>>> 0x800003FC01000 isn't mapped and so it fails - I'm not exactly sure >>>>> how the #PF turns into a #GP, though, maybe because the virtual >>>>> address isn't canonical that point. >>>>> >>>>> Thanks, >>>>> Tom >>>>> >>>>>> >>>>>> I'll added another patch in my code branch to fix this issue later. >>>>>> In the new >>>>> commit, from the perspective of CpuPageTableLib, the whole memory >>>>> can be divided into 3 categories: memory used by page table, guest >>>>> private memory and guest shared memory. CpuPageTableLib will always >>>>> apply the encrypt mask to memory used by page table, which means >>>>> all the non-leaf page table entries are encrypted. For guest >>>>> private memory, this case can be treated as non-1:1 mapping. We can >>>>> apply the encrypt mask by setting the input parameter of >>>>> PageTableMap() API like " Attribute.Uint64 = LinearAddress | >>>>> AddressEncMask". For guest shared memory, this case can be treated as normal 1:1 mapping. >>>>> I'll let you know once the new patch is ready. >>>>>> >>>>>> Thanks, >>>>>> Dun >>>>>> -----Original Message----- >>>>>> From: devel@edk2.groups.io On Behalf Of >>>>> Lendacky, Thomas via groups.io >>>>>> Sent: Thursday, April 13, 2023 3:26 AM >>>>>> To: devel@edk2.groups.io; Tan, Dun >>>>>> Subject: Re: [edk2-devel] [Patch V2 0/8] Use CpuPageTableLib to >>>>>> create >>>>> and update smm page table >>>>>> >>>>>> On 4/12/23 05:17, duntan via groups.io wrote: >>>>>>> Hi Tom, >>>>>>> >>>>>>> This patch set is to change PiSmmCpuDxeSmm code to use >>>>> CpuPageTableLib to create and update SMM page table. The Pcd >>>>> PcdPteMemoryEncryptionAddressOrMask is also used in PiSmmCpuDxeSmm >>>>> code and the whole range covered by page table is mapped encrypted, >>>>> which is different from the situation in DxeIpl module. >>>>>>> So could you also help do a test to make sure the AMD SEV feature >>>>>>> still >>>>> works good in SMM with this patch set? >>>>>>> Here is the code branch in my fork repo: >>>>>>> https://github.com/td36/edk2/commits/SmmPageTable_V2 >>>>>> >>>>>> Hi Dun, >>>>>> >>>>>> I tested at the final commit of the branch and encountered a #GP >>>>>> with an >>>>> SEV guest. It looks like the CpuPageTableLibrary doesn't take the >>>>> encryption bit into account. For example: >>>>>> >>>>>> Line 436 of UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c >>>>>> PagingEntry = (IA32_PAGING_ENTRY >>>>> *)(UINTN)IA32_PNLE_PAGE_TABLE_BASE_ADDRESS (&ParentPagingEntry- >>>>>> Pnle); >>>>>> >>>>>> This will get an address with the encryption bit set and then try >>>>>> to >>>>> reference it. When I clear the encryption bit, the code proceeds a >>>>> bit further, but then encounters a #GP in a different location. >>>>>> >>>>>> So it appears that the CpuPageTableLibrary doesn't deal with the >>>>> encryption bit properly. >>>>>> >>>>>> Also, going through a build/test of each individual patch had mixed results. >>>>>> >>>>>> - With the second patch in the series applied, I get a build error: >>>>>> >>>>>> /root/kernels/ovmf-dun-build-X64/OvmfPkg/OvmfPkgX64.dsc(...): >>>>> error 4000: Instance of library class [CpuPageTableLib] is not >>>>> found >>>>>> in [/root/kernels/ovmf-dun-build- >>>>> X64/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf] [X64] >>>>>> consumed by module [/root/kernels/ovmf-dun-build- >>>>> X64/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf] >>>>>> >>>>>> that isn't resolved until the final patch. >>>>>> >>>>>> Thanks, >>>>>> Tom >>>>>> >>>>>>> >>>>>>> Thanks, >>>>>>> Dun >>>>>>> >>>>>>> -----Original Message----- >>>>>>> From: devel@edk2.groups.io On Behalf Of >>>>> duntan >>>>>>> Sent: Wednesday, April 12, 2023 4:54 PM >>>>>>> To: devel@edk2.groups.io >>>>>>> Subject: [edk2-devel] [Patch V2 0/8] Use CpuPageTableLib to >>>>>>> create and update smm page table >>>>>>> >>>>>>> In V2 patch set: >>>>>>> 1.In 'Refinement to code about updating smm page table', use >>>>>>> QuickSort() >>>>> in BaseLib instead or PerformQuickSort() in BaseSortLib. >>>>>>> 2.Remove the patch to add BaseSortLib in DSC file. >>>>>>> 3.Add a new patch to add CpuPageTableLib in UefiCpuPkg.dsc. >>>>>>> 4.Add a temp patch to add CpuPageTableLib in OvmfPkg dsc files >>>>>>> for test(A previous patch I sent before '[Patch V2 4/8] OvmfPkg: >>>>>>> Add CpuPageTableLib required by DxeIpl in DSC file' contains all >>>>>>> the changes in this patch) >>>>>>> >>>>>>> Dun Tan (8): >>>>>>> OvmfPkg: Add CpuPageTableLib required by PiSmmCpuDxe >>>>>>> UefiPayloadPkg: Add CpuPageTableLib required by PiSmmCpuDxe >>>>>>> UefiCpuPkg: Use CpuPageTableLib to convert SMM paging attribute. >>>>>>> UefiCpuPkg/PiSmmCpuDxeSmm: Avoid setting non-present range >>>>>>> to >>>>> RO/NX >>>>>>> UefiCpuPkg: Extern mSmmShadowStackSize in PiSmmCpuDxeSmm.h >>>>>>> UefiCpuPkg: Refinement to current smm page table generation code >>>>>>> UefiCpuPkg: Refinement to code about updating smm page table >>>>>>> UefiCpuPkg/PiSmmCpuDxeSmm: Remove unnecessary function >>>>>>> >>>>>>> OvmfPkg/CloudHv/CloudHvX64.dsc | 2 +- >>>>>>> OvmfPkg/OvmfPkgIa32.dsc | 3 ++- >>>>>>> OvmfPkg/OvmfPkgIa32X64.dsc | 2 +- >>>>>>> OvmfPkg/OvmfPkgX64.dsc | 2 +- >>>>>>> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 5 +++-- >>>>>>> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c | 3 +-- >>>>>>> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c | 2 +- >>>>>>> UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 132 ----------------- >>>>> ------------------------------------------------------------------- >>>>> - >>>>> -------------------------- >>>>> --------------------- >>>>>>> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 8 ++++++- >>>>> - >>>>>>> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 97 >>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>> ++------------------------------------- >>>>>>> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 1 + >>>>>>> UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 629 >>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>> ++++++++++++++++++++++++++++++++++++++++++------------------------- >>>>> ++++++++++++++++++++++++++++++++++++++++++- >>>>> ------------------------------------------------------------------- >>>>> - >>>>> -------------------------- >>>>> ------------------------------------------------------------------- >>>>> - >>>>> -------------------------- >>>>> ------------------------------------------------------------------- >>>>> - >>>>> -------------------------- >>>>> ----------------------------------------------- >>>>>>> UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 348 >>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>> +++++++++---------------------------------------------------------- >>>>> +++++++++- >>>>> +++++++++-------------------- >>>>> ------------------------------------------------------------------- >>>>> - >>>>> -------------------------- >>>>> -------------------------------------------------- >>>>>>> UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 229 >>>>> ++++++++++++++++++++++++++++++------------------------------------- >>>>> ++++++++++++++++++++++++++++++- >>>>> ++++++++++++++++++++++++++++++-------- >>>>> ------------------------------------------------------------------- >>>>> - >>>>> -------------------------- >>>>> ----------------------------------------------------------- >>>>>>> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c | 3 +-- >>>>>>> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c | 19 ++------- >>>>> ---------- >>>>>>> UefiPayloadPkg/UefiPayloadPkg.dsc | 2 +- >>>>>>> 17 files changed, 510 insertions(+), 977 deletions(-) >>>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>> >>> >>> >>> >>> >>> >>> >>> >>> >>> >>> >>> >> >> >> >> >> > > > > > > > > > >