qemu: enable PV EOI for qemu 1.3
PATCH v2 1/1 qemu: enable PV EOI for qemu 1.3
Enable KVM PV EOI by default. You can still disable it with -kvm_pv_eoi cpu flag. To avoid breaking cross-version migration, enable only for qemu 1.3 (or in the future, newer) machine type.
---
Changes from v1:
Address comments by Eduardo:
use include instead of duplicate definition reduce ifdef spagetti in code using features mask rename init from _pv_eoi to _1_3 to enable adding more stuff in this version
hw/pc_piix.c | 15 ++++++++++++++- target-i386/cpu.c | 33 ++++++++++++++++++++------------- target-i386/cpu.h | 2 ++ 3 files changed, 36 insertions(+), 14 deletions(-)
hw/pc_piix.c +14 -1
@@ -43,6 +43,7 @@#include "xen.h" #include "memory.h" #include "exec-memory.h" +#include "cpu.h" #ifdef CONFIG_XEN # include <xen/hvm/hvm_info_table.h> #endif
@@ -301,6 +302,18 @@ static void pc_init_pci(ram_addr_t ram_size, initrd_filename, cpu_model, 1, 1); } +static void pc_init_pci_1_3 (ram_addr_t ram_size, +const char *boot_device, +const char *kernel_filename, +const char *kernel_cmdline, +const char *initrd_filename, +const char *cpu_model) +{ +enable_kvm_pv_eoi (); +pc_init_pci (ram_size, boot_device, kernel_filename, + kernel_cmdline, initrd_filename, cpu_model); +} + static void pc_init_pci_no_kvmclock(ram_addr_t ram_size, const char *boot_device, const char *kernel_filename,
@@ -353,7 +366,7 @@ static QEMUMachine pc_machine_v1_3 = { .name = "pc-1.3", .alias = "pc", .desc = "Standard PC", - .init = pc_init_pci, + .init = pc_init_pci_1_3, .max_cpus = 255, .is_default = 1, };
target-i386/cpu.c +20 -13
@@ -124,6 +124,20 @@ typedef struct model_features_t {int check_cpuid =0 ;int enforce_cpuid =0 ; +#if defined(CONFIG_KVM ) +static uint32_t kvm_default_features = (1 <<KVM_FEATURE_CLOCKSOURCE ) | + (1 <<KVM_FEATURE_NOP_IO_DELAY ) | + (1 <<KVM_FEATURE_MMU_OP ) | + (1 <<KVM_FEATURE_CLOCKSOURCE2 ) | + (1 <<KVM_FEATURE_ASYNC_PF ) | + (1 <<KVM_FEATURE_STEAL_TIME ) | + (1 <<KVM_FEATURE_CLOCKSOURCE_STABLE_BIT ); +static const uint32_t kvm_pv_eoi_features = (0x1 <<KVM_FEATURE_PV_EOI ); +#else +static uint32_t kvm_default_features =0 ; +static const uint32_t kvm_pv_eoi_features =0 ; +#endif +void host_cpuid (uint32_t function,uint32_t count,uint32_t *eax,uint32_t *ebx,uint32_t *ecx,uint32_t *edx) {
@@ -1107,7 +1121,7 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)/* Features to be added*/ uint32_t plus_features =0 , plus_ext_features =0 ;uint32_t plus_ext2_features =0 , plus_ext3_features =0 ; -uint32_t plus_kvm_features =0 , plus_svm_features =0 ; +uint32_t plus_kvm_features = kvm_default_features, plus_svm_features =0 ;uint32_t plus_7_0_ebx_features =0 ;/* Features to be removed */ uint32_t minus_features =0 , minus_ext_features =0 ;
@@ -1127,18 +1141,6 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)memcpy (x86_cpu_def, def,sizeof (*def)); } -#if defined(CONFIG_KVM ) - plus_kvm_features = (1 <<KVM_FEATURE_CLOCKSOURCE ) | - (1 <<KVM_FEATURE_NOP_IO_DELAY ) | - (1 <<KVM_FEATURE_MMU_OP ) | - (1 <<KVM_FEATURE_CLOCKSOURCE2 ) | - (1 <<KVM_FEATURE_ASYNC_PF ) | - (1 <<KVM_FEATURE_STEAL_TIME ) | - (1 <<KVM_FEATURE_CLOCKSOURCE_STABLE_BIT ); -#else - plus_kvm_features =0 ; -#endif -add_flagname_to_bitmaps ("hypervisor" , &plus_features, &plus_ext_features, &plus_ext2_features, &plus_ext3_features, &plus_kvm_features, &plus_svm_features, &plus_7_0_ebx_features);
@@ -1953,3 +1955,8 @@ static void x86_cpu_register_types(void) }type_init (x86_cpu_register_types) + +void enable_kvm_pv_eoi(void) +{ + kvm_default_features |= kvm_pv_eoi_features; +}
target-i386/cpu.h +2 -0
@@ -1188,4 +1188,6 @@ void do_smm_enter(CPUX86State *env1);void cpu_report_tpr_access (CPUX86State *env,TPRAccess access); +void enable_kvm_pv_eoi (void ); +#endif /* CPU_I386_H */
Signature · 1 line
MST
Am 18.10.2012 00:15, schrieb Michael S. Tsirkin:
Quoted 32 lines
Enable KVM PV EOI by default. You can still disable it with -kvm_pv_eoi cpu flag. To avoid breaking cross-version migration, enable only for qemu 1.3 (or in the future, newer) machine type.
Signed-off-by Michael S. Tsirkin <[email protected]>---
Changes from v1:
Address comments by Eduardo:
use include instead of duplicate definition reduce ifdef spagetti in code using features mask rename init from_pv_eoito _1_3 to enable adding more stuff in this versionhw/pc_piix.c | 15 ++++++++++++++- target-i386/cpu.c | 33 ++++++++++++++++++++------------- target-i386/cpu.h | 2 ++ 3 files changed, 36 insertions(+), 14 deletions(-)hw/pc_piix.c +1
-0@@ -43,6 +43,7 @@#include "xen.h" #include "memory.h" #include "exec-memory.h" +#include "cpu.h" #ifdef CONFIG_XEN # include <xen/hvm/hvm_info_table.h> #endif
hw/pc_piix.c static void pc_init_pci(ram_addr_t ram_size, · line 310
Quoted 9 lines
hw/pc_piix.c +6
-0@@ -301,6 +302,18 @@ static void pc_init_pci(ram_addr_t ram_size, initrd_filename, cpu_model, 1, 1); } +static void pc_init_pci_1_3 (ram_addr_t ram_size, +const char *boot_device, +const char *kernel_filename, +const char *kernel_cmdline, +const char *initrd_filename, +const char *cpu_model)
Indentation was not adapted to name change?
hw/pc_piix.c static void pc_init_pci(ram_addr_t ram_size, · line 314
Quoted 4 lines
hw/pc_piix.c +4
-0+{ +enable_kvm_pv_eoi (); +pc_init_pci (ram_size, boot_device, kernel_filename, + kernel_cmdline, initrd_filename, cpu_model);
There's two tabs here.
Thought you wanted to s/pc_init_pci/pc_init_pci_1_2/g and name this pc_init_pci()? Are you expecting there to be a pc_init_pci_1_4()?
Quoted 68 lines
hw/pc_piix.c +3
-1+} + static void pc_init_pci_no_kvmclock(ram_addr_t ram_size, const char *boot_device, const char *kernel_filename,@@ -353,7 +366,7 @@ static QEMUMachine pc_machine_v1_3 = { .name = "pc-1.3", .alias = "pc", .desc = "Standard PC", - .init = pc_init_pci, + .init = pc_init_pci_1_3, .max_cpus = 255, .is_default = 1, };target-i386/cpu.c +15
-13@@ -124,6 +124,20 @@ typedef struct model_features_t {int check_cpuid =0 ;int enforce_cpuid =0 ; +#if defined(CONFIG_KVM ) +static uint32_t kvm_default_features = (1 <<KVM_FEATURE_CLOCKSOURCE ) | + (1 <<KVM_FEATURE_NOP_IO_DELAY ) | + (1 <<KVM_FEATURE_MMU_OP ) | + (1 <<KVM_FEATURE_CLOCKSOURCE2 ) | + (1 <<KVM_FEATURE_ASYNC_PF ) | + (1 <<KVM_FEATURE_STEAL_TIME ) | + (1 <<KVM_FEATURE_CLOCKSOURCE_STABLE_BIT ); +static const uint32_t kvm_pv_eoi_features = (0x1 <<KVM_FEATURE_PV_EOI ); +#else +static uint32_t kvm_default_features =0 ; +static const uint32_t kvm_pv_eoi_features =0 ; +#endif +void host_cpuid (uint32_t function,uint32_t count,uint32_t *eax,uint32_t *ebx,uint32_t *ecx,uint32_t *edx) {@@ -1107,7 +1121,7 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)/* Features to be added*/ uint32_t plus_features =0 , plus_ext_features =0 ;uint32_t plus_ext2_features =0 , plus_ext3_features =0 ; -uint32_t plus_kvm_features =0 , plus_svm_features =0 ; +uint32_t plus_kvm_features = kvm_default_features, plus_svm_features =0 ;uint32_t plus_7_0_ebx_features =0 ;/* Features to be removed */ uint32_t minus_features =0 , minus_ext_features =0 ;@@ -1127,18 +1141,6 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)memcpy (x86_cpu_def, def,sizeof (*def)); } -#if defined(CONFIG_KVM ) - plus_kvm_features = (1 <<KVM_FEATURE_CLOCKSOURCE ) | - (1 <<KVM_FEATURE_NOP_IO_DELAY ) | - (1 <<KVM_FEATURE_MMU_OP ) | - (1 <<KVM_FEATURE_CLOCKSOURCE2 ) | - (1 <<KVM_FEATURE_ASYNC_PF ) | - (1 <<KVM_FEATURE_STEAL_TIME ) | - (1 <<KVM_FEATURE_CLOCKSOURCE_STABLE_BIT ); -#else - plus_kvm_features =0 ; -#endif -add_flagname_to_bitmaps ("hypervisor" , &plus_features, &plus_ext_features, &plus_ext2_features, &plus_ext3_features, &plus_kvm_features, &plus_svm_features, &plus_7_0_ebx_features);
target-i386/cpu.c static void x86_cpu_register_types(void) · line 1962
Quoted 8 lines
target-i386/cpu.c +5
-0@@ -1953,3 +1955,8 @@ static void x86_cpu_register_types(void) }type_init (x86_cpu_register_types) + +void enable_kvm_pv_eoi(void) +{ + kvm_default_features |= kvm_pv_eoi_features; +}
Indentation is off (7 spaces).
I'd prefer to place this function below the variable definitions, so that type_init() remains at the bottom of the file.
Otherwise I'm happy with the modified cpu.h-based interaction. So far there's no conflicts here, so it could go through either PC maintainer, kvm/uq or qom-cpu.
Andreas
Quoted 11 lines
target-i386/cpu.h +2
-0@@ -1188,4 +1188,6 @@ void do_smm_enter(CPUX86State *env1);void cpu_report_tpr_access (CPUX86State *env,TPRAccess access); +void enable_kvm_pv_eoi (void ); +#endif /* CPU_I386_H */
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Quoted 52 lines · Andreas Färber
Am 18.10.2012 00:15, schrieb Michael S. Tsirkin:
Quoted 32 lines
Enable KVM PV EOI by default. You can still disable it with -kvm_pv_eoi cpu flag. To avoid breaking cross-version migration, enable only for qemu 1.3 (or in the future, newer) machine type.
Signed-off-by Michael S. Tsirkin <[email protected]>---
Changes from v1:
Address comments by Eduardo:
use include instead of duplicate definition reduce ifdef spagetti in code using features mask rename init from_pv_eoito _1_3 to enable adding more stuff in this versionhw/pc_piix.c | 15 ++++++++++++++- target-i386/cpu.c | 33 ++++++++++++++++++++------------- target-i386/cpu.h | 2 ++ 3 files changed, 36 insertions(+), 14 deletions(-)hw/pc_piix.c +1
-0@@ -43,6 +43,7 @@#include "xen.h" #include "memory.h" #include "exec-memory.h" +#include "cpu.h" #ifdef CONFIG_XEN # include <xen/hvm/hvm_info_table.h> #endif hw/pc_piix.c static void pc_init_pci(ram_addr_t ram_size, · line 310
Quoted 9 lines
hw/pc_piix.c +6
-0@@ -301,6 +302,18 @@ static void pc_init_pci(ram_addr_t ram_size, initrd_filename, cpu_model, 1, 1); } +static void pc_init_pci_1_3 (ram_addr_t ram_size, +const char *boot_device, +const char *kernel_filename, +const char *kernel_cmdline, +const char *initrd_filename, +const char *cpu_model)Indentation was not adapted to name change?
hw/pc_piix.c static void pc_init_pci(ram_addr_t ram_size, · line 314
Quoted 4 lines
hw/pc_piix.c +4
-0+{ +enable_kvm_pv_eoi (); +pc_init_pci (ram_size, boot_device, kernel_filename, + kernel_cmdline, initrd_filename, cpu_model);There's two tabs here.
Quoted 2 lines
Thought you wanted to s/pc_init_pci/pc_init_pci_1_2/g and name this
pc_init_pci()?
I decided it's not worth it - better keep the patch small.
Quoted 1 line
Are you expecting there to be a
pc_init_pci_1_4()?
I don't know. Maybe we will be able to stick to .. _1_3 for the next 10 revisions.
Quoted 94 lines
Quoted 68 lines
hw/pc_piix.c +3
-1+} + static void pc_init_pci_no_kvmclock(ram_addr_t ram_size, const char *boot_device, const char *kernel_filename,@@ -353,7 +366,7 @@ static QEMUMachine pc_machine_v1_3 = { .name = "pc-1.3", .alias = "pc", .desc = "Standard PC", - .init = pc_init_pci, + .init = pc_init_pci_1_3, .max_cpus = 255, .is_default = 1, };target-i386/cpu.c +15
-13@@ -124,6 +124,20 @@ typedef struct model_features_t {int check_cpuid =0 ;int enforce_cpuid =0 ; +#if defined(CONFIG_KVM ) +static uint32_t kvm_default_features = (1 <<KVM_FEATURE_CLOCKSOURCE ) | + (1 <<KVM_FEATURE_NOP_IO_DELAY ) | + (1 <<KVM_FEATURE_MMU_OP ) | + (1 <<KVM_FEATURE_CLOCKSOURCE2 ) | + (1 <<KVM_FEATURE_ASYNC_PF ) | + (1 <<KVM_FEATURE_STEAL_TIME ) | + (1 <<KVM_FEATURE_CLOCKSOURCE_STABLE_BIT ); +static const uint32_t kvm_pv_eoi_features = (0x1 <<KVM_FEATURE_PV_EOI ); +#else +static uint32_t kvm_default_features =0 ; +static const uint32_t kvm_pv_eoi_features =0 ; +#endif +void host_cpuid (uint32_t function,uint32_t count,uint32_t *eax,uint32_t *ebx,uint32_t *ecx,uint32_t *edx) {@@ -1107,7 +1121,7 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)/* Features to be added*/ uint32_t plus_features =0 , plus_ext_features =0 ;uint32_t plus_ext2_features =0 , plus_ext3_features =0 ; -uint32_t plus_kvm_features =0 , plus_svm_features =0 ; +uint32_t plus_kvm_features = kvm_default_features, plus_svm_features =0 ;uint32_t plus_7_0_ebx_features =0 ;/* Features to be removed */ uint32_t minus_features =0 , minus_ext_features =0 ;@@ -1127,18 +1141,6 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)memcpy (x86_cpu_def, def,sizeof (*def)); } -#if defined(CONFIG_KVM ) - plus_kvm_features = (1 <<KVM_FEATURE_CLOCKSOURCE ) | - (1 <<KVM_FEATURE_NOP_IO_DELAY ) | - (1 <<KVM_FEATURE_MMU_OP ) | - (1 <<KVM_FEATURE_CLOCKSOURCE2 ) | - (1 <<KVM_FEATURE_ASYNC_PF ) | - (1 <<KVM_FEATURE_STEAL_TIME ) | - (1 <<KVM_FEATURE_CLOCKSOURCE_STABLE_BIT ); -#else - plus_kvm_features =0 ; -#endif -add_flagname_to_bitmaps ("hypervisor" , &plus_features, &plus_ext_features, &plus_ext2_features, &plus_ext3_features, &plus_kvm_features, &plus_svm_features, &plus_7_0_ebx_features);target-i386/cpu.c static void x86_cpu_register_types(void) · line 1962
Quoted 8 lines
target-i386/cpu.c +5
-0@@ -1953,3 +1955,8 @@ static void x86_cpu_register_types(void) }type_init (x86_cpu_register_types) + +void enable_kvm_pv_eoi(void) +{ + kvm_default_features |= kvm_pv_eoi_features; +}Indentation is off (7 spaces).
I'd prefer to place this function below the variable definitions, so that
type_init()remains at the bottom of the file.Otherwise I'm happy with the modified cpu.h-based interaction. So far there's no conflicts here, so it could go through either PC maintainer, kvm/uq or qom-cpu.
Andreas
Quoted 6 lines
target-i386/cpu.h +0
-0@@ -1188,4 +1188,6 @@ void do_smm_enter(CPUX86State *env1);
Quoted 5 lines
target-i386/cpu.h void do_smm_enter(CPUX86State *env1); · line 1193
Quoted 5 lines
target-i386/cpu.h +2
-0void cpu_report_tpr_access (CPUX86State *env,TPRAccess access); +void enable_kvm_pv_eoi (void ); +#endif /* CPU_I386_H */
I'll fix the whitespace and repost.
Thanks for the review.
Quoted 4 lines
Signature · 3 lines
-- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg