Re: [Qemu-devel] [RFC untested PATCH] i386/cpu: make -cpu host support monitor/mwait
On Wed, Jun 13, 2018 at 09:22:10PM +0300, Michael S. Tsirkin wrote:
> On Wed, Jun 13, 2018 at 10:55:43AM +0200, Igor Mammedov wrote:
> > On Tue, 12 Jun 2018 11:49:22 -0300
> > Eduardo Habkost wrote:
> >
> > > On Tue, Jun 12, 2018 at 03:58:03PM +0200, Igor Mammedov wrote:
> > > [...]
> > > > > > > +if (xcc->host_cpuid_required && enable_cpu_pm) {
> > > > > > > +host_cpuid(5, 0, &cpu->mwait.eax, &cpu->mwait.ebx,
> > > > > > > + &cpu->mwait.ecx, &cpu->mwait.edx);
> > > > > > > +}
> > [...]
> > > > > > also max_x86_cpu_initfn() might be better place for filling it up.
> > >
> > > Why?
> > I've missed 'enable_cpu_pm' which is probably a property,
> > so yep it can't go into initfn.
> >
> > However if we not going to migrate this state or use outside of
> > cpu_x86_cpuid(), I don't see why we should add it to X86CPU state
> > and keep around.
> > We can query it on demand from cpu_x86_cpuid() like we do for
> > PMU leaf.
>
> We can do it but the annoying thing is that it spreads
> the info around as we also need to set CPUID_EXT_MONITOR.
>
> This way we set all the info in one place on realize.
I agree. It also makes it easier to make the flags configurable
in the future.
We may eventually move the host-cache-info code to realizefn in
the future, too (if we want to make it work with VCPU topologies
that don't match the host exactly).
--
Eduardo
Re: [Qemu-devel] [RFC untested PATCH] i386/cpu: make -cpu host support monitor/mwait
On Wed, Jun 13, 2018 at 10:55:43AM +0200, Igor Mammedov wrote:
> On Tue, 12 Jun 2018 11:49:22 -0300
> Eduardo Habkost wrote:
>
> > On Tue, Jun 12, 2018 at 03:58:03PM +0200, Igor Mammedov wrote:
> > [...]
> > > > > > +if (xcc->host_cpuid_required && enable_cpu_pm) {
> > > > > > +host_cpuid(5, 0, &cpu->mwait.eax, &cpu->mwait.ebx,
> > > > > > + &cpu->mwait.ecx, &cpu->mwait.edx);
> > > > > > +}
> [...]
> > > > > also max_x86_cpu_initfn() might be better place for filling it up.
> >
> > Why?
> I've missed 'enable_cpu_pm' which is probably a property,
> so yep it can't go into initfn.
>
> However if we not going to migrate this state or use outside of
> cpu_x86_cpuid(), I don't see why we should add it to X86CPU state
> and keep around.
> We can query it on demand from cpu_x86_cpuid() like we do for
> PMU leaf.
We can do it but the annoying thing is that it spreads
the info around as we also need to set CPUID_EXT_MONITOR.
This way we set all the info in one place on realize.
--
MST
Re: [Qemu-devel] [RFC untested PATCH] i386/cpu: make -cpu host support monitor/mwait
On Tue, 12 Jun 2018 11:49:22 -0300
Eduardo Habkost wrote:
> On Tue, Jun 12, 2018 at 03:58:03PM +0200, Igor Mammedov wrote:
> [...]
> > > > > +if (xcc->host_cpuid_required && enable_cpu_pm) {
> > > > > +host_cpuid(5, 0, &cpu->mwait.eax, &cpu->mwait.ebx,
> > > > > + &cpu->mwait.ecx, &cpu->mwait.edx);
> > > > > +}
[...]
> > > > also max_x86_cpu_initfn() might be better place for filling it up.
>
> Why?
I've missed 'enable_cpu_pm' which is probably a property,
so yep it can't go into initfn.
However if we not going to migrate this state or use outside of
cpu_x86_cpuid(), I don't see why we should add it to X86CPU state
and keep around.
We can query it on demand from cpu_x86_cpuid() like we do for
PMU leaf.
Re: [Qemu-devel] [RFC untested PATCH] i386/cpu: make -cpu host support monitor/mwait
On Tue, Jun 12, 2018 at 03:58:03PM +0200, Igor Mammedov wrote:
[...]
> > > > +if (xcc->host_cpuid_required && enable_cpu_pm) {
> > > > +host_cpuid(5, 0, &cpu->mwait.eax, &cpu->mwait.ebx,
> > > > + &cpu->mwait.ecx, &cpu->mwait.edx);
> > > > +}
> > > could this state be migrated? or 'host' is still unmigratable?
> >
> > Host is still unmigratable.
> 'max' cpu model has 'migratable = true' property and 'host' is inherited from
> it,
> hence was the question.
Host can be migratable if and only if the source and destination
CPUs + kernel + QEMU are exactly the same. Not something I would
recommend to anyone, but still something we try not to break.
>
> >
> > > also max_x86_cpu_initfn() might be better place for filling it up.
Why?
--
Eduardo
Re: [Qemu-devel] [RFC untested PATCH] i386/cpu: make -cpu host support monitor/mwait
On Tue, 12 Jun 2018 15:57:19 +0300
"Michael S. Tsirkin" wrote:
> On Tue, Jun 12, 2018 at 02:56:05PM +0200, Igor Mammedov wrote:
> > On Fri, 8 Jun 2018 23:59:19 +0300
> > "Michael S. Tsirkin" wrote:
> >
> > > When guest CPU PM is enabled, and with -cpu host, expose the host CPU
> > > MWAIT leaf to guest so guest can make good PM decisions.
> > >
> > > Signed-off-by: Michael S. Tsirkin
> > > ---
> > >
> > > This builds but is untested. Is this a reasonable way to go about it?
> > >
> > > target/i386/cpu.h | 9 +
> > > target/i386/cpu.c | 18 +-
> > > 2 files changed, 22 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > > index 664504610e..309f804573 100644
> > > --- a/target/i386/cpu.h
> > > +++ b/target/i386/cpu.h
> > > @@ -1378,6 +1378,15 @@ struct X86CPU {
> > > /* if true the CPUID code directly forward host cache leaves to the
> > > guest */
> > > bool cache_info_passthrough;
> > >
> > > +/* if true the CPUID code directly forwards
> > > + * host monitor/mwait leaves to the guest */
> > > +struct {
> > > +uint32_t eax;
> > > +uint32_t ebx;
> > > +uint32_t ecx;
> > > +uint32_t edx;
> > > +} mwait;
> > > +
> > > /* Features that were filtered out because of missing host
> > > capabilities */
> > > uint32_t filtered_features[FEATURE_WORDS];
> > >
> > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > > index 94260412e2..a49443de56 100644
> > > --- a/target/i386/cpu.c
> > > +++ b/target/i386/cpu.c
> > > @@ -3760,11 +3760,11 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t
> > > index, uint32_t count,
> > > }
> > > break;
> > > case 5:
> > > -/* mwait info: needed for Core compatibility */
> > > -*eax = 0; /* Smallest monitor-line size in bytes */
> > > -*ebx = 0; /* Largest monitor-line size in bytes */
> > > -*ecx = CPUID_MWAIT_EMX | CPUID_MWAIT_IBE;
> > > -*edx = 0;
> > > +/* MONITOR/MWAIT Leaf */
> > > +*eax = cpu->mwait.eax; /* Smallest monitor-line size in bytes */
> > > +*ebx = cpu->mwait.ebx; /* Largest monitor-line size in bytes */
> > > +*ecx = cpu->mwait.ecx; /* flags */
> > > +*edx = cpu->mwait.edx; /* mwait substates */
> > > break;
> > > case 6:
> > > /* Thermal and Power Leaf */
> > > @@ -4595,6 +4595,14 @@ static void x86_cpu_realizefn(DeviceState *dev,
> > > Error **errp)
> > > goto out;
> > > }
> > >
> > > +if (xcc->host_cpuid_required && enable_cpu_pm) {
> > > +host_cpuid(5, 0, &cpu->mwait.eax, &cpu->mwait.ebx,
> > > + &cpu->mwait.ecx, &cpu->mwait.edx);
> > > +}
> > could this state be migrated? or 'host' is still unmigratable?
>
> Host is still unmigratable.
'max' cpu model has 'migratable = true' property and 'host' is inherited from
it,
hence was the question.
>
> > also max_x86_cpu_initfn() might be better place for filling it up.
> >
> > > +/* We always wake on interrupt even if host does not have the
> > > capability */
> > > +/* mwait extended info: needed for Core compatibility */
> > > +cpu->mwait.ecx |= CPUID_MWAIT_EMX | CPUID_MWAIT_IBE;
> > > +
> > > if (cpu->apic_id == UNASSIGNED_APIC_ID) {
> > > error_setg(errp, "apic-id property was not initialized
> > > properly");
> > > return;
Re: [Qemu-devel] [RFC untested PATCH] i386/cpu: make -cpu host support monitor/mwait
On Tue, Jun 12, 2018 at 02:56:05PM +0200, Igor Mammedov wrote:
> On Fri, 8 Jun 2018 23:59:19 +0300
> "Michael S. Tsirkin" wrote:
>
> > When guest CPU PM is enabled, and with -cpu host, expose the host CPU
> > MWAIT leaf to guest so guest can make good PM decisions.
> >
> > Signed-off-by: Michael S. Tsirkin
> > ---
> >
> > This builds but is untested. Is this a reasonable way to go about it?
> >
> > target/i386/cpu.h | 9 +
> > target/i386/cpu.c | 18 +-
> > 2 files changed, 22 insertions(+), 5 deletions(-)
> >
> > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > index 664504610e..309f804573 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > @@ -1378,6 +1378,15 @@ struct X86CPU {
> > /* if true the CPUID code directly forward host cache leaves to the
> > guest */
> > bool cache_info_passthrough;
> >
> > +/* if true the CPUID code directly forwards
> > + * host monitor/mwait leaves to the guest */
> > +struct {
> > +uint32_t eax;
> > +uint32_t ebx;
> > +uint32_t ecx;
> > +uint32_t edx;
> > +} mwait;
> > +
> > /* Features that were filtered out because of missing host
> > capabilities */
> > uint32_t filtered_features[FEATURE_WORDS];
> >
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 94260412e2..a49443de56 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -3760,11 +3760,11 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t
> > index, uint32_t count,
> > }
> > break;
> > case 5:
> > -/* mwait info: needed for Core compatibility */
> > -*eax = 0; /* Smallest monitor-line size in bytes */
> > -*ebx = 0; /* Largest monitor-line size in bytes */
> > -*ecx = CPUID_MWAIT_EMX | CPUID_MWAIT_IBE;
> > -*edx = 0;
> > +/* MONITOR/MWAIT Leaf */
> > +*eax = cpu->mwait.eax; /* Smallest monitor-line size in bytes */
> > +*ebx = cpu->mwait.ebx; /* Largest monitor-line size in bytes */
> > +*ecx = cpu->mwait.ecx; /* flags */
> > +*edx = cpu->mwait.edx; /* mwait substates */
> > break;
> > case 6:
> > /* Thermal and Power Leaf */
> > @@ -4595,6 +4595,14 @@ static void x86_cpu_realizefn(DeviceState *dev,
> > Error **errp)
> > goto out;
> > }
> >
> > +if (xcc->host_cpuid_required && enable_cpu_pm) {
> > +host_cpuid(5, 0, &cpu->mwait.eax, &cpu->mwait.ebx,
> > + &cpu->mwait.ecx, &cpu->mwait.edx);
> > +}
> could this state be migrated? or 'host' is still unmigratable?
Host is still unmigratable.
> also max_x86_cpu_initfn() might be better place for filling it up.
>
> > +/* We always wake on interrupt even if host does not have the
> > capability */
> > +/* mwait extended info: needed for Core compatibility */
> > +cpu->mwait.ecx |= CPUID_MWAIT_EMX | CPUID_MWAIT_IBE;
> > +
> > if (cpu->apic_id == UNASSIGNED_APIC_ID) {
> > error_setg(errp, "apic-id property was not initialized properly");
> > return;
Re: [Qemu-devel] [RFC untested PATCH] i386/cpu: make -cpu host support monitor/mwait
On Fri, 8 Jun 2018 23:59:19 +0300
"Michael S. Tsirkin" wrote:
> When guest CPU PM is enabled, and with -cpu host, expose the host CPU
> MWAIT leaf to guest so guest can make good PM decisions.
>
> Signed-off-by: Michael S. Tsirkin
> ---
>
> This builds but is untested. Is this a reasonable way to go about it?
>
> target/i386/cpu.h | 9 +
> target/i386/cpu.c | 18 +-
> 2 files changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 664504610e..309f804573 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1378,6 +1378,15 @@ struct X86CPU {
> /* if true the CPUID code directly forward host cache leaves to the
> guest */
> bool cache_info_passthrough;
>
> +/* if true the CPUID code directly forwards
> + * host monitor/mwait leaves to the guest */
> +struct {
> +uint32_t eax;
> +uint32_t ebx;
> +uint32_t ecx;
> +uint32_t edx;
> +} mwait;
> +
> /* Features that were filtered out because of missing host capabilities
> */
> uint32_t filtered_features[FEATURE_WORDS];
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 94260412e2..a49443de56 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -3760,11 +3760,11 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
> uint32_t count,
> }
> break;
> case 5:
> -/* mwait info: needed for Core compatibility */
> -*eax = 0; /* Smallest monitor-line size in bytes */
> -*ebx = 0; /* Largest monitor-line size in bytes */
> -*ecx = CPUID_MWAIT_EMX | CPUID_MWAIT_IBE;
> -*edx = 0;
> +/* MONITOR/MWAIT Leaf */
> +*eax = cpu->mwait.eax; /* Smallest monitor-line size in bytes */
> +*ebx = cpu->mwait.ebx; /* Largest monitor-line size in bytes */
> +*ecx = cpu->mwait.ecx; /* flags */
> +*edx = cpu->mwait.edx; /* mwait substates */
> break;
> case 6:
> /* Thermal and Power Leaf */
> @@ -4595,6 +4595,14 @@ static void x86_cpu_realizefn(DeviceState *dev, Error
> **errp)
> goto out;
> }
>
> +if (xcc->host_cpuid_required && enable_cpu_pm) {
> +host_cpuid(5, 0, &cpu->mwait.eax, &cpu->mwait.ebx,
> + &cpu->mwait.ecx, &cpu->mwait.edx);
> +}
could this state be migrated? or 'host' is still unmigratable?
also max_x86_cpu_initfn() might be better place for filling it up.
> +/* We always wake on interrupt even if host does not have the capability
> */
> +/* mwait extended info: needed for Core compatibility */
> +cpu->mwait.ecx |= CPUID_MWAIT_EMX | CPUID_MWAIT_IBE;
> +
> if (cpu->apic_id == UNASSIGNED_APIC_ID) {
> error_setg(errp, "apic-id property was not initialized properly");
> return;
Re: [Qemu-devel] [RFC untested PATCH] i386/cpu: make -cpu host support monitor/mwait
On Fri, Jun 08, 2018 at 11:59:19PM +0300, Michael S. Tsirkin wrote: > When guest CPU PM is enabled, and with -cpu host, expose the host CPU > MWAIT leaf to guest so guest can make good PM decisions. > > Signed-off-by: Michael S. Tsirkin > --- > > This builds but is untested. Is this a reasonable way to go about it? Looks good to me. Reviewed-by: Eduardo Habkost -- Eduardo
Re: [Qemu-devel] [RFC untested PATCH] i386/cpu: make -cpu host support monitor/mwait
Hi, This series failed docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. Type: series Message-id: 20180608205830.308627-1-...@redhat.com Subject: [Qemu-devel] [RFC untested PATCH] i386/cpu: make -cpu host support monitor/mwait === TEST SCRIPT BEGIN === #!/bin/bash set -e git submodule update --init dtc # Let docker tests dump environment info export SHOW_ENV=1 export J=8 time make docker-test-mingw@fedora === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 Switched to a new branch 'test' 3b1c49308c i386/cpu: make -cpu host support monitor/mwait === OUTPUT BEGIN === Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc' Cloning into '/var/tmp/patchew-tester-tmp-b0ch0y2u/src/dtc'... Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42' BUILD fedora make[1]: Entering directory '/var/tmp/patchew-tester-tmp-b0ch0y2u/src' GEN /var/tmp/patchew-tester-tmp-b0ch0y2u/src/docker-src.2018-06-08-17.19.44.2407/qemu.tar Cloning into '/var/tmp/patchew-tester-tmp-b0ch0y2u/src/docker-src.2018-06-08-17.19.44.2407/qemu.tar.vroot'... done. Your branch is up-to-date with 'origin/test'. Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc' Cloning into '/var/tmp/patchew-tester-tmp-b0ch0y2u/src/docker-src.2018-06-08-17.19.44.2407/qemu.tar.vroot/dtc'... Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42' Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered for path 'ui/keycodemapdb' Cloning into '/var/tmp/patchew-tester-tmp-b0ch0y2u/src/docker-src.2018-06-08-17.19.44.2407/qemu.tar.vroot/ui/keycodemapdb'... Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce' COPYRUNNER RUN test-mingw in qemu:fedora Packages installed: SDL2-devel-2.0.8-5.fc28.x86_64 bc-1.07.1-5.fc28.x86_64 bison-3.0.4-9.fc28.x86_64 bluez-libs-devel-5.49-3.fc28.x86_64 brlapi-devel-0.6.7-12.fc28.x86_64 bzip2-1.0.6-26.fc28.x86_64 bzip2-devel-1.0.6-26.fc28.x86_64 ccache-3.4.2-2.fc28.x86_64 clang-6.0.0-5.fc28.x86_64 device-mapper-multipath-devel-0.7.4-2.git07e7bd5.fc28.x86_64 findutils-4.6.0-19.fc28.x86_64 flex-2.6.1-7.fc28.x86_64 gcc-8.1.1-1.fc28.x86_64 gcc-c++-8.1.1-1.fc28.x86_64 gettext-0.19.8.1-14.fc28.x86_64 git-2.17.1-2.fc28.x86_64 glib2-devel-2.56.1-3.fc28.x86_64 glusterfs-api-devel-4.0.2-1.fc28.x86_64 gnutls-devel-3.6.2-1.fc28.x86_64 gtk3-devel-3.22.30-1.fc28.x86_64 hostname-3.20-3.fc28.x86_64 libaio-devel-0.3.110-11.fc28.x86_64 libasan-8.1.1-1.fc28.x86_64 libattr-devel-2.4.47-23.fc28.x86_64 libcap-devel-2.25-9.fc28.x86_64 libcap-ng-devel-0.7.9-1.fc28.x86_64 libcurl-devel-7.59.0-3.fc28.x86_64 libfdt-devel-1.4.6-4.fc28.x86_64 libpng-devel-1.6.34-3.fc28.x86_64 librbd-devel-12.2.5-1.fc28.x86_64 libssh2-devel-1.8.0-7.fc28.x86_64 libubsan-8.1.1-1.fc28.x86_64 libusbx-devel-1.0.21-6.fc28.x86_64 libxml2-devel-2.9.7-4.fc28.x86_64 llvm-6.0.0-11.fc28.x86_64 lzo-devel-2.08-12.fc28.x86_64 make-4.2.1-6.fc28.x86_64 mingw32-SDL2-2.0.5-3.fc27.noarch mingw32-bzip2-1.0.6-9.fc27.noarch mingw32-curl-7.57.0-1.fc28.noarch mingw32-glib2-2.54.1-1.fc28.noarch mingw32-gmp-6.1.2-2.fc27.noarch mingw32-gnutls-3.5.13-2.fc27.noarch mingw32-gtk3-3.22.16-1.fc27.noarch mingw32-libjpeg-turbo-1.5.1-3.fc27.noarch mingw32-libpng-1.6.29-2.fc27.noarch mingw32-libssh2-1.8.0-3.fc27.noarch mingw32-libtasn1-4.13-1.fc28.noarch mingw32-nettle-3.3-3.fc27.noarch mingw32-pixman-0.34.0-3.fc27.noarch mingw32-pkg-config-0.28-9.fc27.x86_64 mingw64-SDL2-2.0.5-3.fc27.noarch mingw64-bzip2-1.0.6-9.fc27.noarch mingw64-curl-7.57.0-1.fc28.noarch mingw64-glib2-2.54.1-1.fc28.noarch mingw64-gmp-6.1.2-2.fc27.noarch mingw64-gnutls-3.5.13-2.fc27.noarch mingw64-gtk3-3.22.16-1.fc27.noarch mingw64-libjpeg-turbo-1.5.1-3.fc27.noarch mingw64-libpng-1.6.29-2.fc27.noarch mingw64-libssh2-1.8.0-3.fc27.noarch mingw64-libtasn1-4.13-1.fc28.noarch mingw64-nettle-3.3-3.fc27.noarch mingw64-pixman-0.34.0-3.fc27.noarch mingw64-pkg-config-0.28-9.fc27.x86_64 ncurses-devel-6.1-5.20180224.fc28.x86_64 nettle-devel-3.4-2.fc28.x86_64 nss-devel-3.36.1-1.1.fc28.x86_64 numactl-devel-2.0.11-8.fc28.x86_64 package PyYAML is not installed package libjpeg-devel is not installed perl-5.26.2-411.fc28.x86_64 pixman-devel-0.34.0-8.fc28.x86_64 python3-3.6.5-1.fc28.x86_64 snappy-devel-1.1.7-5.fc28.x86_64 sparse-0.5.2-1.fc28.x86_64 spice-server-devel-0.14.0-4.fc28.x86_64 systemtap-sdt-devel-3.2-11.fc28.x86_64 tar-1.30-3.fc28.x86_64 usbredir-devel-0.7.1-7.fc28.x86_64 virglrenderer-devel-0.6.0-4.20170210git76b3da97b.fc28.x86_64 vte3-devel-0.36.5-6.fc28.x86_64 which-2.21-8.fc28.x86_64 xen-devel-4.10.1-3.fc28.x86_64 zlib-devel-1.2.11-8.fc28.x86_64 Environment variables: TARGET_LIST= PACKAGES=ccache gettext git tar PyYAML sparse flex bison python3 bzip2 hostname gcc gcc-c++ llvm clang make perl which bc findutils glib2-devel libaio-devel pixman-de
