Re: 4MB flash size as build default.

[Prerna <saxenap.ltc@gmail.com>]

On Wed, Jun 6, 2018 at 4:02 PM, Laszlo Ersek <lersek@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-
> bbece5fa65b7@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-
> db629722c70a@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