On Wed, Jun 6, 2018 at 4:02 PM, Laszlo Ersek <ler...@redhat.com> wrote:
> On 06/06/18 05:05, Prerna wrote: > > Hi Laszlo, > > I noticed that the following commit was used to make the 4MB flash layout > > the default for edk2 builds briefly: > > > > bba8dfbec3bb OvmfPkg: make the 4MB flash size the default > > > > However, it was later reverted : > > > > 6e49d01cfb43 Revert "OvmfPkg: make the 4MB flash size the default" > > > > presumably to fix this issue: > > > > 0c79471d6a98 OvmfPkg/PlatformPei: handle non-power-of-two spare > > size for emu variables > > No, that's not exactly the reason. Please see the following sub-thread: > > http://mid.mail-archive.com/bdf196e8-df87-84b9-1d6e- > bbece5fa6...@redhat.com > > Briefly, the 4MB unified pflash size broke two things: > - the EmuVariableFvbRuntimeDxe driver, which would only affect QEMU > command lines with -bios, > - the PlatformPei PEIM, which would affect QEMU command lines with > -pflash too. > > (In the linked message, I explained why I hadn't noticed even regression > #2.) > > The temporary solution was to (a) restore the default to 2MB -- this > would allow people to continue using both -bios and -pflash, and (b) > kludge around the PlatformPei regression, so that the (manually > selected) 4MB build would be usable for -pflash, at the cost of wasting > a little bit of reserved memory. > > In other words, the last two commits that you list have no > inter-dependency between them, instead they mitigated two aspects of the > regression. > > In the longer term, I extended EmuVariableFvbRuntimeDxe (and the > interfacing code in PlatformPei) to handle the new 4MB unified size as > well. Please refer to > <https://bugzilla.tianocore.org/show_bug.cgi?id=527>. This completed the > 4MB support for "-bios" from the OVMF side; however it turned out that > even Xen needed fixes. > > Once Xen too received the necessary patches, we re-enabled the 4MB > default size (commit 1c47fcd465a4 -- basically a re-application of > commit bba8dfbe). Please refer to > <http://mid.mail-archive.com/d327e563-c89a-de28-ae75- > db629722c...@redhat.com>, > and the other message it references. > > > Given that the above commit was also merged, why does edk2 still default > to > > the 2MB flash layout for builds? > > It doesn't; if you build upstream edk2, it defaults to 4MB. > Thank you. It appears I had missed the new commits that re-enabled the 4MB size as default. > > We preserved the 2MB unified size in Gerd's and Fedora's OVMF builds > because the varstore structures are incompatible between the 2MB and the > 4MB builds. (Please refer to section "OVMF Flash Layout" in the > "OvmfPkg/README" file.) If you have a preexistent varstore file, > originally created from the OVMF_VARS.fd template of the 2MB build, and > then you boot the same VM with the OVMF_CODE.fd binary of the 4MB build, > your VM will not boot. (It will just hang with a black screen. If you > capture the OVMF debug log, you will get some error messages, but > practically no user ever captures the debug log.) > > In other words, we preserved the 2MB unified size in Fedora and in > Gerd's repo for staying compatible with the existent VMs of the users of > those distros / repos. > yes, I had played around with these two different layouts. I found my VMs to hang and not boot in case I mismatched the OVMF_VARS.fd. > > As one counter-example, we switched from 2MB to 4MB in RHEL-7.4. This > broke compatibility with VMs created against OVMF in RHEL-7.3, but that > was fine: in RHEL-7.3, OVMF was Tech Preview, and breaking compat with > Tech Preview packages is explicitly permitted. > > Now obviously users of Fedora and Gerd's repo aren't *officially* > entitled to any kind of stability or support (which is why I don't run > Fedora for my day to day work, by the way), so we could have broken > compat there as well, right? Well, yes, if only we had wanted to field > tens or maybe hundreds of complaints on the vfio-users mailing list and > elsewhere. :) Those users most likely didn't *need* the 4MB build -- see > the motivation for the 4MB build in commit b24fca05751f --, hence > keeping compat for them was more important. Regarding direct consumers > of the upstream edk2 *source* tree, we figured they were technical > enough to deal with the change in the default size. > > > Do you see any other issues too with the 4MB flash layout ? > > Nope. > > If you really want to use the 4MB build in Fedora (although I don't see > why you would), it's technically possible to provide the split 4MB build > in addition to the current files (under different filenames). In that > case, please file an RHBZ for the "edk2" component in Fedora (and CC > Gerd on it so he can adapt his package as well, if he intends to). > > I do not necessarily need Fedora. Will be happy to build from upstream. Thank you for patiently explaining the implications of non-power-of-2 for EmuVariableFvbRuntimeDxe. My limited reading had just spotted the breakage for PlatformPei PEIM. This clearly was a commit I had missed from looking at upstream git log. Regards, Prerna _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
