It's an alternative approach to
 1) [PATCH hack dontapply v2 0/7] Dynamic _CST generation
which instead of dynamic AML loading uses static AML with
dynamic values.  It allows us to keep firmware blob static and
to avoid split firmware issue (1) in case of cross version migration.

ABI in this case is confined to cpu hotplug IO registers
(i.e. do it old school way, like we used to do so far).
This way we don't have to add yet another ABI to keep dynamic
AML code under control (1).

Tested  with: XPsp3 - ws2106 guests.

CC: "Michael S. Tsirkin" <mst@redhat.com>


Igor Mammedov (3):
  acpi: add aml_create_byte_field()
  pc: acpi: add _CST support
  acpi: add support for CST update notification

Michael S. Tsirkin (1):
  acpi: aml: add aml_register()

 include/hw/acpi/aml-build.h     |   6 ++
 include/hw/acpi/cpu.h           |  10 +++
 docs/specs/acpi_cpu_hotplug.txt |  21 +++++-
 hw/acpi/aml-build.c             |  28 +++++++
 hw/acpi/cpu.c                   | 158 +++++++++++++++++++++++++++++++++++++++-
 hw/acpi/piix4.c                 |   2 +
 hw/i386/acpi-build.c            |   5 +-
 tests/bios-tables-test.c        |   1 +
 8 files changed, 225 insertions(+), 6 deletions(-)

-- 
2.7.4

On Wed, Aug 08, 2018 at 05:15:45PM +0200, Igor Mammedov wrote:
> It's an alternative approach to
>  1) [PATCH hack dontapply v2 0/7] Dynamic _CST generation
> which instead of dynamic AML loading uses static AML with
> dynamic values.  It allows us to keep firmware blob static and
> to avoid split firmware issue (1) in case of cross version migration.

I think there's a misunderstanding. That patch only declares a couple of
states but that is just for debugging/demonstration purposes.  A typical
real CPU has more states (e.g. some intel CPUs have ~10 levels).


> ABI in this case is confined to cpu hotplug IO registers
> (i.e. do it old school way, like we used to do so far).
> This way we don't have to add yet another ABI to keep dynamic
> AML code under control (1).
> 
> Tested  with: XPsp3 - ws2106 guests.
> 
> CC: "Michael S. Tsirkin" <mst@redhat.com>
> 
> 
> Igor Mammedov (3):
>   acpi: add aml_create_byte_field()
>   pc: acpi: add _CST support
>   acpi: add support for CST update notification
> 
> Michael S. Tsirkin (1):
>   acpi: aml: add aml_register()
> 
>  include/hw/acpi/aml-build.h     |   6 ++
>  include/hw/acpi/cpu.h           |  10 +++
>  docs/specs/acpi_cpu_hotplug.txt |  21 +++++-
>  hw/acpi/aml-build.c             |  28 +++++++
>  hw/acpi/cpu.c                   | 158 +++++++++++++++++++++++++++++++++++++++-
>  hw/acpi/piix4.c                 |   2 +
>  hw/i386/acpi-build.c            |   5 +-
>  tests/bios-tables-test.c        |   1 +
>  8 files changed, 225 insertions(+), 6 deletions(-)
> 
> -- 
> 2.7.4

On Wed, 8 Aug 2018 23:20:25 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Aug 08, 2018 at 05:15:45PM +0200, Igor Mammedov wrote:
> > It's an alternative approach to
> >  1) [PATCH hack dontapply v2 0/7] Dynamic _CST generation
> > which instead of dynamic AML loading uses static AML with
> > dynamic values.  It allows us to keep firmware blob static and
> > to avoid split firmware issue (1) in case of cross version migration.  
> 
> I think there's a misunderstanding. That patch only declares a couple of
> states but that is just for debugging/demonstration purposes.  A typical
> real CPU has more states (e.g. some intel CPUs have ~10 levels).
yep, baremetal has more CStates, but that depends on what we are trying to
achieve here. If goal is to use mwait/monitor instead o halt when idle
to reduce wakeup latencies then 1 entry in _CST is sufficient.
If target is reduce power consumption than we would need more entries.
In the later case _CST is probably not sufficient as it provides only
power/latency pair while intel_idle also provides target residence.
So if power mgmt target is considered, we need to use new _LPI
which means guest will need support for it as well, not sure if x86
kernel supports it nor aware of such support in Windows (hopefully it 
would just ignore unknown method)

> 
> > ABI in this case is confined to cpu hotplug IO registers
> > (i.e. do it old school way, like we used to do so far).
> > This way we don't have to add yet another ABI to keep dynamic
> > AML code under control (1).
> > 
> > Tested  with: XPsp3 - ws2106 guests.
> > 
> > CC: "Michael S. Tsirkin" <mst@redhat.com>
> > 
> > 
> > Igor Mammedov (3):
> >   acpi: add aml_create_byte_field()
> >   pc: acpi: add _CST support
> >   acpi: add support for CST update notification
> > 
> > Michael S. Tsirkin (1):
> >   acpi: aml: add aml_register()
> > 
> >  include/hw/acpi/aml-build.h     |   6 ++
> >  include/hw/acpi/cpu.h           |  10 +++
> >  docs/specs/acpi_cpu_hotplug.txt |  21 +++++-
> >  hw/acpi/aml-build.c             |  28 +++++++
> >  hw/acpi/cpu.c                   | 158 +++++++++++++++++++++++++++++++++++++++-
> >  hw/acpi/piix4.c                 |   2 +
> >  hw/i386/acpi-build.c            |   5 +-
> >  tests/bios-tables-test.c        |   1 +
> >  8 files changed, 225 insertions(+), 6 deletions(-)
> > 
> > -- 
> > 2.7.4  

On Thu, Aug 09, 2018 at 12:13:24PM +0200, Igor Mammedov wrote:
> On Wed, 8 Aug 2018 23:20:25 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Wed, Aug 08, 2018 at 05:15:45PM +0200, Igor Mammedov wrote:
> > > It's an alternative approach to
> > >  1) [PATCH hack dontapply v2 0/7] Dynamic _CST generation
> > > which instead of dynamic AML loading uses static AML with
> > > dynamic values.  It allows us to keep firmware blob static and
> > > to avoid split firmware issue (1) in case of cross version migration.  
> > 
> > I think there's a misunderstanding. That patch only declares a couple of
> > states but that is just for debugging/demonstration purposes.  A typical
> > real CPU has more states (e.g. some intel CPUs have ~10 levels).
> yep, baremetal has more CStates, but that depends on what we are trying to
> achieve here.
> If goal is to use mwait/monitor instead o halt when idle
> to reduce wakeup latencies then 1 entry in _CST is sufficient.

1 entry is not sufficient in my testing. My testing shows
we want to stay in guest for states with latency up to ~200 usec
to get measureable latency gains.


> If target is reduce power consumption than we would need more entries.
> In the later case _CST is probably not sufficient as it provides only
> power/latency pair while intel_idle also provides target residence.
> So if power mgmt target is considered, we need to use new _LPI
> which means guest will need support for it as well, not sure if x86
> kernel supports it nor aware of such support in Windows (hopefully it 
> would just ignore unknown method)

I think we want to address both goals.

Linux on x86 does support _LPI and I agree it's a good idea to be able
to pass that in. Will take a look. But intel_idle seems to just set
target = latency * 2 so I am not sure it's all that important.

> > 
> > > ABI in this case is confined to cpu hotplug IO registers
> > > (i.e. do it old school way, like we used to do so far).
> > > This way we don't have to add yet another ABI to keep dynamic
> > > AML code under control (1).
> > > 
> > > Tested  with: XPsp3 - ws2106 guests.
> > > 
> > > CC: "Michael S. Tsirkin" <mst@redhat.com>
> > > 
> > > 
> > > Igor Mammedov (3):
> > >   acpi: add aml_create_byte_field()
> > >   pc: acpi: add _CST support
> > >   acpi: add support for CST update notification
> > > 
> > > Michael S. Tsirkin (1):
> > >   acpi: aml: add aml_register()
> > > 
> > >  include/hw/acpi/aml-build.h     |   6 ++
> > >  include/hw/acpi/cpu.h           |  10 +++
> > >  docs/specs/acpi_cpu_hotplug.txt |  21 +++++-
> > >  hw/acpi/aml-build.c             |  28 +++++++
> > >  hw/acpi/cpu.c                   | 158 +++++++++++++++++++++++++++++++++++++++-
> > >  hw/acpi/piix4.c                 |   2 +
> > >  hw/i386/acpi-build.c            |   5 +-
> > >  tests/bios-tables-test.c        |   1 +
> > >  8 files changed, 225 insertions(+), 6 deletions(-)
> > > 
> > > -- 
> > > 2.7.4  

On Fri, 10 Aug 2018 02:09:58 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Thu, Aug 09, 2018 at 12:13:24PM +0200, Igor Mammedov wrote:
> > On Wed, 8 Aug 2018 23:20:25 +0300
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >   
> > > On Wed, Aug 08, 2018 at 05:15:45PM +0200, Igor Mammedov wrote:  
> > > > It's an alternative approach to
> > > >  1) [PATCH hack dontapply v2 0/7] Dynamic _CST generation
> > > > which instead of dynamic AML loading uses static AML with
> > > > dynamic values.  It allows us to keep firmware blob static and
> > > > to avoid split firmware issue (1) in case of cross version migration.    
> > > 
> > > I think there's a misunderstanding. That patch only declares a couple of
> > > states but that is just for debugging/demonstration purposes.  A typical
> > > real CPU has more states (e.g. some intel CPUs have ~10 levels).  
> > yep, baremetal has more CStates, but that depends on what we are trying to
> > achieve here.
> > If goal is to use mwait/monitor instead o halt when idle
> > to reduce wakeup latencies then 1 entry in _CST is sufficient.  
> 
> 1 entry is not sufficient in my testing. My testing shows
> we want to stay in guest for states with latency up to ~200 usec
> to get measureable latency gains.
Ok, I'll add support for more CStates

> 
> > If target is reduce power consumption than we would need more entries.
> > In the later case _CST is probably not sufficient as it provides only
> > power/latency pair while intel_idle also provides target residence.
> > So if power mgmt target is considered, we need to use new _LPI
> > which means guest will need support for it as well, not sure if x86
> > kernel supports it nor aware of such support in Windows (hopefully it 
> > would just ignore unknown method)  
> 
> I think we want to address both goals.
> 
> Linux on x86 does support _LPI and I agree it's a good idea to be able
> to pass that in. Will take a look. But intel_idle seems to just set
> target = latency * 2 so I am not sure it's all that important.
it's fairly recent, but if we plan in advance we can support it
as well.

> 
> > >   
> > > > ABI in this case is confined to cpu hotplug IO registers
> > > > (i.e. do it old school way, like we used to do so far).
> > > > This way we don't have to add yet another ABI to keep dynamic
> > > > AML code under control (1).
> > > > 
> > > > Tested  with: XPsp3 - ws2106 guests.
> > > > 
> > > > CC: "Michael S. Tsirkin" <mst@redhat.com>
> > > > 
> > > > 
> > > > Igor Mammedov (3):
> > > >   acpi: add aml_create_byte_field()
> > > >   pc: acpi: add _CST support
> > > >   acpi: add support for CST update notification
> > > > 
> > > > Michael S. Tsirkin (1):
> > > >   acpi: aml: add aml_register()
> > > > 
> > > >  include/hw/acpi/aml-build.h     |   6 ++
> > > >  include/hw/acpi/cpu.h           |  10 +++
> > > >  docs/specs/acpi_cpu_hotplug.txt |  21 +++++-
> > > >  hw/acpi/aml-build.c             |  28 +++++++
> > > >  hw/acpi/cpu.c                   | 158 +++++++++++++++++++++++++++++++++++++++-
> > > >  hw/acpi/piix4.c                 |   2 +
> > > >  hw/i386/acpi-build.c            |   5 +-
> > > >  tests/bios-tables-test.c        |   1 +
> > > >  8 files changed, 225 insertions(+), 6 deletions(-)
> > > > 
> > > > -- 
> > > > 2.7.4