public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 1/1] Add BGRT ACPI table inclusion support in OVMF
@ 2016-12-26  5:25 Bhupesh Sharma
  2017-01-03 14:46 ` Laszlo Ersek
  0 siblings, 1 reply; 3+ messages in thread
From: Bhupesh Sharma @ 2016-12-26  5:25 UTC (permalink / raw)
  To: edk2-devel; +Cc: lersek, bhupesh.linux, Bhupesh Sharma

During debugging Linux kernel for ACPI BGRT support
(especially on VMs), it is sometimes very useful to have
the EFI firmware (OVMF in most cases which use Tianocore)
to export the ACPI BGRT table.

This patch tries to add this support.

I have tested this patch on a RHEL7.3 and Fedora-25 KVM
and ensured that the BGRT logo is properly prepared and
can be viewed with user-space tools (like 'Gwenview' on KDE,
for example):

$ file /sys/firmware/acpi/bgrt/image
/sys/firmware/acpi/bgrt/image: PC bitmap, Windows 3.x format, 193 x 58 x
24

Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
---
 OvmfPkg/OvmfPkgIa32.dsc    | 1 +
 OvmfPkg/OvmfPkgIa32.fdf    | 1 +
 OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
 OvmfPkg/OvmfPkgIa32X64.fdf | 1 +
 OvmfPkg/OvmfPkgX64.dsc     | 1 +
 OvmfPkg/OvmfPkgX64.fdf     | 1 +
 6 files changed, 6 insertions(+)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 81f7521..e97f7f0 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -679,6 +679,7 @@
   OvmfPkg/AcpiTables/AcpiTables.inf
   MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
   MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
+  MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
 
   #
   # Network Support
diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
index 2a5b211..34d57a6 100644
--- a/OvmfPkg/OvmfPkgIa32.fdf
+++ b/OvmfPkg/OvmfPkgIa32.fdf
@@ -278,6 +278,7 @@ INF  OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
 INF  RuleOverride=ACPITABLE OvmfPkg/AcpiTables/AcpiTables.inf
 INF  MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
 INF  MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
+INF  MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
 
 INF  FatPkg/EnhancedFatDxe/Fat.inf
 
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index f7855b6..8e3e04c 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -688,6 +688,7 @@
   OvmfPkg/AcpiTables/AcpiTables.inf
   MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
   MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
+  MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
 
   #
   # Network Support
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index 1c7df21..df55c2b 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -278,6 +278,7 @@ INF  OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
 INF  RuleOverride=ACPITABLE OvmfPkg/AcpiTables/AcpiTables.inf
 INF  MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
 INF  MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
+INF  MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
 
 INF  FatPkg/EnhancedFatDxe/Fat.inf
 
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index e933a41..6ec3fe0 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -686,6 +686,7 @@
   OvmfPkg/AcpiTables/AcpiTables.inf
   MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
   MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
+  MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
 
   #
   # Network Support
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 3bb11cb..5e2e1df 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -278,6 +278,7 @@ INF  OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
 INF  RuleOverride=ACPITABLE OvmfPkg/AcpiTables/AcpiTables.inf
 INF  MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
 INF  MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
+INF  MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
 
 INF  FatPkg/EnhancedFatDxe/Fat.inf
 
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/1] Add BGRT ACPI table inclusion support in OVMF
  2016-12-26  5:25 [PATCH 1/1] Add BGRT ACPI table inclusion support in OVMF Bhupesh Sharma
@ 2017-01-03 14:46 ` Laszlo Ersek
  2017-01-04 19:34   ` Bhupesh SHARMA
  0 siblings, 1 reply; 3+ messages in thread
From: Laszlo Ersek @ 2017-01-03 14:46 UTC (permalink / raw)
  To: Bhupesh Sharma; +Cc: edk2-devel, Jordan Justen (Intel address), Ard Biesheuvel

On 12/26/16 06:25, Bhupesh Sharma wrote:
> During debugging Linux kernel for ACPI BGRT support
> (especially on VMs), it is sometimes very useful to have
> the EFI firmware (OVMF in most cases which use Tianocore)
> to export the ACPI BGRT table.
> 
> This patch tries to add this support.
> 
> I have tested this patch on a RHEL7.3 and Fedora-25 KVM
> and ensured that the BGRT logo is properly prepared and
> can be viewed with user-space tools (like 'Gwenview' on KDE,
> for example):
> 
> $ file /sys/firmware/acpi/bgrt/image
> /sys/firmware/acpi/bgrt/image: PC bitmap, Windows 3.x format, 193 x 58 x
> 24
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> ---
>  OvmfPkg/OvmfPkgIa32.dsc    | 1 +
>  OvmfPkg/OvmfPkgIa32.fdf    | 1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
>  OvmfPkg/OvmfPkgIa32X64.fdf | 1 +
>  OvmfPkg/OvmfPkgX64.dsc     | 1 +
>  OvmfPkg/OvmfPkgX64.fdf     | 1 +
>  6 files changed, 6 insertions(+)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 81f7521..e97f7f0 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -679,6 +679,7 @@
>    OvmfPkg/AcpiTables/AcpiTables.inf
>    MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
>    MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
> +  MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
>  
>    #
>    # Network Support
> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
> index 2a5b211..34d57a6 100644
> --- a/OvmfPkg/OvmfPkgIa32.fdf
> +++ b/OvmfPkg/OvmfPkgIa32.fdf
> @@ -278,6 +278,7 @@ INF  OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
>  INF  RuleOverride=ACPITABLE OvmfPkg/AcpiTables/AcpiTables.inf
>  INF  MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
>  INF  MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
> +INF  MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
>  
>  INF  FatPkg/EnhancedFatDxe/Fat.inf
>  
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index f7855b6..8e3e04c 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -688,6 +688,7 @@
>    OvmfPkg/AcpiTables/AcpiTables.inf
>    MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
>    MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
> +  MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
>  
>    #
>    # Network Support
> diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
> index 1c7df21..df55c2b 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.fdf
> +++ b/OvmfPkg/OvmfPkgIa32X64.fdf
> @@ -278,6 +278,7 @@ INF  OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
>  INF  RuleOverride=ACPITABLE OvmfPkg/AcpiTables/AcpiTables.inf
>  INF  MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
>  INF  MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
> +INF  MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
>  
>  INF  FatPkg/EnhancedFatDxe/Fat.inf
>  
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index e933a41..6ec3fe0 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -686,6 +686,7 @@
>    OvmfPkg/AcpiTables/AcpiTables.inf
>    MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
>    MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
> +  MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
>  
>    #
>    # Network Support
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index 3bb11cb..5e2e1df 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.fdf
> @@ -278,6 +278,7 @@ INF  OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
>  INF  RuleOverride=ACPITABLE OvmfPkg/AcpiTables/AcpiTables.inf
>  INF  MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
>  INF  MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
> +INF  MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
>  
>  INF  FatPkg/EnhancedFatDxe/Fat.inf
>  
> 

(1) This is a good idea. I superficially checked the logic in
"MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.c",
plus the code around the tree that connects with it in one way or
another, and it seems sane.

(2) Also, the fw binary footprint of the new module doesn't seem large,
which is good.

(3) Please change the subject line to:

  OvmfPkg: install BGRT ACPI table

(4) Please also CC Jordan on the OvmfPkg patch, not just me. Consult
Maintainers.txt.

(5) The same feature would be beneficial to ArmVirtPkg as well. Please
add a separate patch to the series that does the same for ArmVirtPkg.
The files you should extend are:

- ArmVirtPkg/ArmVirtQemu.dsc (under comment "ACPI Support")
- ArmVirtPkg/ArmVirtQemuKernel.dsc (under comment "ACPI Support")
- ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc (under comment "ACPI Support")

The subject line should be

  ArmVirtPkg/ArmVirtQemu: install BGRT ACPI table

please CC Ard and me on the patch (see Maintainers.txt).

(6) I would appreciate if you could repeat your BGRT image test in an
aarch64 guest as well (for patch #2).

We can discuss the necessary setup / guest installation steps
separately. (It makes sense to keep at least one long-term aarch64
guest, TCG or KVM.)

(7) Any chance you could regression-test the OvmfPkg change with some
x86-64 Windows guests? Ideally, Windows Server 2008 R2, Windows 8[.1]
and Windows 10 should be covered.

Windows has a proprietary (non-ACPICA) ACPI interpreter, and such
changes are best regression-tested against it as well.

(8) Last but not least, welcome to Red Hat, Bhupesh -- I just noticed :)

Laszlo


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/1] Add BGRT ACPI table inclusion support in OVMF
  2017-01-03 14:46 ` Laszlo Ersek
@ 2017-01-04 19:34   ` Bhupesh SHARMA
  0 siblings, 0 replies; 3+ messages in thread
From: Bhupesh SHARMA @ 2017-01-04 19:34 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: Bhupesh Sharma, Jordan Justen (Intel address), edk2-devel,
	Ard Biesheuvel

Hi Laszlo,

Many thanks for taking out the time to review this.

On Tue, Jan 3, 2017 at 8:16 PM, Laszlo Ersek <lersek@redhat.com> wrote:
> On 12/26/16 06:25, Bhupesh Sharma wrote:
>> During debugging Linux kernel for ACPI BGRT support
>> (especially on VMs), it is sometimes very useful to have
>> the EFI firmware (OVMF in most cases which use Tianocore)
>> to export the ACPI BGRT table.
>>
>> This patch tries to add this support.
>>
>> I have tested this patch on a RHEL7.3 and Fedora-25 KVM
>> and ensured that the BGRT logo is properly prepared and
>> can be viewed with user-space tools (like 'Gwenview' on KDE,
>> for example):
>>
>> $ file /sys/firmware/acpi/bgrt/image
>> /sys/firmware/acpi/bgrt/image: PC bitmap, Windows 3.x format, 193 x 58 x
>> 24
>>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> ---
>>  OvmfPkg/OvmfPkgIa32.dsc    | 1 +
>>  OvmfPkg/OvmfPkgIa32.fdf    | 1 +
>>  OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
>>  OvmfPkg/OvmfPkgIa32X64.fdf | 1 +
>>  OvmfPkg/OvmfPkgX64.dsc     | 1 +
>>  OvmfPkg/OvmfPkgX64.fdf     | 1 +
>>  6 files changed, 6 insertions(+)
>>
>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>> index 81f7521..e97f7f0 100644
>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>> @@ -679,6 +679,7 @@
>>    OvmfPkg/AcpiTables/AcpiTables.inf
>>    MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
>>    MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
>> +  MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
>>
>>    #
>>    # Network Support
>> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
>> index 2a5b211..34d57a6 100644
>> --- a/OvmfPkg/OvmfPkgIa32.fdf
>> +++ b/OvmfPkg/OvmfPkgIa32.fdf
>> @@ -278,6 +278,7 @@ INF  OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
>>  INF  RuleOverride=ACPITABLE OvmfPkg/AcpiTables/AcpiTables.inf
>>  INF  MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
>>  INF  MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
>> +INF  MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
>>
>>  INF  FatPkg/EnhancedFatDxe/Fat.inf
>>
>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>> index f7855b6..8e3e04c 100644
>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>> @@ -688,6 +688,7 @@
>>    OvmfPkg/AcpiTables/AcpiTables.inf
>>    MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
>>    MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
>> +  MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
>>
>>    #
>>    # Network Support
>> diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
>> index 1c7df21..df55c2b 100644
>> --- a/OvmfPkg/OvmfPkgIa32X64.fdf
>> +++ b/OvmfPkg/OvmfPkgIa32X64.fdf
>> @@ -278,6 +278,7 @@ INF  OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
>>  INF  RuleOverride=ACPITABLE OvmfPkg/AcpiTables/AcpiTables.inf
>>  INF  MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
>>  INF  MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
>> +INF  MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
>>
>>  INF  FatPkg/EnhancedFatDxe/Fat.inf
>>
>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>> index e933a41..6ec3fe0 100644
>> --- a/OvmfPkg/OvmfPkgX64.dsc
>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>> @@ -686,6 +686,7 @@
>>    OvmfPkg/AcpiTables/AcpiTables.inf
>>    MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
>>    MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
>> +  MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
>>
>>    #
>>    # Network Support
>> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
>> index 3bb11cb..5e2e1df 100644
>> --- a/OvmfPkg/OvmfPkgX64.fdf
>> +++ b/OvmfPkg/OvmfPkgX64.fdf
>> @@ -278,6 +278,7 @@ INF  OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
>>  INF  RuleOverride=ACPITABLE OvmfPkg/AcpiTables/AcpiTables.inf
>>  INF  MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
>>  INF  MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
>> +INF  MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
>>
>>  INF  FatPkg/EnhancedFatDxe/Fat.inf
>>
>>
>
> (1) This is a good idea. I superficially checked the logic in
> "MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.c",
> plus the code around the tree that connects with it in one way or
> another, and it seems sane.
>
> (2) Also, the fw binary footprint of the new module doesn't seem large,
> which is good.
>
> (3) Please change the subject line to:
>
>   OvmfPkg: install BGRT ACPI table

Sure.

> (4) Please also CC Jordan on the OvmfPkg patch, not just me. Consult
> Maintainers.txt.

Oops. Sorry for missing out Jordan.

> (5) The same feature would be beneficial to ArmVirtPkg as well. Please
> add a separate patch to the series that does the same for ArmVirtPkg.
> The files you should extend are:
>
> - ArmVirtPkg/ArmVirtQemu.dsc (under comment "ACPI Support")
> - ArmVirtPkg/ArmVirtQemuKernel.dsc (under comment "ACPI Support")
> - ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc (under comment "ACPI Support")
>
> The subject line should be
>
>   ArmVirtPkg/ArmVirtQemu: install BGRT ACPI table
>
> please CC Ard and me on the patch (see Maintainers.txt).

Sure, will spin that as a separate patch.

> (6) I would appreciate if you could repeat your BGRT image test in an
> aarch64 guest as well (for patch #2).

Sure I am trying to get hold of a AARCH64 board - will test the 2nd
patch on the same and confirm the test results before sending out
the 2nd patch.

> We can discuss the necessary setup / guest installation steps
> separately. (It makes sense to keep at least one long-term aarch64
> guest, TCG or KVM.)
>
> (7) Any chance you could regression-test the OvmfPkg change with some
> x86-64 Windows guests? Ideally, Windows Server 2008 R2, Windows 8[.1]
> and Windows 10 should be covered.

I am also trying to install a Windows 10 guest at my end. Will try to
check the OvmfPkg change on the same before sending a v2.

> Windows has a proprietary (non-ACPICA) ACPI interpreter, and such
> changes are best regression-tested against it as well.
>
> (8) Last but not least, welcome to Red Hat, Bhupesh -- I just noticed :)

Many thanks Laszlo :)

>
> Laszlo
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Regards,
Bhupesh


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-01-04 19:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-26  5:25 [PATCH 1/1] Add BGRT ACPI table inclusion support in OVMF Bhupesh Sharma
2017-01-03 14:46 ` Laszlo Ersek
2017-01-04 19:34   ` Bhupesh SHARMA

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox