Re: [Qemu-devel] [PATCH 0/3] merge SSDT into DSDT

[Michael S. Tsirkin]

On Tue, Jan 26, 2016 at 10:55:04AM +0100, Igor Mammedov wrote:
> On Mon, 25 Jan 2016 15:01:36 +0200
> "Michael S. Tsirkin" <address@hidden> wrote:
> 
> > On Mon, Jan 25, 2016 at 12:57:28PM +0100, Igor Mammedov wrote:
> > > On Sun, 24 Jan 2016 08:32:23 +0200
> > > "Michael S. Tsirkin" <address@hidden> wrote:
> > >   
> > > > On Fri, Jan 22, 2016 at 03:36:05PM +0100, Igor Mammedov wrote:  
> > > > > Merging both tables will allow for futher ASL
> > > > > simplification and cleanups per device/subsystem
> > > > > And it also allows to reduce number of expected
> > > > > binary blobs for ACPI tests which reduces tests
> > > > > maintenance.  
> > > > 
> > > > What this does break, however, is adding XSDT which
> > > > we might need to do in the future.
> > > > I'd rather do the reverse and have as much as possible
> > > > in the SSDT.  
> > > It doesn't forbid us adding XSDT later,  
> > 
> > Right but we'll have to duplicate dsdt in xsdt, right?
> nope, DSDT is pointed by FADT while SSDTs by RSDT/XSDT.

Oops, I was confused when I wrote this.
You are right, I'll go back and review
everything in light of this.


> The only reason why we have DSDT/SSDT split now is that
> DSDT used to be static while SSDT generated dynamically.
> Now when both tables are dynamically generated there
> is no point to keep them separate.
> 
> I don't see any benefit in keeping empty DSDT as you
> suggest with all the content in SSDT, it the same as
> having only DSDT table but we have to maintain 2 tables
> in code and tests.
> If at some point we use XSDT to hide non XP compatible
> ASL (if that works at all) then SSDT is the place where
> that non compat ASL should be put and pointed only by XSDT,
> while the the rest of compatible code should stay in 
> the main mandatory table (i.e. DSDT).
> > 
> > > DSDT should be kept XP compatible as it's more or less now
> > > and incompatible features we could put in their own SSDTs
> > > if we decide to go for XSDT approach to hide them form XP.
> > > For example I plan to move cpu/memory hotplug ASL into separate
> > > SSDTs and continue to simplify DSDT on top of this series.  
> > 
> > So I'm kind of confused. Why move hotplug to dsdt then move
> > them back out to an SSDT? Let's just put everything in an
> > SSDT ...
> because right now cpu/mem hotplug ASL is split between both
> tables so the first step to isolating and consolidating scattered
> parts is to put everything into one table. After that it
> will be more easy to verify changes that refactoring will
> introduce using bios-tables-test and should simplify refactoring
> patches and review.
> 
> As for moving hotplug into dedicated SSDTs, it's for purpose
> of sharing that code with ARM target and later we possible
> could use it also to hide XP incompatible ASL in them by
> referencing them from only XSDT.
> 
> Why do you insist on putting everything into SSDT instead of DSDT?
> > 
> > > >   
> > > > > Boot tested with RHEL72, WS2003, WS2012R2 guests.
> > > > > 
> > > > > git tree for testing:
> > > > > https://github.com/imammedo/qemu.git merge_ssdt_into_dsdt_v1 
> > > > > 
> > > > > Igor Mammedov (3):
> > > > >   pc: acpi: merge SSDT into DSDT
> > > > >   tests: pc: acpi: drop not needed 'expected SSDT' blobs
> > > > >   tests: pc: acpi: add expected DSDT.bridge blobs and update DSDT
> > > > > blobs
> > > > > 
> > > > >  hw/i386/acpi-build.c                 | 246
> > > > > ++++++++++++++++-------------------
> > > > > tests/acpi-test-data/pc/DSDT         | Bin 3028 -> 5478 bytes
> > > > > tests/acpi-test-data/pc/DSDT.bridge  | Bin 0 -> 7337 bytes
> > > > > tests/acpi-test-data/pc/SSDT         | Bin 2486 -> 0 bytes
> > > > > tests/acpi-test-data/pc/SSDT.bridge  | Bin 4345 -> 0 bytes
> > > > > tests/acpi-test-data/q35/DSDT        | Bin 7666 -> 8321 bytes
> > > > > tests/acpi-test-data/q35/DSDT.bridge | Bin 0 -> 8338 bytes
> > > > > tests/acpi-test-data/q35/SSDT        | Bin 691 -> 0 bytes
> > > > > tests/acpi-test-data/q35/SSDT.bridge | Bin 708 -> 0 bytes 9 files
> > > > > changed, 111 insertions(+), 135 deletions(-) create mode 100644
> > > > > tests/acpi-test-data/pc/DSDT.bridge delete mode 100644
> > > > > tests/acpi-test-data/pc/SSDT delete mode 100644
> > > > > tests/acpi-test-data/pc/SSDT.bridge create mode 100644
> > > > > tests/acpi-test-data/q35/DSDT.bridge delete mode 100644
> > > > > tests/acpi-test-data/q35/SSDT delete mode 100644
> > > > > tests/acpi-test-data/q35/SSDT.bridge
> > > > > 
> > > > > -- 
> > > > > 1.8.3.1  
> > > >   
> >