Re: [Qemu-devel] [RFC 0/9] generate dynamic _CRS for motherboard resources

[Michael S. Tsirkin]

On Mon, Feb 17, 2014 at 11:33:23AM +0100, Igor Mammedov wrote:
> On Sun, 16 Feb 2014 17:53:45 +0200
> "Michael S. Tsirkin" <address@hidden> wrote:
> 
> > On Fri, Feb 07, 2014 at 01:51:27PM +0100, Igor Mammedov wrote:
> > > Since introduction of PCIHP, it became problematic to
> > > punch hole in PCI0._CRS statically since PCI hotplug
> > > region size became runtime changeable.
> > 
> > What makes it runtime changeable?
> piix machine "acpi-pci-hotplug-with-bridge-support=on" effectively
> changes size of pcihp MMIO region

It adds 4 bytes. Let's just reserve a reasonably sized region,
like 512 bytes.

> > 
> > > So replace static hole punching with dynamically consumed
> > > resources in a child device on PCI0 bus. i.e generate
> > > PNP0C02 device as a child of PCI0 bus at runtime and
> > > consume GPE0, PCI/CPU hotplug IO resources in it instead
> > > of punching holes in static PCI0._CRS.
> > 
> > It seems that we are being too exact with
> > IO resources here.
> > Can't we roughly reserve 0xae00 to 0xafff
> > and be done with it?
> that would be easiest way for this specific case if we could agree
> for ranges on PIIX/Q35 machines.
> 
> But I also use it as excuse to introduce ASL like macros so that
> building dynamic SSDT would be easier i.e. replace template patching
> with single place where dynamic device is defined and its values
> are set in simple and straightforward manner.

We don't need excuses really :)
But the approach itself needs some work IMHO, in particular
ASL like syntax by using macros and varargs is not something to strive
for IMHO :)

Why don't we just pass GArrays around?

crs = build_alloc_crs();
build_append_resource(crs, ....)



Or if you really want this, using array of GArray:

build_append_crs(ssdt,
                {
                        build_alloc_resource(...),
                        build_alloc_resource(...),
                        build_alloc_resource(...),
                        build_alloc_resource(...),
                        NULL
                })


> > > Tested with Windows XPsp3, Vista, Windows Server 2003, 2008, 2012r2.
> > > 
> > > PS:
> > > Series adds several ASL like macros to simplify
> > > code for dynamic generation of AML structures.
> > > 
> > > Igor Mammedov (9):
> > >   Revert "pc: Q35 DSDT: exclude CPU hotplug IO range from PCI bus
> > >     resources"
> > >   Revert "pc: PIIX DSDT: exclude CPU/PCI hotplug & GPE0 IO range from
> > >     PCI bus resources"
> > >   Partial revert "pc: ACPI: expose PRST IO range via _CRS"
> > >   acpi: replace opencoded opcodes with defines
> > >   acpi: add PNP0C02 to PCI0 bus
> > >   acpi: consume GPE0 IO resources in PNP0C02 device
> > >   acpi: consume CPU hotplug IO resource in PNP0C02 device
> > >   pcihp: expose PCI hotplug MMIO base/length as properties of piix4pm
> > >   acpi: consume PCIHP IO resource in PNP0C02 device
> > > 
> > >  hw/acpi/pcihp.c                   |   28 ++++++
> > >  hw/acpi/piix4.c                   |    1 +
> > >  hw/i386/acpi-build.c              |  177 
> > > ++++++++++++++++++++++++++++++++++--
> > >  hw/i386/acpi-dsdt-cpu-hotplug.dsl |   11 ---
> > >  hw/i386/acpi-dsdt-pci-crs.dsl     |   15 +++-
> > >  hw/i386/acpi-dsdt.dsl             |   39 --------
> > >  hw/i386/q35-acpi-dsdt.dsl         |   16 ----
> > >  include/hw/acpi/pcihp.h           |    4 +
> > >  8 files changed, 214 insertions(+), 77 deletions(-)
> 
> 
> -- 
> Regards,
>   Igor