public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: Prerna <saxenap.ltc@gmail.com>
Cc: edk2-devel@lists.01.org
Subject: Re: 4MB flash size as build default.
Date: Wed, 6 Jun 2018 12:32:35 +0200	[thread overview]
Message-ID: <972162c0-33f8-8234-0304-855089c094ca@redhat.com> (raw)
In-Reply-To: <CALE=2vs5W59bJrwL-+RQ7XUHStNv6NpmVEv_Cktc-0DfRUS_0g@mail.gmail.com>

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.

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.

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).

Thanks
Laszlo


  reply	other threads:[~2018-06-06 10:32 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-06  3:05 4MB flash size as build default Prerna
2018-06-06 10:32 ` Laszlo Ersek [this message]
2018-06-06 10:57   ` Prerna

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=972162c0-33f8-8234-0304-855089c094ca@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox