Re: [Qemu-devel] [RFC PATCH 00/11] Generate ACPI v5.1 tables and expose it to guest over fw_cfg on ARM

[Laszlo Ersek]

On 01/24/15 10:21, Shannon Zhao wrote:
> This patch series generate seven ACPI v5.1 tables for machine virt on
> ARM.
> The set of generated tables are:
> - RSDP
> - XSDT
> - MADT
> - GTDT
> - FADT
> - FACS
> - DSDT
>
> These tables are created dynamically using the function of
> acpi-build-utils.c, taking into account the needed information passed
> from the virt machine model. When the generation is finalized, it use
> fw_cfg to expose the tables to guest.
>
> This patchset is based on Igor Mammedov's branch which can be found at
> below git tree:
>  https://github.com/imammedo/qemu/commits/ASL_API_v2

Awesome! I didn't know you had been coordinating with Igor. This is the
best (or, put differently, "only" :)) possible way forward. Great!

> And this patchset refers to Alexander Spyridakis's patches which are
> sent to qemu-devel mailing list before.
>  http://lists.gnu.org/archive/html/qemu-devel/2014-10/msg03987.html
>
> As UEFI (ArmVirtualizationQemu) doesn't support downloading ACPI
> tables over fw_cfg, I just do compile test and start a guest with
> UEFI. But I contacted Laszlo Ersek before, he says that if qemu can
> expose the generated ACPI tables over fw_cfg, he can quickly add
> support in UEFI. So just send this out and make it go forward.

I hope I was quick enough:
- patches: http://thread.gmane.org/gmane.comp.bios.tianocore.devel/12158
- branch: https://github.com/lersek/edk2/commits/armvirt_acpi
- binary: http://people.redhat.com/~lersek/armvirt_acpi/QEMU_EFI.fd

>
> Todo:
> 1) add GPIO controller in virt and expose it through ACPI
> 2) add cpu hotplug support
>
> Any comments are welcome.

I answered with a couple of notes and fixes in the thread. I found those
via testing. I won't offer a code review; I hope you don't mind.

For testing on your end (and for further development) before the edk2
series is applied, you can fetch my patches from github, or even use the
binary I built for you.

Note: the binary includes a very small patch that is not upstream. (Well
it includes some other patches too, but they are not relevant.) This is
it:

> --- a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc
> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc
> @@ -87,7 +87,7 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|FALSE
>
>  [PcdsFixedAtBuild.common]
> -  gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000004F
> +  gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8040004F
>
>    gArmPlatformTokenSpaceGuid.PcdFirmwareVendor|"QEMU"

It enables the EFI_D_VERBOSE loglevel. The ACPI code logs quite a bit of
info on this loglevel, so if you build an image yourself, be sure to
enable EFI_D_VERBOSE manually. Otherwise you'll only see a part of this
log fragment:

> Loading driver at 0x000BEE66000 EntryPoint=0x000BEE662B0 AcpiPlatformDxe.efi
> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF BB7BBE98
> ProcessCmdAllocate: File="etc/acpi/rsdp" Alignment=0x10 Zone=2 Size=0x24 
> Address=0xB7048000
> ProcessCmdAllocate: File="etc/acpi/tables" Alignment=0x40 Zone=1 Size=0xBC0 
> Address=0xB7047000
> ProcessCmdAddChecksum: File="etc/acpi/tables" ResultOffset=0x49 Start=0x40 
> Length=0x8F4
> ProcessCmdAddPointer: PointerFile="etc/acpi/tables" 
> PointeeFile="etc/acpi/tables" PointerOffset=0x9B8 PointerSize=8
> ProcessCmdAddPointer: PointerFile="etc/acpi/tables" 
> PointeeFile="etc/acpi/tables" PointerOffset=0x9C0 PointerSize=8
> ProcessCmdAddChecksum: File="etc/acpi/tables" ResultOffset=0x93D Start=0x934 
> Length=0x10C
> ProcessCmdAddChecksum: File="etc/acpi/tables" ResultOffset=0xA49 Start=0xA40 
> Length=0xDC
> ProcessCmdAddChecksum: File="etc/acpi/tables" ResultOffset=0xB25 Start=0xB1C 
> Length=0x60
> ProcessCmdAddPointer: PointerFile="etc/acpi/tables" 
> PointeeFile="etc/acpi/tables" PointerOffset=0xBA0 PointerSize=8
> ProcessCmdAddPointer: PointerFile="etc/acpi/tables" 
> PointeeFile="etc/acpi/tables" PointerOffset=0xBA8 PointerSize=8
> ProcessCmdAddPointer: PointerFile="etc/acpi/tables" 
> PointeeFile="etc/acpi/tables" PointerOffset=0xBB0 PointerSize=8
> ProcessCmdAddChecksum: File="etc/acpi/tables" ResultOffset=0xB85 Start=0xB7C 
> Length=0x44
> ProcessCmdAddPointer: PointerFile="etc/acpi/rsdp" 
> PointeeFile="etc/acpi/tables" PointerOffset=0x18 PointerSize=8
> ProcessCmdAddChecksum: File="etc/acpi/rsdp" ResultOffset=0x8 Start=0x0 
> Length=0x24
> Process2ndPassCmdAddPointer: checking for ACPI header in "etc/acpi/tables" at 
> 0xB7047000 (remaining: 0xBC0): found "FACS" size 0x40
> Process2ndPassCmdAddPointer: checking for ACPI header in "etc/acpi/tables" at 
> 0xB7047040 (remaining: 0xB80): found "DSDT" size 0x8F4
> Process2ndPassCmdAddPointer: checking for ACPI header in "etc/acpi/tables" at 
> 0xB7047934 (remaining: 0x28C): found "FACP" size 0x10C
> Process2ndPassCmdAddPointer: checking for ACPI header in "etc/acpi/tables" at 
> 0xB7047A40 (remaining: 0x180): found "APIC" size 0xDC
> Process2ndPassCmdAddPointer: checking for ACPI header in "etc/acpi/tables" at 
> 0xB7047B1C (remaining: 0xA4): found "GTDT" size 0x60
> Process2ndPassCmdAddPointer: checking for ACPI header in "etc/acpi/tables" at 
> 0xB7047B7C (remaining: 0x44): found "XSDT" size 0x44
> InstallAllQemuLinkedTables: installed 5 tables
> InstallAllQemuLinkedTables: freeing "etc/acpi/rsdp"
> InstallAllQemuLinkedTables: freeing "etc/acpi/tables"

Before you ask:
- The fact that RSDP is not listed in the log above is not a problem;
  it's intentional.
- The fact that XSDT (and RSDT, if it were there) is not installed,
  despite being listed, it's intentional too.
- Yes, your code is correct, and you should continue generating those
  things; I need them.
The short explanation is that UEFI handles these tables automatically in
"some place", but I still need them "elsewhere".

So that's why 5 tables are installed for OSPM: FACS, DSDT, FACP, ACPI,
GTDT.

Also, I decompiled those tables that were visible under
/sys/firmware/acpi/tables/ inside the guest. Please find them attached.
The FACS won't be there, but I explained that in another email -- the
kernel ignores the FACS because it sees the hw-reduced mode.

Yay! :)

Thanks
Laszlo

APIC.dsl
Description: Text Data

DSDT.dsl
Description: Text Data

FACP.dsl
Description: Text Data

GTDT.dsl
Description: Text Data