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

2014-02-19 Thread Igor Mammedov
On Tue, 18 Feb 2014 23:04:13 +0100
Laszlo Ersek ler...@redhat.com wrote:

 On 02/18/14 17:36, Igor Mammedov wrote:
  On Mon, 17 Feb 2014 09:32:35 +0100
  Gerd Hoffmann kra...@redhat.com wrote:
  
  On So, 2014-02-16 at 17:53 +0200, Michael S. Tsirkin 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?
 
  machine type.  q35 / piix map them at different locations.
 
  Also we might want to this also for devices which are
  runtime-configurable (isa-debugcon, pvpanic, ...).
  I'd convert simple devices that conditionally enabled at
  startup time, from static definition + patching into
  completely dynamically generated when device present.
  For example pvpanic falls in to this category.
  
  That would result in smaller ACPI tables guest has to deal with.
 
 I could be mistaken, but AFAIR this caused the windows device manager to
 pop up in windows? Ie. if you have a windows guest and cold-boot it
 twice, once with the device present (generated into ACPI) and once with
 the device absent (not generated into ACPI), then you get hardware
 changes. Whereas, if the device is always present and you only patch
 _STA, then windows doesn't perceive it as a hw change.

Is is irrelevant whether device is statically or dynamically created,
user will face the same issue and has a choice to install driver or tell
windows to ignore device forever.

 Do I recall it right?...
Device manager pop-ups only once when new device is added to install driver.
If later, the device in the same location with the same id appears/disappears,
Device manager handles it silently.

 
 You could argue that a new device indeed warrants a device manager
 popup, but esp. for isa-debugcon and pvpanic, you might want to enable
 those opportunistically, without triggering a new hw dialog. Pvpanic
 triggering the device manager was exactly what drew frowns, for its
 original implementation. IIRC.
the above applies to these cases as well, i.e. if you install driver for
it then there won't be any pop-ups later. If there is no driver
(which is the case) one needs to tell to Device manager to ignore
this device, and then there shouldn't be an additional pop-ups later.

 Anyway pls. feel free to ignore this comment, it just crossed my mind.
 (And of course it's not related to your series.)
 
 Thanks
 Laszlo
 




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

2014-02-18 Thread Igor Mammedov
On Mon, 17 Feb 2014 13:02:18 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 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 m...@redhat.com 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.
Reserve where and for how log it will be enough?
Eventually we will continue punching holes or increasing size of
this one big hole.

Q35 adds more to the problem with guest programmable pm_io_base,
we can't statically punch hole for it.

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

I don't like much macros myself, but compared to alternative
approach ^^^ (I've tried it) it's still harder to read/write
ASL constructs right.

I think that creating AML using ASL like flow and primitives
is better than building it using custom APIs, since it looks
like it was documented in spec.
Macros+vararg provide a necessary degree of flexibility to
create AML using ASL like flow and constructs.

 
 
 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
   })
that would work if there was a way to specify arbitrary statements
inside embedded array (like in the last hunk of [9/9]).

 
 
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




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

2014-02-18 Thread Michael S. Tsirkin
On Tue, Feb 18, 2014 at 12:10:27PM +0100, Igor Mammedov wrote:
 On Mon, 17 Feb 2014 13:02:18 +0200
 Michael S. Tsirkin m...@redhat.com wrote:
 
  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 m...@redhat.com 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.
 Reserve where and for how log it will be enough?
 Eventually we will continue punching holes or increasing size of
 this one big hole.

I agree this might become necessary.  But why not keep it simple while
we can? The advantage of ACPI in qemu is we can fix it without
touching guest code. Let's use it.

So if there's just pm_io_base, we can just write
it in ASL and patch in that value.

 
 Q35 adds more to the problem with guest programmable pm_io_base,
 we can't statically punch hole for it.

Aha. That's a good point.
But I still worry how we'll handle e.g. pvpanic.
Paolo had an idea to scan the memory range,
and punch holes for each section, making an
exception for regions owned by PCI devices.
I thought it would work originally too, but it
seems that e.g. VGA range is also owned by pci
devices sometimes.


  

 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, )
 
 I don't like much macros myself, but compared to alternative
 approach ^^^ (I've tried it) it's still harder to read/write
 ASL constructs right.
 
 I think that creating AML using ASL like flow and primitives
 is better than building it using custom APIs, since it looks
 like it was documented in spec.
 Macros+vararg provide a necessary degree of flexibility to
 create AML using ASL like flow and constructs.

Only you lose all type safety. When compiling ASL with IASL
at least it does some type checking.

  
  
  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
  })
 that would work if there was a way to specify arbitrary statements
 inside embedded array (like in the last hunk of [9/9]).

In C we have functions for this :)
Last chunk:

-if (pm-pcihp_io_base) {
-ACPI_IO(RESBUF, Decode16,
-pm-pcihp_io_base, /* _MIN */
-pm-pcihp_io_base, /* _MAX */
-0x0,   /* _ALN */
-pm-pcihp_io_len); /* _LEN */
-}


+   build_io_resource_16(pm-pcihp_io_base, pm-pcihp_io_base,
 0x0, pm-pcihp_io_len);


Which actually makes one think. Isn't _MAX wrong here?
I'd expect pm-pcihp_io_base + pm-pcihp_io_len - 1

I would also key this off pcihp_io_len.

  
  
 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 

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

2014-02-18 Thread Igor Mammedov
On Tue, 18 Feb 2014 13:33:17 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Tue, Feb 18, 2014 at 12:10:27PM +0100, Igor Mammedov wrote:
  On Mon, 17 Feb 2014 13:02:18 +0200
  Michael S. Tsirkin m...@redhat.com wrote:
  
   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 m...@redhat.com 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.
  Reserve where and for how log it will be enough?
  Eventually we will continue punching holes or increasing size of
  this one big hole.
 
 I agree this might become necessary.  But why not keep it simple while
 we can? The advantage of ACPI in qemu is we can fix it without
 touching guest code. Let's use it.
 
 So if there's just pm_io_base, we can just write
 it in ASL and patch in that value.
 
  
  Q35 adds more to the problem with guest programmable pm_io_base,
  we can't statically punch hole for it.
 
 Aha. That's a good point.
 But I still worry how we'll handle e.g. pvpanic.
 Paolo had an idea to scan the memory range,
 and punch holes for each section, making an
 exception for regions owned by PCI devices.
 I thought it would work originally too, but it
 seems that e.g. VGA range is also owned by pci
 devices sometimes.
from ACPI pov, I think we need to punch holes or define
_CRS in respective consuming device explicitly, doing it
automatically looping through MemoryRegions going to
be tricky.

but for SeaBIOS to find out what ports are reserved
looping might be a right way, if it needs this information.

 
   
 
  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, )
  
  I don't like much macros myself, but compared to alternative
  approach ^^^ (I've tried it) it's still harder to read/write
  ASL constructs right.
  
  I think that creating AML using ASL like flow and primitives
  is better than building it using custom APIs, since it looks
  like it was documented in spec.
  Macros+vararg provide a necessary degree of flexibility to
  create AML using ASL like flow and constructs.
 
 Only you lose all type safety. When compiling ASL with IASL
 at least it does some type checking.
short of embedded IASL solution, it's the same type safety as we
could provide with C, with extra checks inside primitives.
So that API user won't have to worry about it.

 
   
   
   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
 })
  that would work if there was a way to specify arbitrary statements
  inside embedded array (like in the last hunk of [9/9]).
 
 In C we have functions for this :)
I don't say it's impossible to write using C functions to express the same
logic, but it would be harder to read/write code that way. For example below
snippet would look like:

GArray *return_array_if(bool, garray){
 if (bool == true) return garray;
 return empty_garray;
}

build_append_crs(ssdt,
{
build_alloc_resource(...),
...,
return_array_if(have_pcihp,
build_alloc_resource(...)
),

and that will turn out in explosive growth of return_array_if like
functions to accommodate every tweak we would to do.
On 

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

2014-02-18 Thread Igor Mammedov
On Mon, 17 Feb 2014 09:32:35 +0100
Gerd Hoffmann kra...@redhat.com wrote:

 On So, 2014-02-16 at 17:53 +0200, Michael S. Tsirkin 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?
 
 machine type.  q35 / piix map them at different locations.
 
 Also we might want to this also for devices which are
 runtime-configurable (isa-debugcon, pvpanic, ...).
I'd convert simple devices that conditionally enabled at
startup time, from static definition + patching into
completely dynamically generated when device present.
For example pvpanic falls in to this category.

That would result in smaller ACPI tables guest has to deal with.

 
 cheers,
   Gerd
 
 




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

2014-02-18 Thread Igor Mammedov
On Mon, 17 Feb 2014 09:29:20 +0100
Gerd Hoffmann kra...@redhat.com wrote:

 On Fr, 2014-02-07 at 13:51 +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.
  
  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.
 
 Nice.  Can you try to do that for the mmconf xbar in memory space too
 while being at it?
If I manage to convince mst in using macro approach.

PS:
Although his build_append_foo() functions have tremendously improved
dynamic AML generation (made it much less error-prone), the
current acpi-build.c becomes harder to read (just look at new pcihp
parts). So I'm arguing in favor of more simple/intuitive API
approach even if it uses macros (I'm really not fun of it,
but result is much clear/maintainable code).

 
 cheers,
   Gerd
 
 




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

2014-02-18 Thread Laszlo Ersek
On 02/18/14 17:36, Igor Mammedov wrote:
 On Mon, 17 Feb 2014 09:32:35 +0100
 Gerd Hoffmann kra...@redhat.com wrote:
 
 On So, 2014-02-16 at 17:53 +0200, Michael S. Tsirkin 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?

 machine type.  q35 / piix map them at different locations.

 Also we might want to this also for devices which are
 runtime-configurable (isa-debugcon, pvpanic, ...).
 I'd convert simple devices that conditionally enabled at
 startup time, from static definition + patching into
 completely dynamically generated when device present.
 For example pvpanic falls in to this category.
 
 That would result in smaller ACPI tables guest has to deal with.

I could be mistaken, but AFAIR this caused the windows device manager to
pop up in windows? Ie. if you have a windows guest and cold-boot it
twice, once with the device present (generated into ACPI) and once with
the device absent (not generated into ACPI), then you get hardware
changes. Whereas, if the device is always present and you only patch
_STA, then windows doesn't perceive it as a hw change.

Do I recall it right?...

You could argue that a new device indeed warrants a device manager
popup, but esp. for isa-debugcon and pvpanic, you might want to enable
those opportunistically, without triggering a new hw dialog. Pvpanic
triggering the device manager was exactly what drew frowns, for its
original implementation. IIRC.

Anyway pls. feel free to ignore this comment, it just crossed my mind.
(And of course it's not related to your series.)

Thanks
Laszlo




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

2014-02-17 Thread Gerd Hoffmann
On Fr, 2014-02-07 at 13:51 +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.
 
 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.

Nice.  Can you try to do that for the mmconf xbar in memory space too
while being at it?

cheers,
  Gerd





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

2014-02-17 Thread Gerd Hoffmann
On So, 2014-02-16 at 17:53 +0200, Michael S. Tsirkin 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?

machine type.  q35 / piix map them at different locations.

Also we might want to this also for devices which are
runtime-configurable (isa-debugcon, pvpanic, ...).

cheers,
  Gerd





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

2014-02-17 Thread Michael S. Tsirkin
On Mon, Feb 17, 2014 at 09:32:35AM +0100, Gerd Hoffmann wrote:
 On So, 2014-02-16 at 17:53 +0200, Michael S. Tsirkin 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?
 
 machine type.  q35 / piix map them at different locations.

That's not dynamic.  We can load the correct ones per DSDT.

 Also we might want to this also for devices which are
 runtime-configurable (isa-debugcon, pvpanic, ...).

That's more convincing, but I don't want
knowledge of all these devices in acpi-build.
Also we need to make seabios avoid these ranges
when enumerating devices.
How does it know to avoid them ATM?

 
 cheers,
   Gerd
 



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

2014-02-17 Thread Igor Mammedov
On Sun, 16 Feb 2014 17:53:45 +0200
Michael S. Tsirkin m...@redhat.com 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
 
  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.

  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



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

2014-02-17 Thread Gerd Hoffmann
On Mo, 2014-02-17 at 12:28 +0200, Michael S. Tsirkin wrote:
 On Mon, Feb 17, 2014 at 09:32:35AM +0100, Gerd Hoffmann wrote:
  On So, 2014-02-16 at 17:53 +0200, Michael S. Tsirkin 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?
  
  machine type.  q35 / piix map them at different locations.
 
 That's not dynamic.  We can load the correct ones per DSDT.
 
  Also we might want to this also for devices which are
  runtime-configurable (isa-debugcon, pvpanic, ...).
 
 That's more convincing, but I don't want
 knowledge of all these devices in acpi-build.
 Also we need to make seabios avoid these ranges
 when enumerating devices.
 How does it know to avoid them ATM?

seabios maps io ports @ 0xc000 up.
recently it has changed to use 0x1000 - 0xa000 region
in case the hole above 0xc000 is too small.

In other words: It doesn't map anything below 0x1000 and it avoids
0xa000 - 0xbfff.  Hardcoded.  I want lift the later restriction on q35,
by moving pmbase (0xb000 atm) out of the way, so seabios can use the
whole 0x1000 - 0x range, but that is still wip.

cheers,
  Gerd






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

2014-02-17 Thread 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 m...@redhat.com 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



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

2014-02-17 Thread Michael S. Tsirkin
On Mon, Feb 17, 2014 at 11:46:13AM +0100, Gerd Hoffmann wrote:
 On Mo, 2014-02-17 at 12:28 +0200, Michael S. Tsirkin wrote:
  On Mon, Feb 17, 2014 at 09:32:35AM +0100, Gerd Hoffmann wrote:
   On So, 2014-02-16 at 17:53 +0200, Michael S. Tsirkin 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?
   
   machine type.  q35 / piix map them at different locations.
  
  That's not dynamic.  We can load the correct ones per DSDT.
  
   Also we might want to this also for devices which are
   runtime-configurable (isa-debugcon, pvpanic, ...).
  
  That's more convincing, but I don't want
  knowledge of all these devices in acpi-build.
  Also we need to make seabios avoid these ranges
  when enumerating devices.
  How does it know to avoid them ATM?
 
 seabios maps io ports @ 0xc000 up.
 recently it has changed to use 0x1000 - 0xa000 region
 in case the hole above 0xc000 is too small.
 
 In other words: It doesn't map anything below 0x1000 and it avoids
 0xa000 - 0xbfff.  Hardcoded.  I want lift the later restriction on q35,
 by moving pmbase (0xb000 atm) out of the way, so seabios can use the
 whole 0x1000 - 0x range, but that is still wip.
 
 cheers,
   Gerd
 

okay so we'll want some fwcfg for that correct?
whatever fills that, can share logic with acpi
generation :)

That might or might not make it dynamic enough to make it worth
bothering - alternative is just two version of acpi
depending on machine type.

-- 
MST



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

2014-02-16 Thread Michael S. Tsirkin
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?

 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?

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