All of lore.kernel.org
 help / color / mirror / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: Gleb Natapov <gleb@redhat.com>,
	LKML <linux-kernel@vger.kernel.org>, Borislav Petkov <bp@suse.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Andre Przywara <andre@andrep.de>, Joerg Roedel <joro@8bytes.org>,
	X86 ML <x86@kernel.org>, KVM <kvm@vger.kernel.org>,
	qemu-devel@nongnu.org, libvir-list@redhat.com,
	Jiri Denemark <jdenemar@redhat.com>
Subject: Re: [PATCH 1/6] kvm: Add KVM_GET_EMULATED_CPUID
Date: Sat, 28 Sep 2013 12:49:04 +0200	[thread overview]
Message-ID: <20130928104903.GB5956@pd.tnic> (raw)
In-Reply-To: <20130927142100.GB2840@otherpad.lan.raisama.net>

On Fri, Sep 27, 2013 at 11:21:34AM -0300, Eduardo Habkost wrote:
> The problem here is that "requested_features" doesn't include just
> the explicit "+flag" flags, but any flag included in the CPU model
> definition. See the "-cpu n270" example below.

Oh, you mean if requested_features would contain a flag included from
the CPU model definition - a flag which we haven't requested explicitly
- and if kvm emulates that flag, then it will get enabled?

Hmm.

> It should, but your patch will make it stop failing because of MOVBE, as
> now it can be emulated[1].

Right.

> "enforce" makes sure all features are really being enabled. It makes
> QEMU abort if there's any feature that can't be enabled on that host.

Ok.

> [1] Maybe one source of confusion is that the existing code have two
> feature-filtering functions doing basically the same thing:
> filter_features_for_kvm() and kvm_check_features_against_host().  That's

Yes, and the first gets executed unconditionally and does the feature
filtering,  right after the second has run in the kvm_enabled() branch.

> something we must clean up, and they should be unified. "enforce" should
> become synonymous to "make sure filtered_features is all zeroes".  This
> way, libvirt can emulate what 'enforce" does while being able to collect
> detailed error information (which is not easy to do if QEMU simply
> aborts).

Ok, maybe someone who's more knowledgeable with this code should do it -
not me :)

Also, there's another aspect, while we're here: now that QEMU emulates
MOVBE with TCG too, how do we specify on the command line, which
emulation should be used - kvm.ko or QEMU?

Thanks.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

WARNING: multiple messages have this Message-ID (diff)
From: Borislav Petkov <bp@alien8.de>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: KVM <kvm@vger.kernel.org>, Gleb Natapov <gleb@redhat.com>,
	libvir-list@redhat.com, Joerg Roedel <joro@8bytes.org>,
	X86 ML <x86@kernel.org>, LKML <linux-kernel@vger.kernel.org>,
	qemu-devel@nongnu.org, Andre Przywara <andre@andrep.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Jiri Denemark <jdenemar@redhat.com>, Borislav Petkov <bp@suse.de>
Subject: Re: [Qemu-devel] [PATCH 1/6] kvm: Add KVM_GET_EMULATED_CPUID
Date: Sat, 28 Sep 2013 12:49:04 +0200	[thread overview]
Message-ID: <20130928104903.GB5956@pd.tnic> (raw)
In-Reply-To: <20130927142100.GB2840@otherpad.lan.raisama.net>

On Fri, Sep 27, 2013 at 11:21:34AM -0300, Eduardo Habkost wrote:
> The problem here is that "requested_features" doesn't include just
> the explicit "+flag" flags, but any flag included in the CPU model
> definition. See the "-cpu n270" example below.

Oh, you mean if requested_features would contain a flag included from
the CPU model definition - a flag which we haven't requested explicitly
- and if kvm emulates that flag, then it will get enabled?

Hmm.

> It should, but your patch will make it stop failing because of MOVBE, as
> now it can be emulated[1].

Right.

> "enforce" makes sure all features are really being enabled. It makes
> QEMU abort if there's any feature that can't be enabled on that host.

Ok.

> [1] Maybe one source of confusion is that the existing code have two
> feature-filtering functions doing basically the same thing:
> filter_features_for_kvm() and kvm_check_features_against_host().  That's

Yes, and the first gets executed unconditionally and does the feature
filtering,  right after the second has run in the kvm_enabled() branch.

> something we must clean up, and they should be unified. "enforce" should
> become synonymous to "make sure filtered_features is all zeroes".  This
> way, libvirt can emulate what 'enforce" does while being able to collect
> detailed error information (which is not easy to do if QEMU simply
> aborts).

Ok, maybe someone who's more knowledgeable with this code should do it -
not me :)

Also, there's another aspect, while we're here: now that QEMU emulates
MOVBE with TCG too, how do we specify on the command line, which
emulation should be used - kvm.ko or QEMU?

Thanks.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

  reply	other threads:[~2013-09-28 10:49 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-22 14:44 [PATCH 0/6] kvm: Emulate MOVBE, v3 Borislav Petkov
2013-09-22 14:44 ` [PATCH 1/6] kvm: Add KVM_GET_EMULATED_CPUID Borislav Petkov
2013-09-23 16:28   ` Eduardo Habkost
2013-09-23 16:28     ` [Qemu-devel] " Eduardo Habkost
2013-09-23 16:28     ` Eduardo Habkost
2013-09-24  9:57     ` Borislav Petkov
2013-09-24  9:57       ` [Qemu-devel] " Borislav Petkov
2013-09-24 10:04       ` Gleb Natapov
2013-09-24 10:04         ` [Qemu-devel] " Gleb Natapov
2013-09-26 14:19         ` Eduardo Habkost
2013-09-26 14:19           ` [Qemu-devel] " Eduardo Habkost
2013-09-26 18:55           ` Borislav Petkov
2013-09-26 18:55             ` [Qemu-devel] " Borislav Petkov
2013-09-26 19:20             ` Eduardo Habkost
2013-09-26 19:20               ` [Qemu-devel] " Eduardo Habkost
2013-09-26 20:32               ` Borislav Petkov
2013-09-26 20:32                 ` [Qemu-devel] " Borislav Petkov
2013-09-26 20:32                 ` Borislav Petkov
2013-09-27 14:21                 ` Eduardo Habkost
2013-09-27 14:21                   ` [Qemu-devel] " Eduardo Habkost
2013-09-28 10:49                   ` Borislav Petkov [this message]
2013-09-28 10:49                     ` Borislav Petkov
2013-09-30 16:13                     ` Eduardo Habkost
2013-09-30 16:13                       ` [Qemu-devel] " Eduardo Habkost
2013-09-30 16:18                       ` Borislav Petkov
2013-09-30 16:18                         ` [Qemu-devel] " Borislav Petkov
2013-09-22 14:44 ` [PATCH 2/6] kvm, emulator: Use opcode length Borislav Petkov
2013-09-22 14:44 ` [PATCH 3/6] kvm, emulator: Rename VendorSpecific flag Borislav Petkov
2013-09-22 14:44 ` [PATCH 4/6] kvm, emulator: Add initial three-byte insns support Borislav Petkov
2013-10-29  9:50   ` Gleb Natapov
2013-10-29 10:04     ` Borislav Petkov
2013-10-29 10:11       ` Gleb Natapov
2013-09-22 14:44 ` [PATCH 5/6] kvm: Emulate MOVBE Borislav Petkov
2013-09-22 14:44 ` [PATCH 6/6] qemu: Add support for emulated CPU features Borislav Petkov
2013-09-23 17:06   ` Eduardo Habkost
2013-09-23 17:06     ` [Qemu-devel] " Eduardo Habkost
2013-10-29  9:53 ` [PATCH 0/6] kvm: Emulate MOVBE, v3 Gleb Natapov
2013-10-29 10:30   ` Borislav Petkov
2013-10-29 10:35     ` Gleb Natapov
2013-10-29 11:28       ` Borislav Petkov
2013-10-29 11:36         ` Gleb Natapov
2013-10-29 11:53           ` Borislav Petkov
2013-10-29 11:54             ` [PATCH 4/5 -v3.1] kvm, emulator: Add initial three-byte insns support Borislav Petkov
2013-10-29 11:54             ` [PATCH 5/5 -v3.1] kvm: Emulate MOVBE Borislav Petkov
2013-10-30 17:56               ` Paolo Bonzini
2013-10-30 18:03                 ` Borislav Petkov
2013-10-30 18:10 ` [PATCH 0/6] kvm: Emulate MOVBE, v3 Paolo Bonzini
2013-10-30 18:44   ` Borislav Petkov

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=20130928104903.GB5956@pd.tnic \
    --to=bp@alien8.de \
    --cc=andre@andrep.de \
    --cc=bp@suse.de \
    --cc=ehabkost@redhat.com \
    --cc=gleb@redhat.com \
    --cc=hpa@zytor.com \
    --cc=jdenemar@redhat.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=libvir-list@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=x86@kernel.org \
    /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.