All of lore.kernel.org
 help / color / mirror / Atom feed
From: Igor Mammedov <imammedo@redhat.com>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: kvm@vger.kernel.org, "Gleb Natapov" <gleb@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Marcelo Tosatti" <mtosatti@redhat.com>,
	qemu-devel@nongnu.org, "Andreas Färber" <afaerber@suse.de>
Subject: Re: [RFC 02/12] target-i386: Don't set any KVM flag by default if KVM is disabled
Date: Fri, 11 Jan 2013 00:07:40 +0100	[thread overview]
Message-ID: <20130111000740.774084f3@thinkpad.mammed.net> (raw)
In-Reply-To: <1357757632-1950-3-git-send-email-ehabkost@redhat.com>

On Wed,  9 Jan 2013 16:53:42 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> This is a cleanup that tries to solve two small issues:
> 
>  - We don't need a separate kvm_pv_eoi_features variable just to keep a
>    constant calculated at compile-time, and this style would require
>    adding a separate variable (that's declared twice because of the
>    CONFIG_KVM ifdef) for each feature that's going to be enabled/disable
>    by machine-type compat code.
>  - The pc-1.3 code is setting the kvm_pv_eoi flag on cpuid_kvm_features
>    even when KVM is disabled at runtime. This small incosistency in
>    the cpuid_kvm_features field isn't a problem today because
>    cpuid_kvm_features is ignored by the TCG code, but it may cause
>    unexpected problems later when refactoring the CPUID handling code.
> 
> This patch eliminates the kvm_pv_eoi_features variable and simply uses
> kvm_enabled() inside the enable_kvm_pv_eoi() compat function, so it
> enables kvm_pv_eoi only if KVM is enabled. I believe this makes the
> behavior of enable_kvm_pv_eoi() clearer and easier to understand.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Cc: kvm@vger.kernel.org
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Gleb Natapov <gleb@redhat.com>
> Cc: Marcelo Tosatti <mtosatti@redhat.com>
> 
[snip]
>  void host_cpuid(uint32_t function, uint32_t count,
> @@ -1343,13 +1337,15 @@ static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features)
>      unsigned int i;
>      char *featurestr; /* Single 'key=value" string being parsed */
>      /* Features to be added */
> -    FeatureWordArray plus_features = {
> -        [FEAT_KVM] = kvm_default_features,
> -    };
> +    FeatureWordArray plus_features = { 0 };
>      /* Features to be removed */
>      FeatureWordArray minus_features = { 0 };
>      uint32_t numvalue;
>  
> +    if (kvm_enabled()) {
> +        plus_features[FEAT_KVM] = kvm_default_features;
> +    }
While touching it please move setting defaults to cpu_x86_register() or
cpu_x86_find_by_name() to so that cpu_x86_parse_featurestr() would deal only
with custom settings.

[snip]

-- 
Regards,
  Igor

  parent reply	other threads:[~2013-01-10 23:07 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-09 18:53 [RFC 00/12] target-i386: Fix APIC-ID-based topology (v4) Eduardo Habkost
2013-01-09 18:53 ` [RFC 01/12] kvm: add KVM_FEATURE_CLOCKSOURCE_STABLE_BIT fake #define Eduardo Habkost
2013-01-10 23:15   ` Igor Mammedov
2013-01-11  0:05     ` Eduardo Habkost
2013-01-09 18:53 ` [RFC 02/12] target-i386: Don't set any KVM flag by default if KVM is disabled Eduardo Habkost
2013-01-10 11:47   ` Michael S. Tsirkin
2013-01-10 23:07   ` Igor Mammedov [this message]
2013-01-11  0:03     ` Eduardo Habkost
2013-01-09 18:53 ` [RFC 03/12] pc: Reverse pc_init_pci() compatibility logic Eduardo Habkost
2013-01-09 18:53 ` [RFC 04/12] kvm: Create kvm_arch_vcpu_id() function Eduardo Habkost
2013-01-09 18:53 ` [RFC 05/12] target-i386: kvm: Set vcpu_id to APIC ID instead of CPU index Eduardo Habkost
2013-01-09 18:53 ` [RFC 06/12] fw_cfg: Remove FW_CFG_MAX_CPUS from fw_cfg_init() Eduardo Habkost
2013-01-09 18:53 ` [RFC 07/12] target-i386/cpu: Introduce apic_id_for_cpu() function Eduardo Habkost
2013-01-10 23:31   ` Igor Mammedov
2013-01-10 23:51     ` Eduardo Habkost
2013-01-10 23:57     ` Eduardo Habkost
2013-01-09 18:53 ` [RFC 08/12] cpus.h: Make constant smp_cores/smp_threads available on *-user Eduardo Habkost
2013-01-09 18:53 ` [RFC 09/12] pc: Set fw_cfg data based on APIC ID calculation Eduardo Habkost
2013-01-09 18:53 ` [RFC 10/12] tests: Support target-specific unit tests Eduardo Habkost
2013-01-09 18:53 ` [RFC 11/12] target-i386: Topology & APIC ID utility functions Eduardo Habkost
2013-01-09 18:53 ` [RFC 12/12] pc: Generate APIC IDs according to CPU topology Eduardo Habkost
2013-01-10 23:36   ` Igor Mammedov
2013-01-10 23:55     ` Eduardo Habkost
2013-01-17 18:29 ` [RFC 00/12] target-i386: Fix APIC-ID-based topology (v4) 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=20130111000740.774084f3@thinkpad.mammed.net \
    --to=imammedo@redhat.com \
    --cc=afaerber@suse.de \
    --cc=ehabkost@redhat.com \
    --cc=gleb@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=mtosatti@redhat.com \
    --cc=qemu-devel@nongnu.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.