Re: [edk2-devel] Polling Interval in MNP
Hi, Siva The 10ms interval was chosen just by experience. This background polling intends to pick up the coming request (e.g. ARP request) when there is no active polling from upper layer. The ARP time out is 1 second, so using 10ms background polling make the network stack able to receive ARP request among 100 incoming package/second. You can change this to a reasonable time for the inband network. Best Regards Siyuan From: Sivaraman Nainar Sent: 2019年9月10日 17:17 To: devel@edk2.groups.io Cc: Wu, Jiaxin ; Fu, Siyuan Subject: reg: Polling Interval in MNP Hi all: In the Network Package, MNPDxe Driver has the timeout check in MnpCheckPacketTimeout() for the interval of 10 ms. This interval is OK when we have the On board / Add on network controllers. In the cases of Network devices exposed via Inband (Virtual USB exposing network interfaces) when there is a packet read given to USB interfaces and it processed in lower level which could not provide the data within the 10 ms interval and it failed with timeout and communication failed when we perform the HTTP request. Do we have any specific reason to have this as 10 ms? Do we have any max limit for this interval value? -Siva -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47237): https://edk2.groups.io/g/devel/message/47237 Mute This Topic: https://groups.io/mt/34091513/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V2] UefiCpuPkg/CpuExceptionHandlerLib: Fix #AC split lock
Hi Laszlo,
2. Yes, I can change commit message/comments to separate split lock and #AC.
3. Yes it’s a close platform that is enabling #AC which hits double fault
because split lock inside CpuExceptionHandlerLib.
Code:
I was wondering same thing, why are they using locking mechanism. I wasn’t sure
so that’s why keep xchg instead of changing to mov. I have yet to think of any
reasons. I think it's a good idea to use mov instead it accomplishes the same
thing and easier to understand.
Thank you,
John
>-Original Message-
>From: Laszlo Ersek [mailto:ler...@redhat.com]
>Sent: Friday, September 13, 2019 9:32 AM
>To: devel@edk2.groups.io; Lofgren, John E
>Cc: Ni, Ray ; Gao, Liming ; Dong,
>Eric
>Subject: Re: [edk2-devel] [Patch V2] UefiCpuPkg/CpuExceptionHandlerLib: Fix
>#AC split lock
>
>On 09/09/19 20:40, John E Lofgren wrote:
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2150
>>
>> V2 changes:
>> Add xchg 16 bit instructions to handle sgdt and sidt base
>> 63:48 bits and 47:32 bits.
>> Add comment to explain why xchg 64bit isnt being used
>>
>> Fix #AC split lock's caused by seperating base and limit from sgdt and
>> sidt by changing xchg operands to 32-bit to stop from crossing
>> cacheline.
>>
>> Signed-off-by: John E Lofgren
>> ---
>>
>>
>UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nas
>m
>> | 20 ++--
>> 1 file changed, 14 insertions(+), 6 deletions(-)
>
>The commit message (and the bug report) are very difficult to understand.
>
>This is the first time I hear about "split lock" and "alignment check
>exception",
>so please bear with me. This is my take on the problem.
>
>(1) "split lock" is explained well here:
>
>https://lwn.net/Articles/784864/
>
>In short, we can consider it a performance anti-pattern. A locking instruction
>(such as XCHG) is invoked on incorrectly aligned data, which causes
>performance degradation for the whole system. In some cases this can be a
>security issue even, because less privileged code can block (slow down) more
>privileged code running on a different CPU.
>
>(2) Alignment Check Exception is a way to detect the occurrence of "split
>lock". It must be configured explicitly, when the system software wishes to be
>informed explicitly about a split lock event.
>
>Therefore, the "#AC split lock" expression in the commit message is very
>confusing. Those are two different things. One is the problem ("split lock"),
>and the other is the (kind of) solution ("alignment check exception").
>
>We don't care about #AC (the exception) here. My understanding is that the
>open source edk2 tree does not enable #AC. The following include file:
>
> MdePkg/Include/Register/Intel/Msr/P6Msr.h
>
>defines MSR_P6_TEST_CTL (value 0x33), which the above LWN article
>references as MSR_TEST_CTL. The article refers to bit 29, but that seems to be
>part of the "Reserved1" bit-field in the MSR_P6_TEST_CTL_REGISTER.
>
>There seems to be a bit field that could be related (Disable_LOCK, bit#31), but
>again, there is no reference to Disable_LOCK in the edk2 codebase.
>
>So my whole point here is that we should clearly separate "#AC" from "split
>lock" in the commit message (and in the code comment). Those are separate
>things. And "split lock" may apply to edk2, but "#AC" does not (to the open
>source tree anyway).
>
>(3) OK, assuming this code indeed triggers a "split lock" event. Why is that a
>problem? Yes, it may degrade performance for the system. Why do we care?
>We are *already* handling an exception. That should not be a very frequent
>event, for any processor (BSP or AP) in the system.
>
>Is the problem that a closed source platform enables #AC, and then the fault
>handler in CpuExceptionHandlerLib -- which gets invoked due to an
>independent fault -- runs into a *double* fault, due to the split lock raising
>#AC?
>
>This should be clearly explained in the commit message. I'm not prying at
>proprietary platform details, but the circumstances / symptoms of the issue
>should be clearly described.
>
>More comments below, regarding the original code:
>
>>
>> diff --git
>>
>a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.na
>> sm
>>
>b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.na
>> sm
>> index 4db1a09f28..7b7642b290 100644
>> ---
>>
>a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.na
>> sm
>> +++
>b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAs
>> +++ m.nasm
>> @@ -180,21 +180,29 @@ HasErrorCode:
>> pushqword [rbp + 24]
>>
>> ;; UINT64 Gdtr[2], Idtr[2];
>> +; sidt and sgdt saves 10 bytes to memory, 8 bytes = base and 2 bytes =
>limit.
>> +; To avoid #AC split lock when separating base and limit into their
>> +; own separate 64 bit memory, we can’t use 64 bit xchg since base
>> [63:48]
>bits
>> +; may cross the cache line.
>> xor rax, rax
>> pushrax
>> pushrax
>
>So, the contents of RAX is 0 now,
[edk2-devel] [edk2-platforms][PATCH V1 1/1] Platform/Intel/Readme.md: Content update
This change makes the following updates: 1. Add SimicsOpenBoardPkg details. 2. Update Python instructions to Python 3.x and mention Python 2.x deprecation. 3. Add Linux build environment set up instructions. 4. Update supported compiler and tool versions to newer versions tested. * Ubuntu build: 16.04.5 LTS to 18.04.1 LTS * iASL: 20160527 to 20190816 * NASM: 2.11.08 to 2.12.02 5. Update ClevoOpenBoardPkg details. 6. Add instructions to build packages (e.g. AdvancedFeaturePkg). 6. Update Planned Activities and Ideas sections. Cc: Agyeman Prince Cc: Dandan Bi Cc: Sai Chaganty Cc: Chasel Chiu Cc: Liming Gao Cc: Nate DeSimone Cc: Michael D Kinney Cc: Ankit Sinha Cc: Wei David Y Signed-off-by: Michael Kubacki --- Platform/Intel/Readme.md | 105 +--- 1 file changed, 71 insertions(+), 34 deletions(-) diff --git a/Platform/Intel/Readme.md b/Platform/Intel/Readme.md index 3caf362983..334c61e037 100644 --- a/Platform/Intel/Readme.md +++ b/Platform/Intel/Readme.md @@ -56,6 +56,7 @@ A UEFI firmware implementation using MinPlatformPkg is constructed using the fol * The `ClevoOpenBoardPkg` contains board implementations for Clevo systems. * The `KabylakeOpenBoardPkg` contains board implementations for Kaby Lake systems. * The `PurleyOpenBoardPkg` contains board implementations for Purley systems. +* The `SimicsOpenBoardPkg` contains board implementations for the Simics hardware simulator. * The `WhiskeylakeOpenBoardPkg` contains board implementations for Whiskey Lake systems. ## Board Package Organization @@ -83,10 +84,19 @@ return back to the minimum platform caller. - Install into ```C:\ASL``` to match default tools_def.txt configuration. * NASM assembler: Available from: http://www.nasm.us/ - Install into ```C:\NASM``` to match default tools_def.txt configuration. -* Python 2.7.6: Available from: https://www.python.org/download/releases/2.7.6/ - - Install into ```C:\Python27``` to match default tools_def.txt configuration. - - Add C:\Python27 to your path - - Other versions of 2.7 may also work fine. +* Python 3.7.3: Available from: https://www.python.org/downloads/release/python-373/ + - Other versions of Python 3.x should be compatible. + - It is recommended to use the Python launcher to ensure the Python build script is launched using Python 3. +- E.g. "py -3.7 build_bios.py -l" + +## **Linux Build Instructions** + +### Pre-requisites + + * Set up a EDK II build environment for Linux following the instructions in + [Using EDK II with Native GCC](https://github.com/tianocore/tianocore.github.io/wiki/Using-EDK-II-with-Native-GCC). + * Proceed to the [Common EDK II build instructions for Linux](https://github.com/tianocore/tianocore.github.io/wiki/Common-instructions) + to verify your basic EDK II build environment is set up properly. ### Download the required components @@ -106,11 +116,11 @@ return back to the minimum platform caller. * FSP repository * ``git clone https://github.com/IntelFsp/FSP.git`` -### Build +### Board Builds **Building with the python script** -1. Open command window, go to the workspace directory, e.g. c:\Kabylake or ~/Kabylake in the case of a linux OS +1. Open command window, go to the workspace directory, e.g. c:\Edk2Workspace or ~/Edk2Workspace in the case of a linux OS 2. If using a linux OS * Type "cd edk2" * Type "source edksetup.sh" @@ -146,11 +156,19 @@ return back to the minimum platform caller. * Type "python build_bios.py -h" * Note - * Python 2.7.16 and Python 3.7.3 compatible - * Some dependency Python scripts might only support 2.x or 3.x, if that happened use -"py -2" or "py -3" to launch build_bios.py - * This python build script has been tested on Windows 10 and Ubuntu 16.04.5 LTS - * See [cross-platform limitations](#Known-limitations) + * The Python build scripts were compatible with Python 2.7.16. But Python 2.x support is no longer maintained or recommended. + + * This python build script has been tested on Windows 10 and Ubuntu 18.04.1 LTS. + + * Unless otherwise noted, all boards build with the following components and versions: +* Linux build: Ubuntu 18.04.1 LTS with GCC version 5.4.0 +* Windows build: Windows 10 with the Microsoft Visual Studio 2015 compiler +* iASL version: 20190816 +* NASM version: 2.12.02 + + * Unless otherwise noted all boards have been tested for boot to Windows 10 x64 RS3. + + * See [known limitations](#Known-limitations) * Configuration Files * The edk2-platforms\Platform\Intel\build.cfg file contains the default settings used by build_bios.py @@ -185,6 +203,7 @@ return back to the minimum platform caller. | || | build settings, environment variables. | || |---build_board.py: Optional board-specific pre-build, build | ||
Re: [edk2-devel] [PATCH] StandaloneMmPkg: make package .DSC file build again
Reviewed-by: Jiewen Yao > -Original Message- > From: Ard Biesheuvel > Sent: Saturday, September 14, 2019 3:05 AM > To: devel@edk2.groups.io > Cc: ler...@redhat.com; achin.gu...@arm.com; Yao, Jiewen > ; Ard Biesheuvel > Subject: [PATCH] StandaloneMmPkg: make package .DSC file build again > > The StandaloneMmPkg .DSC file went out of sync with the changes > applied to the package when I enabled this code on the Synquacer > platform in edk2-platforms. So apply the necessary changes to make > this package build in isolation. > > Signed-off-by: Ard Biesheuvel > --- > StandaloneMmPkg/StandaloneMmPkg.dsc | 19 +++ > 1 file changed, 11 insertions(+), 8 deletions(-) > > diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc > b/StandaloneMmPkg/StandaloneMmPkg.dsc > index 8c5b9b3a3d47..8a68d397469b 100644 > --- a/StandaloneMmPkg/StandaloneMmPkg.dsc > +++ b/StandaloneMmPkg/StandaloneMmPkg.dsc > @@ -39,29 +39,32 @@ >BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf >DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf > > DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDe > bugPrintErrorLevelLib.inf > + > ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/P > rePiExtractGuidedSectionLib.inf >FvLib|StandaloneMmPkg/Library/FvLib/FvLib.inf > - > HobLib|StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmC > oreHobLib.inf > + > HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLi > b.inf >IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf > > MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMe > mLib.inf > > MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmCoreMemoryAl > locationLib/StandaloneMmCoreMemoryAllocationLib.inf > + > MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/Standalo > neMmServicesTableLib.inf >PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf > + > PeCoffExtraActionLib|StandaloneMmPkg/Library/StandaloneMmPeCoffExtraAct > ionLib/StandaloneMmPeCoffExtraActionLib.inf >PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf >PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf > > ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseRepo > rtStatusCodeLibNull.inf > - > - # > - # Entry point > - # > - > StandaloneMmDriverEntryPoint|StandaloneMmPkg/Library/StandaloneMmDriv > erEntryPoint/StandaloneMmDriverEntryPoint.inf > + > StandaloneMmCoreEntryPoint|StandaloneMmPkg/Library/StandaloneMmCoreE > ntryPoint/StandaloneMmCoreEntryPoint.inf > + > StandaloneMmDriverEntryPoint|MdePkg/Library/StandaloneMmDriverEntryPoin > t/StandaloneMmDriverEntryPoint.inf > > [LibraryClasses.AARCH64] >ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf > > StandaloneMmMmuLib|ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStand > aloneMmLib.inf >ArmSvcLib|ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf > > CacheMaintenanceLib|ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMa > intenanceLib.inf > - > PeCoffExtraActionLib|StandaloneMmPkg/Library/StandaloneMmPeCoffExtraAct > ionLib/StandaloneMmPeCoffExtraActionLib.inf > > - > StandaloneMmCoreEntryPoint|StandaloneMmPkg/Library/StandaloneMmCoreE > ntryPoint/StandaloneMmCoreEntryPoint.inf > + NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf > + NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > + > +[LibraryClasses.common.MM_CORE_STANDALONE] > + > HobLib|StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmC > oreHobLib.inf > > > # > ### > # > -- > 2.17.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47228): https://edk2.groups.io/g/devel/message/47228 Mute This Topic: https://groups.io/mt/34130722/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] SimicsOpenBoardPkg: Change to gEfiSmmSmramMemoryGuid to fix build error
MinPlatform PKG now use gEfiSmmSmramMemoryGuid instead of gEfiSmmPeiSmramMemoryReserveGuid to build memory HOB. SimicsOpenBoardPkg need to update for it accordingly. Cc: Hao Wu Cc: Liming Gao Cc: Ankit Sinha Cc: Agyeman Prince Cc: Kubacki Michael A Cc: Nate DeSimone Cc: Michael D Kinney Signed-off-by: David Wei --- Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c | 2 +- Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c index 90e6d1d3cf..ee0eead5a8 100644 --- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c +++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c @@ -449,7 +449,7 @@ QemuInitializeRam ( SmramRanges = 1; Hob.Raw = BuildGuidHob( - , + , BufferSize ); ASSERT(Hob.Raw); diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf index ccc7037d75..e466d57e4e 100644 --- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf +++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf @@ -39,7 +39,7 @@ [Guids] gEfiMemoryTypeInformationGuid - gEfiSmmPeiSmramMemoryReserveGuid ## CONSUMES + gEfiSmmSmramMemoryGuid ## CONSUMES [LibraryClasses] BaseLib -- 2.16.2.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47227): https://edk2.groups.io/g/devel/message/47227 Mute This Topic: https://groups.io/mt/34131830/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFC] EDK II Continuous Integration Phase 1
These tests require using the "edk2-pytool" stuff but are easy to integrate with the github PR or CI flow. Example of it running the code compliance tests is here: https://dev.azure.com/tianocore/edk2-ci-play/_build?definitionId=13&_a=summary A test run takes about 3 minutes. The past week or so we also have been working on a more complete test run which adds to the above test suite three more tests. Compile each package for Debug, Release, and Host Based Unit tests. Obviously this adds more time and takes more resources but at the moment we have it down to 13 minutes. You can see it here: https://dev.azure.com/tianocore/edk2-ci-play/_build?definitionId=12&_a=summary To enable this framework you need the following. 1. pip install a couple of edk2-pytool packages (see requirements.txt file) 2. Add the pytool based plugins to the edk2 code tree See here: https://github.com/spbrogan/edk2-staging/tree/edk2-stuart-ci-latest/BaseTools/Plugin/Ci 3. Add a CiSettings.py file that satisfies the setup, update, and ci_build See here: https://github.com/spbrogan/edk2-staging/blob/edk2-stuart-ci-latest/CISettings.py 4. Add a *.ci.yaml file to each package to configure test settings, ignore files, and other configuration for testing the package. Example here: https://github.com/spbrogan/edk2-staging/blob/edk2-stuart-ci-latest/MdeModulePkg/MdeModulePkg.ci.yaml 5. For Azure pipeline support (cloud based CI) you will need to add Azure pipeline.yaml flles. This can all be seen in the branch here. https://github.com/spbrogan/edk2-staging/tree/edk2-stuart-ci-latest Be aware that in that same branch we are enabling "Host based unit tests" which also requires more code changes so if diffing with clean edk2 you can ignore those (although that is all part of another RFC for unit test so any feedback on that would be great too). Feedback would be appreciated. I am hoping this RFC can move forward in the next few weeks. Thanks Sean -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47226): https://edk2.groups.io/g/devel/message/47226 Mute This Topic: https://groups.io/mt/33072637/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] StandaloneMmPkg: make package .DSC file build again
The StandaloneMmPkg .DSC file went out of sync with the changes applied to the package when I enabled this code on the Synquacer platform in edk2-platforms. So apply the necessary changes to make this package build in isolation. Signed-off-by: Ard Biesheuvel --- StandaloneMmPkg/StandaloneMmPkg.dsc | 19 +++ 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc b/StandaloneMmPkg/StandaloneMmPkg.dsc index 8c5b9b3a3d47..8a68d397469b 100644 --- a/StandaloneMmPkg/StandaloneMmPkg.dsc +++ b/StandaloneMmPkg/StandaloneMmPkg.dsc @@ -39,29 +39,32 @@ BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf + ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.inf FvLib|StandaloneMmPkg/Library/FvLib/FvLib.inf - HobLib|StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf + HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.inf MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmCoreMemoryAllocationLib/StandaloneMmCoreMemoryAllocationLib.inf + MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf + PeCoffExtraActionLib|StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf - - # - # Entry point - # - StandaloneMmDriverEntryPoint|StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf + StandaloneMmCoreEntryPoint|StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf + StandaloneMmDriverEntryPoint|MdePkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf [LibraryClasses.AARCH64] ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf StandaloneMmMmuLib|ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf ArmSvcLib|ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf CacheMaintenanceLib|ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.inf - PeCoffExtraActionLib|StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf - StandaloneMmCoreEntryPoint|StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf + NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf + NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf + +[LibraryClasses.common.MM_CORE_STANDALONE] + HobLib|StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf # -- 2.17.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47225): https://edk2.groups.io/g/devel/message/47225 Mute This Topic: https://groups.io/mt/34130722/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-platforms PATCH 1/1] Platform/RPi3: Add missing header files in INF file
On Sat, Aug 31, 2019 at 03:08:50PM +0100, Pete Batard wrote: > The header files are used but missing in INF, which causes > warning message when building them. > > Signed-off-by: Pete Batard Reviewed-by: Leif Lindholm Pushed as e60b57484b8a. Thanks! > --- > Platform/RaspberryPi/RPi3/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.inf >| 1 + > Platform/RaspberryPi/RPi3/Drivers/ConfigDxe/ConfigDxe.inf >| 1 + > Platform/RaspberryPi/RPi3/Drivers/DisplayDxe/DisplayDxe.inf >| 1 + > Platform/RaspberryPi/RPi3/Drivers/DwUsbHostDxe/DwUsbHostDxe.inf >| 2 ++ > Platform/RaspberryPi/RPi3/Drivers/MmcDxe/MmcDxe.inf >| 1 + > Platform/RaspberryPi/RPi3/Drivers/VarBlockServiceDxe/VarBlockServiceDxe.inf >| 1 + > > Platform/RaspberryPi/RPi3/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > | 1 + > 7 files changed, 8 insertions(+) > > diff --git > a/Platform/RaspberryPi/RPi3/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.inf > b/Platform/RaspberryPi/RPi3/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.inf > index 3da379b99bbb..487b7e3592ab 100644 > --- a/Platform/RaspberryPi/RPi3/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.inf > +++ b/Platform/RaspberryPi/RPi3/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.inf > @@ -16,6 +16,7 @@ [Defines] >ENTRY_POINT= MMCInitialize > > [Sources.common] > + ArasanMmcHostDxe.h >ArasanMmcHostDxe.c > > [Packages] > diff --git a/Platform/RaspberryPi/RPi3/Drivers/ConfigDxe/ConfigDxe.inf > b/Platform/RaspberryPi/RPi3/Drivers/ConfigDxe/ConfigDxe.inf > index 6d6c90b78408..2fc4302526a1 100644 > --- a/Platform/RaspberryPi/RPi3/Drivers/ConfigDxe/ConfigDxe.inf > +++ b/Platform/RaspberryPi/RPi3/Drivers/ConfigDxe/ConfigDxe.inf > @@ -23,6 +23,7 @@ [Defines] > > [Sources] >ConfigDxe.c > + ConfigDxeFormSetGuid.h >ConfigDxeHii.vfr >ConfigDxeHii.uni > > diff --git a/Platform/RaspberryPi/RPi3/Drivers/DisplayDxe/DisplayDxe.inf > b/Platform/RaspberryPi/RPi3/Drivers/DisplayDxe/DisplayDxe.inf > index 652f6827f351..11271045bdd9 100644 > --- a/Platform/RaspberryPi/RPi3/Drivers/DisplayDxe/DisplayDxe.inf > +++ b/Platform/RaspberryPi/RPi3/Drivers/DisplayDxe/DisplayDxe.inf > @@ -26,6 +26,7 @@ [Defines] > # > > [Sources] > + DisplayDxe.h >DisplayDxe.c >Screenshot.c >ComponentName.c > diff --git a/Platform/RaspberryPi/RPi3/Drivers/DwUsbHostDxe/DwUsbHostDxe.inf > b/Platform/RaspberryPi/RPi3/Drivers/DwUsbHostDxe/DwUsbHostDxe.inf > index 6eaca35aef55..f86480c035ba 100644 > --- a/Platform/RaspberryPi/RPi3/Drivers/DwUsbHostDxe/DwUsbHostDxe.inf > +++ b/Platform/RaspberryPi/RPi3/Drivers/DwUsbHostDxe/DwUsbHostDxe.inf > @@ -16,6 +16,8 @@ [Defines] >ENTRY_POINT= DwUsbHostEntryPoint > > [Sources.common] > + DwcHw.h > + DwUsbHostDxe.h >DwUsbHostDxe.c >DriverBinding.c >ComponentName.c > diff --git a/Platform/RaspberryPi/RPi3/Drivers/MmcDxe/MmcDxe.inf > b/Platform/RaspberryPi/RPi3/Drivers/MmcDxe/MmcDxe.inf > index 2c71bb624387..0690f9da9c4f 100644 > --- a/Platform/RaspberryPi/RPi3/Drivers/MmcDxe/MmcDxe.inf > +++ b/Platform/RaspberryPi/RPi3/Drivers/MmcDxe/MmcDxe.inf > @@ -18,6 +18,7 @@ [Defines] > > [Sources.common] >ComponentName.c > + Mmc.h >Mmc.c >MmcBlockIo.c >MmcIdentification.c > diff --git > a/Platform/RaspberryPi/RPi3/Drivers/VarBlockServiceDxe/VarBlockServiceDxe.inf > b/Platform/RaspberryPi/RPi3/Drivers/VarBlockServiceDxe/VarBlockServiceDxe.inf > index 8f99528f8963..394a4f61a5b8 100644 > --- > a/Platform/RaspberryPi/RPi3/Drivers/VarBlockServiceDxe/VarBlockServiceDxe.inf > +++ > b/Platform/RaspberryPi/RPi3/Drivers/VarBlockServiceDxe/VarBlockServiceDxe.inf > @@ -27,6 +27,7 @@ [Defines] > > [Sources] >FvbInfo.c > + VarBlockService.h >VarBlockService.c >VarBlockServiceDxe.c >FileIo.c > diff --git > a/Platform/RaspberryPi/RPi3/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > > b/Platform/RaspberryPi/RPi3/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > index 7ce3e95c80d6..e1b132a0ae3a 100644 > --- > a/Platform/RaspberryPi/RPi3/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > +++ > b/Platform/RaspberryPi/RPi3/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > @@ -25,6 +25,7 @@ [Defines] > # > > [Sources] > + PlatformBm.h >PlatformBm.c > > [Packages] > -- > 2.21.0.windows.1 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47223): https://edk2.groups.io/g/devel/message/47223 Mute This Topic: https://groups.io/mt/33090523/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 1/1] ArmPlatformPkg: Add missing header files in INF file
On Sat, Aug 31, 2019 at 03:07:29PM +0100, Pete Batard wrote: > The header files are used but missing in INF, which causes > warning message when building them. > > Signed-off-by: Pete Batard Thanks for the cleanup. Reviewed-by: Leif Lindholm Pushed as 9b5a1c789d39. > --- > ArmPlatformPkg/Library/ArmMaliDp/ArmMaliDp.inf | 1 + > ArmPlatformPkg/Library/HdLcd/HdLcd.inf | 1 + > ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.inf | 1 + > ArmPlatformPkg/PrePi/PeiMPCore.inf | 1 + > ArmPlatformPkg/PrePi/PeiUniCore.inf| 1 + > 5 files changed, 5 insertions(+) > > diff --git a/ArmPlatformPkg/Library/ArmMaliDp/ArmMaliDp.inf > b/ArmPlatformPkg/Library/ArmMaliDp/ArmMaliDp.inf > index c3f2eb03f01c..d9d9bbd30e4c 100644 > --- a/ArmPlatformPkg/Library/ArmMaliDp/ArmMaliDp.inf > +++ b/ArmPlatformPkg/Library/ArmMaliDp/ArmMaliDp.inf > @@ -16,6 +16,7 @@ [Defines] >LIBRARY_CLASS = LcdHwLib > > [Sources.common] > + ArmMaliDp.h >ArmMaliDp.c > > [Packages] > diff --git a/ArmPlatformPkg/Library/HdLcd/HdLcd.inf > b/ArmPlatformPkg/Library/HdLcd/HdLcd.inf > index c14f387b57d0..bc80e1adea27 100644 > --- a/ArmPlatformPkg/Library/HdLcd/HdLcd.inf > +++ b/ArmPlatformPkg/Library/HdLcd/HdLcd.inf > @@ -17,6 +17,7 @@ [Defines] >LIBRARY_CLASS = LcdHwLib > > [Sources.common] > + HdLcd.h >HdLcd.c > > [Packages] > diff --git a/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.inf > b/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.inf > index 8b7cc48bd60d..757348c19659 100644 > --- a/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.inf > +++ b/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.inf > @@ -16,6 +16,7 @@ [Defines] >LIBRARY_CLASS = LcdHwLib > > [Sources.common] > + PL111Lcd.h >PL111Lcd.c > > [Packages] > diff --git a/ArmPlatformPkg/PrePi/PeiMPCore.inf > b/ArmPlatformPkg/PrePi/PeiMPCore.inf > index 8be222893e2a..9c5da0d42a7b 100644 > --- a/ArmPlatformPkg/PrePi/PeiMPCore.inf > +++ b/ArmPlatformPkg/PrePi/PeiMPCore.inf > @@ -15,6 +15,7 @@ [Defines] >VERSION_STRING = 1.0 > > [Sources] > + PrePi.h >PrePi.c >MainMPCore.c > > diff --git a/ArmPlatformPkg/PrePi/PeiUniCore.inf > b/ArmPlatformPkg/PrePi/PeiUniCore.inf > index 95919b784185..ee9b05b25337 100644 > --- a/ArmPlatformPkg/PrePi/PeiUniCore.inf > +++ b/ArmPlatformPkg/PrePi/PeiUniCore.inf > @@ -15,6 +15,7 @@ [Defines] >VERSION_STRING = 1.0 > > [Sources] > + PrePi.h >PrePi.c >MainUniCore.c > > -- > 2.21.0.windows.1 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47222): https://edk2.groups.io/g/devel/message/47222 Mute This Topic: https://groups.io/mt/33090510/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] SimicsOpenBoardPkg: Always set the boot priority by default
When running networks of multiple machines, Simics assigns different disk IDs
to each disk created.
this change the boot priority and can't boot from SATA HDD directly.
Clear boot priority in NVRAM can fix this issue.
Cc: Hao Wu
Cc: Liming Gao
Cc: Ankit Sinha
Cc: Agyeman Prince
Cc: Kubacki Michael A
Cc: Nate DeSimone
Cc: Michael D Kinney
Signed-off-by: David Wei
---
.../Library/PlatformBootManagerLib/BdsPlatform.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git
a/Platform/Intel/SimicsOpenBoardPkg/Library/PlatformBootManagerLib/BdsPlatform.c
b/Platform/Intel/SimicsOpenBoardPkg/Library/PlatformBootManagerLib/BdsPlatform.c
index 953a4a6c15..926ed94ca1 100644
---
a/Platform/Intel/SimicsOpenBoardPkg/Library/PlatformBootManagerLib/BdsPlatform.c
+++
b/Platform/Intel/SimicsOpenBoardPkg/Library/PlatformBootManagerLib/BdsPlatform.c
@@ -338,10 +338,18 @@ PlatformBootManagerBeforeConsole (
VOID
)
{
-// EFI_HANDLEHandle;
-// EFI_STATUSStatus;
+ EFI_BOOT_MANAGER_LOAD_OPTION *NvBootOptions;
+ UINTN NvBootOptionCount;
+ UINTN Index;
+ EFI_STATUSStatus;
+
+ DEBUG((EFI_D_INFO, "PlatformBootManagerBeforeConsole\n"));
+
+ NvBootOptions = EfiBootManagerGetLoadOptions(,
LoadOptionTypeBoot);
+ for (Index = 0; Index < NvBootOptionCount; Index++) {
+Status =
EfiBootManagerDeleteLoadOptionVariable(NvBootOptions[Index].OptionNumber,
LoadOptionTypeBoot);
+ }
- DEBUG ((EFI_D_INFO, "PlatformBootManagerBeforeConsole\n"));
InstallDevicePathCallback ();
VisitAllInstancesOfProtocol (,
--
2.16.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#47221): https://edk2.groups.io/g/devel/message/47221
Mute This Topic: https://groups.io/mt/34129361/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V2] UefiCpuPkg/CpuExceptionHandlerLib: Fix #AC split lock
On 09/09/19 20:40, John E Lofgren wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2150
>
> V2 changes:
> Add xchg 16 bit instructions to handle sgdt and sidt base
> 63:48 bits and 47:32 bits.
> Add comment to explain why xchg 64bit isnt being used
>
> Fix #AC split lock's caused by seperating base and limit from sgdt
> and sidt by changing xchg operands to 32-bit to stop from crossing
> cacheline.
>
> Signed-off-by: John E Lofgren
> ---
> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm | 20
> ++--
> 1 file changed, 14 insertions(+), 6 deletions(-)
The commit message (and the bug report) are very difficult to understand.
This is the first time I hear about "split lock" and "alignment check
exception", so please bear with me. This is my take on the problem.
(1) "split lock" is explained well here:
https://lwn.net/Articles/784864/
In short, we can consider it a performance anti-pattern. A locking
instruction (such as XCHG) is invoked on incorrectly aligned data, which
causes performance degradation for the whole system. In some cases this
can be a security issue even, because less privileged code can block
(slow down) more privileged code running on a different CPU.
(2) Alignment Check Exception is a way to detect the occurrence of
"split lock". It must be configured explicitly, when the system software
wishes to be informed explicitly about a split lock event.
Therefore, the "#AC split lock" expression in the commit message is very
confusing. Those are two different things. One is the problem ("split
lock"), and the other is the (kind of) solution ("alignment check
exception").
We don't care about #AC (the exception) here. My understanding is that
the open source edk2 tree does not enable #AC. The following include file:
MdePkg/Include/Register/Intel/Msr/P6Msr.h
defines MSR_P6_TEST_CTL (value 0x33), which the above LWN article
references as MSR_TEST_CTL. The article refers to bit 29, but that seems
to be part of the "Reserved1" bit-field in the MSR_P6_TEST_CTL_REGISTER.
There seems to be a bit field that could be related (Disable_LOCK,
bit#31), but again, there is no reference to Disable_LOCK in the edk2
codebase.
So my whole point here is that we should clearly separate "#AC" from
"split lock" in the commit message (and in the code comment). Those are
separate things. And "split lock" may apply to edk2, but "#AC" does not
(to the open source tree anyway).
(3) OK, assuming this code indeed triggers a "split lock" event. Why is
that a problem? Yes, it may degrade performance for the system. Why do
we care? We are *already* handling an exception. That should not be a
very frequent event, for any processor (BSP or AP) in the system.
Is the problem that a closed source platform enables #AC, and then the
fault handler in CpuExceptionHandlerLib -- which gets invoked due to an
independent fault -- runs into a *double* fault, due to the split lock
raising #AC?
This should be clearly explained in the commit message. I'm not prying
at proprietary platform details, but the circumstances / symptoms of the
issue should be clearly described.
More comments below, regarding the original code:
>
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
> index 4db1a09f28..7b7642b290 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
> @@ -180,21 +180,29 @@ HasErrorCode:
> pushqword [rbp + 24]
>
> ;; UINT64 Gdtr[2], Idtr[2];
> +; sidt and sgdt saves 10 bytes to memory, 8 bytes = base and 2 bytes =
> limit.
> +; To avoid #AC split lock when separating base and limit into their
> +; own separate 64 bit memory, we can’t use 64 bit xchg since base
> [63:48] bits
> +; may cross the cache line.
> xor rax, rax
> pushrax
> pushrax
So, the contents of RAX is 0 now, and we've made 16 bytes (filled with
0) room on the stack.
> sidt[rsp]
This instruction writes 10 bytes at the base of that "room" on the
stack. Offsets 0 and 1 contain the "limit", offsets 2-9 (inclusive)
contain the "base address".
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|L|L|B|B|B|B|B|B|B|B|0|0|0|0|0|0|
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> -xchgrax, [rsp + 2]
(I guess this is the instruction that splits the lock.)
Now RAX has the base address, and the stack looks like:
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|L|L|0|0|0|0|0|0|0|0|0|0|0|0|0|0|
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> -xchgrax, [rsp]
Now RAX has the limit, and the stack is:
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|B|B|B|B|B|B|B|B|0|0|0|0|0|0|0|0|
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> -xchgrax, [rsp + 8]
Now RAX is zero again, and the stack is:
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|B|B|B|B|B|B|B|B|L|L|0|0|0|0|0|0|
[edk2-devel] [PATCH 11/11] OvmfPkg/XenBusDxe: Fix XenStoreWaitForEvent use during EBS
XenStoreWaitForEvent is going to be called when the ExitBootServices
is signaled, but both CreateEvent and WaitForEvent can't be used.
CreateEvent allocate some memory and WaitForEvent can only be used
when TPL is TPL_APPLICATION.
When ExitBootServices has been called, simply return immediately and
let caller of XenStoreWaitForEvent do a busy loop.
Also cleanup error handling in XenStoreWaitForEvent, WaitForEvent
shouldn't return EFI_UNSUPPORTED anymore.
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2190
Signed-off-by: Anthony PERARD
---
OvmfPkg/XenBusDxe/XenBusDxe.c | 2 ++
OvmfPkg/XenBusDxe/XenBusDxe.h | 1 +
OvmfPkg/XenBusDxe/XenStore.c | 13 +
3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/OvmfPkg/XenBusDxe/XenBusDxe.c b/OvmfPkg/XenBusDxe/XenBusDxe.c
index c71966a666..eb1503ad2b 100644
--- a/OvmfPkg/XenBusDxe/XenBusDxe.c
+++ b/OvmfPkg/XenBusDxe/XenBusDxe.c
@@ -264,6 +264,8 @@ NotifyExitBoot (
Dev = Context;
+ Dev->ExitBootNotified = TRUE;
+
//
// First, ask every driver using xenbus to disconnect without
// deallocating memory.
diff --git a/OvmfPkg/XenBusDxe/XenBusDxe.h b/OvmfPkg/XenBusDxe/XenBusDxe.h
index 0e91c24338..80162fc3ff 100644
--- a/OvmfPkg/XenBusDxe/XenBusDxe.h
+++ b/OvmfPkg/XenBusDxe/XenBusDxe.h
@@ -79,6 +79,7 @@ struct _XENBUS_DEVICE {
EFI_HANDLEControllerHandle;
XENIO_PROTOCOL*XenIo;
EFI_EVENT ExitBootEvent;
+ BOOLEAN ExitBootNotified;
EFI_DEVICE_PATH_PROTOCOL *DevicePath;
LIST_ENTRYChildList;
diff --git a/OvmfPkg/XenBusDxe/XenStore.c b/OvmfPkg/XenBusDxe/XenStore.c
index 4026c8a829..4f126109d4 100644
--- a/OvmfPkg/XenBusDxe/XenStore.c
+++ b/OvmfPkg/XenBusDxe/XenStore.c
@@ -401,17 +401,22 @@ XenStoreWaitForEvent (
EFI_EVENT TimerEvent;
EFI_EVENT WaitList[2];
+ //
+ // If the ExitBootServices event have been signaled, simply allow to have
+ // busy loop in the caller.
+ //
+ if (xs.Dev->ExitBootNotified) {
+return EFI_SUCCESS;
+ }
+
gBS->CreateEvent (EVT_TIMER, 0, NULL, NULL, );
gBS->SetTimer (TimerEvent, TimerRelative, Timeout);
WaitList[0] = xs.EventChannelEvent;
WaitList[1] = TimerEvent;
Status = gBS->WaitForEvent (2, WaitList, );
- ASSERT (Status != EFI_INVALID_PARAMETER);
+ ASSERT_EFI_ERROR (Status);
gBS->CloseEvent (TimerEvent);
- if (Status == EFI_UNSUPPORTED) {
-return EFI_SUCCESS;
- }
if (Index == 1) {
return EFI_TIMEOUT;
} else {
--
Anthony PERARD
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#47218): https://edk2.groups.io/g/devel/message/47218
Mute This Topic: https://groups.io/mt/34128216/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 10/11] OvmfPkg/XenPvBlkDxe: Use XenBusIo->RegisterExitCallback
In order to be able to reset the backend before handing it to the next
operating system, it should be reset properly. This patch register a
callback function to be called by XenBusDxe during the
ExitBootServices event.
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2190
Signed-off-by: Anthony PERARD
---
OvmfPkg/XenPvBlkDxe/BlockFront.c | 37 ---
OvmfPkg/XenPvBlkDxe/BlockFront.h | 12 +-
OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.c | 4 ++--
3 files changed, 47 insertions(+), 6 deletions(-)
diff --git a/OvmfPkg/XenPvBlkDxe/BlockFront.c b/OvmfPkg/XenPvBlkDxe/BlockFront.c
index 25a398ccc4..7c166888bd 100644
--- a/OvmfPkg/XenPvBlkDxe/BlockFront.c
+++ b/OvmfPkg/XenPvBlkDxe/BlockFront.c
@@ -309,6 +309,8 @@ XenPvBlockFrontInitialization (
Dev->MediaInfo.Sectors, Dev->MediaInfo.SectorSize));
*DevPtr = Dev;
+
+ XenBusIo->RegisterExitCallback (XenBusIo, XenPvBlockFrontReset, Dev);
return EFI_SUCCESS;
Error2:
@@ -326,13 +328,16 @@ XenPvBlockFrontInitialization (
VOID
XenPvBlockFrontShutdown (
- IN XEN_BLOCK_FRONT_DEVICE *Dev
+ IN XEN_BLOCK_FRONT_DEVICE *Dev,
+ IN BOOLEANResetOnly
)
{
XENBUS_PROTOCOL *XenBusIo = Dev->XenBusIo;
XENSTORE_STATUS Status;
UINT64 Value;
+ XenBusIo->RegisterExitCallback (XenBusIo, NULL, NULL);
+
XenPvBlockSync (Dev);
Status = XenBusIo->SetState (XenBusIo, XST_NIL, XenbusStateClosing);
@@ -393,12 +398,38 @@ XenPvBlockFrontShutdown (
}
Close:
- XenBusIo->UnregisterWatch (XenBusIo, Dev->StateWatchToken);
+ if (!ResetOnly) {
+XenBusIo->UnregisterWatch (XenBusIo, Dev->StateWatchToken);
+ }
XenBusIo->XsRemove (XenBusIo, XST_NIL, "ring-ref");
XenBusIo->XsRemove (XenBusIo, XST_NIL, "event-channel");
XenBusIo->XsRemove (XenBusIo, XST_NIL, "protocol");
- XenPvBlockFree (Dev);
+ if (ResetOnly) {
+XenBusIo->GrantEndAccess (XenBusIo, Dev->RingRef);
+XenBusIo->EventChannelClose (XenBusIo, Dev->EventChannel);
+ } else {
+XenPvBlockFree (Dev);
+ }
+}
+
+/**
+ To be called when ExitBootServices has been called.
+
+ This should reset the PV backend without using memory allocation
+ services.
+**/
+VOID
+EFIAPI
+XenPvBlockFrontReset (
+ IN VOID *Context
+ )
+{
+ XEN_BLOCK_FRONT_DEVICE *Dev;
+
+ Dev = Context;
+
+ XenPvBlockFrontShutdown (Dev, TRUE);
}
STATIC
diff --git a/OvmfPkg/XenPvBlkDxe/BlockFront.h b/OvmfPkg/XenPvBlkDxe/BlockFront.h
index 7c2d7f2c9e..ebce4fe434 100644
--- a/OvmfPkg/XenPvBlkDxe/BlockFront.h
+++ b/OvmfPkg/XenPvBlkDxe/BlockFront.h
@@ -67,9 +67,19 @@ XenPvBlockFrontInitialization (
OUT XEN_BLOCK_FRONT_DEVICE **DevPtr
);
+/**
+ @param ResetOnly Set to TRUE when called during the ExitBootServices.
+**/
VOID
XenPvBlockFrontShutdown (
- IN XEN_BLOCK_FRONT_DEVICE *Dev
+ IN XEN_BLOCK_FRONT_DEVICE *Dev,
+ IN BOOLEANResetOnly
+ );
+
+VOID
+EFIAPI
+XenPvBlockFrontReset (
+ IN VOID *Context
);
VOID
diff --git a/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.c
b/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.c
index bfe7b1a754..3031406980 100644
--- a/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.c
+++ b/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.c
@@ -312,7 +312,7 @@ XenPvBlkDxeDriverBindingStart (
UninitBlockFront:
FreePool (Media);
- XenPvBlockFrontShutdown (Dev);
+ XenPvBlockFrontShutdown (Dev, FALSE);
CloseProtocol:
gBS->CloseProtocol (ControllerHandle, ,
This->DriverBindingHandle, ControllerHandle);
@@ -377,7 +377,7 @@ XenPvBlkDxeDriverBindingStop (
Media = BlockIo->Media;
Dev = XEN_BLOCK_FRONT_FROM_BLOCK_IO (BlockIo);
- XenPvBlockFrontShutdown (Dev);
+ XenPvBlockFrontShutdown (Dev, FALSE);
FreePool (Media);
--
Anthony PERARD
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#47219): https://edk2.groups.io/g/devel/message/47219
Mute This Topic: https://groups.io/mt/34128218/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 07/11] OvmfPkg/XenBusDxe: Use on stack buffer in internal functions
We will use a buffer on the stack instead of allocating memory for
internal functions that are expecting a reply from xenstore.
The external interface XENBUS_PROTOCOL isn't changed yet, so
allocation are made for XsRead and XsBackendRead.
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2190
Signed-off-by: Anthony PERARD
---
OvmfPkg/XenBusDxe/XenBus.c | 40 ++--
OvmfPkg/XenBusDxe/XenStore.c | 115 ---
OvmfPkg/XenBusDxe/XenStore.h | 17 +++---
3 files changed, 95 insertions(+), 77 deletions(-)
diff --git a/OvmfPkg/XenBusDxe/XenBus.c b/OvmfPkg/XenBusDxe/XenBus.c
index bb8ddbc4d4..78835ec7b3 100644
--- a/OvmfPkg/XenBusDxe/XenBus.c
+++ b/OvmfPkg/XenBusDxe/XenBus.c
@@ -89,19 +89,18 @@ XenBusReadDriverState (
IN CONST CHAR8 *Path
)
{
- XenbusState State;
- CHAR8 *Ptr = NULL;
+ XenbusState State;
+ CHAR8 Buffer[4];
+ UINTN BufferSize;
XENSTORE_STATUS Status;
- Status = XenStoreRead (XST_NIL, Path, "state", NULL, (VOID **));
+ BufferSize = sizeof (Buffer) - 1;
+ Status = XenStoreRead (XST_NIL, Path, "state", , Buffer);
if (Status != XENSTORE_STATUS_SUCCESS) {
State = XenbusStateClosed;
} else {
-State = AsciiStrDecimalToUintn (Ptr);
- }
-
- if (Ptr != NULL) {
-FreePool (Ptr);
+Buffer[BufferSize] = '\0';
+State = AsciiStrDecimalToUintn (Buffer);
}
return State;
@@ -129,8 +128,11 @@ XenBusAddDevice (
if (XenStorePathExists (XST_NIL, DevicePath, "")) {
XENBUS_PRIVATE_DATA *Child;
-enum xenbus_state State;
-CHAR8 *BackendPath;
+enum xenbus_state State;
+CHAR8 BackendPath[XENSTORE_ABS_PATH_MAX + 1];
+UINTN BackendPathSize;
+
+BackendPathSize = sizeof (BackendPath);
Child = XenBusDeviceInitialized (Dev, DevicePath);
if (Child != NULL) {
@@ -155,17 +157,18 @@ XenBusAddDevice (
}
StatusXenStore = XenStoreRead (XST_NIL, DevicePath, "backend",
- NULL, (VOID **) );
+ , BackendPath);
if (StatusXenStore != XENSTORE_STATUS_SUCCESS) {
DEBUG ((EFI_D_ERROR, "xenbus: %a no backend path.\n", DevicePath));
Status = EFI_NOT_FOUND;
goto out;
}
+BackendPath[BackendPathSize] = '\0';
Private = AllocateCopyPool (sizeof (*Private), );
Private->XenBusIo.Type = AsciiStrDup (Type);
Private->XenBusIo.Node = AsciiStrDup (DevicePath);
-Private->XenBusIo.Backend = BackendPath;
+Private->XenBusIo.Backend = AsciiStrDup (BackendPath);
Private->XenBusIo.DeviceId = (UINT16)AsciiStrDecimalToUintn (Id);
Private->Dev = Dev;
@@ -309,17 +312,20 @@ XenBusSetState (
)
{
enum xenbus_state CurrentState;
- XENSTORE_STATUS Status;
- CHAR8 *Temp;
+ XENSTORE_STATUS Status;
+ CHAR8 Buffer[4];
+ UINTN BufferSize;
+
+ BufferSize = sizeof (Buffer) - 1;
DEBUG ((EFI_D_INFO, "XenBus: Set state to %d\n", NewState));
- Status = XenStoreRead (Transaction, This->Node, "state", NULL, (VOID
**));
+ Status = XenStoreRead (Transaction, This->Node, "state", ,
Buffer);
if (Status != XENSTORE_STATUS_SUCCESS) {
goto Out;
}
- CurrentState = AsciiStrDecimalToUintn (Temp);
- FreePool (Temp);
+ Buffer[BufferSize] = '\0';
+ CurrentState = AsciiStrDecimalToUintn (Buffer);
if (CurrentState == NewState) {
goto Out;
}
diff --git a/OvmfPkg/XenBusDxe/XenStore.c b/OvmfPkg/XenBusDxe/XenStore.c
index 004d3b6022..b9588bb8c6 100644
--- a/OvmfPkg/XenBusDxe/XenStore.c
+++ b/OvmfPkg/XenBusDxe/XenStore.c
@@ -756,8 +756,9 @@ XenStoreGetError (
@param RequestTypeThe type of message to send.
@param WriteRequest Pointers to the body sections of the request.
@param NumRequestsThe number of body sections in the request.
- @param LenPtr The returned length of the reply.
- @param ResultPtr The returned body of the reply.
+ @param BufferSize IN: size of the buffer
+OUT: The returned length of the reply.
+ @param Buffer The returned body of the reply.
@return XENSTORE_STATUS_SUCCESS on success. Otherwise an errno indicating
the cause of failure.
@@ -769,15 +770,13 @@ XenStoreTalkv (
IN enum xsd_sockmsg_type RequestType,
IN CONST WRITE_REQUEST *WriteRequest,
IN UINT32 NumRequests,
- OUT UINT32 *LenPtr OPTIONAL,
- OUT VOID**ResultPtr OPTIONAL
+ IN OUT UINTN*BufferSize OPTIONAL,
+ OUT VOID*Buffer OPTIONAL
)
{
struct xsd_sockmsg Message;
UINTN Index;
XENSTORE_STATUSStatus;
- VOID *Buffer;
- UINTN BufferSize;
if (Transaction == XST_NIL) {
Message.tx_id = 0;
@@ -805,32 +804,15 @@ XenStoreTalkv (
}
}
- if (ResultPtr) {
-Buffer = AllocatePool (XENSTORE_PAYLOAD_MAX + 1);
-BufferSize = XENSTORE_PAYLOAD_MAX;
- } else {
-Buffer =
[edk2-devel] [PATCH 06/11] OvmfPkg/XenBusDxe: Rework XenStoreProcessMessage to avoid allocating memory
This patch rework XenStoreProcessMessage in order to avoid memory
allocation when a reply is expected. Instead of allocating a buffer
for this reply, we are going to copy to a buffer passed by the caller.
For messages that aren't fully received, they will be stored in a
buffer that have been allocated at the initialisation of the driver.
A temporary memory allocation is made in XenStoreTalkv but that will
be removed in a further patch.
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2190
Signed-off-by: Anthony PERARD
---
OvmfPkg/XenBusDxe/XenStore.c | 297 +++
1 file changed, 130 insertions(+), 167 deletions(-)
diff --git a/OvmfPkg/XenBusDxe/XenStore.c b/OvmfPkg/XenBusDxe/XenStore.c
index ca7be12d68..004d3b6022 100644
--- a/OvmfPkg/XenBusDxe/XenStore.c
+++ b/OvmfPkg/XenBusDxe/XenStore.c
@@ -72,27 +72,6 @@ struct _XENSTORE_WATCH
#define XENSTORE_WATCH_FROM_LINK(l) \
CR (l, XENSTORE_WATCH, Link, XENSTORE_WATCH_SIGNATURE)
-
-/**
- * Structure capturing messages received from the XenStore service.
- */
-#define XENSTORE_MESSAGE_SIGNATURE SIGNATURE_32 ('X', 'S', 's', 'm')
-typedef struct {
- UINT32 Signature;
- LIST_ENTRY Link;
-
- struct xsd_sockmsg Header;
-
- union {
-/* Queued replies. */
-struct {
- CHAR8 *Body;
-} Reply;
- } u;
-} XENSTORE_MESSAGE;
-#define XENSTORE_MESSAGE_FROM_LINK(r) \
- CR (r, XENSTORE_MESSAGE, Link, XENSTORE_MESSAGE_SIGNATURE)
-
/**
* Container for all XenStore related state.
*/
@@ -105,21 +84,6 @@ typedef struct {
XENBUS_DEVICE *Dev;
- /**
- * A list of replies to our requests.
- *
- * The reply list is filled by xs_rcv_thread(). It
- * is consumed by the context that issued the request
- * to which a reply is made. The requester blocks in
- * XenStoreReadReply ().
- *
- * /note Only one requesting context can be active at a time.
- */
- LIST_ENTRY ReplyList;
-
- /** Lock protecting the reply list. */
- EFI_LOCK ReplyLock;
-
/**
* List of registered watches.
*/
@@ -136,6 +100,13 @@ typedef struct {
/** Handle for XenStore events. */
EFI_EVENT EventChannelEvent;
+
+ /** Buffer used to copy payloads from the xenstore ring */
+ // The + 1 is to allow to have a \0.
+ CHAR8 Buffer[XENSTORE_PAYLOAD_MAX + 1];
+
+ /** ID used when sending messages to xenstored */
+ UINTN NextRequestId;
} XENSTORE_PRIVATE;
//
@@ -148,6 +119,12 @@ static XENSTORE_PRIVATE xs;
// Private Utility Functions
//
+STATIC
+XENSTORE_STATUS
+XenStoreGetError (
+ CONST CHAR8 *ErrorStr
+ );
+
/**
Count and optionally record pointers to a number of NUL terminated
strings in a buffer.
@@ -613,70 +590,106 @@ XenStoreReadStore (
Block reading the next message from the XenStore service and
process the result.
+ @param ExpectedRequestId Block until a reply to with this ID is seen.
+ @param ExpectedTransactionId Idem, but should also match this ID.
+ @param BufferSize IN: size of the buffer
+OUT: The returned length of the reply.
+ @param Buffer The returned body of the reply.
+
@return XENSTORE_STATUS_SUCCESS on success. Otherwise an errno value
indicating the type of failure encountered.
**/
STATIC
XENSTORE_STATUS
XenStoreProcessMessage (
- VOID
+ IN UINT32ExpectedRequestId,
+ IN UINT32ExpectedTransactionId,
+ IN OUT UINTN *BufferSize OPTIONAL,
+ IN OUT CHAR8 *Buffer OPTIONAL
)
{
- XENSTORE_MESSAGE *Message;
- CHAR8 *Body;
- XENSTORE_STATUS Status;
-
- Message = AllocateZeroPool (sizeof (XENSTORE_MESSAGE));
- Message->Signature = XENSTORE_MESSAGE_SIGNATURE;
- Status = XenStoreReadStore (>Header, sizeof (Message->Header));
- if (Status != XENSTORE_STATUS_SUCCESS) {
-FreePool (Message);
-DEBUG ((EFI_D_ERROR, "XenStore: Error read store (%d)\n", Status));
-return Status;
- }
-
- Body = AllocatePool (Message->Header.len + 1);
- Status = XenStoreReadStore (Body, Message->Header.len);
- if (Status != XENSTORE_STATUS_SUCCESS) {
-FreePool (Body);
-FreePool (Message);
-DEBUG ((EFI_D_ERROR, "XenStore: Error read store (%d)\n", Status));
-return Status;
- }
- Body[Message->Header.len] = '\0';
+ struct xsd_sockmsg Header;
+ CHAR8 *Payload;
+ XENSTORE_STATUSStatus;
- if (Message->Header.type == XS_WATCH_EVENT) {
-CONST CHAR8*WatchEventPath;
-CONST CHAR8*WatchEventToken;
-VOID *ConvertedToken;
-XENSTORE_WATCH *Watch;
+ while (TRUE) {
-//
-// Parse WATCH_EVENT messages
-// \0\0
-//
-WatchEventPath = Body;
-WatchEventToken = WatchEventPath + AsciiStrSize (WatchEventPath);
+Status = XenStoreReadStore (, sizeof (Header));
+if (Status != XENSTORE_STATUS_SUCCESS) {
+ DEBUG ((DEBUG_ERROR, "XenStore: Error read store (%d)\n", Status));
+ return Status;
+}
-ConvertedToken = (VOID *) AsciiStrHexToUintn (WatchEventToken);
+
[edk2-devel] [PATCH 08/11] OvmfPkg/XenBus: Change XENBUS_PROTOCOL to not return allocated memory
XsRead and XsBackendRead of the XENBUS_PROTOCOL return allocated
memory but this isn't allowed during the ExitBootServices call. We
need XsRead and XsBackendRead to disconnect from the device so
XENBUS_PROTOCOL is changed to use a buffer supplied by a child driver.
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2190
Signed-off-by: Anthony PERARD
---
OvmfPkg/Include/Protocol/XenBus.h | 32 --
OvmfPkg/XenBusDxe/XenStore.c | 44 +-
OvmfPkg/XenBusDxe/XenStore.h | 6 +++--
OvmfPkg/XenPvBlkDxe/BlockFront.c | 45 ++-
4 files changed, 54 insertions(+), 73 deletions(-)
diff --git a/OvmfPkg/Include/Protocol/XenBus.h
b/OvmfPkg/Include/Protocol/XenBus.h
index 8ff5ca3575..c22bdfb368 100644
--- a/OvmfPkg/Include/Protocol/XenBus.h
+++ b/OvmfPkg/Include/Protocol/XenBus.h
@@ -35,6 +35,12 @@ typedef struct
#define XST_NIL ((XENSTORE_TRANSACTION *) NULL)
+//
+// When reading a node from xenstore, if the size of the data to be read is
+// unknown, this value can be use for the size of the buffer.
+//
+#define XENSTORE_PAYLOAD_MAX 4096
+
typedef enum {
XENSTORE_STATUS_SUCCESS = 0,
XENSTORE_STATUS_FAIL,
@@ -64,19 +70,17 @@ typedef enum {
///
/**
- Get the contents of the node Node of the PV device. Returns the contents in
- *Result which should be freed after use.
+ Get the contents of the node Node of the PV device.
@param This A pointer to XENBUS_PROTOCOL instance.
@param TransactionThe XenStore transaction covering this request.
@param Node The basename of the file to read.
- @param Result The returned contents from this file.
+ @param BufferSize On input, a pointer to the size of the buffer at
Buffer.
+On output, the size of the data written to Buffer.
+ @param Buffer A pointer to a buffer into which the data read will be
saved.
@return On success, XENSTORE_STATUS_SUCCESS. Otherwise an errno value
indicating the type of failure.
-
- @note The results buffer is malloced and should be free'd by the
-caller.
**/
typedef
XENSTORE_STATUS
@@ -84,23 +88,22 @@ XENSTORE_STATUS
IN XENBUS_PROTOCOL *This,
IN CONST XENSTORE_TRANSACTION *Transaction,
IN CONST CHAR8 *Node,
- OUT VOID **Result
+ IN OUT UINTN *BufferSize,
+ OUT VOID *Buffer
);
/**
- Get the contents of the node Node of the PV device's backend. Returns the
- contents in *Result which should be freed after use.
+ Get the contents of the node Node of the PV device's backend.
@param This A pointer to XENBUS_PROTOCOL instance.
@param TransactionThe XenStore transaction covering this request.
@param Node The basename of the file to read.
- @param Result The returned contents from this file.
+ @param BufferSize On input, a pointer to the size of the buffer at
Buffer.
+On output, the size of the data written to Buffer.
+ @param Buffer A pointer to a buffer into which the data read will be
saved.
@return On success, XENSTORE_STATUS_SUCCESS. Otherwise an errno value
indicating the type of failure.
-
- @note The results buffer is malloced and should be free'd by the
-caller.
**/
typedef
XENSTORE_STATUS
@@ -108,7 +111,8 @@ XENSTORE_STATUS
IN XENBUS_PROTOCOL *This,
IN CONST XENSTORE_TRANSACTION *Transaction,
IN CONST CHAR8 *Node,
- OUT VOID **Result
+ IN OUT UINTN *BufferSize,
+ OUT VOID *Buffer
);
/**
diff --git a/OvmfPkg/XenBusDxe/XenStore.c b/OvmfPkg/XenBusDxe/XenStore.c
index b9588bb8c6..cb2d9e1215 100644
--- a/OvmfPkg/XenBusDxe/XenStore.c
+++ b/OvmfPkg/XenBusDxe/XenStore.c
@@ -1336,27 +1336,11 @@ XenBusXenStoreRead (
IN XENBUS_PROTOCOL *This,
IN CONST XENSTORE_TRANSACTION *Transaction,
IN CONST CHAR8 *Node,
- OUT VOID **Value
+ IN OUT UINTN *BufferSize,
+ OUT VOID *Buffer
)
{
- XENSTORE_STATUS Status;
- UINTN BufferSize;
- VOID*Buffer;
-
- BufferSize = XENSTORE_PAYLOAD_MAX + 1;
- Buffer = AllocatePool (BufferSize);
- if (Buffer == NULL) {
-return XENSTORE_STATUS_ENOMEM;
- }
-
- Status = XenStoreRead (Transaction, This->Node, Node, , Buffer);
-
- if (Status == XENSTORE_STATUS_SUCCESS) {
-*Value = Buffer;
- } else {
-FreePool (Buffer);
- }
- return Status;
+ return XenStoreRead (Transaction, This->Node, Node, BufferSize, Buffer);
}
XENSTORE_STATUS
@@ -1365,27 +1349,11 @@ XenBusXenStoreBackendRead (
IN XENBUS_PROTOCOL *This,
IN CONST XENSTORE_TRANSACTION *Transaction,
IN CONST CHAR8 *Node,
- OUT VOID **Value
+ IN OUT UINTN *BufferSize,
+ OUT VOID *Buffer
)
{
-
[edk2-devel] [PATCH 00/11] OvmfPkg/XenBusDxe: Fix ExitBootServices handler to avoid allocation
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2190 Patch series available in this git branch: https://xenbits.xen.org/git-http/people/aperard/ovmf.git br.xenbusdxe-fix-exitbootservices-v1 Hi, This patch series works toward removing usage of Memory Allocation Services in XenBusDxe when ExitBootServices() is called by the next operating system. Since that in order to reset a backend, communication needs to happened via xenstore, this series focus mostly on getting rid of allocation in the xenstore driver. There are still some left but that's in function that aren't needed after EBS is called. In some places (like XenStoreVSPrint), instead of allocating a buffer, the buffer (4k) is on the stack. Thanks, Anthony PERARD (11): OvmfPkg/XenBusDxe: Fix missing \n in DEBUG messages OvmfPkg/XenBusDxe: Have XenStoreFindWatch take a pointer OvmfPkg/XenBusDxe: Rework watch events reception OvmfPkg/XenBusDxe: Avoid Allocate in XenStoreVSPrint OvmfPkg/XenBusDxe: Construct paths without allocation OvmfPkg/XenBusDxe: Rework XenStoreProcessMessage to avoid allocating memory OvmfPkg/XenBusDxe: Use on stack buffer in internal functions OvmfPkg/XenBus: Change XENBUS_PROTOCOL to not return allocated memory OvmfPkg/XenBusDxe: Fix NotifyExitBoot to avoid Memory Allocation Services OvmfPkg/XenPvBlkDxe: Use XenBusIo->RegisterExitCallback OvmfPkg/XenBusDxe: Fix XenStoreWaitForEvent use during EBS OvmfPkg/Include/Protocol/XenBus.h | 67 +++- OvmfPkg/XenBusDxe/EventChannel.c | 3 +- OvmfPkg/XenBusDxe/XenBus.c| 58 ++- OvmfPkg/XenBusDxe/XenBusDxe.c | 29 +- OvmfPkg/XenBusDxe/XenBusDxe.h | 3 + OvmfPkg/XenBusDxe/XenStore.c | 577 +- OvmfPkg/XenBusDxe/XenStore.h | 44 ++- OvmfPkg/XenPvBlkDxe/BlockFront.c | 82 +++-- OvmfPkg/XenPvBlkDxe/BlockFront.h | 12 +- OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.c | 4 +- 10 files changed, 483 insertions(+), 396 deletions(-) -- Anthony PERARD -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47208): https://edk2.groups.io/g/devel/message/47208 Mute This Topic: https://groups.io/mt/34128009/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 02/11] OvmfPkg/XenBusDxe: Have XenStoreFindWatch take a pointer
Rework XenStoreFindWatch() to be able to search for a registered watch
with a pointer instead of a string.
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2190
Signed-off-by: Anthony PERARD
---
OvmfPkg/XenBusDxe/XenStore.c | 20 +++-
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/OvmfPkg/XenBusDxe/XenStore.c b/OvmfPkg/XenBusDxe/XenStore.c
index 7253d8ae37..727641a0fe 100644
--- a/OvmfPkg/XenBusDxe/XenStore.c
+++ b/OvmfPkg/XenBusDxe/XenStore.c
@@ -253,14 +253,12 @@ Split (
STATIC
XENSTORE_WATCH *
XenStoreFindWatch (
- IN CONST CHAR8 *Token
+ IN VOID *Token
)
{
- XENSTORE_WATCH *Watch, *WantedWatch;
+ XENSTORE_WATCH *Watch;
LIST_ENTRY *Entry;
- WantedWatch = (VOID *) AsciiStrHexToUintn (Token);
-
if (IsListEmpty ()) {
return NULL;
}
@@ -268,7 +266,7 @@ XenStoreFindWatch (
!IsNull (, Entry);
Entry = GetNextNode (, Entry)) {
Watch = XENSTORE_WATCH_FROM_LINK (Entry);
-if (Watch == WantedWatch)
+if ((VOID *) Watch == Token)
return Watch;
}
@@ -632,12 +630,16 @@ XenStoreProcessMessage (
Body[Message->Header.len] = '\0';
if (Message->Header.type == XS_WATCH_EVENT) {
+VOID *ConvertedToken;
+
Message->u.Watch.Vector = Split(Body, Message->Header.len,
>u.Watch.VectorSize);
+ConvertedToken =
+ (VOID *) AsciiStrHexToUintn (Message->u.Watch.Vector[XS_WATCH_TOKEN]);
+
EfiAcquireLock ();
-Message->u.Watch.Handle =
- XenStoreFindWatch (Message->u.Watch.Vector[XS_WATCH_TOKEN]);
+Message->u.Watch.Handle = XenStoreFindWatch (ConvertedToken);
DEBUG ((EFI_D_INFO, "XenStore: Watch event %a\n",
Message->u.Watch.Vector[XS_WATCH_TOKEN]));
if (Message->u.Watch.Handle != NULL) {
@@ -1384,8 +1386,7 @@ XenStoreUnregisterWatch (
ASSERT (Watch->Signature == XENSTORE_WATCH_SIGNATURE);
- AsciiSPrint (Token, sizeof (Token), "%p", (VOID *) Watch);
- if (XenStoreFindWatch (Token) == NULL) {
+ if (XenStoreFindWatch (Watch) == NULL) {
return;
}
@@ -1393,6 +1394,7 @@ XenStoreUnregisterWatch (
RemoveEntryList (>Link);
EfiReleaseLock ();
+ AsciiSPrint (Token, sizeof (Token), "%p", (VOID *) Watch);
XenStoreUnwatch (Watch->Node, Token);
/* Cancel pending watch events. */
--
Anthony PERARD
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#47211): https://edk2.groups.io/g/devel/message/47211
Mute This Topic: https://groups.io/mt/34128012/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 01/11] OvmfPkg/XenBusDxe: Fix missing \n in DEBUG messages
Fix missing \n in DEBUG messages in XenBusDxe and use DEBUG_*.
Signed-off-by: Anthony PERARD
---
OvmfPkg/XenBusDxe/EventChannel.c | 3 ++-
OvmfPkg/XenBusDxe/XenStore.c | 6 +++---
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/OvmfPkg/XenBusDxe/EventChannel.c b/OvmfPkg/XenBusDxe/EventChannel.c
index 6900071782..c6b3871781 100644
--- a/OvmfPkg/XenBusDxe/EventChannel.c
+++ b/OvmfPkg/XenBusDxe/EventChannel.c
@@ -44,7 +44,8 @@ XenBusEventChannelAllocate (
EVTCHNOP_alloc_unbound,
);
if (ReturnCode != 0) {
-DEBUG ((EFI_D_ERROR, "ERROR: alloc_unbound failed with rc=%d",
ReturnCode));
+DEBUG ((DEBUG_ERROR, "ERROR: alloc_unbound failed with rc=%d\n",
+ReturnCode));
return ReturnCode;
}
*Port = Parameter.port;
diff --git a/OvmfPkg/XenBusDxe/XenStore.c b/OvmfPkg/XenBusDxe/XenStore.c
index 34890ae40b..7253d8ae37 100644
--- a/OvmfPkg/XenBusDxe/XenStore.c
+++ b/OvmfPkg/XenBusDxe/XenStore.c
@@ -738,7 +738,7 @@ XenStoreReadReply (
XENSTORE_STATUS Status;
Status = XenStoreProcessMessage ();
if (Status != XENSTORE_STATUS_SUCCESS && Status != XENSTORE_STATUS_EAGAIN)
{
- DEBUG ((EFI_D_ERROR, "XenStore, error while reading the ring (%d).",
+ DEBUG ((DEBUG_ERROR, "XenStore, error while reading the ring (%d).\n",
Status));
return Status;
}
@@ -1076,7 +1076,7 @@ XenStoreDeinit (
if (!IsListEmpty ()) {
XENSTORE_WATCH *Watch;
LIST_ENTRY *Entry;
-DEBUG ((EFI_D_WARN, "XenStore: RegisteredWatches is not empty, cleaning
up..."));
+DEBUG ((DEBUG_WARN, "XenStore: RegisteredWatches is not empty, cleaning
up...\n"));
Entry = GetFirstNode ();
while (!IsNull (, Entry)) {
Watch = XENSTORE_WATCH_FROM_LINK (Entry);
@@ -1092,7 +1092,7 @@ XenStoreDeinit (
//
if (!IsListEmpty ()) {
LIST_ENTRY *Entry;
-DEBUG ((EFI_D_WARN, "XenStore: WatchEvents is not empty, cleaning up..."));
+DEBUG ((DEBUG_WARN, "XenStore: WatchEvents is not empty, cleaning
up...\n"));
Entry = GetFirstNode ();
while (!IsNull (, Entry)) {
XENSTORE_MESSAGE *Message = XENSTORE_MESSAGE_FROM_LINK (Entry);
--
Anthony PERARD
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#47209): https://edk2.groups.io/g/devel/message/47209
Mute This Topic: https://groups.io/mt/34128010/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 03/11] OvmfPkg/XenBusDxe: Rework watch events reception
This patch rework the reception of xenstore watch event to avoid
allocation.
Instead of queuing watch events, we simply mark a XENSTORE_WATCH as
"triggered". We don't need to know how many time we received the
event, only that it happened. That avoid to allocate a
XENSTORE_MESSAGE for every watch events.
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2190
Signed-off-by: Anthony PERARD
---
OvmfPkg/XenBusDxe/XenStore.c | 125 ++-
1 file changed, 35 insertions(+), 90 deletions(-)
diff --git a/OvmfPkg/XenBusDxe/XenStore.c b/OvmfPkg/XenBusDxe/XenStore.c
index 727641a0fe..5cc900190a 100644
--- a/OvmfPkg/XenBusDxe/XenStore.c
+++ b/OvmfPkg/XenBusDxe/XenStore.c
@@ -65,6 +65,8 @@ struct _XENSTORE_WATCH
/* Path being watched. */
CHAR8 *Node;
+
+ BOOLEAN Triggered;
};
#define XENSTORE_WATCH_FROM_LINK(l) \
@@ -86,13 +88,6 @@ typedef struct {
struct {
CHAR8 *Body;
} Reply;
-
-/* Queued watch events. */
-struct {
- XENSTORE_WATCH *Handle;
- CONST CHAR8 **Vector;
- UINT32 VectorSize;
-} Watch;
} u;
} XENSTORE_MESSAGE;
#define XENSTORE_MESSAGE_FROM_LINK(r) \
@@ -133,14 +128,6 @@ typedef struct {
/** Lock protecting the registered watches list. */
EFI_LOCK RegisteredWatchesLock;
- /**
- * List of pending watch callback events.
- */
- LIST_ENTRY WatchEvents;
-
- /** Lock protecting the watch calback list. */
- EFI_LOCK WatchEventsLock;
-
/**
* The event channel for communicating with the
* XenStore service.
@@ -630,29 +617,32 @@ XenStoreProcessMessage (
Body[Message->Header.len] = '\0';
if (Message->Header.type == XS_WATCH_EVENT) {
-VOID *ConvertedToken;
+CONST CHAR8*WatchEventPath;
+CONST CHAR8*WatchEventToken;
+VOID *ConvertedToken;
+XENSTORE_WATCH *Watch;
-Message->u.Watch.Vector = Split(Body, Message->Header.len,
->u.Watch.VectorSize);
+//
+// Parse WATCH_EVENT messages
+// \0\0
+//
+WatchEventPath = Body;
+WatchEventToken = WatchEventPath + AsciiStrSize (WatchEventPath);
-ConvertedToken =
- (VOID *) AsciiStrHexToUintn (Message->u.Watch.Vector[XS_WATCH_TOKEN]);
+ConvertedToken = (VOID *) AsciiStrHexToUintn (WatchEventToken);
EfiAcquireLock ();
-Message->u.Watch.Handle = XenStoreFindWatch (ConvertedToken);
-DEBUG ((EFI_D_INFO, "XenStore: Watch event %a\n",
-Message->u.Watch.Vector[XS_WATCH_TOKEN]));
-if (Message->u.Watch.Handle != NULL) {
- EfiAcquireLock ();
- InsertHeadList (, >Link);
- EfiReleaseLock ();
+Watch = XenStoreFindWatch (ConvertedToken);
+DEBUG ((DEBUG_INFO, "XenStore: Watch event %a\n", WatchEventToken));
+if (Watch != NULL) {
+ Watch->Triggered = TRUE;
} else {
DEBUG ((EFI_D_WARN, "XenStore: Watch handle %a not found\n",
- Message->u.Watch.Vector[XS_WATCH_TOKEN]));
- FreePool((VOID*)Message->u.Watch.Vector);
- FreePool(Message);
+ WatchEventToken));
}
EfiReleaseLock ();
+FreePool (Message);
+FreePool (Body);
} else {
Message->u.Reply.Body = Body;
EfiAcquireLock ();
@@ -936,40 +926,29 @@ XenStoreUnwatch (
STATIC
XENSTORE_STATUS
XenStoreWaitWatch (
- VOID *Token
+ IN VOID *Token
)
{
- XENSTORE_MESSAGE *Message;
- LIST_ENTRY *Entry = NULL;
- LIST_ENTRY *Last = NULL;
+ XENSTORE_WATCH *Watch;
XENSTORE_STATUS Status;
+ EfiAcquireLock ();
+ Watch = XenStoreFindWatch (Token);
+ EfiReleaseLock ();
+ if (Watch == NULL) {
+return XENSTORE_STATUS_EINVAL;
+ }
+
while (TRUE) {
-EfiAcquireLock ();
-if (IsListEmpty () ||
-Last == GetFirstNode ()) {
- EfiReleaseLock ();
- Status = XenStoreProcessMessage ();
- if (Status != XENSTORE_STATUS_SUCCESS && Status !=
XENSTORE_STATUS_EAGAIN) {
-return Status;
- }
- continue;
+if (Watch->Triggered) {
+ Watch->Triggered = FALSE;
+ return XENSTORE_STATUS_SUCCESS;
}
-for (Entry = GetFirstNode ();
- Entry != Last && !IsNull (, Entry);
- Entry = GetNextNode (, Entry)) {
- Message = XENSTORE_MESSAGE_FROM_LINK (Entry);
- if (Message->u.Watch.Handle == Token) {
-RemoveEntryList (Entry);
-EfiReleaseLock ();
-FreePool((VOID*)Message->u.Watch.Vector);
-FreePool(Message);
-return XENSTORE_STATUS_SUCCESS;
- }
+Status = XenStoreProcessMessage ();
+if (Status != XENSTORE_STATUS_SUCCESS && Status != XENSTORE_STATUS_EAGAIN)
{
+ return Status;
}
-Last = GetFirstNode ();
-EfiReleaseLock ();
}
}
@@ -1052,12 +1031,10 @@ XenStoreInit (
xs.XenStore, xs.EventChannel));
InitializeListHead ();
- InitializeListHead ();
InitializeListHead ();
EfiInitializeLock (, TPL_NOTIFY);
EfiInitializeLock (, TPL_NOTIFY);
- EfiInitializeLock (, TPL_NOTIFY);
/* Initialize the
[edk2-devel] [PATCH 09/11] OvmfPkg/XenBusDxe: Fix NotifyExitBoot to avoid Memory Allocation Services
This patch fix the EVT_SIGNAL_EXIT_BOOT_SERVICES handler to avoid
using the Memory Allocation Services.
This comes with a new interface named RegisterExitCallback so that PV
drivers can disconnect from the backend before XenBusDxe is teared
down.
Instead of using Disconnect() to tear down the XenBus driver and the
children drivers, we are going to ask every driver using
XENBUS_PROTOCOL to disconnect from the hardware via the callback set
with RegisterExitCallback, then reset the xenstore shared ring and
the grant table.
Since there are no driver using RegisterExitCallback yet, no driver are
going to be disconnected. Linux can deal with that. And that will be
fixed by the next patch with a change for XenPvBlkDxe.
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2190
Signed-off-by: Anthony PERARD
---
OvmfPkg/Include/Protocol/XenBus.h | 35 +++
OvmfPkg/XenBusDxe/XenBus.c| 18
OvmfPkg/XenBusDxe/XenBusDxe.c | 27 +---
OvmfPkg/XenBusDxe/XenBusDxe.h | 2 ++
OvmfPkg/XenBusDxe/XenStore.c | 22 ++-
OvmfPkg/XenBusDxe/XenStore.h | 21 +++
6 files changed, 121 insertions(+), 4 deletions(-)
diff --git a/OvmfPkg/Include/Protocol/XenBus.h
b/OvmfPkg/Include/Protocol/XenBus.h
index c22bdfb368..04986747c8 100644
--- a/OvmfPkg/Include/Protocol/XenBus.h
+++ b/OvmfPkg/Include/Protocol/XenBus.h
@@ -373,6 +373,38 @@ XENSTORE_STATUS
IN VOID *Token
);
+typedef
+VOID
+(EFIAPI *XENBUS_EXIT_CALLBACK)(
+ IN VOID *Context
+ );
+
+/**
+ Register a function to be called during the ExitBootServices event.
+
+ NotifyFunction will be called when XenBusDxe is notified of
+ EVT_SIGNAL_EXIT_BOOT_SERVICES. The function should follow the same
+ requirements as if it as registered an event on
+ EVT_SIGNAL_EXIT_BOOT_SERVICES, i.e. no use of the Memory Allocation
+ Services.
+
+ To unregister the function, call RegisterExitCallback with
+ NotifyFunction=NULL.
+
+ @note There can only be one callback per driver.
+
+ @param This A pointer to the XENBUS_PROTOCOL.
+ @param NotifyFunction The function to be called.
+ @param NotifyContextA context.
+**/
+typedef
+VOID
+(EFIAPI *XENBUS_SET_EXIT_CALLBACK) (
+ IN XENBUS_PROTOCOL *This,
+ IN XENBUS_EXIT_CALLBACK NotifyFunction,
+ IN VOID *NotifyContext
+ );
+
///
/// Protocol structure
@@ -400,6 +432,9 @@ struct _XENBUS_PROTOCOL {
XENBUS_REGISTER_WATCH_BACKEND RegisterWatchBackend;
XENBUS_UNREGISTER_WATCH UnregisterWatch;
XENBUS_WAIT_FOR_WATCH WaitForWatch;
+
+ XENBUS_SET_EXIT_CALLBACK RegisterExitCallback;
+
//
// Protocol data fields
//
diff --git a/OvmfPkg/XenBusDxe/XenBus.c b/OvmfPkg/XenBusDxe/XenBus.c
index 78835ec7b3..54166505d2 100644
--- a/OvmfPkg/XenBusDxe/XenBus.c
+++ b/OvmfPkg/XenBusDxe/XenBus.c
@@ -343,6 +343,23 @@ XenBusSetState (
return Status;
}
+STATIC
+VOID
+EFIAPI
+XenBusRegisterExitCallback (
+ IN XENBUS_PROTOCOL *This,
+ IN XENBUS_EXIT_CALLBACK NotifyFunction,
+ IN VOID *NotifyContext
+ )
+{
+ XENBUS_PRIVATE_DATA *Dev;
+
+ Dev = XENBUS_PRIVATE_DATA_FROM_THIS (This);
+
+ Dev->ExitCallback = NotifyFunction;
+ Dev->ExitCallbackContext = NotifyContext;
+}
+
STATIC XENBUS_PRIVATE_DATA gXenBusPrivateData = {
XENBUS_PRIVATE_DATA_SIGNATURE,// Signature
{ NULL, NULL }, // Link
@@ -364,6 +381,7 @@ STATIC XENBUS_PRIVATE_DATA gXenBusPrivateData = {
XenBusRegisterWatchBackend, // XenBusIo.RegisterWatchBackend
XenBusUnregisterWatch, // XenBusIo.UnregisterWatch
XenBusWaitForWatch, // XenBusIo.WaitForWatch
+XenBusRegisterExitCallback, // XenBusIo.RegisterExitCallback
NULL, // XenBusIo.Type
0, // XenBusIo.DeviceId
diff --git a/OvmfPkg/XenBusDxe/XenBusDxe.c b/OvmfPkg/XenBusDxe/XenBusDxe.c
index 634c7b71eb..c71966a666 100644
--- a/OvmfPkg/XenBusDxe/XenBusDxe.c
+++ b/OvmfPkg/XenBusDxe/XenBusDxe.c
@@ -258,10 +258,31 @@ NotifyExitBoot (
IN VOID *Context
)
{
- XENBUS_DEVICE *Dev = Context;
+ XENBUS_DEVICE *Dev;
+ LIST_ENTRY *Entry;
+ XENBUS_PRIVATE_DATA *Child;
- gBS->DisconnectController(Dev->ControllerHandle,
-Dev->This->DriverBindingHandle, NULL);
+ Dev = Context;
+
+ //
+ // First, ask every driver using xenbus to disconnect without
+ // deallocating memory.
+ //
+ for (Entry = GetFirstNode (>ChildList);
+ !IsNodeAtEnd (>ChildList, Entry);
+ Entry = GetNextNode (>ChildList, Entry)) {
+Child = XENBUS_PRIVATE_DATA_FROM_LINK (Entry);
+if (Child->ExitCallback != NULL) {
+ Child->ExitCallback (Child->ExitCallbackContext);
+}
+ }
+
+ //
+ // Now, we can reset the devices used by XenBusDxe.
+ //
+ XenStoreResetWatches ();
+ XenStoreResetRing (Dev);
+
Re: [edk2-devel] [PATCH v3] ArmVirtPkg: increase FD/FV size for NOOPT builds
On Fri, 13 Sep 2019 at 13:44, Laszlo Ersek wrote:
>
> On 09/12/19 18:58, Ard Biesheuvel wrote:
> > After upgrading the CI system we use for building the ArmVirtPkg
> > targets, we started seeing failures due to the NOOPT build running
> > out of space when using the CLANG38 toolchain definition combined
> > with clang 7.
> >
> > We really don't want to increase the FD/FV sizes in general to
> > accommodate this, so parameterize the relevant quantities and
> > increase them by 50% for NOOPT builds.
> >
> > Signed-off-by: Ard Biesheuvel
> > ---
> > v3: - don't rely on fragile ordering of DEFINEs for the target dependent
> > default value, but instead, use a single FD_SIZE_IN_MB macro whose
> > default is DEFINEd either to 2 or 3 depend on the build target. That
> > permits switching back to 2 MB for NOOPT builds if desired while
> > changing the default to 3 MB
> > - fix a few image header definitions that I missed for ARM32 + Xen
> >
> > v2: implement suggestions by Laszlo on 1) how to parameterize this further,
> > and b) to avoid adding another .inc file
> > update kernel header field, as pointed out by Philippe
> >
> >
> > ArmVirtPkg/ArmVirt.dsc.inc | 15
> > ArmVirtPkg/ArmVirtQemu.fdf | 14 +---
> > ArmVirtPkg/ArmVirtQemuKernel.fdf | 24 +---
> > ArmVirtPkg/ArmVirtXen.fdf| 24 +---
> > 4 files changed, 68 insertions(+), 9 deletions(-)
> >
> > diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> > index a4ae25d982a2..10037c938eb8 100644
> > --- a/ArmVirtPkg/ArmVirt.dsc.inc
> > +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> > @@ -10,6 +10,21 @@
> > [Defines]
> >DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x804F
> >
> > +!if $(TARGET) != NOOPT
> > + DEFINE FD_SIZE_IN_MB= 2
> > +!else
> > + DEFINE FD_SIZE_IN_MB= 3
> > +!endif
> > +
> > +!if $(FD_SIZE_IN_MB) == 2
> > + DEFINE FD_SIZE = 0x20
> > + DEFINE FD_NUM_BLOCKS= 0x200
> > +!endif
> > +!if $(FD_SIZE_IN_MB) == 3
> > + DEFINE FD_SIZE = 0x30
> > + DEFINE FD_NUM_BLOCKS= 0x300
> > +!endif
> > +
> >
> > [BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
> >GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
> >
> > diff --git a/ArmVirtPkg/ArmVirtQemu.fdf b/ArmVirtPkg/ArmVirtQemu.fdf
> > index c2169cb7964b..2c8936a1ae15 100644
> > --- a/ArmVirtPkg/ArmVirtQemu.fdf
> > +++ b/ArmVirtPkg/ArmVirtQemu.fdf
> > @@ -20,14 +20,22 @@
> > #
> >
> >
> >
> > +[Defines]
> > +!if $(FD_SIZE_IN_MB) == 2
> > + DEFINE FVMAIN_COMPACT_SIZE = 0x1ff000
> > +!endif
> > +!if $(FD_SIZE_IN_MB) == 3
> > + DEFINE FVMAIN_COMPACT_SIZE = 0x2ff000
> > +!endif
> > +
> > [FD.QEMU_EFI]
> > BaseAddress = 0x|gArmTokenSpaceGuid.PcdFdBaseAddress # QEMU
> > assigns 0 - 0x800 for a BootROM
> > -Size = 0x0020|gArmTokenSpaceGuid.PcdFdSize # The size
> > in bytes of the FLASH Device
> > +Size = $(FD_SIZE)|gArmTokenSpaceGuid.PcdFdSize # The size
> > in bytes of the FLASH Device
> > ErasePolarity = 1
> >
> > # This one is tricky, it must be: BlockSize * NumBlocks = Size
> > BlockSize = 0x1000
> > -NumBlocks = 0x200
> > +NumBlocks = $(FD_NUM_BLOCKS)
> >
> >
> >
> > #
> > @@ -59,7 +67,7 @@ DATA = {
> > !endif
> > }
> >
> > -0x1000|0x001ff000
> > +0x1000|$(FVMAIN_COMPACT_SIZE)
> > gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
> > FV = FVMAIN_COMPACT
> >
> > diff --git a/ArmVirtPkg/ArmVirtQemuKernel.fdf
> > b/ArmVirtPkg/ArmVirtQemuKernel.fdf
> > index f675b6d65ee1..72fc8dd698f8 100644
> > --- a/ArmVirtPkg/ArmVirtQemuKernel.fdf
> > +++ b/ArmVirtPkg/ArmVirtQemuKernel.fdf
> > @@ -20,14 +20,22 @@
> > #
> >
> >
> >
> > +[Defines]
> > +!if $(FD_SIZE_IN_MB) == 2
> > + DEFINE FVMAIN_COMPACT_SIZE = 0x1f8000
> > +!endif
> > +!if $(FD_SIZE_IN_MB) == 3
> > + DEFINE FVMAIN_COMPACT_SIZE = 0x2f8000
> > +!endif
> > +
> > [FD.QEMU_EFI]
> > BaseAddress = 0x|gArmTokenSpaceGuid.PcdFdBaseAddress # QEMU
> > assigns 0 - 0x800 for a BootROM
> > -Size = 0x0020|gArmTokenSpaceGuid.PcdFdSize # The size
> > in bytes of the FLASH Device
> > +Size = $(FD_SIZE)|gArmTokenSpaceGuid.PcdFdSize # The size
> > in bytes of the FLASH Device
> > ErasePolarity = 1
> >
> > # This one is tricky, it must be: BlockSize * NumBlocks = Size
> > BlockSize = 0x1000
> > -NumBlocks = 0x200
> > +NumBlocks = $(FD_NUM_BLOCKS)
> >
> >
> >
> > #
> > @@ -56,7 +64,12 @@ DATA = {
>
Re: [edk2-devel] [PATCH v3] ArmVirtPkg: increase FD/FV size for NOOPT builds
On 09/12/19 18:58, Ard Biesheuvel wrote:
> After upgrading the CI system we use for building the ArmVirtPkg
> targets, we started seeing failures due to the NOOPT build running
> out of space when using the CLANG38 toolchain definition combined
> with clang 7.
>
> We really don't want to increase the FD/FV sizes in general to
> accommodate this, so parameterize the relevant quantities and
> increase them by 50% for NOOPT builds.
>
> Signed-off-by: Ard Biesheuvel
> ---
> v3: - don't rely on fragile ordering of DEFINEs for the target dependent
> default value, but instead, use a single FD_SIZE_IN_MB macro whose
> default is DEFINEd either to 2 or 3 depend on the build target. That
> permits switching back to 2 MB for NOOPT builds if desired while
> changing the default to 3 MB
> - fix a few image header definitions that I missed for ARM32 + Xen
>
> v2: implement suggestions by Laszlo on 1) how to parameterize this further,
> and b) to avoid adding another .inc file
> update kernel header field, as pointed out by Philippe
>
>
> ArmVirtPkg/ArmVirt.dsc.inc | 15
> ArmVirtPkg/ArmVirtQemu.fdf | 14 +---
> ArmVirtPkg/ArmVirtQemuKernel.fdf | 24 +---
> ArmVirtPkg/ArmVirtXen.fdf| 24 +---
> 4 files changed, 68 insertions(+), 9 deletions(-)
>
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index a4ae25d982a2..10037c938eb8 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -10,6 +10,21 @@
> [Defines]
>DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x804F
>
> +!if $(TARGET) != NOOPT
> + DEFINE FD_SIZE_IN_MB= 2
> +!else
> + DEFINE FD_SIZE_IN_MB= 3
> +!endif
> +
> +!if $(FD_SIZE_IN_MB) == 2
> + DEFINE FD_SIZE = 0x20
> + DEFINE FD_NUM_BLOCKS= 0x200
> +!endif
> +!if $(FD_SIZE_IN_MB) == 3
> + DEFINE FD_SIZE = 0x30
> + DEFINE FD_NUM_BLOCKS= 0x300
> +!endif
> +
>
> [BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
>GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>
> diff --git a/ArmVirtPkg/ArmVirtQemu.fdf b/ArmVirtPkg/ArmVirtQemu.fdf
> index c2169cb7964b..2c8936a1ae15 100644
> --- a/ArmVirtPkg/ArmVirtQemu.fdf
> +++ b/ArmVirtPkg/ArmVirtQemu.fdf
> @@ -20,14 +20,22 @@
> #
>
>
>
> +[Defines]
> +!if $(FD_SIZE_IN_MB) == 2
> + DEFINE FVMAIN_COMPACT_SIZE = 0x1ff000
> +!endif
> +!if $(FD_SIZE_IN_MB) == 3
> + DEFINE FVMAIN_COMPACT_SIZE = 0x2ff000
> +!endif
> +
> [FD.QEMU_EFI]
> BaseAddress = 0x|gArmTokenSpaceGuid.PcdFdBaseAddress # QEMU
> assigns 0 - 0x800 for a BootROM
> -Size = 0x0020|gArmTokenSpaceGuid.PcdFdSize # The size
> in bytes of the FLASH Device
> +Size = $(FD_SIZE)|gArmTokenSpaceGuid.PcdFdSize # The size
> in bytes of the FLASH Device
> ErasePolarity = 1
>
> # This one is tricky, it must be: BlockSize * NumBlocks = Size
> BlockSize = 0x1000
> -NumBlocks = 0x200
> +NumBlocks = $(FD_NUM_BLOCKS)
>
>
>
> #
> @@ -59,7 +67,7 @@ DATA = {
> !endif
> }
>
> -0x1000|0x001ff000
> +0x1000|$(FVMAIN_COMPACT_SIZE)
> gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
> FV = FVMAIN_COMPACT
>
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.fdf
> b/ArmVirtPkg/ArmVirtQemuKernel.fdf
> index f675b6d65ee1..72fc8dd698f8 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.fdf
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.fdf
> @@ -20,14 +20,22 @@
> #
>
>
>
> +[Defines]
> +!if $(FD_SIZE_IN_MB) == 2
> + DEFINE FVMAIN_COMPACT_SIZE = 0x1f8000
> +!endif
> +!if $(FD_SIZE_IN_MB) == 3
> + DEFINE FVMAIN_COMPACT_SIZE = 0x2f8000
> +!endif
> +
> [FD.QEMU_EFI]
> BaseAddress = 0x|gArmTokenSpaceGuid.PcdFdBaseAddress # QEMU
> assigns 0 - 0x800 for a BootROM
> -Size = 0x0020|gArmTokenSpaceGuid.PcdFdSize # The size
> in bytes of the FLASH Device
> +Size = $(FD_SIZE)|gArmTokenSpaceGuid.PcdFdSize # The size
> in bytes of the FLASH Device
> ErasePolarity = 1
>
> # This one is tricky, it must be: BlockSize * NumBlocks = Size
> BlockSize = 0x1000
> -NumBlocks = 0x200
> +NumBlocks = $(FD_NUM_BLOCKS)
>
>
>
> #
> @@ -56,7 +64,12 @@ DATA = {
>0x01, 0x00, 0x00, 0x10, # code0: adr x1, .
>0xff, 0x1f, 0x00, 0x14, # code1: b 0x8000
>0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, # text_offset: 512 KB
> +!if $(FD_SIZE_IN_MB) == 2
>0x00, 0x00, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, #
