On 07/07/16 10:14, Fan, Jeff wrote: > Laszlo, > > Ovmf is just required to build resource HOB as blew on S3 path. > AddMemoryRangeHob (0, BASE_512KB + BASE_128KB);
That could work for me, yes (I'm about to test it). > And It is responsibility of consumer (like CpuMpPei or other PEI module) to > build allocation HOB. In general I agree with this, but on the S3 path, I disagree. On the S3 path, that memory is *not free* for consumption. Any random PEIM is *not allowed* to allocate that memory and just scribble over it. The memory belongs to the OS. If I installed the memory HOB only, and did not install an allocation HOB for it as well, then any random PEIM could think that the memory is free for the taking. The PEIM would be right, and OVMF would be completely wrong to allow that on the S3 resume path. We need a dedicated way to say that a given memory range is okay to use temporarily, but *only if* the contents are restored at End-of-PEI. Thanks Laszlo > > Thanks! > Jeff > > -----Original Message----- > From: Laszlo Ersek [mailto:[email protected]] > Sent: Thursday, July 07, 2016 3:04 PM > To: Fan, Jeff; Kinney, Michael D; Justen, Jordan L > Cc: Paolo Bonzini; edk2-devel-01 > Subject: Re: [edk2] multiprocessing in PEI? > > On 07/07/16 04:52, Fan, Jeff wrote: >> Laszlo, >> >> On PEI phase, whatever it is normal boot or S3 boot, PEI memory allocation >> services should not allocate memory < 1 MB address. >> >> If any other PEIM module wants to allocate memory < 1 MB address, it has to >> scan Memory resource HOB to find the available memory range and build memory >> allocation HOB like what CpuMpPei did. >> >> If this method is OK, CpuMpPei or other module are required to check >> existing both memory resource HOB and memory allocation HOB. >> >> If we just borrow one piece of range < 1 MB address around each of >> INIT-SIPI-SIPI to AP, we still need to get the available memory address. >> >> So, platform still needs to build memory resource HOB < 1MB on S3 boot like >> normal boot. I think this update on platform should be simpler than new >> pair of PCDs introducing. >> >> What do you think of it? > > * First, at this point I'm now thinking that "borrowing" and "allocating" are > actually separate things: > > (a) For allocating memory under 1MB, scanning both the memory resource > HOBs and the memory allocation HOBs would be necessary, yes. > > (b) However, in case the memory under 1MB is only borrowed (and the > backup buffer is allocated with the PEI serivces), the HOB scanning > should not be necessary at all (for any kinds of HOBs). CpuMpPei > is intimately tied to Intel real mode anyway (hence the < 1MB > requirement in the first place), so why can't CpuMpPei just borrow > a few pages at a fixed low address (under 640KB)? That memory is > required to exist on platforms where CpuMpPei is supposed to run, > isn't it? > > * Second, the reason I'm reluctant to produce the same set of memory resource > and alloc HOBs on the S3 resume path as on the normal boot path is the > following: > > In my opinion, that would be misleading. Other PEIMs that execute (a) -- that > is, allocation and not borrowing -- might think that any memory that is > covered by a resource descriptor HOB, but not covered by any allocation HOBs, > is "free memory". While in fact that memory could well be used by the OS. > > In other words, the reason why we don't produce any of these HOBs is exactly > to communicate that there is zero free memory *for allocation*, outside of > the PEI memory services. Any PEIM that wants to allocate (as opposed to > borrow) *should* fail, unless it uses the PEI memory services. And if a PEIM > wants to *borrow* (as opposed to allocate), then it shouldn't need to scan > HOBs. > > * If there is a requirement in the PI spec that the HOBs be produced on the > S3 path as well, then I guess we'll have to comply with that. With my current > understanding though, I feel that producing the same set of HOBs would give > other PEIMs an incorrect view of memory ownership. The result of > > all system memory HOBs minus all allocation HOBs > > would appear as > > memory that is free for further allocation > > which would be false. > > * So my proposal would be to allow the platform to advertize a range for > CpuMpPei to borrow from, with two new PCDs. > > If you are strongly opposed to that, then I guess I can write a patch for > OvmfPkg that, on the S3 resume path, produces one (1) system memory HOB as an > *exception*, specifically to allow CpuMpPei's scanning to succeed (with an > appropriate code comment of course). I could also cover the entire area in > question with a memory allocation HOB as well, hoping that it would keep away > other PEIMs that look for "free memory", like in (a). Something like: > > // > // Allow CpuMpPei to borrow memory from this range for the AP startup > // code, but prevent other PEIMs from thinking the range is "free > // memory". > // > AddMemoryRangeHob (0, BASE_512KB + BASE_128KB); > BuildMemoryAllocationHob (0, BASE_512KB + BASE_128KB, > EfiBootServicesData); > > Actually, let me try that, and see where I land. > > Thanks, > Laszlo > > >> -----Original Message----- >> From: Laszlo Ersek [mailto:[email protected]] >> Sent: Thursday, July 07, 2016 2:52 AM >> To: Fan, Jeff; Kinney, Michael D; Justen, Jordan L >> Cc: Paolo Bonzini; edk2-devel-01 >> Subject: Re: [edk2] multiprocessing in PEI? >> >> On 07/06/16 19:08, Laszlo Ersek wrote: >>> Hi Jeff, >>> >>> On 07/05/16 15:50, Fan, Jeff wrote: >>>> When CPU MP PPI installed, CPU MP PPI Services will be fully usable. >>> >>> I included CpuMpPei in OVMF, and it works fine on the normal boot path. >>> (I haven't done anything special with it thus far, just built it into >>> the firmware, and it starts up fine.) >>> >>> However, on the S3 resume path, it runs into an assertion failure: >>> the >>> GetWakeupBuffer() function fails. That is actually expected, because >>> at that point the runtime OS being resumed owns all memory under 1MB. >>> >>> On the S3 resume path, OVMF does install a tiny bit of permanent PEI >>> RAM: 32 KB in size (PcdS3AcpiReservedMemorySize), starting at 8288 KB >>> (PcdS3AcpiReservedMemoryBase). On the normal boot path, we reserve >>> this area as AcpiNVS type memory, so that the runtime OS stays out of it. >>> >>> Is CpuMpPei meant to be used on the S3 resume path in general? >> >> It looks like CpuMpPei expects memory resource descriptor HOBs to exist on >> the S3 resume path as well. Then it seems to lay the wakeup buffer over >> OS-owned memory under 1MB. But, it saves the original memory contents into a >> backup buffer allocated from permanent PEI RAM. And, at end-of-PEI, the >> memory contents are restored. >> >> OVMF does not produce memory / IO / MMIO resource descriptors on the S3 >> resume path. The assumption is that no PEIM will directly look for any such >> HOBs on the S3 path, only use the permament PEI RAM for memory allocation. >> Does this conflict with the PI spec? >> >> I wonder if GetWakeupBuffer()'s scanning of EFI_RESOURCE_SYSTEM_MEMORY >> descriptor HOBs is a good idea. When it finds a location that is suitable as >> a wakeup buffer, it "claims" that area by building a memory allocation HOB >> that covers the area. I think that's fine; however, the scanning itself does >> not consider any other memory allocation HOBs that may have been carved out >> of system memory by other PEIMs, earlier. >> >> I wonder if platforms would be better served by a (base, size) pair of PCDs. >> The platform could set these PCDs, and make sure that both the OS and other >> firmware modules (PEIMs and DXE drivers) stay out of this area. (For >> example, if S3 is not supported or enabled on the platform, then the >> platform could allocate this area as Boot Services Data. >> Otherwise, it would be AcpiNVS.) CpuMpPei could then remove the >> resource HOB scanning, and it would only need to validate if (a) the >> size PCD was big enough to contain the wakeup buffer, and (b) the base >> PCD was low enough to keep the wakeup buffer under 1MB, (c) any >> necessary alignments. Basically, ask the platform to preallocate the >> wakeup buffer. (The minimal suitable size can be found statically, by >> trial and >> error.) The backup buffer (in permanent PEI RAM) could also be dropped, >> decreasing footprint. >> >> What do you think? >> >> Thanks >> Laszlo >> > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
