Re: ArmPlatformPkg: Allocate VRAM as RuntimeServicesData

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

On 11 October 2016 at 18:44, Ryan Harkin <ryan.harkin@linaro.org> wrote:
> Hi Evan,
>
> This was sent to the list with no subject line and I wasn't on CC, so
> I didn't see it.
>
> Are you still using this patch and want it in, i.e. does it need
> review and test?
>

I'm sure it works, but I don't think we should take it.
RuntimeServicesData can never be released to the OS, so taking an 8MB
chunk just in case the OS may decide to drive the framebuffer using
the firmware's protocol rather than via a native driver is not
something we should have in reference code.

The GOP protocol is arguably a hack anyway, since the entire protocol
database and driver tree are torn down after ExitBootServices(), while
the GOP leaves a live memory range in place that happens to keep
operating as a framebuffer. If the OS wants to use this protocol
during normal operation, it should take care to reserve this memory
region itself.

I am aware that not all OSes may behave correctly in this regard. This
is mainly due to the fact that GOP is usually implemented by a PCI
device, which exposes the framebuffer via a PCI BAR rather than via a
system memory range.



> On 4 March 2016 at 15:57,  <evan.lloyd@arm.com> wrote:
>> Code at: https://github.com/EvanLloyd/tianocore/commit/
>> From: Sami Mujawar <sami.mujawar@arm.com>
>> Date: Thu, 25 Feb 2016 15:07:40 +0000
>> Subject: [PATCH] ArmPlatformPkg: Allocate VRAM as RuntimeServicesData
>>
>> The UEFI specification allows the operating system (OS) to use the
>> Graphics Output Protocol (GOP) in the following scenarios:
>>  a. as part of the startup process and
>>  b. prior to loading of a high performance OS graphics driver
>>
>> If the VRAM is allocated as BootServicesData, then it is unmapped on
>> exit boot services. This prevents GOP usage by the OS post exit boot
>> services (the second scenario); as it results in a crash when the VRAM
>> is accessed.
>>
>> This patch fixes the issue by allocating VRAM as RuntimeServicesData.
>>
>> Code at: https://github.com/EvanLloyd/tianocore/commit/18fab16a63c59c84c71cd81089a55a4081ebe253
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Alexei Fedorov <alexei.fedorov@arm.com>
>> Signed-off-by: Girish Pathak <girish.pathak@arm.com>
>> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
>> Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>
>> ---
>>  ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
>> index a578467..4ab8862 100644
>> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
>> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
>> @@ -133,7 +133,7 @@ LcdPlatformGetVram (
>>    } else {
>>      AllocationType = AllocateAddress;
>>    }
>> -  Status = gBS->AllocatePages (AllocationType, EfiBootServicesData, EFI_SIZE_TO_PAGES(((UINTN)LCD_VRAM_SIZE)), VramBaseAddress);
>> +  Status = gBS->AllocatePages (AllocationType, EfiRuntimeServicesData, EFI_SIZE_TO_PAGES(((UINTN)LCD_VRAM_SIZE)), VramBaseAddress);
>>    if (EFI_ERROR(Status)) {
>>      return Status;
>>    }
>> --
>> 2.7.0
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel