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

[Igor Mammedov]

On Tue, 9 Jun 2015 08:35:38 +0200
"Michael S. Tsirkin" <address@hidden> wrote:

> 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.
We've had PCI0._CRS method with invalid for XP opcode and
it didn't BSOD unless execution reached it. (it was 64-bit PCI window if I 
recall right)

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