From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web11.59150.1682359654173538410 for ; Mon, 24 Apr 2023 11:07:34 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=KuBkiKg5; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 9662961A0D for ; Mon, 24 Apr 2023 18:07:33 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 06AE4C433D2 for ; Mon, 24 Apr 2023 18:07:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1682359653; bh=8NM56PCa0DF0x9WOHyytGXeQH9nEJrpsR37YxU4ulT0=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=KuBkiKg5s2ePzUC3vVFgl5m1oD+NOQA3UbvD108UMt3u8Xcm97zr9ujYAUBFkQCyF Z06D466m62npC9rIf8j4n4sDkNuYrFpfZuKzYwz3N1ztMj+s1Jt/g4Nacdk8GkV1Fd skWdOBOelog3V5z6gBjb0LvsZi/B9zEm8IJ9EjgJxOBmTPCsKywR3S92/pGir4x94t YNZRoafFgp3Rimxs4+hV9ZVK0jAh2MZHl1I6mwR3Mz/U6H66rRuuGPXtIYMhFkEhTC IaSRao4kvGgmudNb5xhadLB9sALDO/ZvQh1Q6biU7B8ztMTxaRw6vsJK9W65vl7Gt0 ywk4XM1Bamg3w== Received: by mail-lf1-f48.google.com with SMTP id 2adb3069b0e04-4efef769000so1748663e87.3 for ; Mon, 24 Apr 2023 11:07:32 -0700 (PDT) X-Gm-Message-State: AAQBX9fwwa++1fJTHvab92k5hhaahCNFwNY2VJ54Fm7kUVsPASgH6TGC h2ROf7t581Ka+4TsXDIJx82F/g5FphNCtXsHTl4= X-Google-Smtp-Source: AKy350ZzOOyC5stX4bNDNHtNM77JGebcScrxC6URI4jtdr+IwOADkpvXcSpIDa+x6+is+jvdMwKfshvsbFjG07hrpCM= X-Received: by 2002:a05:6512:15d:b0:4db:28ce:e600 with SMTP id m29-20020a056512015d00b004db28cee600mr4154445lfo.1.1682359650972; Mon, 24 Apr 2023 11:07:30 -0700 (PDT) MIME-Version: 1.0 References: <20230424100552.2718-1-dun.tan@intel.com> In-Reply-To: From: "Ard Biesheuvel" Date: Mon, 24 Apr 2023 20:07:19 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [Patch V3 0/8] Create page table by CpuPageTableLib in DxeIpl To: "Kinney, Michael D" Cc: "devel@edk2.groups.io" , "Tan, Dun" Content-Type: text/plain; charset="UTF-8" On Mon, 24 Apr 2023 at 19:51, Kinney, Michael D wrote: > > Hi Ard, > > Thanks for the feedback. Let's work on this approach. > > Are there similar needs for CPU related services in the DXE Core before > the CPU AP is loaded? > > If we are going to define a new lib class to abstract DXE IPL requirements, > it would be good to cover DXE Core requirements too. > Yeah, excellent point. The problem I have had to work around in my strict permissions series (which includes the linked patch) is that there is a window from the moment DXE core is dispatched until the moment the CPU arch protocol DXE driver is dispatched where we don't have an architectural means to manipulate memory permissions. So what we'd need here is a library version of the following method typedef EFI_STATUS (EFIAPI *EFI_CPU_SET_MEMORY_ATTRIBUTES)( IN EFI_CPU_ARCH_PROTOCOL *This, IN EFI_PHYSICAL_ADDRESS BaseAddress, IN UINT64 Length, IN UINT64 Attributes ); *However*, I am aware that the X86 DXE IPL code deviates from this, as it needs to build long mode compatible page tables before switching from IA32 to X64, right? So whether that use case can be served with this prototype is doubtful, but I guess it /would/ make sense to define a single library abstraction that can accommodate all of these use cases. Happy to discuss this in more detail, > > > -----Original Message----- > > From: devel@edk2.groups.io On Behalf Of Ard Biesheuvel > > Sent: Monday, April 24, 2023 10:24 AM > > To: devel@edk2.groups.io; Tan, Dun > > Subject: Re: [edk2-devel] [Patch V3 0/8] Create page table by CpuPageTableLib in DxeIpl > > > > On Mon, 24 Apr 2023 at 12:06, duntan wrote: > > > > > > In V3 patch set: > > > 1. Add a new patch 'MdePkg: Move CpuPageTableLib defination to MdePkg' to Move CpuPageTableLib defination from UefiCpuPkg to > > MdePkg. > > > So that MdeModulePkg doesn't need to depend on UefiCpuPkg. > > > > As I replied to the other patch, I think adding CpuPageTableLib to > > MdePkg in its current form (even only the interface) is not the right > > approach here. The function protoypes and enums exposed by this > > library are highly specific to a particular implementation. > > > > There is a clear use case here, which is the DXE IPL code, and as has > > been suggested in the other thread, it would be more appropriate to > > define an abstraction regarding this use case, and define it as a > > library class (with a NULL implementation) in MdeModulePkg. That way, > > UefiCpuPkg can provide the x86 implementation based on > > CpuPageTableLilb, and other architectures can do likewise. > > > > Please refer to the patch below, which illustrates why there are other > > requirements in this area: > > > > https://edk2.groups.io/g/devel/message/101122 > > > > -- > > Ard. > > > > > > > > > > > 2. Modify the patch 'MdeModulePkg/DxeIpl: Create page table by CpuPageTableLib' to set GHCB page to be mapped as unencrypted > > for each CPU for AMD SEV feature. > > > > > > Dun Tan (8): > > > MdePkg: Move CpuPageTableLib defination to MdePkg > > > EmulatorPkg: Add CpuPageTableLib required by DxeIpl in DSC > > > IntelFsp2Pkg: Add CpuPageTableLib required by DxeIpl in DSC > > > MdeModulePkg: Add CpuPageTableLib required by DxeIpl in DSC > > > OvmfPkg: Add CpuPageTableLib required by DxeIpl in DSC file > > > MdeModulePkg/DxeIpl: Create page table by CpuPageTableLib > > > MdeModulePkg/DxeIpl: Remove duplicated code to enable NX > > > MdeModulePkg/DxeIpl: Refinement to the code to set PageTable as RO > > > > > > EmulatorPkg/EmulatorPkg.dsc | 3 ++- > > > IntelFsp2Pkg/Tools/Tests/QemuFspPkg.dsc | 3 ++- > > > MdeModulePkg/Core/DxeIplPeim/DxeIpl.h | 3 ++- > > > MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 5 ++++- > > > MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 112 ++++---------------------------------------------------------- > > -------------------------------------------------- > > > MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c | 5 +++-- > > > MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 720 > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > ++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------------------------------------- > > -------------------------------------------------------------------------------------------------------------------------------- > > -------------------------------------------------------------------------------------------------------------------------------- > > -------------------------------------------------------------------------------------------------------------------------------- > > -------------------------------------------------------------------------------- > > > MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h | 182 ++++++++++---------------------------------------------------- > > ------------------------------------------------------------------------------------------------------------------------ > > > MdeModulePkg/MdeModulePkg.dsc | 3 ++- > > > {UefiCpuPkg => MdePkg}/Include/Library/CpuPageTableLib.h | 0 > > > MdePkg/MdePkg.dec | 5 ++++- > > > OvmfPkg/AmdSev/AmdSevX64.dsc | 2 +- > > > OvmfPkg/Bhyve/BhyveX64.dsc | 3 ++- > > > OvmfPkg/CloudHv/CloudHvX64.dsc | 2 +- > > > OvmfPkg/Microvm/MicrovmX64.dsc | 2 +- > > > OvmfPkg/OvmfPkgIa32.dsc | 3 ++- > > > OvmfPkg/OvmfPkgIa32X64.dsc | 2 +- > > > OvmfPkg/OvmfPkgX64.dsc | 2 +- > > > OvmfPkg/OvmfXen.dsc | 2 +- > > > UefiCpuPkg/UefiCpuPkg.dec | 3 --- > > > 20 files changed, 211 insertions(+), 851 deletions(-) > > > rename {UefiCpuPkg => MdePkg}/Include/Library/CpuPageTableLib.h (100%) > > > > > > -- > > > 2.31.1.windows.1 > > > > > > > > > > > > > > > > > > > > > > > > > > >