Re: [PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes

[Paolo Bonzini]

On 06/05/2017 18:48, Gabriel L. Somlo wrote:
> So, in conclusion; it's not important to *me* that this old machine
> keeps working, I'm just volunteering test data points. So please don't
> feel obligated in any way to go out of your way on my account. OTOH,
> I'm happy to provide feedback as long as you would like me to.
> 
> Along the same lines: Paolo, as the author of commit 2c82878b0cb38fd,
> is the Xeon chip listed above one of the "obsolete for virtualization"
> models ?

Yes - I hadn't tested this model in particular, and this one is a little
less obsolete compared to the ones I found without NMI support (a 64-bit
Prescott and a 32-bit Yonah), but I still believe it's saner to treat
them as obsolete.

Can you please run vmxcap (from QEMU's git repository) on that processor
and include the output?

Paolo

> In that case, it makes no sense for me to keep using it for
> tests, and the fact that it misbehaves with L1 MWAIT should also not
> matter at all.

Re: [PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes

[Paolo Bonzini]

On 06/05/2017 18:48, Gabriel L. Somlo wrote:
> So, in conclusion; it's not important to *me* that this old machine
> keeps working, I'm just volunteering test data points. So please don't
> feel obligated in any way to go out of your way on my account. OTOH,
> I'm happy to provide feedback as long as you would like me to.
> 
> Along the same lines: Paolo, as the author of commit 2c82878b0cb38fd,
> is the Xeon chip listed above one of the "obsolete for virtualization"
> models ?

Yes - I hadn't tested this model in particular, and this one is a little
less obsolete compared to the ones I found without NMI support (a 64-bit
Prescott and a 32-bit Yonah), but I still believe it's saner to treat
them as obsolete.

Can you please run vmxcap (from QEMU's git repository) on that processor
and include the output?

Paolo

> In that case, it makes no sense for me to keep using it for
> tests, and the fact that it misbehaves with L1 MWAIT should also not
> matter at all.

Re: [PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes

[Gabriel L. Somlo]

On Thu, May 04, 2017 at 08:07:15PM +0200, Radim Krčmář wrote:
> 2017-05-04 13:56-0400, Gabriel L. Somlo:
> > If I wanted to test this (e.g. with OS X 10.8 guests on several of my older
> > Mac boxes running Fedora), which git repo would you have me use? (The series
> > won't apply directly on top of 
> > git://git.kernel.org/pub/scm/virt/kvm/kvm.git).
> 
> The queue branch of that repo.  This series depends on a patch that is
> applied there:
> 
>   668fffa3f838 kvm: better MWAIT emulation for guests
> 
> I forgot to mention that, sorry.

OK, here's where I'm at right now:

With this series applied on top of 'queue', my MacbookAir4,2 running
F25 (with the kvm/queue kernel) works fine, i.e. loads the kvm-intel
module successfully, and mwaits in L1 guest mode, reporting 400% cpu
but staying cool (guest started with -smp 4).

So far, so good.

On the MacPro1,1, I first had to revert 2c82878b0cb38fd
("KVM: VMX: require virtual NMI support") to get around this error:

# modprobe -v kvm-intel
insmod /lib/modules/4.11.0-rc3+/kernel/virt/lib/irqbypass.ko
insmod /lib/modules/4.11.0-rc3+/kernel/arch/x86/kvm/kvm.ko
insmod /lib/modules/4.11.0-rc3+/kernel/arch/x86/kvm/kvm-intel.ko
modprobe: ERROR: could not insert 'kvm_intel': Input/output error

Next, it turns out that on the MacPro1,1 kvm_mwait_in_guest() returns
TRUE, which causes OS X 10.7 (the one that mwaits without checking
CPUID) to misbehave. Forcing the function to return 0 (FALSE) solves
the problem:

diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index b49add7..249362c 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -216,9 +216,12 @@ static inline u64 nsec_to_cycles(struct kvm_vcpu *vcpu, 
u64 nsec)
 
 static inline bool kvm_mwait_in_guest(void)
 {
-   return boot_cpu_has(X86_FEATURE_MWAIT) &&
+   bool ret;
+   ret =  boot_cpu_has(X86_FEATURE_MWAIT) &&
!boot_cpu_has_bug(X86_BUG_AMD_E400) &&
!boot_cpu_has_bug(X86_BUG_MONITOR);
+   printk(KERN_INFO "kvm_mwait_in_guest: %d\n", ret);
+   return 0;
 }
 
 #endif

After this change, I get:

[ 1201.529002] kvm_mwait_in_guest: 1
[ 1201.529024] kvm_mwait_in_guest: 1
[ 1201.529029] kvm_mwait_in_guest: 1
[ 1201.529038] kvm_mwait_in_guest: 1
[ 1201.529047] kvm_mwait_in_guest: 1
[ 1225.150235] kvm: MONITOR instruction emulated as NOP!
[ 1225.150240] kvm: MWAIT instruction emulated as NOP!

indicating that it *would* have returned TRUE if I let it :)

This is a 2x dual-core Xeon, cca 2006 vintage, and the last (4th) CPU
in /proc/cpuinfo returns:

processor   : 3
vendor_id   : GenuineIntel
cpu family  : 6
model   : 15
model name  : Intel(R) Xeon(R) CPU5150  @ 2.66GHz
stepping: 6
microcode   : 0xd2
cpu MHz : 2659.977
cache size  : 4096 KB
physical id : 3
siblings: 2
core id : 0
cpu cores   : 2
apicid  : 6
initial apicid  : 6
fpu : yes
fpu_exception   : yes
cpuid level : 10
wp  : yes
flags   : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov 
pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx lm 
constant_tsc arch_perfmon pebs bts rep_good nopl cpuid aperfmperf pni dtes64 
monitor ds_cpl vmx est tm2 ssse3 cx16 xtpr pdcm dca lahf_lm tpr_shadow dtherm
bugs:
bogomips: 5320.03
clflush size: 64
cache_alignment : 64
address sizes   : 36 bits physical, 48 bits virtual
power management:

So, in conclusion; it's not important to *me* that this old machine
keeps working, I'm just volunteering test data points. So please don't
feel obligated in any way to go out of your way on my account. OTOH,
I'm happy to provide feedback as long as you would like me to.

Along the same lines: Paolo, as the author of commit 2c82878b0cb38fd,
is the Xeon chip listed above one of the "obsolete for virtualization"
models ? In that case, it makes no sense for me to keep using it for
tests, and the fact that it misbehaves with L1 MWAIT should also not
matter at all.

Let me know what you all think.

Thanks,
--Gabriel

Re: [PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes

[Gabriel L. Somlo]

On Thu, May 04, 2017 at 08:07:15PM +0200, Radim Krčmář wrote:
> 2017-05-04 13:56-0400, Gabriel L. Somlo:
> > If I wanted to test this (e.g. with OS X 10.8 guests on several of my older
> > Mac boxes running Fedora), which git repo would you have me use? (The series
> > won't apply directly on top of 
> > git://git.kernel.org/pub/scm/virt/kvm/kvm.git).
> 
> The queue branch of that repo.  This series depends on a patch that is
> applied there:
> 
>   668fffa3f838 kvm: better MWAIT emulation for guests
> 
> I forgot to mention that, sorry.

OK, here's where I'm at right now:

With this series applied on top of 'queue', my MacbookAir4,2 running
F25 (with the kvm/queue kernel) works fine, i.e. loads the kvm-intel
module successfully, and mwaits in L1 guest mode, reporting 400% cpu
but staying cool (guest started with -smp 4).

So far, so good.

On the MacPro1,1, I first had to revert 2c82878b0cb38fd
("KVM: VMX: require virtual NMI support") to get around this error:

# modprobe -v kvm-intel
insmod /lib/modules/4.11.0-rc3+/kernel/virt/lib/irqbypass.ko
insmod /lib/modules/4.11.0-rc3+/kernel/arch/x86/kvm/kvm.ko
insmod /lib/modules/4.11.0-rc3+/kernel/arch/x86/kvm/kvm-intel.ko
modprobe: ERROR: could not insert 'kvm_intel': Input/output error

Next, it turns out that on the MacPro1,1 kvm_mwait_in_guest() returns
TRUE, which causes OS X 10.7 (the one that mwaits without checking
CPUID) to misbehave. Forcing the function to return 0 (FALSE) solves
the problem:

diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index b49add7..249362c 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -216,9 +216,12 @@ static inline u64 nsec_to_cycles(struct kvm_vcpu *vcpu, 
u64 nsec)
 
 static inline bool kvm_mwait_in_guest(void)
 {
-   return boot_cpu_has(X86_FEATURE_MWAIT) &&
+   bool ret;
+   ret =  boot_cpu_has(X86_FEATURE_MWAIT) &&
!boot_cpu_has_bug(X86_BUG_AMD_E400) &&
!boot_cpu_has_bug(X86_BUG_MONITOR);
+   printk(KERN_INFO "kvm_mwait_in_guest: %d\n", ret);
+   return 0;
 }
 
 #endif

After this change, I get:

[ 1201.529002] kvm_mwait_in_guest: 1
[ 1201.529024] kvm_mwait_in_guest: 1
[ 1201.529029] kvm_mwait_in_guest: 1
[ 1201.529038] kvm_mwait_in_guest: 1
[ 1201.529047] kvm_mwait_in_guest: 1
[ 1225.150235] kvm: MONITOR instruction emulated as NOP!
[ 1225.150240] kvm: MWAIT instruction emulated as NOP!

indicating that it *would* have returned TRUE if I let it :)

This is a 2x dual-core Xeon, cca 2006 vintage, and the last (4th) CPU
in /proc/cpuinfo returns:

processor   : 3
vendor_id   : GenuineIntel
cpu family  : 6
model   : 15
model name  : Intel(R) Xeon(R) CPU5150  @ 2.66GHz
stepping: 6
microcode   : 0xd2
cpu MHz : 2659.977
cache size  : 4096 KB
physical id : 3
siblings: 2
core id : 0
cpu cores   : 2
apicid  : 6
initial apicid  : 6
fpu : yes
fpu_exception   : yes
cpuid level : 10
wp  : yes
flags   : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov 
pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx lm 
constant_tsc arch_perfmon pebs bts rep_good nopl cpuid aperfmperf pni dtes64 
monitor ds_cpl vmx est tm2 ssse3 cx16 xtpr pdcm dca lahf_lm tpr_shadow dtherm
bugs:
bogomips: 5320.03
clflush size: 64
cache_alignment : 64
address sizes   : 36 bits physical, 48 bits virtual
power management:

So, in conclusion; it's not important to *me* that this old machine
keeps working, I'm just volunteering test data points. So please don't
feel obligated in any way to go out of your way on my account. OTOH,
I'm happy to provide feedback as long as you would like me to.

Along the same lines: Paolo, as the author of commit 2c82878b0cb38fd,
is the Xeon chip listed above one of the "obsolete for virtualization"
models ? In that case, it makes no sense for me to keep using it for
tests, and the fact that it misbehaves with L1 MWAIT should also not
matter at all.

Let me know what you all think.

Thanks,
--Gabriel

Re: [PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes

[Gabriel L. Somlo]

On Thu, May 04, 2017 at 08:07:15PM +0200, Radim Krčmář wrote:
> 2017-05-04 13:56-0400, Gabriel L. Somlo:
> > If I wanted to test this (e.g. with OS X 10.8 guests on several of my older
> > Mac boxes running Fedora), which git repo would you have me use? (The series
> > won't apply directly on top of 
> > git://git.kernel.org/pub/scm/virt/kvm/kvm.git).
> 
> The queue branch of that repo.  This series depends on a patch that is
> applied there:
> 
>   668fffa3f838 kvm: better MWAIT emulation for guests
> 
> I forgot to mention that, sorry.

Thanks; right now, I get this:

# modprobe -v kvm-intel
insmod /lib/modules/4.11.0-rc3+/kernel/virt/lib/irqbypass.ko 
insmod /lib/modules/4.11.0-rc3+/kernel/arch/x86/kvm/kvm.ko 
insmod /lib/modules/4.11.0-rc3+/kernel/arch/x86/kvm/kvm-intel.ko 
modprobe: ERROR: could not insert 'kvm_intel': Input/output error

but that appears to have nothing to do with the MWAIT patches. I'm
bisecting to find the root cause, but it's really slow...

I'll follow up when I know more... Thanks,
--Gabriel

Re: [PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes

[Gabriel L. Somlo]

On Thu, May 04, 2017 at 08:07:15PM +0200, Radim Krčmář wrote:
> 2017-05-04 13:56-0400, Gabriel L. Somlo:
> > If I wanted to test this (e.g. with OS X 10.8 guests on several of my older
> > Mac boxes running Fedora), which git repo would you have me use? (The series
> > won't apply directly on top of 
> > git://git.kernel.org/pub/scm/virt/kvm/kvm.git).
> 
> The queue branch of that repo.  This series depends on a patch that is
> applied there:
> 
>   668fffa3f838 kvm: better MWAIT emulation for guests
> 
> I forgot to mention that, sorry.

Thanks; right now, I get this:

# modprobe -v kvm-intel
insmod /lib/modules/4.11.0-rc3+/kernel/virt/lib/irqbypass.ko 
insmod /lib/modules/4.11.0-rc3+/kernel/arch/x86/kvm/kvm.ko 
insmod /lib/modules/4.11.0-rc3+/kernel/arch/x86/kvm/kvm-intel.ko 
modprobe: ERROR: could not insert 'kvm_intel': Input/output error

but that appears to have nothing to do with the MWAIT patches. I'm
bisecting to find the root cause, but it's really slow...

I'll follow up when I know more... Thanks,
--Gabriel

Re: [PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes

[Radim Krčmář]

2017-05-04 13:56-0400, Gabriel L. Somlo:
> If I wanted to test this (e.g. with OS X 10.8 guests on several of my older
> Mac boxes running Fedora), which git repo would you have me use? (The series
> won't apply directly on top of git://git.kernel.org/pub/scm/virt/kvm/kvm.git).

The queue branch of that repo.  This series depends on a patch that is
applied there:

  668fffa3f838 kvm: better MWAIT emulation for guests

I forgot to mention that, sorry.

Re: [PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes

[Radim Krčmář]

2017-05-04 13:56-0400, Gabriel L. Somlo:
> If I wanted to test this (e.g. with OS X 10.8 guests on several of my older
> Mac boxes running Fedora), which git repo would you have me use? (The series
> won't apply directly on top of git://git.kernel.org/pub/scm/virt/kvm/kvm.git).

The queue branch of that repo.  This series depends on a patch that is
applied there:

  668fffa3f838 kvm: better MWAIT emulation for guests

I forgot to mention that, sorry.

Re: [PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes

[Gabriel L. Somlo]

Hi Radim,

On Wed, May 03, 2017 at 09:37:29PM +0200, Radim Krčmář wrote:
> kvm_mwait_in_guest() was overcomplicated and also missed one AMD bug
> that should prevent MWAIT pass through.
> 
> This series ignores errata that don't have any Linux bug defined;
> I know of two minor (not affecting the host) Core 2 errata:
>   AG36.  Split Locked Stores May not Trigger the Monitoring Hardware
>   AG106.  A REP STOS/MOVS to a MONITOR/MWAIT Address Range May Prevent
>   Triggering of the Monitoring Hardware
> 
> None of them are really worthy of a new condition if Linux never hit
> them ... we still have the OS X bug that Gabriel is hitting, but I'm ok
> with the original approach that sacrificed it for "greater good".

If I wanted to test this (e.g. with OS X 10.8 guests on several of my older
Mac boxes running Fedora), which git repo would you have me use? (The series
won't apply directly on top of git://git.kernel.org/pub/scm/virt/kvm/kvm.git).

Thanks much,
--Gabriel

> Radim Krčmář (4):
>   KVM: svm: prevent MWAIT in guest with erratum 400
>   KVM: x86: prevent MWAIT in guest with buggy MONITOR
>   KVM: x86: drop bogus MWAIT check
>   KVM: x86: simplify kvm_mwait_in_guest()
> 
>  arch/x86/kvm/x86.h | 33 +++--
>  1 file changed, 3 insertions(+), 30 deletions(-)
> 
> -- 
> 2.12.2
> 

Re: [PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes

[Gabriel L. Somlo]

Hi Radim,

On Wed, May 03, 2017 at 09:37:29PM +0200, Radim Krčmář wrote:
> kvm_mwait_in_guest() was overcomplicated and also missed one AMD bug
> that should prevent MWAIT pass through.
> 
> This series ignores errata that don't have any Linux bug defined;
> I know of two minor (not affecting the host) Core 2 errata:
>   AG36.  Split Locked Stores May not Trigger the Monitoring Hardware
>   AG106.  A REP STOS/MOVS to a MONITOR/MWAIT Address Range May Prevent
>   Triggering of the Monitoring Hardware
> 
> None of them are really worthy of a new condition if Linux never hit
> them ... we still have the OS X bug that Gabriel is hitting, but I'm ok
> with the original approach that sacrificed it for "greater good".

If I wanted to test this (e.g. with OS X 10.8 guests on several of my older
Mac boxes running Fedora), which git repo would you have me use? (The series
won't apply directly on top of git://git.kernel.org/pub/scm/virt/kvm/kvm.git).

Thanks much,
--Gabriel

> Radim Krčmář (4):
>   KVM: svm: prevent MWAIT in guest with erratum 400
>   KVM: x86: prevent MWAIT in guest with buggy MONITOR
>   KVM: x86: drop bogus MWAIT check
>   KVM: x86: simplify kvm_mwait_in_guest()
> 
>  arch/x86/kvm/x86.h | 33 +++--
>  1 file changed, 3 insertions(+), 30 deletions(-)
> 
> -- 
> 2.12.2
> 

Re: [PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes

[Alexander Graf]

On 03.05.17 21:37, Radim Krčmář wrote:

kvm_mwait_in_guest() was overcomplicated and also missed one AMD bug
that should prevent MWAIT pass through.

This series ignores errata that don't have any Linux bug defined;
I know of two minor (not affecting the host) Core 2 errata:
  AG36.  Split Locked Stores May not Trigger the Monitoring Hardware
  AG106.  A REP STOS/MOVS to a MONITOR/MWAIT Address Range May Prevent
  Triggering of the Monitoring Hardware

None of them are really worthy of a new condition if Linux never hit
them ... we still have the OS X bug that Gabriel is hitting, but I'm ok
with the original approach that sacrificed it for "greater good".


I like the series :)

Reviewed-by: Alexander Graf 


Alex

Re: [PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes

[Alexander Graf]

On 03.05.17 21:37, Radim Krčmář wrote:

kvm_mwait_in_guest() was overcomplicated and also missed one AMD bug
that should prevent MWAIT pass through.

This series ignores errata that don't have any Linux bug defined;
I know of two minor (not affecting the host) Core 2 errata:
  AG36.  Split Locked Stores May not Trigger the Monitoring Hardware
  AG106.  A REP STOS/MOVS to a MONITOR/MWAIT Address Range May Prevent
  Triggering of the Monitoring Hardware

None of them are really worthy of a new condition if Linux never hit
them ... we still have the OS X bug that Gabriel is hitting, but I'm ok
with the original approach that sacrificed it for "greater good".


I like the series :)

Reviewed-by: Alexander Graf 


Alex

[PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes

[Radim Krčmář]

kvm_mwait_in_guest() was overcomplicated and also missed one AMD bug
that should prevent MWAIT pass through.

This series ignores errata that don't have any Linux bug defined;
I know of two minor (not affecting the host) Core 2 errata:
  AG36.  Split Locked Stores May not Trigger the Monitoring Hardware
  AG106.  A REP STOS/MOVS to a MONITOR/MWAIT Address Range May Prevent
  Triggering of the Monitoring Hardware

None of them are really worthy of a new condition if Linux never hit
them ... we still have the OS X bug that Gabriel is hitting, but I'm ok
with the original approach that sacrificed it for "greater good".


Radim Krčmář (4):
  KVM: svm: prevent MWAIT in guest with erratum 400
  KVM: x86: prevent MWAIT in guest with buggy MONITOR
  KVM: x86: drop bogus MWAIT check
  KVM: x86: simplify kvm_mwait_in_guest()

 arch/x86/kvm/x86.h | 33 +++--
 1 file changed, 3 insertions(+), 30 deletions(-)

-- 
2.12.2

[PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes

[Radim Krčmář]

kvm_mwait_in_guest() was overcomplicated and also missed one AMD bug
that should prevent MWAIT pass through.

This series ignores errata that don't have any Linux bug defined;
I know of two minor (not affecting the host) Core 2 errata:
  AG36.  Split Locked Stores May not Trigger the Monitoring Hardware
  AG106.  A REP STOS/MOVS to a MONITOR/MWAIT Address Range May Prevent
  Triggering of the Monitoring Hardware

None of them are really worthy of a new condition if Linux never hit
them ... we still have the OS X bug that Gabriel is hitting, but I'm ok
with the original approach that sacrificed it for "greater good".


Radim Krčmář (4):
  KVM: svm: prevent MWAIT in guest with erratum 400
  KVM: x86: prevent MWAIT in guest with buggy MONITOR
  KVM: x86: drop bogus MWAIT check
  KVM: x86: simplify kvm_mwait_in_guest()

 arch/x86/kvm/x86.h | 33 +++--
 1 file changed, 3 insertions(+), 30 deletions(-)

-- 
2.12.2