Re: [PATCH 18/19] ArmPlatformPkg: Reserving framebuffer at build

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 1 December 2017 at 16:56, Evan Lloyd <Evan.Lloyd@arm.com> wrote:
> Responses inline
>
>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: 25 October 2017 19:10
>> To: Evan Lloyd <Evan.Lloyd@arm.com>
>> Cc: edk2-devel@lists.01.org; "ard.biesheuvel@linaro.org"@arm.com;
>> "leif.lindholm@linaro.org"@arm.com;
>> "Matteo.Carlini@arm.com"@arm.com; "nd@arm.com"@arm.com
>> Subject: Re: [PATCH 18/19] ArmPlatformPkg: Reserving framebuffer at
>> build
>>
>> On 26 September 2017 at 21:15,  <evan.lloyd@arm.com> wrote:
>> > From: Girish Pathak <girish.pathak@arm.com>
>> >
>> > Currently frame buffer memory is either reserved in special VRAM or
>> > dynamically allocated using boot services memory allocation functions.
>> > When allocated using boot services calls the memory has to be
>> > allocated as EfiBootServicesData. Unfortunately failures have been
>> > seen with this case.  There is also an unfortunate lack of control on
>> > the placement of the frmae buffer.
>> >
>> > This change introduces two PCDs, PcdArmLcdFrameBufferBase and
>> > PcdArmLcdFrameBufferSize which enable build time reservation of the
>> > frame buffer, avoiding the need to allocate dynamically.  This allows
>> > the frame buffer to appear as "I/O memory" outside of the normal RAM
>> > map, which is similar to the "VRAM" case.
>> >
>> > This change has no impact on current code, only enables the option of
>> > build time reservation of frame buffers.
>> >
>>
>> Where is the memory actually being reserved? And if it is reserved, how can
>> the OS reclaim it if it is not interested in using the GOP?
>
> [[Evan Lloyd]] The memory is reserved by whatever sets the Pcd - normally the .dsc.  It may or may not be system RAM.
> If it is excised from system memory, then there is no way for the OS to reclaim it, as it can't differentiate that from the VRAM case.
> I'm not sure I see the relevance of that anyway.  Is our objective to provide restricted firmware tightly tuned for a specific operating system in an unrealistic mode, or to provide a generic example of firmware exercising the standard interfaces and testing real use cases?
>

Is that intended as a rhetorical question? If you introduce a feature
that may take away RAM from the OS without a way to reclaim it, that
deserves a comment. At the very least, it would have saved me from
having to ask about it specifically.

Whether doing so is justified is another question, but that depends on
the expected users of this code (hence the need to document it)

-- 
Ard.