Re: [Qemu-devel] [PATCH v2 0/4] acpi: xsdt support

[Laszlo Ersek]

On 06/08/15 20:14, Michael S. Tsirkin wrote:
> XSDT support allows using ACPI 2 features while
> avoiding breaking legacy windows XP guests:
> ACPI 2 tables are linked from XSDT only,
> ACPI 1 tables from both RSDT and XSDT, this way
> XP does not see ACPI 2 tables.
> 
> As a first step, this patchset generates v2 RSDP
> and fills in XSDT matching RSDT exactly.
> 
> ARM can switch to XSDT as well, I'm not bothering
> until there's an easy way to test that.
> 
> Note: unit test files need to be updated with this,
> I'm not bothering with posting them.
> 
> Changes from v1:
>     xsdt address is 64 bit
>     arm patch is now tested
> 
> Michael S. Tsirkin (4):
>   acpi: add API for 64 bit offsets
>   i386/acpi: collect 64 bit offsets for xsdt
>   i386/acpi: add XSDT
>   acpi: unify rsdp generation
> 
>  include/hw/acpi/acpi-defs.h | 15 +++++--
>  include/hw/acpi/aml-build.h |  7 +++-
>  hw/acpi/aml-build.c         | 99 
> +++++++++++++++++++++++++++++++++++++--------
>  hw/arm/virt-acpi-build.c    | 39 +++---------------
>  hw/i386/acpi-build.c        | 64 +++++++++++------------------
>  5 files changed, 129 insertions(+), 95 deletions(-)
> 

I tested this series with ArmVirtQemu (aka AAVMF) and OVMF too. (They
use common code in edk2.)

The ARM build works indeed, but that's only because in patch #4 we have

  build_rsdp(tables->rsdp, tables->linker, rsdt, 0);

ie. there's only an RSDT.

Due to patch #3 however, the OVMF client breaks:

  build_rsdp(tables->rsdp, tables->linker, rsdt, xsdt);

The problem is that the "directed acyclic graph" of ADD_POINTER edges is
no longer a tree. At least some tables can be reached on multiple paths.
(Eg. the FADT can be reached via RSDP->RSDT-> FADT, and also
RSDP->XSDT->FADT.)

This is a problem because EFI_ACPI_TABLE_PROTOCOL has "builtin
knowledge" about what tables may not be installed only once vs. several
times. Unfortunately, in this case both decisions have bad consequences:

- When FADT is passed to EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable() for
the second time, the function returns EFI_ACCESS_DENIED, and the
linker/loader client bails out.

- When (eg.) the same SSDT is passed to
EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable() for the second time, the
function will happily install (a copy of) it again, and then we'll end
up with two copies of the same SSDT.

EFI_ACPI_TABLE_PROTOCOL manages both the RSDT and the XSDT internally.
As far as I can see, it puts each table in both. The RSDT and the XSDT
are not distinguished even on the UEFI spec level; it lumps them
together under "RSDT/XSDT" every time.

This is probably very frustrating; sorry about that.

Laszlo