Re: [Qemu-devel] [RFC 0/9] generate dynamic _CRS for motherboard resources
On Tue, 18 Feb 2014 23:04:13 +0100 Laszlo Ersek wrote: > On 02/18/14 17:36, Igor Mammedov wrote: > > On Mon, 17 Feb 2014 09:32:35 +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. > >> > >> 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
On 02/18/14 17:36, Igor Mammedov wrote: > On Mon, 17 Feb 2014 09:32:35 +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. >> >> 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
On Mon, 17 Feb 2014 09:29:20 +0100 Gerd Hoffmann 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
On Mon, 17 Feb 2014 09:32:35 +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. > > 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
On Tue, 18 Feb 2014 13:33:17 +0200
"Michael S. Tsirkin" 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" 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" 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(...),
...,
Re: [Qemu-devel] [RFC 0/9] generate dynamic _CRS for motherboard resources
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" 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" 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 resou
Re: [Qemu-devel] [RFC 0/9] generate dynamic _CRS for motherboard resources
On Mon, 17 Feb 2014 13:02:18 +0200
"Michael S. Tsirkin" 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" 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
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
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" 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
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
On Sun, 16 Feb 2014 17:53:45 +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? 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
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
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
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
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(-)
[Qemu-devel] [RFC 0/9] generate dynamic _CRS for motherboard resources
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. 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(-)
