Re: [PATCH 2/3] OvmfPkg: introduce FD_SIZE_4MB (mainly) for Windows HCK

[Jordan Justen <jordan.l.justen@intel.com>]

On 2017-05-02 12:31:39, Laszlo Ersek wrote:
> On 05/02/17 20:22, Jordan Justen wrote:
> > On 2017-05-02 07:39:04, Laszlo Ersek wrote:
> >>
> >> I wouldn't mind if we made more room for the varstore in the 2MB build,
> >> even at the expense of FVMAIN_COMPACT, if we also kept the current 2MB
> >> build the default, so that the "new" (incompatible) 2MB build doesn't
> >> come as a surprise to unsuspecting downstreams.
> >>
> >> Regarding the 4MB build:
> >> - we can discuss that on top of the above "new" 2MB build,
> >> - we can discuss it instead of the above "new" 2MB build,
> >> - we can postpone it for now, for upstream.
> > 
> > I was hoping there was a way to avoid the need to add 4MB, but you
> > needing to support the layout until 2024 really adds risk to the 2MB
> > image. I think there is a decent chance 2MB would work until then, but
> > I can also see how it adds significant risk.
> > 
> > If we are adding the 4MB layout, then we may as well make it the
> > default for debug builds.
> 
> OK, I think that's technically doable. Based on your commit e3dca1859b24
> ("OvmfPkg: Increase default RELEASE build image size to 2MB",
> 2016-01-29), the $(TARGET) macro can be used in FDF files.
> 
> > I'm not sure what to do about 2MB then. In
> > the short term, I say we do nothing.
> 
> Do you mean "do nothing about 2MB", or "do nothing at all in the fdf.inc"?
> 
> (You have to be really specific with me these days, sorry...)
> 
> If I understand correctly, we'd have to reinstate FD_SIZE_2MB then, so
> that people that want to stick with the 2MB build for DEBUG (and NOOPT)
> can select it. Given that 4MB would become the new default for those
> targets.

Ah. I guess I dropped FD_SIZE_2MB in e3dca1859b24, which I don't think
I should have done. Going forward, I think we should allow
FD_SIZE_1/2/4MB.

Regarding RELEASE builds, I'm not sure what we should do. Should we
just change it to 4MB as well? In the past, I preferred to allow
release builds to use the smaller size, since it fit. But, in this
case we also know that leaving 2MB size will mean a known test will
fail. The test failing doesn't mean a real user is likely to be
impacted, but I guess Microsoft feels the larger size may be required
in some scenarios.

What do you think? (Maybe not a fair question since you don't use the
release build.) I guess the safe option is to just bump the default
for both the debug and release builds to the ridiculously large (er, I
mean luxuriously spacious :) 4MB image.

> > I feel fairly confident of the 4MB image supporting your code size
> > needs until 2024. What seems less certain in future varstore
> > requirements. Right now the requirement is 120~128k. I think rather
> > than 248k in the 4MB layout, we should make it 256k. (Since these
> > kinds of things often progress in powers-of-two.) It will make for a
> > couple unfriendly offsets, but it seems worth it to avoid being 8k shy
> > of the next power-of-two size.
> > 
> > In my other email, I mentioned the event-log. I did ask around a bit
> > about this, but I didn't find anyone willing to fight for more space.
> > Therefore, I think we should just keep it at 4k.
> 
> That means 256K for the varstore, 4K for the event log, 4K for the FTW
> working block.
> 
> How much for the spare area? Currently the spare area equals the sum of
> the former three. The spare area is used both while reclaiming the
> varstore, and while reclaiming the FTW working block. (Not sure about
> the event log.) So I'd say we should stick with our tradition, and make
> the spare area 256K + 4K + 4K = 264K in size. That would result in a
> 528K NVRAM. (Which is 16K larger than in the current patch.)
> 
> In turn, I wouldn't increase FVMAIN_COMPACT by 1664K, to 3376K, but by
> 16K less (1648K) to 3360K. The full FD size would remain 4M.
> 
> Does this sound okay?

Yes.

This will leave the split rom sizes being a multiple of 16k rather
than 512k. Today they are a multiple of 128k. I don't expect this
would be an issue for qemu/kvm. Do you agree?

-Jordan