All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <bonzini@gnu.org>
To: Wanpeng Li <kernellwp@gmail.com>,
	kvm@vger.kernel.org, qemu-devel@nongnu.org
Cc: Wanpeng Li <wanpeng.li@hotmail.com>,
	Richard Henderson <rth@twiddle.net>,
	Eduardo Habkost <ehabkost@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [PATCH] target-i386: fix losing XCR0 processor state component bits
Date: Wed, 28 Sep 2016 09:54:27 +0200	[thread overview]
Message-ID: <b8f568ac-64a3-852c-deb5-fa67fc147005@gnu.org> (raw)
In-Reply-To: <1475040669-29085-1-git-send-email-wanpeng.li@hotmail.com>



On 28/09/2016 07:31, Wanpeng Li wrote:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
> 
> Commit 96193c22a "target-i386: Move xsave component mask to features array"
> leverages features array to handle XCR0 processor state component bits, 
> however, it introduces a regression:
> 
> warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 0]
> warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 1]
> warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 2]
> 
> My desktop doesn't have enough advance features, so just X87,SSE,AVX 
> warnings are splat when I boot a guest.
> 
> The get migratable flags logic in x86_cpu_filter_features() path will 
> filter out the feature flags which are unsupported and unmigratable. 
> However, the bits of XCR0 processor state component featureword don't 
> have feat_names, and some features like SSE/AVX etc have feat_names in 
> CPUID.01H:EDX, CPUID.01H:ECX, so they are treated as unsupported.
> 
> This patch fix it by don't filter out XCR0 processor state components 
> bits though they don't have feat_names just as before commit 96193c22ab3.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
>  target-i386/cpu.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index ad09246..9d24eff 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2156,6 +2156,10 @@ static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w,
>          r = kvm_arch_get_supported_cpuid(kvm_state, wi->cpuid_eax,
>                                                      wi->cpuid_ecx,
>                                                      wi->cpuid_reg);
> +        if ((w == FEAT_XSAVE_COMP_LO) ||
> +            (w == FEAT_XSAVE_COMP_HI)) {
> +            return r;
> +        }
>      } else if (tcg_enabled()) {
>          r = wi->tcg_features;
>      } else {
> 

I think the right place to add the test is x86_cpu_get_migratable_flags.

Paolo

WARNING: multiple messages have this Message-ID (diff)
From: Paolo Bonzini <bonzini@gnu.org>
To: Wanpeng Li <kernellwp@gmail.com>,
	kvm@vger.kernel.org, qemu-devel@nongnu.org
Cc: Wanpeng Li <wanpeng.li@hotmail.com>,
	Richard Henderson <rth@twiddle.net>,
	Eduardo Habkost <ehabkost@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] target-i386: fix losing XCR0 processor state component bits
Date: Wed, 28 Sep 2016 09:54:27 +0200	[thread overview]
Message-ID: <b8f568ac-64a3-852c-deb5-fa67fc147005@gnu.org> (raw)
In-Reply-To: <1475040669-29085-1-git-send-email-wanpeng.li@hotmail.com>



On 28/09/2016 07:31, Wanpeng Li wrote:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
> 
> Commit 96193c22a "target-i386: Move xsave component mask to features array"
> leverages features array to handle XCR0 processor state component bits, 
> however, it introduces a regression:
> 
> warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 0]
> warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 1]
> warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 2]
> 
> My desktop doesn't have enough advance features, so just X87,SSE,AVX 
> warnings are splat when I boot a guest.
> 
> The get migratable flags logic in x86_cpu_filter_features() path will 
> filter out the feature flags which are unsupported and unmigratable. 
> However, the bits of XCR0 processor state component featureword don't 
> have feat_names, and some features like SSE/AVX etc have feat_names in 
> CPUID.01H:EDX, CPUID.01H:ECX, so they are treated as unsupported.
> 
> This patch fix it by don't filter out XCR0 processor state components 
> bits though they don't have feat_names just as before commit 96193c22ab3.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
>  target-i386/cpu.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index ad09246..9d24eff 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2156,6 +2156,10 @@ static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w,
>          r = kvm_arch_get_supported_cpuid(kvm_state, wi->cpuid_eax,
>                                                      wi->cpuid_ecx,
>                                                      wi->cpuid_reg);
> +        if ((w == FEAT_XSAVE_COMP_LO) ||
> +            (w == FEAT_XSAVE_COMP_HI)) {
> +            return r;
> +        }
>      } else if (tcg_enabled()) {
>          r = wi->tcg_features;
>      } else {
> 

I think the right place to add the test is x86_cpu_get_migratable_flags.

Paolo

  reply	other threads:[~2016-09-28  7:54 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-28  5:31 [PATCH] target-i386: fix losing XCR0 processor state component bits Wanpeng Li
2016-09-28  5:31 ` [Qemu-devel] " Wanpeng Li
2016-09-28  7:54 ` Paolo Bonzini [this message]
2016-09-28  7:54   ` Paolo Bonzini
2016-09-28  8:38   ` Wanpeng Li
2016-09-28  8:38     ` [Qemu-devel] " Wanpeng Li
2016-09-28 14:57   ` Eduardo Habkost
2016-09-28 14:57     ` [Qemu-devel] " Eduardo Habkost
2016-09-28 15:01     ` Paolo Bonzini
2016-09-28 15:01       ` [Qemu-devel] " Paolo Bonzini
2016-09-28 15:05       ` Eduardo Habkost
2016-09-28 15:05         ` [Qemu-devel] " Eduardo Habkost
2016-09-28 15:09         ` Paolo Bonzini
2016-09-28 15:09           ` [Qemu-devel] " Paolo Bonzini
2016-09-28 15:59           ` Eduardo Habkost
2016-09-28 15:59             ` [Qemu-devel] " Eduardo Habkost
2016-09-28 16:07             ` Paolo Bonzini
2016-09-28 16:07               ` [Qemu-devel] " Paolo Bonzini
2016-09-28 16:13               ` Eduardo Habkost
2016-09-28 16:13                 ` [Qemu-devel] " Eduardo Habkost
2016-09-28 16:29                 ` Eduardo Habkost
2016-09-28 16:29                   ` [Qemu-devel] " Eduardo Habkost

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b8f568ac-64a3-852c-deb5-fa67fc147005@gnu.org \
    --to=bonzini@gnu.org \
    --cc=ehabkost@redhat.com \
    --cc=kernellwp@gmail.com \
    --cc=kvm@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    --cc=wanpeng.li@hotmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.