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

[Shannon Zhao]

On 2015/1/25 7:31, Laszlo Ersek wrote:
> 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!
> 

Thanks, Laszlo. Igor Mammedov's work is awesome and make the ACPI generation 
simpler.

>> 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
> 

Great work!

>>
>> 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.
> 

Ok, thanks.

> 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:
> 

Ok, I see.


>> 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.
> 

Thanks for your explanation and great support.
Thanks,
Shannon