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

[Michael S. Tsirkin]

On Tue, Jun 09, 2015 at 02:02:31AM -0400, Paolo Bonzini wrote:
> 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


It does BSOD.
Skipping RSDT sounds good.

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