Re: [edk2-devel] HII Keyword utility
I like the idea; it sounds useful for automation. Although more and more of that is moving to Redfish now. No idea where it should go, though. *Brian J. Johnson *Enterprise X86 Lab Hewlett Packard Enterprise *From:* Jeff Brasen via groups.io [mailto:jbrasen=nvidia@groups.io] *Sent:* Monday, May 13, 2024 at 11:27 AM *To:* devel@edk2.groups.io *Subject:* [edk2-devel] HII Keyword utility I have a UEFI shell application that can set/get UEFI x-UEFI HII Keyword values. edk2-nvidia/Silicon/NVIDIA/Application/HiiKeywordUtil at main · NVIDIA/edk2-nvidia (github.com) <https://github.com/NVIDIA/edk2-nvidia/tree/main/Silicon/NVIDIA/Application/HiiKeywordUtil> I have used this to configure iSCSI from the shell. Is this something that is desired to be in the main edk2 repo? If so, what package should this go under? If this is generally useful I'll do some cleanup and make a push request after the stable202405 release is out. -Jeff Brasen -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118928): https://edk2.groups.io/g/devel/message/118928 Mute This Topic: https://groups.io/mt/106076569/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024
On 5/3/24 12:38, Pedro Falcato wrote: On Thu, May 2, 2024 at 7:17 PM Kinney, Michael D wrote: -Original Message- From: r...@edk2.groups.io On Behalf Of Pedro Falcato Sent: Thursday, May 2, 2024 10:51 AM To: devel@edk2.groups.io; Kinney, Michael D Cc: r...@edk2.groups.io; Leif Lindholm ; Andrew Fish (af...@apple.com) Subject: Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024 On Wed, May 1, 2024 at 6:44 PM Michael D Kinney via groups.io wrote: * All contributors, maintainers, and reviewers must have GitHub IDs. * The commit message would no longer require Cc:, Reviewed-by:, Acked- by: or Tested-by: tags. The only required tag would be Signed-off-by. I'd just like to note that losing the CC:, Reviewed-by:, etc is a big loss. Gerrit auto-adds Rb's, github PR's do not (I'd guess there's a way to pull that off with github actions, but I haven't looked). It'll be a mess if I have to go through online GH PR backlogs just to find who to CC/add-to-review. It kills the decentralized bit off of git too :) Can you provide more details on the impact of the loss? In my view, commits should be fairly self-describing. What changes, why, are obvious, but who looked at it, who reviewed it, who was cc'd but didn't respond, who tested are also pretty important. Git is supposed to be decentralized, let's not forget. If we ever migrate from GH, if GH ever goes down, if the links ever go down, you'll never be able to know who looked at it. If you're looking at an EDK2 commit deep into an Intel-internal fork, you won't know what "PR #478" is (heck, rebase-and-merge doesn't reference PRs either). Well said. That's my concern as well: TianoCore won't use GitHub forever, and any GitHub metadata (PR numbers, GitHub IDs, bug numbers, etc.) will become meaningless once we change. Never mind that the code can be disassociated from the metadata simply by forking to a new repository, as Pedro said Side-note: How are we supposed to find the PR for a given commit? Searching doesn't seem to work well. For instance, I picked a random non-trivial commit out of the current open PRs: MdeModulePkg/Bus/Spi/SpiBus: Adding SpiBus Drivers. https://github.com/tianocore/edk2/pulls?q=is%3Apr+is%3Aopen+MdeModulePkg%2FBus%2FSpi%2FSpiBus%3A+Adding+SpiBus+Drivers has no matches? I am curious how other GitHub projects handle this topic. I see it I don't think they do, sadly. But I also don't know many people with a positive opinion on GH PRs :) Yeah... my opinions are decidedly mixed. They are convenient, but have some serious gaps around archiving, auditing, and versioning of review requests. They don't even let you review the commit messages (one of their most serious flaws!) It is sad that we're moving to PRs after I finally got a nice and sane(ish!) email workflow (openfw.io + b4). Otherwise, no objections, it's better than edk2.git's half-email half-PR frankenprocess. I'd guess this change only encompasses edk2.git? How about the other repos? Any timeline for those? The plan is to apply this to all repos, one at a time. Need to get the revised process documented and working in one repo before applying to all. Gotcha, thanks! -- Brian J. Johnson Hewlett Packard Enterprise -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118826): https://edk2.groups.io/g/devel/message/118826 Mute This Topic: https://groups.io/mt/105873467/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024
On 5/1/24 18:19, Dionna Glaze via groups.io wrote: On Wed, May 1, 2024 at 11:12 AM Leif Lindholm via groups.io wrote: On 2024-05-01 18:43, Michael D Kinney wrote: Hello, I would like to propose that TianoCore move all code review from email based code reviews to GitHub Pull Requests based code reviews. The proposed date to switch would be immediately after the next stable tag which is currently scheduled for May 24, 2024. Thanks Mike. I'm in favour of this change, and the date. I still want us to try to figure out how to retain review history beyond what github decides we need, but I don't think it justifies indefinitely delaying the switchover. And frankly, it will be easier to experiment with what works and not after the switch. +1. UI-based interactions don't export well for archival-permalinking reasons, and Github archive behavior is for repositories only, not the reviews. But yes, wouldn't want to delay for a bot to echo conversations to devel@edk2.groups.io or some other solution. +1 from me as well. We need to maintain review history in some fairly permanent manner, both the reviewed code and review comments. / Leif Updates to the following Wiki page would be required to describe the required process when using GitHub Pull Requests for all code review related activity. https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process A couple examples of the changes that would need to be documented are: * All contributors, maintainers, and reviewers must have GitHub IDs. It looks like this is already resolved for the existing Maintainers.txt with the `[username]` syntax, but I don't see any explanation of that expectation. Seems fine to me. * The commit message would no longer require Cc:, Reviewed-by:, Acked-by: or Tested-by: tags. The only required tag would be Signed-off-by. Would those tags be optional? Test and ack info can be helpful when researching a change, to find people who may be knowledgeable about it. Similarly, the Reviewed-by info is nice to have in the history, without having to dig it out of archives. But it's a bit awkward to add on github: you have to push new commits with the Reviewed-by tags, but that changes the SHAs, so it's not obvious that the commits you're merging have the same code as the ones which were reviewed. For the email flow, we trust maintainers to get this right. For the github flow, are we deciding to rely exclusively on the PR archives? What if a maintainer decides to tweak a commit before merging it, eg. to fix a trivial typo? With the email flow they just go ahead and do it. With the github flow, would they need to post another PR, so it could make it through the process and be merged? * The Pull Request submitter is required to invite the required maintainers and reviewers to the pull request. This is the same set of maintainers and reviewers that are required to be listed in Cc: tags in today's process. This is not configured on tianocore/edk2 at the moment. I have no way to invite a reviewer. Is this a planned fix? * Maintainers are responsible for verifying that all conversations in the code review are resolved and that all review approvals from the required set of maintainers are present before setting the 'push' label. Will there be documentation on how to use the conversation resolution feature? It's unclear to me whether the PR owner or the reviewer is responsible for marking a conversation "resolved." Please provide feedback 1) If you are not in favor of this change. 2) If you are not in favor of the proposed date of this change. 3) On the process changes you would like to see documented in the Wiki pages related to using GitHub Pull Request based code reviews. There is some prototype work to automate/simplify some of the PR based code review process steps. Those could be added over time as resources are available to finish and support them. Best regards, Mike -- Brian "The answers we have found only serve to raise a whole set of new questions. In some ways we feel we are as confused as ever, but we are confused on a higher level and about more important things." -- Anonymous -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118524): https://edk2.groups.io/g/devel/message/118524 Mute This Topic: https://groups.io/mt/105847510/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should not be set by default in CR0
On 1/18/24 09:46, Gerd Hoffmann wrote: On Wed, Jan 10, 2024 at 04:43:47PM +, West, Catharine wrote: Disabling cache by default results in violation of BTG protections (if BTG enabled). BIOS cannot assume that cache is disabled before it executes as ACM may be required to enable NEM. Whatever solution needs to be done here cannot evict ACM-enabled NEM. Well, it's OVMF in a virtual machine. No boot guard involved. So we could probably go for a OVMF-specific patch here. But I'd prefer to figure what exactly is happening here before going down that route. An extreme slowdown just because we flip that bit doesn't make sense to me. Why is boot time increasing? Not clear. It seems to be the lzma uncompress of the firmware volume in rom / pflash which is very slow. Also it is apparently only triggered in case pci device assignment is used. I've seen extreme slowness on physical platforms when we've mixed up the MTRRs or page tables, causing code to be mapped uncached. Lzma uncompress of ROM could be pretty slow as well, if the ROM is being read uncached. Lzma probably reads the data a byte at a time, which is the worst case for uncached accesses. Since this is a VM, it's not actually uncached at the hardware level, but I don't know how QEMU/KVM handles uncached guest mappings It may be doing a VMEXIT for every byte. Anyway, I suggest double-checking your page tables and MTRRs. -- Brian J. Johnson Enterprise X86 Lab Hewlett Packard Enterprise -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114145): https://edk2.groups.io/g/devel/message/114145 Mute This Topic: https://groups.io/mt/100367559/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg: Initialise serial port early in StandaloneMmEntryPoint
On 1/10/24 11:16, levi.yun wrote: Hi, Laszlo. This will keep initing the serial port upon every API call until the global variable becomes writeable, and then the next API call will init the serial port for one last time, and also prevent further page table checks. The CheckWritable() function is an implementation detail. In the DXE phase, it could be implemented (I think?) with the GetMemorySpaceDescriptor() DXE service, or perhaps even the EFI_MEMORY_ATTRIBUTE_PROTOCOL.GetMemoryAttributes() UEFI protocol member function. In the standalone MM core, CheckWritable() could walk the page tables explicitly. The idea is, either way, to *predict* whether writing to "mInitialized" would trap. >> I think it wouldn't proper, to DxeCore and StMM too. IIUC, before CoreInitializeMemoryServices is called, we couldn't use that method in case DxeCore. And the problem now I face is also StMM before populating memory information (done in LibConstructor). Now I think that speculative / out of order execution could actually trigger the trap *before* GlobalsWriteable is calculated; however, I think such a trap should be architecturally hidden (i.e., invisible). I think at worst we could need a compiler barrier (maybe throw in some "volatile" for GlobalsWriteable and mInitialized), so that the *compiler* not try to reorder the accesses. But even that sounds like a stretch. Agree if we develop CheckPerm?? Currently, (In my narrow thinking) there is no more generic solution than create new interface SerialPortInitilizeEarly. Am I missing? Many Thanks. Sincerely, Levi. Levi, FWIW I prefer your original approach: explicitly call SerialPortInitialize() early enough that you don't need to worry about output occurring before that point. Then you can also use a SerialPortLib instance which assumes that this has already been done and doesn't try to re-initialize the port, which saves some overhead. Constructors in DebugLib, SerialPortLib, and other very low-level libraries are problematic due to the issue you ran in to, so it seems best to just avoid them altogether. Ard didn't want a SerialPortInitialize() call directly in the all-platform StandaloneMmCore _ModuleEntryPoint() function, which is understandable. So perhaps you could either: 1. Propose a platform-specific callout at that point and a library class to implement it, with an empty instance for general use and your own platform-specific instance which calls SerialPortInitialize(). or 2. Write your own platform-specific version of StandaloneMmCoreEntryPoint. -- Brian J. Johnson Enterprise X86 Lab Hewlett Packard Enterprise -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113540): https://edk2.groups.io/g/devel/message/113540 Mute This Topic: https://groups.io/mt/103540969/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] question about cxl device enumeration in pci bus driver
I see a long discussion on lore.kernel.org: https://lore.kernel.org/linux-cxl/byapr12mb3336f5addedbb6245bc2b48dbd...@byapr12mb3336.namprd12.prod.outlook.com/T/ This document also has some info on how CXL hotplug is handled: https://cdrdv2-public.intel.com/643805/643805_CXL%20Memory%20Device%20SW%20Guide_Rev1p0.pdf *Brian J. Johnson *Enterprise X86 Lab Hewlett Packard Enterprise *From:* Yoshinoya [mailto:yoshinoyat...@163.com] *Sent:* Monday, November 6, 2023 at 5:20 AM *To:* devel@edk2.groups.io *Cc:* jonathan.came...@huawei.com, Laszlo Ersek , kra...@redhat.com, Ni, Ray , Sayanta Pattanayak *Subject:* [edk2-devel] question about cxl device enumeration in pci bus driver Hi, I have a question about cxl memory device hot-plug. For example: 1. When cold boot, the platform has only 512GB cxl type-3 memory 2. During OS runtime, user hot-plug another cxl type-3 memory device expanding to 1TB. So, how did OS identify another 512GB space newly added without a reboot? Could anyone help to explain the procedure draftly? Thanks 在 2023-10-27 09:29:31，"Yoshinoya" 写道： Hi, Thanks for reply! I download code from this git https://github.com/SayantaP-arm/edk2-platforms/ For this ARM edk2 sample package, it provided cxldxe driver which being executed after UEFI DXE PciBus enumeration finishes. This ppt (https://lpc.events/event/16/contributions/1254/ <https://lpc.events/event/16/contributions/1254/>) describes good. Intel also provied a CXL Type 3 memory device software guide. This guide also describes system firmware boot sequence and uefi boot sequence. But i could not match these describes with standard UEFI BIOS Boot flow, such as dxe phase's standard pci enumeration driver. It sees needing add some cxl discovery code into dxe pci bus driver. Thanks At 2023-10-26 21:35:38, "Jonathan Cameron via groups.io" wrote: >On Thu, 26 Oct 2023 11:49:28 +0200 >"Laszlo Ersek" wrote: > >> On 10/26/23 10:33, Gerd Hoffmann wrote: >> > On Thu, Oct 26, 2023 at 10:36:35AM +0800, Yoshinoya wrote: >> > >> >> CXL Host Bridge / Root Port / Switch / Device enumeration / HDM Config, maybe could be integrated into pci drivers stack. >> > >> > Point being? Can or should the firmware do anything useful with >> > the CXL hardware? If so, what exactly and why? >> > >> > Current state of affairs is that the PCI stack does the usual PCI >> > initialization (enumerate, assign resources to PCI bars) and leaves >> > everything else to the OS. >> >> (I don't know what "HDM Config" stands for.) >> >> The only utility for driving CXL devices from the firmware could be, AFAICT: >> >> - booting off of such a device (or at least "supporting OS boot" in some >> manner) >> >> - using such a device for UEFI console purposes > >There are different models for how to use CXL devices and what's possible depends on the >version of CXL. CXL 1.1 wasn't great for standards defined discovery, so >EDK2 platform logic basically has to do everything. > >The one mostly expected for early CXL servers, for backwards compatibility, makes >setting up the CXL memory decoders (Host managed Device Memory - HDM) in all the >components in the path to memory + locking them down an EDK2 problem. They are then >presented in the memory map and in SRAT, HMAT etc the same as normal DDR memory. >Idea being that an old OS will be fine with that and doesn't have to be CXL aware >at all. Note this also involves walking the CDAT tables via DOE mailboxes in PCI >config space to get the magic numbers needed to compute HMAT. > >The other model is to do very little in EDK2 and make entirely a problem for the OS. >The logic is necessary anyway if you want to support hotplug etc, so use it for the >cold plug paths 2. That's all we've currently supported on QEMU. > >There was a presentation at Linux Plumbers last year on some out of tree support >from ARM for doing the setup on a CXL 2.0 platform (I think) in EDK2 >https://lpc.events/event/16/contributions/1254/ <https://lpc.events/event/16/contributions/1254/> >But I guess it never went upstream. >https://github.com/Saya
Re: 回复: [edk2-devel] [PATCH 00/29] CryptoPkg: Update OpenSSL submodule to 3.0.9
Liming and Jiewen, I asked Terry Lee to give this patchset a spin, and it seemed to work well for our h/w use case. The size increase vs. OpenSSL 1.1.1n was noticeable, but workable. We're good with merging it. Brian J. Johnson HP Enterprise Misison-Critical Systems Original Message From: gaoliming via groups.io [mailto:gaoliming=byosoft.com...@groups.io] Sent: Friday, August 4, 2023 at 4:44 AM To: , , 'Kinney, Michael D' , 'Li, Yi1' , 'Andrew Fish' Cc: 'Yao, Jiewen' , 'Lu, Xiaoyu1' , 'Jiang, Guomin' , 'Gerd Hoffmann' , 'Ard Biesheuvel' Subject: 回复: [edk2-devel] [PATCH 00/29] CryptoPkg: Update OpenSSL submodule to 3.0.9 Hi, all We are near to the soft feature freeze for the stable tag 202308. Please give your opinion for this patch set to catch 202308 release. Now, Ard, Jiewen (Crypto Package Maintainer), Yi (Patch Contributor) opinion is to merge this patch set if no other comments in one week. Leif opinion is to agree with the package maintainer. All changes of this patch set are in CryptoPkg. So, I also agree with CryptoPkg maintainer. Thanks Liming -邮件原件- 发件人: devel@edk2.groups.io 代表 Leif Lindholm 发送时间: 2023年8月3日 2:46 收件人: Kinney, Michael D ; devel@edk2.groups.io; Gao, Liming ; Li, Yi1 抄送: Yao, Jiewen ; Lu, Xiaoyu1 ; Jiang, Guomin ; 'Gerd Hoffmann' ; Andrew Fish (af...@apple.com) 主题: Re: [edk2-devel] [PATCH 00/29] CryptoPkg: Update OpenSSL submodule to 3.0.9 I am a little bit nervous about introducing this massive change so late in the cycle, and am not sure whether any deferral of the soft freeze would be sufficient to change that. My preference would be having this introduced right after the stable tag, giving it a full cycle of enforced testing before the next stable tag. *But* I'm probably less vested in that outcome than some others, and am happy to leave the call to the CryptoPkg (and other affected) maintainers. / Leif -Original Message- From: Kinney, Michael D Sent: Wednesday, August 2, 2023 5:42 PM To: devel@edk2.groups.io; Gao, Liming ; Li, Yi1 Cc: Yao, Jiewen ; Lu, Xiaoyu1 ; Jiang, Guomin ; 'Gerd Hoffmann' ; Andrew Fish (af...@apple.com) ; Leif Lindholm ; Kinney, Michael D Subject: RE: [edk2-devel] [PATCH 00/29] CryptoPkg: Update OpenSSL submodule to 3.0.9 WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros. Hi Liming, I have added Andrew and Leif to this thread so we can discuss if the release date need to be adjusted. Mike -Original Message- From: devel@edk2.groups.io On Behalf Of gaoliming via groups.io Sent: Wednesday, August 2, 2023 3:07 AM To: devel@edk2.groups.io; Li, Yi1 Cc: Yao, Jiewen ; Lu, Xiaoyu1 ; Jiang, Guomin ; 'Gerd Hoffmann' Subject: 回复: [edk2-devel] [PATCH 00/29] CryptoPkg: Update OpenSSL submodule to 3.0.9 Yi: Thanks for your great work to update openssl 3.0. The commit message shows this patch set must catch edk2 202308 stable tag. Right? Edk2 202308 stable tag will start soft feature free from Aug 7th (next Monday). That means this patch set needs to pass code review in one week. Jiwen, Gerd: Can you give your comments for this patch set this week? If you need more time, I will raise the request to defer the soft feature freeze. Thanks Liming -邮件原件- 发件人: devel@edk2.groups.io 代表 Li, Yi 发送时间: 2023年7月28日 14:40 收件人: devel@edk2.groups.io 抄送: Yi Li ; Jiewen Yao ; Xiaoyu Lu ; Guomin Jiang ; Gerd Hoffmann 主题: [edk2-devel] [PATCH 00/29] CryptoPkg: Update OpenSSL submodule to 3.0.9 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3466 According to https://www.OpenSSL.org/policies/releasestrat.html , OpenSSL Version 1.1.1 will be supported until 2023-09-11 (LTS). Need to upgrade OpenSsl to 3.0.9 before 1.1.1 support stopping. PR： https://github.com/tianocore/edk2/pull/4692 Cc: Jiewen Yao Cc: Xiaoyu Lu Cc: Guomin Jiang Cc: Gerd Hoffmann Gerd Hoffmann (15): CryptoPkg/openssl: update submodule to openssl-3.0.9 CryptoPkg/openssl: cleanup all openssl1.1.1 generated files and code CryptoPkg/openssl: update Openssl*.inf files for openssl 3.0 CryptoPkg/openssl: add openssl3 configure scripts CryptoPkg/openssl: UefiAsm.conf update for openssl 3.0 CryptoPkg/BaseCryptLib: no openssl deprecation warnings please CryptoPkg/BaseCryptLib: adapt CryptSm3.c to openssl 3.0 changes. CryptoPkg/BaseCryptLib: drop BIO_* dummy functions CryptoPkg/TlsLib: ERR_GET_FUNC is gone CryptoPkg/openssl: adapt rand_pool.c to openssl 3.0 changes CryptoPkg/openssl: move compiler_flags to buildinf.c CryptoPkg/openssl: store dummy update for openssl 3.0 CryptoPkg/openssl: adapt EcSm2Null.c for openssl 3.0 CryptoPkg/TlsLib: use unsigned long for ErrorCode CryptoPkg/openssl: update CI config for openssl 3.0 Yi Li (14): CryptoPkg: Move all UEFI implement of openssl to OpensslStub CryptoPkg: use UEFI provider as default CryptoPkg: adapt 3.0
Re: [edk2-devel] [Help] in Setting up EFI Shell in QEMU to allow for HTTP Requests
Are you sure you have the HttpDxe driver in your OVMF build? Looks like you'd need to build with either -D NETWORK_HTTP_ENABLE or -D NETWORK_HTTP_BOOT_ENABLE. See https://github.com/tianocore/edk2/blob/master/OvmfPkg/README for details on HTTPS. *Brian J. Johnson *Hewlett Packard Enterprise ** *From:* CrossedCarpet [mailto:crossedcar...@hotmail.com] *Sent:* Monday, February 13, 2023 at 2:47 AM *To:* devel@edk2.groups.io *Subject:* [edk2-devel] [Help] in Setting up EFI Shell in QEMU to allow for HTTP Requests Greetings, I want to create an UEFI App with internet connection. Before I start developing the code, I wanted to make sure my setup is working properly so that the app can make http requests. To that end, I spent the last days trying to make the `http` command work in the EFI Shell launched inside QEMU. I can get the ping command to work properly, but calling: http httpbin.org/get Always returns: Unable to open http protocol on `eth0` - Unsupported Unable to download the file `/get` on `eth0` - Unsupported This is my startup.nsh script to configure the interface: connect ifconfig -r eth0 ifconfig -s eth0 dhcp ifconfig -l eth0 These were my different attempts at invoking Qemu properly: -netdev user,id=mynet0,hostfwd=tcp::8080-:80 -device e1000,netdev=mynet0 \ -netdev user,id=user.0 -device e1000,netdev=user.0 \ -nic user,ipv6=off,model=e1000,mac=52:54:98:76:54:32 \ And following this guide <https://gist.github.com/extremecoders-re/e8fd8a67a515fee0c873dcafc81d811c> I tried to setup a tap, but no dice: -netdev tap,id=mynet0,ifname=tap0,script=no,downscript=no -device e1000,netdev=mynet0,mac=52:55:00:d1:55:01 \ Have you ever achieved this? What was your setup like? Thank you for your time. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100329): https://edk2.groups.io/g/devel/message/100329 Mute This Topic: https://groups.io/mt/96939495/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 1/5] UefiCpuPkg/SmmBaseHob.h: Add SMM Base HOB Data
On 2/2/23 06:51, Gerd Hoffmann wrote:
Hi,
- With relatively many elements fitting into a single HOB, on most
platforms, just one HOB is going to be used. While that may be good for
performance, it is not good for code coverage (testing). The quirky
indexing method will not be exercised by most platforms.
TRUE so I propose that the first version of the code change only expects
the HOB.NumberOfCpus equals to the NumberOfCpus returned from MP
service, meaning the code logic only supports single instance of the HOB.
When a platform that contains >8000 cpu threads resulting in multiple HOBs
produced, the expectation will break and remind us that the CpuSmm driver
needs to update to handle multiple HOBs.
Given that this is already the second case where we hit the 64k size
limit and I expect it will not be the last one: I think it makes sense
to introduce a generic and reusable concept of chunked HOBs, so you can
add helper functions to HobLib for splitting and reassembling, with a
struct along the lines of:
typedef struct {
// offset and size of this particular chunk
UINT32 ChunkOffset;
UINT32 ChunkSize;
// number of chunks and size of all chunks combined.
UINT32 ChunkCount;
UINT32 TotalSize;
// chunk data
UINT8 Data[0];
} EFI_HOB_CHUNK;
take care,
Gerd
Gerd's suggestion could be handy. Here's another approach when data is
too large to fit in a HOB, which doesn't require splitting up the data:
PEI tracks page allocations by generating memory allocation HOBs
(EFI_HOB_TYPE_MEMORY_ALLOCATION.) The EFI_HOB_MEMORY_ALLOCATION_HEADER
structure in the HOB contains a "Name" field of type EFI_GUID which can
be used to track the purpose of that particular page allocation. It's
zeroed by BuildMemoryAllocationHob(), and not usually used. But if you
put your own GUID in there, you can use it to track which memory
allocation HOB corresponds to your data, without having to manage a
separate HOB with a pointer. The allocation will be automatically
tracked through pre-RAM PEI, post-RAM PEI, and DXE, and the pages
(although not the HOB) will even persist into Runtime (if you use an
EfiRuntimeServices memory type.)
That wouldn't help the OP with SMM, though. They would still have to
copy the pages into SMRAM somehow.
Unfortunately, neither HobLib nor AllocatePages() has an interface for
setting the "Name" field. But you can call AllocatePages(), then search
the HOB list for the resulting HOB, and update it's AllocDescriptor.Name
field.
--
Brian J. Johnson
Enterprise X86 Lab
Hewlett Packard Enterprise
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99504): https://edk2.groups.io/g/devel/message/99504
Mute This Topic: https://groups.io/mt/96350760/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] edk2setup.sh shortcomings
> 1) It exports PYTHONHASHSEED=1 (needed?); Setting PYTHONHASHSEED causes python hashes to be iterated in a deterministic order. The autogen tools use hashes internally, so setting PYTHONHASHSEED to a fixed value causes them to produce identical output each time they are run with identical input. That helps avoid unnecessary rebuilds of code which hasn't actually changed. It affects items like the order of dependencies in the generated makefiles. Setting PYTHONHASHSEED knocked several minutes off of the rebuild time for one of our platforms. Recommended. Brian J. Johnson HPE Original Message From: tlaro...@polynum.com [mailto:tlaro...@polynum.com] Sent: Thursday, February 2, 2023 at 11:59 AM To: Gerd Hoffmann Cc: devel@edk2.groups.io, Andrew Fish , Leif Lindholm , Michael D Kinney Subject: [edk2-devel] edk2setup.sh shortcomings Le Thu, Feb 02, 2023 at 05:50:32PM +0100, Gerd Hoffmann a écrit : On Thu, Feb 02, 2023 at 12:29:32PM +0100, tlaro...@polynum.com wrote: edk2setup.sh has shortcomings. To list some: - The functions return a status but it is not tested; hence the script goes to the end with a final "return $?" that simply returns the status of the last command that is "unset" which always successfully unsets, even a not set variable. Hence a script can not catch a failure by testing the end status that is always 0; - If WORKSPACE is set, --reconfig does nothing; - If EDK_TOOLS_PATH and PACKAGES_PATH are set, even to incorrect values, the script succeeds even if BaseTools/ is not found anywhere; - The comments are obsolete (1): bash(1) is required because the syntax is not POSIX.2 sh(1) compliant and because some Makefile recipes have "bash'isms" (indeed, a GMAKE variable should be exported with a definition of "/path/to/gnu/make SHELL=/path/to/bash" and a canonical call should be "$GMAKE ..."); - The comments are obsolete (2): CYGWIN is not treated in anyway specifically and, on the contrary, the regexp translation of ':' in spaces for PACKAGES_PATH would be sure to create a mess with a MS Windows like path; - The settings have obviously evolved and the help message does not list all the variables that can be set and that do modify the way the setting is done; - Some commands (notably whereis(1)) are not standard utilities, not to be found on all Unix like systems and, even if found, have greatly diverging behaviors. What is the preferred procedure? Ignore it and to just use BaseTools/BuildEnv directly? I'm not fully sure what value it adds ... That's the problem: it does not add much, but it adds some things and the problem (for me) is to know if these are used or not: 1) It exports PYTHONHASHSEED=1 (needed?); 2) Undocumented features allow to use something else instead of BaseTools/BuildEnv... I also think that it would be far better to state clearly what is needed (including with BaseTools/BuildEnv and the like) and let builders simply set the correct environment with defined variables being used afterwards (MAKE, SHELL, PYTHON and so on). To state, for example the Python major version required and that everything depends on GNU make and bash(1) (hence the /path/to/gnu/make SHELL=/path/to/bash invocation) and that would be it. But: who uses it (edk2setup.sh) and with what? This question, I can not answer, obviously... Should I file BZ to list all the problems so that someone authorized may address them? Or can I propose a patch to address these (keeping it backward compatible with a present correct use) with a reasonable hope that, as an exception that will not become a rule, it will not be ignored? Sending patches has a much higher chance to succeed, although there is no guarantee unfortunately. Hum... There is a very lethal weapon actually in use: the pillow. I already sent various patches and they are silently ignored... If my contribution will be ignored as others have been till now, honesty should be to clearly state: "we don't care and we won't care" so that I can use my time trying to solve the problems I want to address at a different level than UEFI... Best, -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#99503): https://edk2.groups.io/g/devel/message/99503 Mute This Topic: https://groups.io/mt/96697952/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v7 2/2] tools_def: add -fno-omit-frame-pointer to GCC48_{IA32,X64}_CC_FLAGS
OK. Doesn't look like a big impact.
Thanks,
Brian J. Johnson
Original Message
From: Gerd Hoffmann [mailto:kra...@redhat.com]
Sent: Thursday, December 1, 2022 at 2:38 AM
To: Brian J. Johnson
Cc: devel@edk2.groups.io, Alexey Kardashevskiy , Liming
Gao , Erdem Aktas ,
Pawel Polawski , Jordan Justen
, Ard Biesheuvel ,
Yuwei Chen , Tom Lendacky
, James Bottomley , Oliver
Steffen , Jiewen Yao , Min Xu
, Brijesh Singh , Bob Feng
Subject: [edk2-devel] [PATCH v7 2/2] tools_def: add
-fno-omit-frame-pointer to GCC48_{IA32,X64}_CC_FLAGS
On Wed, Nov 30, 2022 at 01:28:42PM -0600, Brian J. Johnson wrote:
Gerd,
Sorry, gotta ask: does this make much difference in the size of the
compiled code? That's a constraint on many real-hardware X64 platforms,
especially for 32-bit code.
Quick test with OvmfPkg/OvmfPkgIa32X64.dsc (sec/pei ia32, dxe x64):
master branch:
FV Space Information
SECFV [11%Full] 212992 (0x34000) total, 23728 (0x5cb0) used, 189264
(0x2e350) free
PEIFV [34%Full] 917504 (0xe) total, 319416 (0x4dfb8) used, 598088
(0x92048) free
DXEFV [49%Full] 12582912 (0xc0) total, 6268032 (0x5fa480) used,
6314880 (0x605b80) free
FVMAIN_COMPACT [98%Full] 1753088 (0x1ac000) total, 1725328 (0x1a5390)
used, 27760 (0x6c70) free
with patch applied:
FV Space Information
SECFV [11%Full] 212992 (0x34000) total, 23728 (0x5cb0) used, 189264
(0x2e350) free
PEIFV [34%Full] 917504 (0xe) total, 319416 (0x4dfb8) used, 598088
(0x92048) free
DXEFV [50%Full] 12582912 (0xc0) total, 6335936 (0x60adc0) used,
6246976 (0x5f5240) free
FVMAIN_COMPACT [99%Full] 1753088 (0x1ac000) total, 1738176 (0x1a85c0)
used, 14912 (0x3a40) free
So slightly more for 64-bit code.
take care,
Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96837): https://edk2.groups.io/g/devel/message/96837
Mute This Topic: https://groups.io/mt/95354912/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v7 2/2] tools_def: add -fno-omit-frame-pointer to GCC48_{IA32,X64}_CC_FLAGS
Gerd,
Sorry, gotta ask: does this make much difference in the size of the
compiled code? That's a constraint on many real-hardware X64 platforms,
especially for 32-bit code.
Brian J. Johnson
Original Message
From: Gerd Hoffmann [mailto:kra...@redhat.com]
Sent: Wednesday, November 30, 2022 at 3:44 AM
To: devel@edk2.groups.io
Cc: Alexey Kardashevskiy , Liming Gao
, Erdem Aktas , Pawel
Polawski , Jordan Justen
, Ard Biesheuvel ,
Yuwei Chen , Tom Lendacky
, James Bottomley , Gerd
Hoffmann , Oliver Steffen ,
Jiewen Yao , Min Xu , Brijesh
Singh , Bob Feng
Subject: [edk2-devel] [PATCH v7 2/2] tools_def: add
-fno-omit-frame-pointer to GCC48_{IA32,X64}_CC_FLAGS
Fixes problems due to code assuming it runs with frame pointers and thus
updates rbp / ebp registers when switching stacks.
Signed-off-by: Gerd Hoffmann
---
BaseTools/Conf/tools_def.template | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/BaseTools/Conf/tools_def.template
b/BaseTools/Conf/tools_def.template
index 73f95b2a3a9f..f1fd6a003062 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -1888,8 +1888,8 @@ DEFINE GCC_DEPS_FLAGS = -MMD -MF $@.deps
DEFINE GCC48_ALL_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS)
-ffunction-sections -fdata-sections -DSTRING_ARRAY_NAME=$(BASE_NAME)Strings
DEFINE GCC48_IA32_X64_DLINK_COMMON = -nostdlib
-Wl,-n,-q,--gc-sections -z common-page-size=0x20
-DEFINE GCC48_IA32_CC_FLAGS = DEF(GCC48_ALL_CC_FLAGS) -m32
-march=i586 -malign-double -fno-stack-protector -D EFI32
-fno-asynchronous-unwind-tables -Wno-address
-DEFINE GCC48_X64_CC_FLAGS= DEF(GCC48_ALL_CC_FLAGS) -m64
-fno-stack-protector "-DEFIAPI=__attribute__((ms_abi))"
-maccumulate-outgoing-args -mno-red-zone -Wno-address -mcmodel=small
-fpie -fno-asynchronous-unwind-tables -Wno-address
+DEFINE GCC48_IA32_CC_FLAGS = DEF(GCC48_ALL_CC_FLAGS) -m32
-march=i586 -malign-double -fno-stack-protector -D EFI32
-fno-asynchronous-unwind-tables -Wno-address -fno-omit-frame-pointer
+DEFINE GCC48_X64_CC_FLAGS= DEF(GCC48_ALL_CC_FLAGS) -m64
-fno-stack-protector "-DEFIAPI=__attribute__((ms_abi))"
-maccumulate-outgoing-args -mno-red-zone -Wno-address -mcmodel=small
-fpie -fno-asynchronous-unwind-tables -Wno-address -fno-omit-frame-pointer
DEFINE GCC48_IA32_X64_ASLDLINK_FLAGS =
DEF(GCC48_IA32_X64_DLINK_COMMON) -Wl,--entry,ReferenceAcpiTable -u
ReferenceAcpiTable
DEFINE GCC48_IA32_X64_DLINK_FLAGS=
DEF(GCC48_IA32_X64_DLINK_COMMON) -Wl,--entry,$(IMAGE_ENTRY_POINT) -u
$(IMAGE_ENTRY_POINT)
-Wl,-Map,$(DEST_DIR_DEBUG)/$(BASE_NAME).map,--whole-archive
DEFINE GCC48_IA32_DLINK2_FLAGS =
-Wl,--defsym=PECOFF_HEADER_SIZE=0x220 DEF(GCC_DLINK2_FLAGS_COMMON)
--
Brian
"I love deadlines. I like the whooshing sound they make as they fly by."
-- Douglas Adams
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96746): https://edk2.groups.io/g/devel/message/96746
Mute This Topic: https://groups.io/mt/95354912/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] Access 64bit address space in 32bit mode
Slight correction: PAE paging can access up to 52 physical address bits, for 4 PBytes of memory. Section 4.4 of the Intel® 64 and IA-32 Architectures Software Developer’s Manual Volume 3 (3A, 3B, 3C & 3D): System Programming Guide covers it. Brian J. Johnson *From:* Andrew Fish via groups.io [mailto:afish=apple@groups.io] *Sent:* Wednesday, November 9, 2022 at 11:20 AM *To:* devel@edk2.groups.io, yoshinoyat...@163.com *Subject:* [edk2-devel] Access 64bit address space in 32bit mode On Nov 7, 2022, at 7:16 PM, Yoshinoya wrote: Hello Is it possible to access 64bit address space in 32bit mode? I assume you are talking about x86? For example, opcode prefix 0x66/67 could let code running in 16bit mode to access 32bit data/address. This is more complex than just instruction prefix. You need the CPU to be setup in big real mode via the GDT, so you basically have a 32-bit environment setup via GDT etc. Also the prefix opcodes have different meaning in different modes. I don’t think there is a way to make 32-bit code access 64-bit data via instruction prefix even if a 64-bit GDT was setup with paging enabled. Or, establishing page table is a must requirement for accessing 64bit address space. For x86 you have to have 64-bit versions of the IDT, GDT, and you need to enable paging to enter 64-bit Long Mode. In a 32-bit x86 world you can access up to 64 GB of physical memory via using 32-bit page table using PAE [1]. PAE is a 32-bit virtual address space, but with support for a 36-bit physical address. I think in the olden days of 32-bit x86 EFI servers would have custom EFI code that enabled paging in 32-bit and carved out a chunk of the 32-bit memory space that could be mapped to 36-bit physical addresses. I think this was platform specific code and I don’t know of any open source version. The 32-bit Long Mode EFI does not have paging enabled, so adding PAE means enabling paging yourself. The edk2 has the opposite version of this code so you can call 16-bit really mode (Legacy BIOS) from 32-bit Protected mode, or 64-bit Long Mode. This is the code to Thunk for 32-bit/64-bit mode to 16-bit code [2]/ [1] https://en.wikipedia.org/wiki/Physical_Address_Extensio <https://en.wikipedia.org/wiki/Physical_Address_Extension>n <https://en.wikipedia.org/wiki/Physical_Address_Extension> [2] https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseLib/X86Thunk.c edk2.png edk2/Thunk16.nasm at master · tianocore/edk2 <https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseLib/X64/Thunk16.nasm> github.com <https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseLib/X64/Thunk16.nasm> <https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseLib/X64/Thunk16.nasm> edk2.png edk2/Thunk16.nasm at master · tianocore/edk2 <https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseLib/Ia32/Thunk16.nasm> github.com <https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseLib/Ia32/Thunk16.nasm> <https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseLib/Ia32/Thunk16.nasm> Thanks, Andrew Fish Thanks -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#96171): https://edk2.groups.io/g/devel/message/96171 Mute This Topic: https://groups.io/mt/94885261/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] How to guard CAR's stack overflow
I've never tried protecting the stack like this, so I'm not sure exactly what you're running in to. The documentation says the base address is specified a bit differently for expand-down descriptors. It needs to point to the page below the stack (lower addresses.) The descriptors for the other segments would still need to cover the entire address space (limit 0x, page-granular, expand-up.) And you'd need to use a different segment selector (and hence a different GDT entry) for the stack segment (ss register) than for the regular data segments. Most of the early PEI code I've seen uses the same selector for ds, ef, fs, gs, and ss. Hope that helps. This is mostly theoretical. I don't actually have a lot of experience with x86 segment programming. Brian J. Johnson *From:* Tiger Liu(BJ-RD) [mailto:tiger...@zhaoxin.com] *Sent:* Wednesday, September 21, 2022, 10:32 PM *To:* devel@edk2.groups.io , Ni, Ray , brian.john...@hpe.com *Subject:* [edk2-devel] How to guard CAR's stack overflow Hi, Johnson: Thanks for your reply! I tried and found it seemed causing some other problems. It hang in eary pei stage. It seems below code could also cause an exception if using expand-down mode in CAR phase’s stack established. mov eax, ss:[ebx] mov eax, [ebp] mov eax, [esp] Thanks *发件人:*Brian J. Johnson *收件人:*devel@edk2.groups.io; ray...@intel.com; Tiger Liu(BJ-RD) *抄送:*Fan, Jeff You could also try modifying the Ia32 segment descriptors to mark the stack segment as an "expand down" type with a limit set just below the low end of the stack area. That should generate a stack-fault exception if the stack overflows, and wouldn't require building page tables. See sections 5.1 - 5.3 of the Intel SDM, volume 3. Brian J. Johnson *From:*Ni, Ray [mailto:ray...@intel.com <mailto:ray...@intel.com>] *Sent:*Wednesday, September 14, 2022, 10:25 PM *To:*devel@edk2.groups.io <mailto:devel@edk2.groups.io>, tiger...@zhaoxin.com <mailto:tiger...@zhaoxin.com> *Cc:*Fan, Jeff <mailto:fanjianf...@byosoft.com.cn> *Subject:*[edk2-devel] How to guard CAR's stack overflow It’s doable. You need to enable paging and mark the very low 4K area of the stack as not-present. You could use the UefiCpuPkg/Library/CpuPageTableLib to help you create the 1:1 page table with the specific 4K area as not-present (if you are using x86 processors). Thanks, Ray *From:*devel@edk2.groups.io <mailto:devel@edk2.groups.io> *On Behalf Of *Tiger Liu(BJ-RD) *Sent:* Thursday, September 15, 2022 8:50 AM *To:* devel@edk2.groups.io *Subject:* [edk2-devel] How to guard CAR's stack overflow Hi, Experts: Usually, we use Cache As Ram to setup stack and heap for C language running environment before permanent memory has been initialized. So, is there a method to guard this phase’s stack overflow? Note: I find udk has introduced a method to guard stack overflow after memory has been initialized and discovered. Thanks 保密声明： 本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。 /CONFIDENTIAL NOTE: / /This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited./ _._,_._,_ Groups.io Links: You receive all messages sent to this group. View/Reply Online (#93797) <https://edk2.groups.io/g/devel/message/93797> | | Mute This Topic <https://groups.io/mt/93691088/1761811> | New Topic <https://edk2.groups.io/g/devel/post> Your Subscription <https://edk2.groups.io/g/devel/editsub/1761811> | Contact Group Owner <mailto:devel+ow...@edk2.groups.io> | Unsubscribe <https://edk2.groups.io/g/devel/unsub> [brian.john...@hpe.com] _ 保密声明： 本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。 /CONFIDENTIAL NOTE: / /This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited./ -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#94142): https://edk2.groups.io/g/devel/message/94142 Mute This Topic: https://groups.io/mt/93691088/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] How to guard CAR's stack overflow
You could also try modifying the Ia32 segment descriptors to mark the stack segment as an "expand down" type with a limit set just below the low end of the stack area. That should generate a stack-fault exception if the stack overflows, and wouldn't require building page tables. See sections 5.1 - 5.3 of the Intel SDM, volume 3. Brian J. Johnson *From:* Ni, Ray [mailto:ray...@intel.com] *Sent:* Wednesday, September 14, 2022, 10:25 PM *To:* devel@edk2.groups.io , tiger...@zhaoxin.com *Cc:* Fan, Jeff *Subject:* [edk2-devel] How to guard CAR's stack overflow It’s doable. You need to enable paging and mark the very low 4K area of the stack as not-present. You could use the UefiCpuPkg/Library/CpuPageTableLib to help you create the 1:1 page table with the specific 4K area as not-present (if you are using x86 processors). Thanks, Ray *From:*devel@edk2.groups.io *On Behalf Of *Tiger Liu(BJ-RD) *Sent:* Thursday, September 15, 2022 8:50 AM *To:* devel@edk2.groups.io *Subject:* [edk2-devel] How to guard CAR's stack overflow Hi, Experts: Usually, we use Cache As Ram to setup stack and heap for C language running environment before permanent memory has been initialized. So, is there a method to guard this phase’s stack overflow? Note: I find udk has introduced a method to guard stack overflow after memory has been initialized and discovered. Thanks 保密声明： 本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。 /CONFIDENTIAL NOTE: / /This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited./ _._,_._,_ Groups.io Links: You receive all messages sent to this group. View/Reply Online (#93797) <https://edk2.groups.io/g/devel/message/93797> | | Mute This Topic <https://groups.io/mt/93691088/1761811> | New Topic <https://edk2.groups.io/g/devel/post> Your Subscription <https://edk2.groups.io/g/devel/editsub/1761811> | Contact Group Owner <mailto:devel+ow...@edk2.groups.io> | Unsubscribe <https://edk2.groups.io/g/devel/unsub> [brian.john...@hpe.com] _ -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#93972): https://edk2.groups.io/g/devel/message/93972 Mute This Topic: https://groups.io/mt/93691088/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/2] OvmfPkg: Introduce NULL class library to inhibit driver load
quot;MdePkg/Library/UefiDriverEntryPoint/DriverEntryPoint.c":
- One is inside the _ModuleEntryPoint() function.
This call is reached only when the function designated as ENTRY_POINT
in the driver's INF file returns (note, said function is not the
actual entry point function of the driver -- the actual entry point is
the _ModuleEntryPoint() function).
When gBS->Exit() is called, the CoreExit() function
[MdeModulePkg/Core/Dxe/Image/Image.c] long-jumps back to
CoreStartImage(), and no part of the driver's _ModuleEntryPoint() is
again used. So the first ProcessLibraryDestructorList() call site,
namely the one in ModuleEntryPoint(), is not reached when gBS->Exit()
is called.
- The other ProcessLibraryDestructorList() call site is in
_DriverUnloadHandler()
[MdePkg/Library/UefiDriverEntryPoint/DriverEntryPoint.c].
Now it's not easy at all to say whether gBS->Exit() utilizes this
function or not, when it unloads the image (because, per spec,
gBS->Exit() *is* responsible for unloading the image).
However, we need not track that down right now, to see that the
proposed patch is unsafe in this aspect. That's because
_ModuleEntryPoint() does the following:
//
// Call constructor for all libraries
//
ProcessLibraryConstructorList (ImageHandle, SystemTable);
//
// Install unload handler...
//
if (_gDriverUnloadImageCount != 0) {
Status = gBS->HandleProtocol (
ImageHandle,
,
(VOID **)
);
ASSERT_EFI_ERROR (Status);
LoadedImage->Unload = _DriverUnloadHandler;
}
In other words, even if CoreExit() might call Unload -->
_DriverUnloadHandler() --> ProcessLibraryDestructorList() somewhere,
_ModuleEntryPoint() sets "Unload" to "_DriverUnloadHandler" only
*after* ProcessLibraryConstructorList() returns. And the proposed
patch calls gBS->Exit() from *inside* ProcessLibraryConstructorList(),
that is, when "Unload" is not set yet.
On physical machines, I've seen firmware options for disabling the IP
stack entirely; I wonder how those firmwares do it...
I don't know how any physical machine handles that particular option.
But one approach would be to add a GUID to the depex of the module you
want to control, and install it only when you want the module to be
dispatched. That's pretty straightforward, although it does result in
"Driver %g was discovered but not loaded!!" messages from
CoreDisplayDiscoveredNotDispatched() if sufficient debugging is enabled.
Brian J. Johnson
Laszlo
On 08/15/22 11:40, Ard Biesheuvel wrote:
+ }
+ return EFI_SUCCESS;
+}
diff --git a/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.inf
b/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.inf
new file mode 100644
index ..ed521d12d335
--- /dev/null
+++ b/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.inf
@@ -0,0 +1,28 @@
+## @file
+# Copyright (c) 2022, Google LLC. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION= 1.29
+ BASE_NAME = DriverLoadInhibitorLib
+ FILE_GUID = af4c2c0b-f7ed-4d61-ad97-5953982c3531
+ MODULE_TYPE= DXE_DRIVER
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = NULL
+ CONSTRUCTOR= DriverLoadInhibitorLibConstructor
+
+[Sources]
+ DriverLoadInhibitorLib.c
+
+[LibraryClasses]
+ QemuFwCfgSimpleParserLib
+ UefiBootServicesTableLib
+
+[Packages]
+ MdePkg/MdePkg.dec
+ OvmfPkg/OvmfPkg.dec
+
+[FixedPcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdDriverInhibitorFwCfgVarName
diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 5af76a540529..e9a22cab088c 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -399,6 +399,10 @@ [PcdsFixedAtBuild]
## The Tdx accept page size. 0x1000(4k),0x20(2M)
gUefiOvmfPkgTokenSpaceGuid.PcdTdxAcceptPageSize|0x20|UINT32|0x65
+ ## The QEMU fw_cfg variable that DriverLoadInhibitorLib will check to
+ # decide whether to abort dispatch of the driver it is linked into.
+ gUefiOvmfPkgTokenSpaceGuid.PcdDriverInhibitorFwCfgVarName|""|VOID*|0x68
+
[PcdsDynamic, PcdsDynamicEx]
gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10
--
Brian
"You're the flavor packet in the Ramen noodle brick of life, Roy."
-- Three Fellows
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92489): https://edk2.groups.io/g/devel/message/92489
Mute This Topic: https://groups.io/mt/93032846/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] FreePool in PEI (was: Clarification of Memory management in PEI phase)
I've been looking at this a bit. It appears that implementing PEI FreePool in this way would require: * Adding a new HOB type EFI_HOB_TYPE_FREE_POOL to PiHob.h * Adding a new FreePool PEI service to PiPeiCis.h * Implementing the service in PeiMain.c and MemoryServices.c * Adding PeiServicesFreePool() to PeiServicesLib * Modifying PeiMemoryAllocationLib to call PeiServicesFreePool() when freeing EfiBootServicesData, while ignoring free requests for other memory types The last step is a bit problematic, since PeiMemoryAllocationLib implements non-EfiBootServicesData pool allocations using AllocatePages(). FreePool() doesn't take a MemoryType parameter, so it just has to trust that it has been given a EfiBootServicesData buffer which was allocated from the HOB list. If a caller tries to free non-EfiBootServicesData memory, eg. memory allocated via AllocateRuntimePool(), the PEI service will return an error. PeiMemoryAllocationLib can ignore the error, since a failed free is essentially a no-op, which is the traditional implementation of FreePool() in PEI anyway. Does that sound like an acceptable level of complexity and impact to the specifications? I'd rather not start the code first process if there's no chance it will go through. An alternative would be to implement pool allocation and freeing entirely in PeiMemoryAllocationLib, instead of adding a new PEI service. If the code is too big, PeiMemoryAllocationLib could wrap a PPI. Then we could have different implementations, either backed by the HOB list like the current allocator, or backed by pages with a more sophisticated allocator. Brian Original Message From: Vincent Zimmer [mailto:vincent.zim...@gmail.com] Sent: Wednesday, June 22, 2022, 4:59 PM To: devel@edk2.groups.io, brian.john...@hpe.com Subject: [edk2-devel] Clarification of Memory management in PEI phase sounds like a good idea. As a next step, perhaps a candidate activity for https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Code-First-Process in anticipation of future inclusion in PI? Vincent On Wed, Jun 22, 2022 at 2:41 PM Brian J. Johnson wrote: Andrew, Yes, adding a new HOB type to represent free pool would probably be the easiest. Or we could write or borrow a traditional malloc() implementation, similar to DXE's pool allocator, and back it with memory from AllocatePages(). That would probably have better performance, and would avoid fragmenting the pool memory when non-pool HOBs are added. It would probably be more code, though. Brian J. Johnson From: Andrew Fish [mailto:af...@apple.com] Sent: Wednesday, June 22, 2022, 3:54 PM To: Brian J. Johnson Cc: devel@edk2.groups.io, ayushdevel1...@gmail.com Subject: [edk2-devel] Clarification of Memory management in PEI phase Brian, I think all the PEI Allocate Pool does is make a HOB [1]. I don’t think we can remove HOBs when memory is freed as that would change pointers to pool. It may be possible to mark a region as free and allocate from that list batch 1st, and just over allocate space if needed. It could be as simple as just adding a new HOB type to represent free pool. [1] https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/Memory/MemoryServices.c#L878 Thanks, Andrew Fish On Jun 22, 2022, at 12:39 PM, Brian J. Johnson -- Brian J. Johnson Enterprise X86 Lab Hewlett Packard Enterprise brian.john...@hpe.com -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#91002): https://edk2.groups.io/g/devel/message/91002 Mute This Topic: https://groups.io/mt/92114859/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] Clarification of Memory management in PEI phase
Andrew, Yes, adding a new HOB type to represent free pool would probably be the easiest. Or we could write or borrow a traditional malloc() implementation, similar to DXE's pool allocator, and back it with memory from AllocatePages(). That would probably have better performance, and would avoid fragmenting the pool memory when non-pool HOBs are added. It would probably be more code, though. Brian J. Johnson *From:* Andrew Fish [mailto:af...@apple.com] *Sent:* Wednesday, June 22, 2022, 3:54 PM *To:* Brian J. Johnson *Cc:* devel@edk2.groups.io, ayushdevel1...@gmail.com *Subject:* [edk2-devel] Clarification of Memory management in PEI phase Brian, I think all the PEI Allocate Pool does is make a HOB [1]. I don’t think we can remove HOBs when memory is freed as that would change pointers to pool. It may be possible to mark a region as free and allocate from that list batch 1st, and just over allocate space if needed. It could be as simple as just adding a new HOB type to represent free pool. [1] https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/Memory/MemoryServices.c#L878 Thanks, Andrew Fish On Jun 22, 2022, at 12:39 PM, Brian J. Johnson Sorry for the late response to this thread... PEI has grown greatly in complexity, as Andrew pointed out. Do we (TianoCore community) think it's time to add a real pool manager to PEI? There's getting to be quite a bit of post-DRAM-initialization PEI code on some platforms. And really, having a limited amount of pre-DRAM memory available is an even better reason for having an effective pool allocator, so the memory can be freed and reused. FWIW we (HPE) needed to add a private pool allocator to manage memory for a proprietary h/w initialization module which needs to run in post-DRAM PEI, and depends on allocating and freeing memory in different phases of its operation. Brian J. Johnson *From:* Ayush Singh [mailto:ayushdevel1...@gmail.com] *Sent:* Friday, June 10, 2022, 12:22 AM *To:* Andrew Fish *Cc:* devel@edk2.groups.io *Subject:* [edk2-devel] Clarification of Memory management in PEI phase Thanks for the wonderful answer. Ayush Singh On Thu, Jun 9 2022 at 01:26:58 PM -0700, Andrew Fish wrote: On Jun 9, 2022, at 10:28 AM, Ayush Singh include: 1. InstallPeiMemory() This is basically: (*PeiServices)->InstallPeiMemory (PeiServices, MemoryBegin, MemoryLength); This is how you tell the PEI Core the location of the memory that will can be used in PEI. 2. AllocatePages() 3. AllocatePool() 4. CopyMem() 5. SetMem() 6. FreePages() However, no `FreePool()` service seems to be present. So how is the memory allocated using `AllocatePool()` freed? It basically gets Freed when you transition to the DXE phase. To step back for a minute I think it is important to remember that the main job of PEI is to initialize DRAM, and deal with S3 (resuming from suspend to RAM). So as soon as you have DRAM you are kind done and ready for the DXE IPL so you can load the DXE Phase and start up EFI. Remember PEI is Pre EFI. The reality is programming DRAM is complex and lots of code got written, then lots more code got written and PEI has become large for some ports. That was never the intent. PEI is designed as a way to run C code when you code is running from ROM and you donā��t have any DRAM. For x86 not having DRAM means you are using the cache as RAM. For some SoCs there is actually an SRAM you can use. Thus the PEI memory allocation scheme is designed to deal with this very constrained environment. You start PEI with a heap and stack. You can also allocate HOBs (Hand Off Blocks). A pool allocation in PEI is just a HOB. See [1]. There is no way to free a HOB. So the AllocatePool() kind of leaks into DXE too as an entry in the HOB list. But when the OS called gBS->ExitBootServices() that frees all non runtime memory back to the OS. If you look a AllocatePages/FreePages you will see AllocatePages creates a HOB that points to the memory region, and FreePages just marks that HOB as not used. That code is also in this file [1]. TL;DR there is no pool manager in PEI. [1] https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/Memory/MemoryServices.c#L878 Thanks, Andrew Fish Ayush Singh Brian -- "There is the greatest practical benefit in making a few failures early in life." -- Thomas Henry Huxley -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#90700): https://edk2.groups.io/g/devel/message/90700 Mute This Topic: https://groups.io/mt/91651322/21656 Group
Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI
Nate, FWIW I welcome having a standard interface to write EFI variables in PEI. Not all platforms store variables in flash, so they don't all have tricky issues with fault-tolerant writes to h/w which is also being used for code fetches. And writing variables early makes it possible to modify settings in response to h/w changes and external requests, without having to boot all the way to DXE, write the variables, then trigger a reset. Brian J. Johnson Original Message From: Nate DeSimone [mailto:nathaniel.l.desim...@intel.com] Sent: Monday, June 13, 2022, 4:31 PM To: Yao, Jiewen , devel@edk2.groups.io , michael.kuba...@outlook.com Cc: Wang, Jian J , Gao, Liming , Kinney, Michael D , Oram, Isaac W , Chiu, Chasel , Cheng, Gao , Zhang, Di , Bu, Daocheng , Kubacki, Michael Subject: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI Hi Jiewen, I am fine with deferring the submission of this to edk2 until the implementation is ready for review. I just wanted to get feedback on the API so that once the implementation patch series arrives we will at least that that piece of the review done. I would say this thread achieved that goal. Thank you for the pointer to the protected variable code, we will review it and make sure that no issues would arise from the pre-memory PEI implementation. Thanks, Nate -Original Message- From: Yao, Jiewen Sent: Friday, June 10, 2022 6:09 PM To: Desimone, Nathaniel L ; devel@edk2.groups.io; michael.kuba...@outlook.com Cc: Wang, Jian J ; Gao, Liming ; Kinney, Michael D ; Oram, Isaac W ; Chiu, Chasel ; Cheng, Gao ; Zhang, Di ; Bu, Daocheng ; Kubacki, Michael Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI Thanks for the response. 1) Why we need "enable UEFI variable write before permanent memory is available"? 2) If the implementation is not ready, I do have concern to add it so early in EDKII. If I don’t have a big picture, I am not sure how to review the completeness. Can we put it to EDKII-staging (https://github.com/tianocore/edk2-staging) for a moment? I don’t see the need to add the interface now for work-in-progress feature, since there is no consumer and no producer. Another reason is that I happen to know other feature (in EDKII stage) is impacting variable driver. https://github.com/tianocore/edk2-staging/tree/ProtectedVariable/libs Please do consider that as well - how to write a protected variable in PEI phase. Thank you Yao Jiewen -Original Message- From: Desimone, Nathaniel L Sent: Saturday, June 11, 2022 5:49 AM To: Yao, Jiewen ; devel@edk2.groups.io; michael.kuba...@outlook.com Cc: Wang, Jian J ; Gao, Liming ; Kinney, Michael D ; Oram, Isaac W ; Chiu, Chasel ; Cheng, Gao ; Zhang, Di ; Bu, Daocheng ; Kubacki, Michael Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI Hi Jiewen, Thanks for the feedback, per your questions: 1. The primary use case for this is to enable UEFI variable writes before permanent memory is available. 2. The implementation is a work in progress. We will provide it shortly. As this will be a rather large patch set, I would like to get this piece in place beforehand so that the reviewers can focus on the implementation separate from the API definition. 3. No impact to secure boot. We are not going to support writing to authenticated variables in PEI. As mentioned in the comments, if a PEIM wishes to update any of the authenticated variables it must use the existing HOB mechanism to have a later DXE phase perform the update. 4. With regard to atomicity, we have a complete implementation of the fault tolerant write services operational in Pre-Memory PEI. 5. Good point on the S3 resume, we will need to add an SMI to have the variable services re-initialize the mNvVariableCache. Hope that helps, Nate -Original Message- From: Yao, Jiewen Sent: Friday, June 10, 2022 9:56 AM To: devel@edk2.groups.io; michael.kuba...@outlook.com; Desimone, Nathaniel L Cc: Wang, Jian J ; Gao, Liming ; Kinney, Michael D ; Oram, Isaac W ; Chiu, Chasel ; Cheng, Gao ; Zhang, Di ; Bu, Daocheng ; Kubacki, Michael Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI Hi I am curious why we need this interface. Why we need write variable capability in PEI phase? Where is the implementation of this? I prefer to see an implementation submitted together with header file. For example, what is the impact to secure boot related feature, how to write auth variable in PEI, how PEI write variable cowork with SMM version in S3 resume phase, how to support variable atomicity, etc. Thank you Yao Jiewen -Original Message- From: devel@edk2.groups.io On Behalf Of Michael Kubacki Sent: Friday, June 10, 2022 10:00 AM To: devel@edk2.groups.io; Desimone, Nathaniel L Cc: Wang, Jian J ; Gao, Limin
Re: [edk2-devel] Clarification of Memory management in PEI phase
Sorry for the late response to this thread... PEI has grown greatly in complexity, as Andrew pointed out. Do we (TianoCore community) think it's time to add a real pool manager to PEI? There's getting to be quite a bit of post-DRAM-initialization PEI code on some platforms. And really, having a limited amount of pre-DRAM memory available is an even better reason for having an effective pool allocator, so the memory can be freed and reused. FWIW we (HPE) needed to add a private pool allocator to manage memory for a proprietary h/w initialization module which needs to run in post-DRAM PEI, and depends on allocating and freeing memory in different phases of its operation. Brian J. Johnson *From:* Ayush Singh [mailto:ayushdevel1...@gmail.com] *Sent:* Friday, June 10, 2022, 12:22 AM *To:* Andrew Fish *Cc:* devel@edk2.groups.io *Subject:* [edk2-devel] Clarification of Memory management in PEI phase Thanks for the wonderful answer. Ayush Singh On Thu, Jun 9 2022 at 01:26:58 PM -0700, Andrew Fish wrote: On Jun 9, 2022, at 10:28 AM, Ayush Singh InstallPeiMemory() This is basically: (*PeiServices)->InstallPeiMemory (PeiServices, MemoryBegin, MemoryLength); This is how you tell the PEI Core the location of the memory that will can be used in PEI. 2. AllocatePages() 3. AllocatePool() 4. CopyMem() 5. SetMem() 6. FreePages() However, no `FreePool()` service seems to be present. So how is the memory allocated using `AllocatePool()` freed? It basically gets Freed when you transition to the DXE phase. To step back for a minute I think it is important to remember that the main job of PEI is to initialize DRAM, and deal with S3 (resuming from suspend to RAM). So as soon as you have DRAM you are kind done and ready for the DXE IPL so you can load the DXE Phase and start up EFI. Remember PEI is Pre EFI. The reality is programming DRAM is complex and lots of code got written, then lots more code got written and PEI has become large for some ports. That was never the intent. PEI is designed as a way to run C code when you code is running from ROM and you donā��t have any DRAM. For x86 not having DRAM means you are using the cache as RAM. For some SoCs there is actually an SRAM you can use. Thus the PEI memory allocation scheme is designed to deal with this very constrained environment. You start PEI with a heap and stack. You can also allocate HOBs (Hand Off Blocks). A pool allocation in PEI is just a HOB. See [1]. There is no way to free a HOB. So the AllocatePool() kind of leaks into DXE too as an entry in the HOB list. But when the OS called gBS->ExitBootServices() that frees all non runtime memory back to the OS. If you look a AllocatePages/FreePages you will see AllocatePages creates a HOB that points to the memory region, and FreePages just marks that HOB as not used. That code is also in this file [1]. TL;DR there is no pool manager in PEI. [1] https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/Memory/MemoryServices.c#L878 Thanks, Andrew Fish Ayush Singh Brian -- "It's OK to be stuck. 99% of the time in your own [research] work, you're stuck." -- Mark Lawrence -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#90695): https://edk2.groups.io/g/devel/message/90695 Mute This Topic: https://groups.io/mt/91651322/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] Running and Testing Modules and Applications
Qemu's virtual VFAT (vvfat) disk type is a convenient way to test UEFI applications. It presents a folder on the host as a VFAT file system to the guest. It's not the fastest or the most stable disk type (be careful not to modify files from the host while the guest is running), but it's really handy. Another way to put a file on a UEFI VFAT disk image for qemu is to use mtools (https://www.gnu.org/software/mtools/), a set of user-mode programs which can manipulate FAT disk images. You can write some scripts around them to automate your workflow, similarly to uefi-run. I've done that quite a bit in the past. Good luck, Brian J. Johnson Original Message From: Ayush Singh [mailto:ayushdevel1...@gmail.com] Sent: Friday, June 3, 2022, 11:49 AM To: edk2-devel-groups-io Subject: [edk2-devel] Running and Testing Modules and Applications Hello everyone, I wanted to ask everyone how most modules and applications are run/tested in edk2. I will be working on Adding Rust support for edk2 during GSoC and thus will probably have to do a lot of primitive testing. I did look at the EmulationPkg but didn't really understand how to use it. It simply drops me into gdb, although maybe that's what it is supposed to do? There were also some GUI programs (VisualUefi) that can be used in windows, but since I am in Linux, they aren't much useful. I also found a tutorial to run it in a physical machine (https://tait.tech/2021/04/18/uefi-development-environment/ ), but that seems more for the final testing rather than testing during development. I have also tried using qemu for running applications, and I guess I was somewhat successful by using the script: `https://github.com/Richard-W/uefi-run` to test out uefi applications in qemu. However, it builds a FAT filesystem around the EFI application, so I was wondering if there was a better and simpler way to do it. Ayush Singh -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#90200): https://edk2.groups.io/g/devel/message/90200 Mute This Topic: https://groups.io/mt/91525281/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-discuss] GSoC Proposal
On 4/15/22 23:23, Desimone, Nathaniel L wrote: Hi Marvin, -Original Message- From: Marvin Häuser Sent: Friday, April 15, 2022 9:44 AM To: disc...@edk2.groups.io; Johnson, Brian ; Desimone, Nathaniel L ; Andrew Fish ; devel@edk2.groups.io Cc: Pedro Falcato ; adachristin...@gmail.com; Shi, Steven Subject: Re: [edk2-devel] [edk2-discuss] GSoC Proposal Hey Brian, On 15.04.22 18:22, Brian J. Johnson wrote: Nate, Andrew, Marvin, Pedro, Ada, et al, This is a great discussion. I've been debating where to weigh in... Personally, I don't think this topic can get enough attention. Thanks! :) I agree that some sort of library sharing to reduce image size would be very helpful. Although some parts of the BIOS are built separately, large parts of it are compiled at the same time, so there should be ample opportunity for tool-directed sharing. Some form of pre-linking modules together may be the easiest way to do that. But however it gets implemented, we should try to make it automatic: requiring library writers to manually add a lot of metadata or write thunks or shims would be a barrier to adoption. My suggestion to use a private function array would indeed require shims, however this would be the "better than nothing I guess" solution where only the most used and most painful to duplicate functions are shared, e.g., memory and maybe common string printing functions. This would quickly and efficiently achieve the goal of "reduce size". And some form of sophisticated prelinking would be my proposal for something more systematic, which might come a lot cheaper with the security features to be added. I also agree that PEI has acquired far too much functionality over time, and we badly need to find some way to reverse that. PEI is pulling in more and more of the h/w initialization, leaving DXE mainly to provide the boot interface (ACPI, UEFI services, setup screens, etc.) That puts more and more pressure on XIP storage and cache-as-RAM. How can we encourage a change? I'm not in the loop enough to comment on the XIP and CAR issues. However, HW init moving to PEI (or more generally, into a single stage) is not necessarily a bad thing, is it? As I've said before, I'm not sure what else to think of PEI than "DXE light". Both "initialise some of the hardware" with no clear scope boundary but with similar interfaces. If they were sort of fused to provide a minimal "pre-memory PEI" and a more DXE-like "post-memory PEI" without having a clear boundary between HW init steps, would that be too bad? Now I'm *really* just blindly guessing because I barely looked into this project, but isn't that somewhat the idea of SlimBootloader? Your description of DXE sort of is it collapsing into an advanced BDS. Nate earlier said he'd like a clear distinction between DXE and BDS, which I didn't really get the point of. I guess what I'm trying to express is where does DXE end and where does BDS begin? A lot of drivers that are only used in BDS get dispatched early in DXE. Conversely, it is entirely possible for some DXE drivers to only run after we have technically entered the BDS phase. The line where DXE ends and BDS begins is blurry and not really a line but more like a rectangle. To Isaac's point, there is a good argument to be made for beefing up PEI's capabilities to the point that DXE and BDS more or less become one thing. But what does that mean for PEI? Do we change from the 1-D PPI database to the 2-D Handle database in order to express device topologies better for example? That would be useful for stuff like the PEI UFS drivers as they run into difficulties expressing LUN IDs that the DXE version does not for example. Not sure if I would actually formally propose that in the PI WG but interesting stuff to think about regardless. > Andrew pointed out that PEI was originally intended for memory init, and DXE for the rest. One nice aspect of that is that there's a simple, architected, consolidated handoff of state between them: the HOB list. That makes it easier to do "unusual" things in SEC+PEI (special security, address map changes, etc.) but have it all hidden from DXE, where the majority of "standard" code lives. The trend of moving more and more into PEI is eroding that advantage. Maybe a similar handoff of state is needed between pre-memory and post-memory PEI? Or between DXE and BDS? One could argue that we already have that for BDS: the UEFI boot services. But if PEI consumed the HW init portions of DXE, a more advanced BDS (which also powers e.g. the HID stuff on the way) would be the logical remainder stage to separately support UEFI booting as a payload (for SlimBootloader, coreboot, ...). All of that was logically speaking. Even if the development was driven to its extreme, PEI and DXE cores would still share a bunch of code and it might make sense to share a foundation technically.
Re: [edk2-devel] [edk2-discuss] GSoC Proposal
Nate, Andrew, Marvin, Pedro, Ada, et al, This is a great discussion. I've been debating where to weigh in... I agree that some sort of library sharing to reduce image size would be very helpful. Although some parts of the BIOS are built separately, large parts of it are compiled at the same time, so there should be ample opportunity for tool-directed sharing. Some form of pre-linking modules together may be the easiest way to do that. But however it gets implemented, we should try to make it automatic: requiring library writers to manually add a lot of metadata or write thunks or shims would be a barrier to adoption. I also agree that PEI has acquired far too much functionality over time, and we badly need to find some way to reverse that. PEI is pulling in more and more of the h/w initialization, leaving DXE mainly to provide the boot interface (ACPI, UEFI services, setup screens, etc.) That puts more and more pressure on XIP storage and cache-as-RAM. How can we encourage a change? Thanks, Brian J. Johnson On 4/15/22 03:15, Nate DeSimone wrote: Hi Andrew, On 4/14/22, 7:43 PM, "Andrew Fish" wrote: On Apr 14, 2022, at 6:06 PM, Nate DeSimone wrote: Hi Marvin, -Original Message- From: devel@edk2.groups.io On Behalf Of Marvin Häuser Sent: Thursday, April 14, 2022 12:56 AM To: disc...@edk2.groups.io; Desimone, Nathaniel L Cc: Pedro Falcato ; edk2-devel-groups-io ; adachristin...@gmail.com; Shi, Steven Subject: Re: [edk2-devel] [edk2-discuss] GSoC Proposal I feel like there are so many much easier solutions to this problem that are at most limited by the clear specification. The UEFI specification with regards to booting and all of that obviously is of utmost importance. If you have a better idea that retains compatibility with the existing UEFI PI then I would be happy to hear it. Ultimately anything we do needs to be a pure extension that retains compatibility with old code. Given that restriction having the ability to coalesce all the LibraryClasses into a single module and have cross-module linking seems like the best way to handle it to me. The UEFI PI specification parts that deal about internal structure, as far as I know, are only in place to make it easy to integrate Intel IP. Its not just for Intel. The biggest reason for it to increase the standardization of the boot flow across the PC ecosystem. We have learned from experience that firmware is super critical to get a product out the door but it is also difficult to write. So we try to make it as reusable as humanly possible. In fact, I don’t *know*, but I’m pretty sure the very strict separation between PEI and DXE was preserved mostly because MRC was 32-bit-only for a long time. Glad that seems to have been resolved, AMD does memory init by PSP nowadays. Having less complex early stages chain load more complex later stages is a common design pattern in firmware, not just UEFI. For example, your typical ARM system loads kinda like this: PBL (SoC ROM) --> SBL (RAM Init) --> ARM Trust Zone --> Hypervisor --> EDK II or U-Boot or LittleKernel (which runs android fastboot) Comparing relative complexity I believe the Intel UEFI PI design is actually pretty simple when you consider how much it gets done: Ucode ROM --> SEC + PEI --> DXE + SMM + BDS My biggest criticism of the design is that the strict separation between PEI and DXE doesn't exist between DXE, SMM, and BDS There are a few reasons why PEI was 32-bit for quite some time. The biggest one is the code size increase, 64-bit x86 code is 20-30% larger than 32-bit x86 code. Since the only RAM Pre-Memory code has access to is the cache onboard the processor and for security reasons all that code has to fit within that RAM we generally do everything we can to make that image as small as possible. Second, 64-bit requires a page table and since we desired to keep PEI simple we tried to avoid that. Finally, the PI spec didn't allow a 64-bit PEI until recently. MRC is 32-bit code just because that is what PEI happens to be. Porting it to 64-bit is not terribly difficult. Ultimately the mix of 32/64-bit does cause some difficulties. Any data structures that get shared between PEI and DXE (HOBs, PCDs, etc.) need to resolve to the same size and packing. LibraryClasses need to be written to compile properly in both modes. In the case of FSP API mode you need to resort to thunking between 32 and 64-bit modes during DXE. More or less we decided that the costs are starting to outweigh the advantages. I’d also point out that x86 VMs use X64 (x86-64) PEI today and it works so the 32-bit/64-bit mix has nothing to do with UEFI/PI/edk2. In the PC ecosystem a single chipset family can power thousands of unique designs. So the DRAM memory needs to be external, support lots of different chipset packages(signal integrity...), support the lowest cost through the highest cost DRAM and
Re: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg
I'll add my agreement to Marvin and Jeff: a low-level sort routine like this should let the caller be in charge of memory allocation, so it can be used in the widest variety of contexts (SEC, exception handlers, APs, etc.) So let's make the BufferOneElement parameter mandatory. Brian J. Johnson Original Message From: Ni, Ray [mailto:ray...@intel.com] Sent: Monday, September 27, 2021, 8:49 PM To: Marvin Häuser , fanjianf...@byosoft.com.cn , devel@edk2.groups.io , 'gaoliming' , Chan, Amy , 'Andrew Fish' Cc: Kinney, Michael D , 'Gao, Liming' , Liu, Zhiguang , Wang, Jian J , Gao, Zhichao Subject: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg Marvin, I agree with your concerns, in a certain level. But I didn't treat it as a very big problem of having caller pass the BufferOneElement "intelligently". I am ok to use your option 1), making BufferOneElement mandatory. Caller should always supply the buffer that's sufficient to hold one element. By the way, I don't want to propose "swap-without-using-temporary-value" method to avoid using the "BufferOneElement"? Because that makes the simple thing complicated! Thanks, Ray -Original Message- From: Marvin Häuser Sent: Monday, September 27, 2021 5:09 PM To: fanjianf...@byosoft.com.cn; devel@edk2.groups.io; Ni, Ray ; 'gaoliming' ; Chan, Amy ; 'Andrew Fish' Cc: Kinney, Michael D ; 'Gao, Liming' ; Liu, Zhiguang ; Wang, Jian J ; Gao, Zhichao Subject: Re: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg On 27/09/2021 10:50, fanjianf...@byosoft.com.cn wrote: For former caller, they could still keep as is to call the old API in MdeModulePkg. I think Ray's design is compatible change for existing code. Only when the existing code wants to remove the dependency on MdeModuelPkg, they could migrate to the new API in baselib. I agree with one split-API desgin what you mentioned. But my point is to define one API in baselib and to make the baselib not depend on memory allocation. And another wrapper API could be designed to be placed in any other class. Oh sure, it could totally live in another class. I'd just like to have those two models (caller- and callee-owned buffer) strictly separate, I personally do not mind where exactly they are implemented. Thanks! Best regards, Marvin Jeff fanjianf...@byosoft.com.cn *From:* Marvin Häuser <mailto:mhaeu...@posteo.de> *Date:* 2021-09-27 16:14 *To:* fanjianf...@byosoft.com.cn <mailto:fanjianf...@byosoft.com.cn>; devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Ni, Ray <mailto:ray...@intel.com>; 'gaoliming' <mailto:gaolim...@byosoft.com.cn>; Chan, Amy <mailto:amy.c...@intel.com>; 'Andrew Fish' <mailto:af...@apple.com> *CC:* Kinney, Michael D <mailto:michael.d.kin...@intel.com>; 'Gao, Liming' <mailto:liming@intel.com>; Liu, Zhiguang <mailto:zhiguang@intel.com>; Wang, Jian J <mailto:jian.j.w...@intel.com>; Gao, Zhichao <mailto:zhichao@intel.com> *Subject:* Re: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg On 27/09/2021 02:45, fanjianf...@byosoft.com.cn wrote: > Making baselib implementation depend on MemoryAllocationLib > (indirectly on Pei Service and gBS), it may prevent > this base API using at some seneraio. i don't think it's better. That is why I asked about a split-API scenario, one of which does not depend on dynamic memory allocation (SetVariable-like) and one does (wrapper-like). > Add this parameter and make this parameter is optional, > 1, when NULL, use the local 256 bytes stack > 2, if 256 bytes stack is not enough, return RETURE_BUFFER_TOO_SAMLL, > 3, caller check the return status, to do nothing or to allocate enough > buffer for retry > > This is just like SetVariable()'s implementation. Yes, and because that is SetVariable's implementation, we have library functions to make it less error-prone and more convenient [1]. As a matter of fact, we have a (semi-lax) policy in our codebases to avoid such functions like the plague and use those library wrappers where-ever it can make sense. The only super-rare exceptions I can think of are when we know the size of the element ahead of time. Also SetVariable has no arbitrary constraint on when it may work the first time, and there is code that will fail when the first return is not EFI_BUFFER_TOO_SMALL. This solution honestly yields even more problems, because it introduces a Status return which was not there before. For common code safety and quality policy, this requires the value *must* be retrieved and checked in some fashion. So all callers, no matter the pr
Re: [edk2-devel] [PATCH v1] OvmfPkg: Add build options for 8MB and 16MB X64 OVMF images
, 0xDF, 0x00, 0x00,
!endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if ($(FD_SIZE_IN_KB) == 4096) || ($(FD_SIZE_IN_KB) == 8192) ||
($(FD_SIZE_IN_KB) == 16384)
# Size: 0x4
(gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) -
# 0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) = 0x3ffb8
# This can speed up the Variable Dispatch a bit.
@@ -83,7 +83,7 @@ DATA = {
!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
0xe000|0x1000
!endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if ($(FD_SIZE_IN_KB) == 4096) || ($(FD_SIZE_IN_KB) == 8192) ||
($(FD_SIZE_IN_KB) == 16384)
0x0004|0x1000
!endif
#NV_EVENT_LOG
@@ -91,7 +91,7 @@ DATA = {
!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
0xf000|0x1000
!endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if ($(FD_SIZE_IN_KB) == 4096) || ($(FD_SIZE_IN_KB) == 8192) ||
($(FD_SIZE_IN_KB) == 16384)
0x00041000|0x1000
!endif
#NV_FTW_WORKING
@@ -109,7 +109,7 @@ DATA = {
!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
0x0001|0x0001
!endif
-!if $(FD_SIZE_IN_KB) == 4096
+!if ($(FD_SIZE_IN_KB) == 4096) || ($(FD_SIZE_IN_KB) == 8192) ||
($(FD_SIZE_IN_KB) == 16384)
0x00042000|0x00042000
!endif
#NV_FTW_SPARE
I'm providing minimal feedback here just to get this review off my plate
as quickly as possible. Sorry, I'm collapsing under my TODO list.
(1) Every such change is compatibility breaking, so we *must* use the
opportunity at once to *significantly increase* the non-volatile
variable store size as well.
We need to discuss this question with OS vendors and hardware platform
vendors on this list, to see what physical flash sizes are expected in
the future, and we must add a good safety margin on top of that.
> The primary concern is with the dbx variable growing without bounds over
time.
Once we introduce a new FD_SIZE_IN_KB option, we're stuck with its
varstore layout forever, so we'd better get it right and future-proof at
once.
(2) [FD.MEMFD] should immediately benefit from this change, even if your
downstream populates FVMAIN_COMPACT with something else than PEIFV and
DXEFV. First, we're almost out of (uncompressed) DXEFV space again.
Second, especially the confidential computing technologies have been
gobbling up the nice, low, free space in FD.MEMFD the way a kid with a
sweet tooth empties a cookie jar. This change is already compat
breaking, so I'd like to see *some* proposal (separate patches) for
enlarging *and pushing up* PEIFV and DXEFV.
(3) Unfortunately, I have to agree that introducing *both* a 8MB option
*and* a 16MB option is justified, per QEMU commit 0657c657eb37
("hw/i386/pc: add max combined fw size as machine configuration option",
2020-12-09).
However, please add each option in a separate patch.
(4) Dumping a bunch of magic numbers on reviewers is unhelpful. I'll
need to sit down with a calculator and go through the patch with a
magnifying glass. Please support that work by creating a commit message
(summary table) similar to the one in commit b24fca05751f ("OvmfPkg:
introduce 4MB flash image (mainly) for Windows HCK", 2017-05-05).
I've found it very helpful to create a spreadsheet to calculate the
offsets based on the region sizes, and add it to the tree. It can also
calculate the related PCDs, if any. That makes it a lot easier to
verify the numbers, and to make changes in the future.
(5) Modifying *only* "OvmfPkg/OvmfPkgX64.dsc" doesn't seem right, there
are other DSC files (platforms) in OvmfPkg that would benefit. Without
much thinking for now, I'd say the new options should be available in
each DSC (platform description), even the 32-bit ones.
I'm extremely annoyed by the general trend that the firmware (the OS
under the OS) keeps growing. Because of that, Linuxboot is a fantastic
project. I'd like OVMF to support the development of Linuxboot. I
welcome this patch for that reason.
But I'd also like OVMF to benefit from this change even when it is built
with a traditional -- and regrettably, ever-growing -- DXE phase. I
welcome this patch for that reason too.
Thank you,
Laszlo
--
Brian J. Johnson
Enterprise X86 Lab
Hewlett Packard Enterprise
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#75838): https://edk2.groups.io/g/devel/message/75838
Mute This Topic: https://groups.io/mt/83106841/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 1/1] UefiCpuPkg/MpInitLib: Reduce reset vector memory pressure
On 9/24/20 1:22 AM, Laszlo Ersek wrote: - I don't remember if it's required that the APIC ID space be densely populated. For example, if we have a topology with 7 possible (= maximum) logical CPUs, I'm unsure if a spec forbids any of those CPUs from having APIC ID 7. That could cause a problem in MpInitLibSevEsAPReset(), I assume. FWIW, there are many bare metal processors with non-contiguous APIC IDs. Intel puts the socket ID in the upper bits of the APIC ID. So if the socket doesn't have a power-of-two number of cores, there is always a gap. Plus, the BSP doesn't always have APIC ID 0 -- it depends on which physical cores passed the manufacturing tests for that particular part. That has broken various kernels, BIOSes, and other software at times. So it's best not to make assumptions. I don't know if that applies to VMs, though. The standards may be different (if there are any standards at all in that area.) -- Brian J. Johnson Enterprise X86 Lab Hewlett Packard Enterprise -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#65632): https://edk2.groups.io/g/devel/message/65632 Mute This Topic: https://groups.io/mt/77041010/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-rfc] MdeModulePkg/StatusCodeHandler: Separate NULL class libraries for Memory and serial handlers from MdeModulePkg/Universal/StatusCodeHandler modules
Thanks for the link. I agree that this change will make the StatusCodeHandler driver more modular, and is a step in the right direction. But I think it could go further, with almost no additional work, and simplify the overall Status Code mechanism, not just the StatusCodeHandler driver. Currently, the StatusCodeHandler driver entry routines run some initialization code, register callbacks (eg. for ExitBootServices and SetVirtualAddressMap), and call the RscHandler PPI/Protocol to register the worker routines. If I'm understanding the proposal correctly, all that code will be moved to the individual NULL libraries, since any particular library may or may not need any of it. Then the StatusCodeHandler modules will be left with no code of their own at all: they will only be wrappers for the NULL libraries. Their entry routines will do nothing except return EFI_SUCCESS! (1) It seems strange and wasteful to keep around empty modules like this. So I'm suggesting adding the NULL libraries to the StatusCodeRouter modules instead. They would need to export the protocol/PPI routines to the NULL libraries via a header file, so they could call them directly instead of looking up the protocol/PPI. But that's a minor change. Then you could remove the empty StatusCodeHandler modules entirely. The advantage would be that there would be fewer modules in the build, simplifying the .dsc and .fdf files slightly. It would also reduce code size a bit by sharing common library routines, such as BaseLib, with the StatusCodeRouter modules. If those don't seem like worthwhile advantages, that's OK with me. I don't want to belabor the point, or impede progress. If others are OK with the proposal as it stands, then I am too. Thanks, *Brian J. Johnson *Enterprise X86 Lab Hewlett Packard Enterprise (1) The StatusCodeHandlerRuntimeDxe driver also handles PcdStatusCodeReplayIn as part of its entry code. That code would probably have to stay in a separate module rather than being linked to StatusCodeRouter as a NULL library. That way it could be dispatched after the ReportStatusCode protocol is available. *From:* Dong, Eric [mailto:eric.d...@intel.com] *Sent:* Thursday, June 25, 2020, 10:41 AM *To:* Brian J. Johnson , Bi, Dandan , Andrew Fish , edk2-devel-groups-io *Cc:* r...@edk2.groups.io , Ni, Ray , Wang, Jian J , Wu, Hao A , Tan, Ming , Laszlo Ersek *Subject:* [edk2-devel] [edk2-rfc] MdeModulePkg/StatusCodeHandler: Separate NULL class libraries for Memory and serial handlers from MdeModulePkg/Universal/StatusCodeHandler modules Hi Brian, In this new design, we still use register status code handler Protocol/Ppi to register the handler logic. We just want to change the StatusCodeHandler driver. We try to split the register logic to NULL library to make the code more modularity. We already created sample library in Edk2-Platforms repo https://github.com/tianocore/edk2-platforms/tree/master/Features/Intel/Debugging/PostCodeDebugFeaturePkg/Library/PostCodeStatusCodeHandlerLib. You can check this code to understand more about what we want to do. Thanks, Eric *From:* Brian J. Johnson *Sent:* Thursday, June 25, 2020 4:25 AM *To:* Bi, Dandan ; Andrew Fish ; edk2-devel-groups-io *Cc:* r...@edk2.groups.io; Dong, Eric ; Ni, Ray ; Wang, Jian J ; Wu, Hao A ; Tan, Ming ; Laszlo Ersek *Subject:* Re: [edk2-devel] [edk2-rfc] MdeModulePkg/StatusCodeHandler: Separate NULL class libraries for Memory and serial handlers from MdeModulePkg/Universal/StatusCodeHandler modules Dandan, The Status Code Protocol/PPI is the high-level interface which is being implemented. The ReportStatusCodeRouter module implements this in terms of the ReportStatusCodeHandler Protocol/PPI. That allows multiple ReportStatusCodeHandler modules to be used at once, so they can each react to different types of status codes, or report them through multiple channels. That sort of multiplexing seems like a useful feature. Now we're considering adding a mechanism which allows registering status code handlers via NULL libraries, rather than via the protocol/PPI. That sounds like exactly what ReportStatusCodeRouter is intended for. It wouldn't really change its scope, it would just make it more flexible. Adding this feature via a StatusCodeHandler module wouldn't improve modularity, it would just add complexity. As an OEM, adding a custom handler would look the same to me either way: I would have to add the NULL class library to a MdeModulePkg driver's entry in my .dsc file. It doesn't matter to me whether it's the ReportStatusCodeRouter or StatusCodeHandler module. And if I can do it in ReportStatusCodeRouter, then I don't need to include any StatusCodeHandler modules in the build at all. That saves code space and reduces the number of modules in the APRIORI list, which are both good things
Re: [edk2-devel] [edk2-rfc] MdeModulePkg/StatusCodeHandler: Separate NULL class libraries for Memory and serial handlers from MdeModulePkg/Universal/StatusCodeHandler modules
Dandan, The Status Code Protocol/PPI is the high-level interface which is being implemented. The ReportStatusCodeRouter module implements this in terms of the ReportStatusCodeHandler Protocol/PPI. That allows multiple ReportStatusCodeHandler modules to be used at once, so they can each react to different types of status codes, or report them through multiple channels. That sort of multiplexing seems like a useful feature. Now we're considering adding a mechanism which allows registering status code handlers via NULL libraries, rather than via the protocol/PPI. That sounds like exactly what ReportStatusCodeRouter is intended for. It wouldn't really change its scope, it would just make it more flexible. Adding this feature via a StatusCodeHandler module wouldn't improve modularity, it would just add complexity. As an OEM, adding a custom handler would look the same to me either way: I would have to add the NULL class library to a MdeModulePkg driver's entry in my .dsc file. It doesn't matter to me whether it's the ReportStatusCodeRouter or StatusCodeHandler module. And if I can do it in ReportStatusCodeRouter, then I don't need to include any StatusCodeHandler modules in the build at all. That saves code space and reduces the number of modules in the APRIORI list, which are both good things. ReportStatusCodeRouterPei already has to track registered handlers in PEI when running from ROM (it uses a HOB.) Tracking handlers from NULL libraries wouldn't be any harder. In fact, it looks like it could just export the Register() function to the NULL libraries, and they could call it in their constructors. I think using NULL libraries for status code handlers is a great idea. I'd just like to take that opportunity to reduce the complexity of the overall status code stack while we're at it. Thanks, *Brian J. Johnson *Enterprise X86 Lab Hewlett Packard Enterprise *From:* Bi, Dandan [mailto:dandan...@intel.com] *Sent:* Monday, June 22, 2020, 2:27 AM *To:* Andrew Fish , edk2-devel-groups-io , brian.john...@hpe.com *Cc:* r...@edk2.groups.io , Dong, Eric , Ni, Ray , Wang, Jian J , Wu, Hao A , Tan, Ming , Laszlo Ersek , Bi, Dandan *Subject:* [edk2-devel] [edk2-rfc] MdeModulePkg/StatusCodeHandler: Separate NULL class libraries for Memory and serial handlers from MdeModulePkg/Universal/StatusCodeHandler modules Hi Brian, Personally, I prefer to add the NULL class Library to StatusCodeHandler modules. 1. I think we should make the functionality of each module clear and separated. It may also be why we added ReportStatusCodeRouter and StatusCodeHandler modules in edk2 repo before. ReportStatusCodeRouter modules are responsible for producing Status Code Protocol/PPI and Report Status Code Handler Protocol/PPI, and StatusCodeHandler modules are responsible for producing handlers (Handlers can be provided by NULL class Libraries in this RFC). So, that’s why I don’t want to add the NULL class Library to ReportStatusCodeRouter modules directly, which change the functionality scope of existing modules. 2. I agree that we have a lot of layers of indirection now, but what we may gain is the good modularity. And you also mentioned that one or more StatusCodeHandler Modules may be used. We also want to achieve that only the StatusCodeHandler modules in MdeModulePkg can be used after this separation, platform can only add its own handler Libs to meet its requirement. 3. As Andrew mentioned below, if add the libraries to ReportStatusCodeRouter, there will be some issue we need to fix, which seems also make the code logic a little tricky to me. Thanks, Dandan *From:* Andrew Fish *Sent:* Saturday, June 20, 2020 2:04 AM *To:* edk2-devel-groups-io ; brian.john...@hpe.com *Cc:* Bi, Dandan ; r...@edk2.groups.io; Dong, Eric ; Ni, Ray ; Wang, Jian J ; Wu, Hao A ; Tan, Ming *Subject:* Re: [edk2-devel] [edk2-rfc] MdeModulePkg/StatusCodeHandler: Separate NULL class libraries for Memory and serial handlers from MdeModulePkg/Universal/StatusCodeHandler modules On Jun 19, 2020, at 10:29 AM, Brian J. Johnson mailto:brian.john...@hpe.com>wrote: On 6/18/20 2:01 AM, Dandan Bi wrote: Hi All, REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2816 <https://bugzilla.tianocore.org/show_bug.cgi?id=2816> We plan to separate two kinds of NULL class libraries for Memory and serial handlers from*MdeModulePkg/Universal/StatusCodeHandler/…/ StatusCodeHandlerPei/RuntimeDxe/Smm*modules. The benefit we want to gain from this separation is to 1) make the code clear and easy to maintain, 2) make platform flexible to choose any handler library they need, and it also can reduce image size since the unused handlers can be excluded. If you have an
Re: [edk2-devel] [edk2-rfc] MdeModulePkg/StatusCodeHandler: Separate NULL class libraries for Memory and serial handlers from MdeModulePkg/Universal/StatusCodeHandler modules
On 6/18/20 2:01 AM, Dandan Bi wrote:
Hi All,
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2816
<https://bugzilla.tianocore.org/show_bug.cgi?id=2816>
We plan to separate two kinds of NULL class libraries for Memory and
serial handlers from *MdeModulePkg/Universal/StatusCodeHandler/…/
StatusCodeHandlerPei/RuntimeDxe/Smm* modules.
The benefit we want to gain from this separation is to 1) make the
code clear and easy to maintain, 2) make platform flexible to choose
any handler library they need, and it also can reduce image size since
the unused handlers can be excluded.
If you have any concern or comments for this separation, please let me
know.
We plan to add new separated NULL class library
*MemoryStausCodeHandlerLib *and*SerialStatusCodeHandlerLib *with
different phase implementation into *MdeModulePkg\Library\* directory.
The main tree structure may like below:
MdeModulePkg\Library
|--*MemoryStausCodeHandlerLib*
|--|-- PeiMemoryStausCodeHandlerLib.inf
|--|-- RuntimeDxeMemoryStatusCodeHandlerLib.inf
|--|-- SmmMemoryStausCodeHandlerLib.inf
|--*SerialStatusCodeHandlerLib*
|--|-- PeiSerialStatusCodeHandlerLib.inf
|--|-- RuntimeDxeSerialStatusCodeHandlerLib.inf
|--|-- SmmSerialStatusCodeHandlerLib.inf
**
**
We will update existing platform use cases in edk2 and edk2-platform
repo to cover the new NULL class library to make sure this change
doesn’t impact any platform.
After this separation, StatusCodeHandler module usage will like below,
and it’s also very flexible for platform to cover more handler
libraries to meet their requirements.
MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.inf {
NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/PeiMemoryStausCodeHandlerLib.inf
NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/PeiSerialStatusCodeHandlerLib.inf
…
}
MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf
{
NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/RuntimeDxeMemoryStausCodeHandlerLib.inf
NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/RuntimeDxeSerialStatusCodeHandlerLib.inf
…
}
MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.inf {
NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/SmmMemoryStausCodeHandlerLib.inf
NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/SmmSerialStatusCodeHandlerLib.inf
…
}
Thanks,
Dandan
Dandan,
We'll have a lot of layers of indirection The
ReportStatusCodeRouter modules will call one or more
StatusCodeHandlerModules, and the standard StatusCodeHandler modules
will call multiple StatusCodeHandlerLib libraries.
How about adding StatusCodeHandlerLib support directly to the
ReportStatusCodeRouter modules? Then platforms could omit the
StatusCodeHandler modules if they're only using the open-source code.
That sounds like less overhead since fewer modules would be needed.
Thanks,
--
*Brian J. Johnson
*Enterprise X86 Lab
Hewlett Packard Enterprise
*hpe.com* <3D"hpe.com">
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#61533): https://edk2.groups.io/g/devel/message/61533
Mute This Topic: https://groups.io/mt/74953841/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v7 00/43] SEV-ES guest support
FWIW, I kind of like the idea of having a way to provide an exception handler in all situations via a library. We use a custom fatal exception handler which prints extra debugging information. So currently we have to modify CpuExceptionHandlerLib, or explicitly hook every exception as early as we can. Neither is ideal: modifying the library becomes a maintenance burden, and hooking the exceptions via RegisterInterruptHandler() has limitations, as Ray pointed out. I think that shows that we don't currently have a good way to write a "system" exception handler to provide underlying support to the rest of BIOS. CpuExceptionHandlerLib is large and complex, but monolithic. I don't have a solution to propose, but maybe it will get someone thinking -- Brian J. Johnson Enterprise X86 Lab Hewlett Packard Enterprise brian.john...@hpe.com Original Message From: Lendacky, Thomas [mailto:thomas.lenda...@amd.com] Sent: Friday, May 15, 2020, 9:30 AM To: Ni, Ray , devel@edk2.groups.io , af...@apple.com Cc: Justen, Jordan L , Laszlo Ersek , Ard Biesheuvel , Kinney, Michael D , Gao, Liming , Dong, Eric , Brijesh Singh , You, Benjamin , Bi, Dandan , Dong, Guo , Wu, Hao A , Wang, Jian J , Ma, Maurice , Fan Jeff Subject: [edk2-devel] [PATCH v7 00/43] SEV-ES guest support On 5/15/20 12:47 AM, Ni, Ray wrote: I just realized my solution doesn't cover some scenarios: 1. SMM 2. S3 boot path 3. CapsuleX64 If we want to hook #29 in all scenarios, your way directly modifying CpuExceptionHandlerLib is the easiest way because RegisterInterruptHandler() is supported only in DXE/SMM case. There is no way to use RegisterXXX() API for #2 and #3 because PEI instance doesn't support. Do we need to hook in all scenarios? What instructions could cause #29? A #VC can only be genereted by an SEV-ES guest, so in the standard case the hook will never be called. For an SEV-ES guest, a number of different instructions can cause a #VC. These include IO instructions (such as used to output debug message to the serial port), CPUID instructions, anything performing MMIO, RDMSR/WRMSR instructions, etc. So we need to hook it in all SEV-ES scenarios. To eliminate all of the handler code in the standard case, I'm planning on providing a NULL VmgExitLib library that has a (near) empty #VC handler so that the full #VC handler code will only be in the Ovmf package. I don't see much difference between the new way introducing OverrideCpuExceptionHandler () and directly modifying the CpuExceptionHandlerLib. Both ways modifies the library. Introducing OverrideCpuExceptionHandler() might be worse because it creates an interface which encourages anyone to hook any exceptions. Your current way only hooks #29. Ok, I can go back to the explicit check for exception #29. I'll work to get these changes and changes from other feedback into the next version and out for review early next week. Thanks for taking the time to work through this with me! Tom Thanks, Ray -Original Message- From: devel@edk2.groups.io On Behalf Of Lendacky, Thomas Sent: Friday, May 15, 2020 1:59 AM To: Ni, Ray ; devel@edk2.groups.io; af...@apple.com Cc: Justen, Jordan L ; Laszlo Ersek ; Ard Biesheuvel ; Kinney, Michael D ; Gao, Liming ; Dong, Eric ; Brijesh Singh ; You, Benjamin ; Bi, Dandan ; Dong, Guo ; Wu, Hao A ; Wang, Jian J ; Ma, Maurice ; Fan Jeff Subject: Re: [edk2-devel] [PATCH v7 00/43] SEV-ES guest support On 5/14/20 8:10 AM, Ni, Ray wrote: Tom, Hi Ray, I just discussed with original CPU owner Jeff and went through how IDT is setup in the boot flow. Here is what I think you can do to avoid modifying the CpuExceptionHandlerLib. 1. SecPlatformMain() modifies IDT[29] to point to your VC handler. This step helps to build the VC handler in whole 32bit mode SEC+PEI. That can probably be done, but duplicates a lot of code - all of the exception entry assembler code. Additionally, UefiCpuPkg/CpuMpPei/CpuMpPei.c will also invoke InitializeCpuExceptionHandlers() registering a new IDT[29] entry. 2. Create a new DXE driver with dependency set to TRUE and call RegisterCpuInteruptHandler(29, xx) in its entrypoint to register VC handler for whole 64bit mode DXE. 3. Platform FDF uses apriori file mechanism to make sure the driver created in step #2 is dispatched as the 1st driver in DXE phase. This step is optional if you accept there is some time that VC handler is not setup in early DXE phase. Tracing the execution of an apriori driver shows that this happens after DXE has initialized its exception handler and #VCs occur before a handler can be reigstered by the new driver, causing a failure. 4. In the new DXE driver, gets the EFI_VECTOR_HANDOFF_INFO (MdePkg\Include\Ppi\VectorHandoffInfo.h) from configuration table. It reports failure if the vector_handoff table says DO_NOT_HOOK for #29. It re-produces vector_handoff table with #29 set to DO_NOT_HOOK so that no
Re: [edk2-devel] [PATCH V4 00/27] Disabling safe string constraint assertions
As am I -- I don't see that they add value. Brian Original Message From: Bret Barkelew via groups.io [mailto:bret.barkelew=microsoft@groups.io] Sent: Wednesday, May 13, 2020, 9:41 AM To: devel@edk2.groups.io , ler...@redhat.com , Kinney, Michael D , Vitaly Cheptsov Cc: Andrew Fish , Marvin Häuser , liming.gao , Gao, Zhichao Subject: [EXTERNAL] Re: [edk2-devel] [PATCH V4 00/27] Disabling safe string constraint assertions O. Does that mean we get to start on DebugLibEx? In all seriousness, I’m also in the camp of “can’t we just drop these assertions”? - Bret *From:* devel@edk2.groups.io on behalf of Laszlo Ersek via groups.io *Sent:* Wednesday, May 13, 2020 2:16:12 AM *To:* Kinney, Michael D ; devel@edk2.groups.io ; Vitaly Cheptsov *Cc:* Andrew Fish ; Marvin Häuser ; liming.gao ; Gao, Zhichao *Subject:* [EXTERNAL] Re: [edk2-devel] [PATCH V4 00/27] Disabling safe string constraint assertions Hi Mike, On 05/12/20 20:18, Kinney, Michael D wrote: What if there is a DebugLib implementation of the DebugLib class that does not depend on DebugCommonLib. There need not be a link failure in this case either, if the DebugLib instance in question provides the DebugCommonLib API implementations too. Anyway I don't want to obsess about this. I'm just sad there are zero acceptable solutions apparently to the 100% valid problem statement that Vitaly submitted last August, in TianoCore#2054. (Asserting properties of untrusted external data is *asinine*.) But then, if Vitaly proposes to update all DebugLib instances one by one, that gets shot down because "too many DebugLib instances in platforms". And if Vitaly extracts the common bits so that only the common bits have to be updated, that gets shot down by "we don't support this kind of dependency, please update all DebugLib instances instead". Let's just be honest and call DebugLib frozen forever. Laszlo -- Brian "We are Microsoft. UNIX is irrelevant. OS/2 is irrelevant. Openness is futile. Prepare to be assimilated." -- p...@turing.org -- (quoted by Eric Berggren) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#59458): https://edk2.groups.io/g/devel/message/59458 Mute This Topic: https://groups.io/mt/74190937/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFC][PATCH v1] UefiCpuPkg/MpInitLib DXE: Reduce AP status check interval
On 3/23/20 9:37 AM, Ni, Ray wrote: Laszlo, Adding a PCD means platform integrators need to consider which value to set. Most of the time, they may just use the default PCD value. Then, why not we add the PCD later when a real case is met? The patch changes existent behavior; it is not for a newly introduced feature. Because most platforms are not in the edk2 tree, we don't know what platforms could be regressed by increasing the polling frequency tenfold. (And remember that the polling action has O(n) cost, where "n" is the logical processor count.) Under your suggestion, the expression "real case is met" amounts to "someone reports a regression" (possibly after the next stable tag, even). I don't think that's a good idea. In particular, the patch is motivated by RegisterCpuFeaturesLib -- the CpuFeaturesInitialize() function -- on some platform(s) that Hao uses. But there are platforms that don't use RegisterCpuFeaturesLib, and still use MpInitLib. OK. I agree with your suggestion. Thank you. I agree with Laszlo: MP initialization is tricky to scale, and platform dependent. My group builds real machines with thousands of APs, and we have had to do various tweaks to the MP init. code. Having a PCD to adjust this timeout will be very useful. Thanks, -- Brian J. Johnson Enterprise X86 Lab Hewlett Packard Enterprise brian.john...@hpe.com -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#56112): https://edk2.groups.io/g/devel/message/56112 Mute This Topic: https://groups.io/mt/71925352/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 6/6] OvmfPkg IA32: add support for loading X64 images
On 2/26/20 1:43 PM, Ard Biesheuvel wrote:
This is the UEFI counterpart to my Linux series which generalizes
mixed mode support into a feature that requires very little internal
knowledge about the architecture specifics of booting Linux on the
part of the bootloader or firmware.
Instead, we add a .compat PE/COFF header containing an array of
PE_COMPAT nodes containing tuples that
describe alternate entrypoints into the image for different native
machine types, e.g., IA-32 in a 64-bit image so it can be booted
from IA-32 firmware.
This patch implements the PE/COFF emulator protocol to take this new
section into account, so that such images can simply be loaded via
LoadImage/StartImage, e.g., straight from the shell.
This feature is based on the EDK2 specific PE/COFF emulator protocol
that was introduced in commit 57df17fe26cd ("MdeModulePkg/DxeCore:
invoke the emulator protocol for foreign images", 2019-04-14).
Signed-off-by: Ard Biesheuvel
Acked-by: Laszlo Ersek
---
OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c | 139
OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf | 36 +
OvmfPkg/OvmfPkgIa32.dsc | 5 +
OvmfPkg/OvmfPkgIa32.fdf | 4 +
4 files changed, 184 insertions(+)
diff --git a/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c
b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c
new file mode 100644
index ..6dc07f467752
--- /dev/null
+++ b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c
@@ -0,0 +1,139 @@
+/** @file
+ * PE/COFF emulator protocol implementation to start Linux kernel
+ * images from non-native firmware
+ *
+ * Copyright (c) 2020, ARM Ltd. All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-2-Clause-Patent
+ *
+ */
+
+#include
+
+#include
+#include
+#include
+
+#include
+
+#pragma pack (1)
+typedef struct {
+ UINT8 Type;
+ UINT8 Size;
+ UINT16 MachineType;
+ UINT32 EntryPoint;
+} PE_COMPAT_TYPE1;
+#pragma pack ()
+
+STATIC
+BOOLEAN
+EFIAPI
+IsImageSupported (
+ IN EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL*This,
+ IN UINT16 ImageType,
+ IN EFI_DEVICE_PATH_PROTOCOL*DevicePath OPTIONAL
+ )
+{
+ return ImageType == EFI_IMAGE_SUBSYSTEM_EFI_APPLICATION;
+}
+
+STATIC
+EFI_IMAGE_ENTRY_POINT
+EFIAPI
+GetCompatEntryPoint (
+ IN EFI_PHYSICAL_ADDRESS ImageBase
+ )
+{
+ EFI_IMAGE_DOS_HEADER *DosHdr;
+ UINTN PeCoffHeaderOffset;
+ EFI_IMAGE_NT_HEADERS32*Pe32;
+ EFI_IMAGE_SECTION_HEADER *Section;
+ UINTN NumberOfSections;
+ PE_COMPAT_TYPE1 *PeCompat;
+
+ DosHdr = (EFI_IMAGE_DOS_HEADER *)(UINTN)ImageBase;
+ if (DosHdr->e_magic != EFI_IMAGE_DOS_SIGNATURE) {
+return NULL;
+ }
+
+ PeCoffHeaderOffset = DosHdr->e_lfanew;
+ Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINTN)ImageBase + PeCoffHeaderOffset);
+
+ Section = (EFI_IMAGE_SECTION_HEADER *)((UINTN)>OptionalHeader +
+
Pe32->FileHeader.SizeOfOptionalHeader);
+ NumberOfSections = (UINTN)Pe32->FileHeader.NumberOfSections;
+
+ while (NumberOfSections--) {
+if (!CompareMem (Section->Name, ".compat", sizeof (Section->Name))) {
+ //
+ // Dereference the section contents to find the mixed mode entry point
+ //
+ PeCompat = (PE_COMPAT_TYPE1 *)((UINTN)ImageBase +
Section->VirtualAddress);
+
+ while (PeCompat->Type != 0) {
+if (PeCompat->Type == 1 &&
+PeCompat->Size >= sizeof (PE_COMPAT_TYPE1) &&
+EFI_IMAGE_MACHINE_TYPE_SUPPORTED (PeCompat->MachineType)) {
+
+ return (EFI_IMAGE_ENTRY_POINT)((UINTN)ImageBase +
PeCompat->EntryPoint);
+}
+PeCompat = (PE_COMPAT_TYPE1 *)((UINTN)PeCompat + PeCompat->Size);
+ }
Ard,
Cool patch series!
I'm not an official reviewer, but I'd feel better about this patch if
you added a condition to exit the "while (PeCompat->Type != 0)" loop if
PeCompat ever gets pointed outside of the section. Otherwise a
malformed or corrupted .compat section could send you off dereferencing
anything at all.
Similarly, it wouldn't hurt to sanity check the header fields, such as
e_lfanew, OptionalHeader, SizeOfOptionalHeader, and NumberOfSections (or
at least verify that all pointers you calculate from them point within
the overall image. Or has that already been done by the PeCoff loader
by the time this code is called?
Thanks,
Brian J. Johnson
+}
+Section++;
+ }
+ return NULL;
+}
+
+STATIC
+EFI_STATUS
+EFIAPI
+RegisterImage (
+ IN EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL*This,
+ IN EFI_PHYSICAL_ADDRESSImageBase,
+ IN UINT64 ImageSize,
+
Re: [edk2-devel] [Patch 0/2] UefiCpuPkg: Default avoid print.
uld require all DebugLib instances to change, which is something you were trying to avoid. However, it's not always practical to track down all uses of DEBUG(). An AP can easily call a library routine which uses DEBUG() rather than AP_DEBUG(), buried under several layers of transitive library dependencies. In other words, it's not always practical to determine ahead of time if a given DEBUG() call may be done on an AP. I know that AP code runs in a very restricted environment and that people who use MpServices are supposed to understand the repercussions, but it gets very difficult when libraries are involved. :( So would a better solution be to modify the common unsafe DebugLib instances to have DebugPrintEnabled() return FALSE on APs? That would probably require a new BaseLib interface to determine if the caller is running on the BSP or an AP. (For IA32/X64 this isn't too hard -- it just needs to check a bit in the local APIC. I have no idea about other architectures.) That wouldn't solve the problem everywhere -- anyone using a custom DebugLib would have to update it themselves. But it would solve it solidly in the majority of cases. Thoughts? -- Brian J. Johnson Enterprise X86 Lab Hewlett Packard Enterprise -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44685): https://edk2.groups.io/g/devel/message/44685 Mute This Topic: https://groups.io/mt/32664465/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] TianoCore Community Design Meeting Minutes
On 5/2/19 2:33 PM, sean.brogan via groups.io wrote: Brian, I would really like to hear about the challenges your team faced and issues that caused those solutions to be unworkable. Project Mu has and continues to invest a lot in testing capabilities, build automation, and finding ways to improve quality that scale. Our products depend on a reference BIOS tree provided to us by a major processor vendor. That tree includes portions of Edk2, plus numerous proprietary additions. Each new platform starts with a new drop of vendor code. They provide additional drops throughout the platform's life. In the past these were distributed as zip files, but more recently they have transitioned to git. We end up having to make extensive changes to their code to port it to our platform. In addition, we maintain internally several packages of code used on all our platforms, designed to be platform-independent, plus a platform-dependent package which is intended to be modified for each platform. When we first started using git, we looked for a way to share our all-platform code among platforms, and move our platform-dependent code easily to new platforms, while making it easy to integrate new drops from our vendor. We considered using git submodules, but decided that would be too awkward. Modifying code in a submodule involves committing in the submodule, then committing in the module containing it. This seemed like too much trouble for our developers, who were all new to git. Plus, it didn't interact well at all with our internal bug tracking system. Basically, there was no good way to tie commits in various sub- and super-modules together in a straightforward, trackable way. We tried a package called gitslave (http://gitslave.sourceforge.net/), which automates running git commands across a super-repo and various sub-repos, with some sugar for aggregating the results into a readable whole. It's a bit more transparent than submodules. But at the end of the day, you're still trying to coordinate multiple git repositories. We gave it a try for a month or two, but having to manage multiple repositories for day-to-day work, and the lack of a single commit history spanning the entire tree doomed that scheme. Developers rebelled. Ever since, we've used a single git repo per platform. We keep the vendor code in a "base" branch, which we update as they provide drops, then merge into our master branch. When we start a new platform, we use git filter-branch to extract our all-platform and platform-dependent code into a new branch, which we move to the new platform's repo and merge into master. It's possible to re-extract the code if we need to pick up updates. This doesn't provide total flexibility... for instance, backporting a fix in our all-platform code back to a previous platform involves manual cherrypicking. But for day-to-day development, it lets us work in a single git tree, with a bisectable history, working git-blame, commit IDs which tie directly to our bug tracker, and no external tooling. It's a bit of a pain to merge a new drop (shell scripts are our friends), but we're optimizing for ease of local development. That seems like the best use of our resources. So I'm leery of any scheme which involves multiple repos managed by an external tool. It sounds like a difficult way to do day-to-day development. If Edk2 does move to split repos, we could filter-branch and merge them all together into a single branch for internal use, I suppose. But that does make it harder to push fixes upstream. (Not that we end up doing a lot of that... we're not developing an open-source BIOS, just making use of open-source upstream components. So our use case is quite a bit different from Laszlo's.) We're also generally focusing on one platform at a time, not trying to update shared code across many at once. So our use case may be different from Sean's. This got rather long... I hope it helps explain where we're coming from. -- Brian J. Johnson Enterprise X86 Lab Hewlett Packard Enterprise brian.john...@hpe.com -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#39961): https://edk2.groups.io/g/devel/message/39961 Mute This Topic: https://groups.io/mt/31242794/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 2/6] MdePkg/BaseLib: Use PcdSpeculationBarrierType
On 4/29/19 8:30 PM, Michael D Kinney wrote:
Use PcdSpeculationBarrierType in the x86 implementation
of SpeculationBarrier() to select between AsmLfence(),
AsmCpuid(), and no operation.
Cc: Liming Gao
Signed-off-by: Michael D Kinney
---
MdePkg/Library/BaseLib/BaseLib.inf | 1 +
MdePkg/Library/BaseLib/X86SpeculationBarrier.c | 8 ++--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/MdePkg/Library/BaseLib/BaseLib.inf
b/MdePkg/Library/BaseLib/BaseLib.inf
index 533e83e0b2..3586beb0ab 100644
--- a/MdePkg/Library/BaseLib/BaseLib.inf
+++ b/MdePkg/Library/BaseLib/BaseLib.inf
@@ -394,6 +394,7 @@ [Pcd]
gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength ##
SOMETIMES_CONSUMES
gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength ##
SOMETIMES_CONSUMES
gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPropertyMask ##
SOMETIMES_CONSUMES
+ gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType ##
SOMETIMES_CONSUMES
[FeaturePcd]
gEfiMdePkgTokenSpaceGuid.PcdVerifyNodeInList ## CONSUMES
diff --git a/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
b/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
index 8e5f983bb8..b28fd8de9b 100644
--- a/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
+++ b/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
@@ -1,7 +1,7 @@
/** @file
SpeculationBarrier() function for IA32 and x64.
- Copyright (C) 2018, Intel Corporation. All rights reserved.
+ Copyright (C) 2018 - 2019, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -22,5 +22,9 @@ SpeculationBarrier (
VOID
)
{
- AsmLfence ();
+ if (PcdGet8 (PcdSpeculationBarrierType) == 0x01) {
+AsmLfence ();
+ } else if (PcdGet8 (PcdSpeculationBarrierType) == 0x02) {
+AsmCpuid (0x01, NULL, NULL, NULL, NULL);
+ }
}
Looks good. I'm not a maintainer, but FWIW:
Reviewed-by: Brian J. Johnson
--
Brian J. Johnson
Enterprise X86 Lab
Hewlett Packard Enterprise
brian.john...@hpe.com
+1 651 683 7521 Office
Eagan, MN
hpe.com
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#39883): https://edk2.groups.io/g/devel/message/39883
Mute This Topic: https://groups.io/mt/31415901/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/4] MdePkg/BaseLib: Verify SSE2 support in IA32 AsmLfence()
On 4/26/19 3:27 PM, Laszlo Ersek wrote: On 04/25/19 19:53, Michael D Kinney wrote: Use CPUID in IA32 implementation of AsmLfence() to verify that SSE2 is supported before using the LFENCE instruction. Cc: Liming Gao Signed-off-by: Michael D Kinney --- MdePkg/Library/BaseLib/Ia32/Lfence.nasm | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/MdePkg/Library/BaseLib/Ia32/Lfence.nasm b/MdePkg/Library/BaseLib/Ia32/Lfence.nasm index 44478be35f..0a60ae1d57 100644 --- a/MdePkg/Library/BaseLib/Ia32/Lfence.nasm +++ b/MdePkg/Library/BaseLib/Ia32/Lfence.nasm @@ -1,5 +1,5 @@ ;-- ; -; Copyright (c) 2018, Intel Corporation. All rights reserved. +; Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved. ; SPDX-License-Identifier: BSD-2-Clause-Patent ; ; Module Name: @@ -26,5 +26,17 @@ ;-- global ASM_PFX(AsmLfence) ASM_PFX(AsmLfence): +; +; Use CPUID instruction (CPUID.01H:EDX.SSE2[bit 26] = 1) to test whether the +; processor supports SSE2 instruction. Save/restore non-volatile register +; EBX that is modified by CPUID +; +pushebx +mov eax, 1 +cpuid +bt edx, 26 +jnc Done lfence +Done: +pop ebx ret The SDM seems to confirm that lfence depends on SSE2. However, that raises another question: AsmLfence() is used for implementing SpeculationBarrier() on IA32/X64. And so I wonder: the plaforms where lfence is unavailable, do they *not* need a speculation barrier at all, or do they need a *different* implementation? Thanks, Laszlo Several issues: This patch introduces stronger fencing than is required. The SDM says, "Reads or writes cannot be reordered with I/O instructions, locked instructions, or serializing instructions." (vol 3a, sec 8.2.2.) The cpuid instruction is a "serializing instruction" (sec 8.3). So the cpuid is essentially a load fence plus a store fence (plus other functionality, such as draining the memory queues.) That makes the lfence following it redundant. Cpuid is a fairly heavyweight instruction due to its serializing behavior. It provides the load fencing semantics of lfence, but can introduce a significant performance hit if it's called often. Lfence is a lot lighter weight instruction. So using cpuid in AsmLfence may make it a lot slower than the caller expects. Also, skipping a fencing instruction if it's not supported doesn't seem like the right approach in any case. The caller expects AsmLfence to provide certain fencing semantics... the implementation isn't free to just do nothing (unless it's on an architecture where load fencing is not required, since memory is always ordered.) Plus, the routine is called "AsmLfence", so I'd expect it to always use lfence, and cause an exception if the instruction isn't implemented. I'd think the callers should be modified to call AsmLfence or routines using other instructions (such as cpuid) to provide fencing according to the CPU architecture they are on. Is there a way to make this a compile-time decision? I haven't tried to track down all the callers of AsmLfence Thanks, -- Brian J. Johnson Enterprise X86 Lab Hewlett Packard Enterprise -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#39666): https://edk2.groups.io/g/devel/message/39666 Mute This Topic: https://groups.io/mt/31345225/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] TianoCore Community Design Meeting Minutes
On 4/23/19 1:22 PM, Laszlo Ersek wrote: On 04/19/19 07:55, Ni, Ray wrote: Hi everyone, In the first design meeting, Matthew and Sean from Microsoft presented the Mu tools. Below are some notes Mike and I captured from the meeting. Please reply to this mail for any questions and comments. Matthew Carlson / Sean Brogan - Microsoft - Project Mu Tools https://urldefense.proofpoint.com/v2/url?u=https-3A__edk2.groups.io_g_devel_files_Designs_2019_0418_2019-2D04-2D18-2520Microsoft-2520-2D-2520Build-2520Tools-2520-2D-2520Design-2520Review-2520.pdf=DwIC-g=C5b8zRQO1miGmBeVZ2LFWg=joEypYTP_0CJDmGFXzPM2s0mxEmiZkE9j8XY2t0muB0=JrIFm-OW7EUMJO_bZcr5RkYsyHrao3YmmSYnCOCMAAg=f18bByZUCGrcf2VKMVUAoPNNBz2TKQFLJw1BNphrDc0= I've checked the slides; I'd like to comment on / ask about one particular topic. The following three items relate to that topic: (1): Background [...] - Splitting the code: A platform only needs to see the code the platform uses to build. (2): Build a platform through PlatformBuild.py - Starts with ~1% of platform code - Dependencies resolving phase pulls additional platform code * Multiple GIT repos are needed by platform. The dep resolving phase simplifies the code setup. "setup" phase is isolated and can be skipped or replaced with other similar tools. (3): slide 25 / 34: How do you discover what repos you need? Platforms define what they need to build and SDE finds it and "SDE" is explained earlier on slide 22 / 34, "Self Describing Environment": Verifies dependencies declared thru ext_deps and updates as needed While I agree that a platform need not "see" more code than it requires for being built, the platform is also not *hurt* by seeing more than it strictly requires. On the other hand, under a split repos approach, how are inter-dependencies (between sub-repos) tracked, and navigated? Assume there is a regression (encountered in platform testing) -- how do you narrow down the issue to a specific commit of a specific sub-repo? And, how do you automate said narrowing-down? In a common git repository / with a common git history, the inter-dependencies are tracked implicitly, and they aren't hard to navigate, manually or automatically. Such navigation doesn't need external tooling; it's all built into git (for example into "git checkout" and "git bisect"). git supports submodules internally, but that feature exists to mirror the boundaries that already exist between developer communities. For example, OpenSSL's developer community and edk2's developer community, are mostly distinct. Their workflows differ, their release criteria differ, their testing expectations differ, so it makes sense for edk2 to consume OpenSSL via a submodule. But, I don't think the same applies to core modules in e.g. MdeModulePkg and UefiCpuPkg, vs. *open* firmware platforms. Those development communities overlap (or should overlap) to a good extent; we shouldn't fragment them by splitting repos. (Separate subsystem repos and mailing lists are fine as long as everything is git-merged ultimately into the central repo.) Note: I'm not arguing what Project Mu should do for its own sake. I'm arguing against adopting some Project Mu workflow bits for upstream edk2, at the level I currently understand those workflow bits. My understanding of Project Mu could be very lacking. (I missed the design meeting due to an unresolvable, permanent conflict.) Slide 12/34 says, "Next Steps: Propose RFC to TianoCore community: Create 3 git repositories". I hope I can check that out in more detail. Thanks, Laszlo I noticed similar things, and agree with Laszlo's points. My group has attempted to develop a complex edk2-based project using multiple repos and some external tooling in the past, and found it completely unworkable. Perhaps Project Mu's tooling is better than ours was. But for modules which are developed together by the same group of people, keeping all the code in a single git repo lets you make the best use of git, and removes a lot of room for errors when committing code across multiple modules. -- Brian J. Johnson Enterprise X86 Lab Hewlett Packard Enterprise -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#39428): https://edk2.groups.io/g/devel/message/39428 Mute This Topic: https://groups.io/mt/31242794/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
