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

[Paolo Bonzini]

I would just patch OVMF to ignore the RSDT if there is an XSDT.

Alternatively, can you check for ACPI 2.0 support via _OSI, and load the ACPI 2.0 bits via LoadTable? Hopefully XP does not BSOD if the invalid (for ACPI 1.0) opcodes are in a Then block or in a separate method... Then you can use just an RSDT.

Paolo


-----Original Message-----
From: Michael S. Tsirkin address@hidden
Received: martedì, 09 giu 2015, 7:31
To: Laszlo Ersek address@hidden
CC: address@hidden, address@hidden, address@hidden, address@hidden, address@hidden
Subject: Re: [PATCH v2 0/4]  acpi: xsdt support

On Tue, Jun 09, 2015 at 02:08:03AM +0200, Laszlo Ersek wrote:
> 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

Thanks for the info!  This is worth fixing.  Can you fix this without
protocol changes, or should we change the protocol to pass a hint that a
pointer is to another instance of a previously used table?

--
MST