target/i386: kvm: print info when the kernel doesn't support ioctl(KVM_CAP_GET_MSR_FEATURES)

[PATCH] target/i386: kvm: print info when the kernel doesn't support ioctl(KVM_CAP_GET_MSR_FEATURES)

Li Qiang posted 1 patch 5 years, 10 months ago

Download mbox

Test asan failed

Test checkpatch failed

Test FreeBSD failed

Test docker-mingw@fedora failed

Test docker-clang@ubuntu failed

Test docker-quick@centos7 failed

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200103143224.49187-1-liq3ea@163.com

Maintainers: Richard Henderson <rth@twiddle.net>, Paolo Bonzini <pbonzini@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, Marcelo Tosatti <mtosatti@redhat.com>

target/i386/kvm.c | 2 ++
1 file changed, 2 insertions(+)

Expand all Fold all

[PATCH] target/i386: kvm: print info when the kernel doesn't support ioctl(KVM_CAP_GET_MSR_FEATURES)

Posted by Li Qiang 5 years, 10 months ago

The ioctl(KVM_CAP_GET_MSR_FEATURES) is quite new. In old platform that
doesn't support this ioctl will sometimes make the user confusion. For
example, when we do nested virtualiztion using host-passthrough model
the VM will has quite different cpu feature with the host.

Signed-off-by: Li Qiang <liq3ea@163.com>
---
 target/i386/kvm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 0b511906e3..9688f7a167 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1916,6 +1916,8 @@ static int kvm_get_supported_feature_msrs(KVMState *s)
     }
 
     if (!kvm_check_extension(s, KVM_CAP_GET_MSR_FEATURES)) {
+        info_report("ioctl(KVM_CAP_GET_MSR_FEATURES) is "
+                    "not supported by this kernel.");
         return 0;
     }
 
-- 
2.17.1

Re: [PATCH] target/i386: kvm: print info when the kernel doesn't support ioctl(KVM_CAP_GET_MSR_FEATURES)

Posted by Philippe Mathieu-Daudé 5 years, 10 months ago

On 1/3/20 3:32 PM, Li Qiang wrote:
> The ioctl(KVM_CAP_GET_MSR_FEATURES) is quite new. In old platform that

Can you refer to the kernel version? "is quite new (introduced in vX.Y)"

> doesn't support this ioctl will sometimes make the user confusion. For
> example, when we do nested virtualiztion using host-passthrough model

"virtualization"

> the VM will has quite different cpu feature with the host.

I'm not native English speaking but I'd use:
"... will have quite different features than the host"

> 
> Signed-off-by: Li Qiang <liq3ea@163.com>
> ---
>   target/i386/kvm.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 0b511906e3..9688f7a167 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -1916,6 +1916,8 @@ static int kvm_get_supported_feature_msrs(KVMState *s)
>       }
>   
>       if (!kvm_check_extension(s, KVM_CAP_GET_MSR_FEATURES)) {
> +        info_report("ioctl(KVM_CAP_GET_MSR_FEATURES) is "
> +                    "not supported by this kernel.");

Please remove the trailing '.'

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>           return 0;
>       }
>   
>

Re: [PATCH] target/i386: kvm: print info when the kernel doesn't support ioctl(KVM_CAP_GET_MSR_FEATURES)

Posted by Paolo Bonzini 5 years, 10 months ago

On 03/01/20 15:32, Li Qiang wrote:
> The ioctl(KVM_CAP_GET_MSR_FEATURES) is quite new. In old platform that
> doesn't support this ioctl will sometimes make the user confusion. For
> example, when we do nested virtualiztion using host-passthrough model
> the VM will has quite different cpu feature with the host.
> 
> Signed-off-by: Li Qiang <liq3ea@163.com>
> ---
>  target/i386/kvm.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 0b511906e3..9688f7a167 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -1916,6 +1916,8 @@ static int kvm_get_supported_feature_msrs(KVMState *s)
>      }
>  
>      if (!kvm_check_extension(s, KVM_CAP_GET_MSR_FEATURES)) {
> +        info_report("ioctl(KVM_CAP_GET_MSR_FEATURES) is "
> +                    "not supported by this kernel.");
>          return 0;
>      }

Does this matter with host-passthrough?  As long as "-cpu host,+vmx"
works with an old kernel, there's no reason to warn---and even then the
warning should:

1) be restricted to the nested case;

2) be emitted even if MSR features are supported but VMX MSRs are not.

Paolo