Re: [libvirt] [PATCH qom-cpu 02/11] target-i386: Disable kvm_mmu_op by default on pc-1.4

2013-01-07 Thread Eduardo Habkost 
On Sun, Jan 06, 2013 at 03:38:28PM +0200, Gleb Natapov wrote:
 On Fri, Jan 04, 2013 at 08:01:03PM -0200, Eduardo Habkost wrote:
  The kvm_mmu_op feature was removed from the kernel since v3.3 (released
  in March 2012), it was marked for removal since January 2011 and it's
  slower than shadow or hardware assisted paging (see kernel commit
  fb92045843). It doesn't make sense to keep it enabled by default.
  
 Actually it was effectively removed Oct 1 2009 by a68a6a7282373. After 3
 and a half years of not having it I think we can safely drop it without
 trying to preserve it in older machine types.

Agreed. Especially considering that the check/enforce code for KVM flags
is currently broken. So probably people using pc-1.0, pc-1.1, pc-1.2 are
probably _not_ getting the kvm_mmu feature exposed to the guest.

 
  Also, keeping it enabled by default would cause unnecessary hassle when
  libvirt start using the enforce option.
  
  Signed-off-by: Eduardo Habkost ehabk...@redhat.com
  ---
  Cc: k...@vger.kernel.org
  Cc: Michael S. Tsirkin m...@redhat.com
  Cc: Gleb Natapov g...@redhat.com
  Cc: Marcelo Tosatti mtosa...@redhat.com
  Cc: libvir-list@redhat.com
  Cc: Jiri Denemark jdene...@redhat.com
  
  I was planning to reverse the logic of the compat init functions and
  make pc_init_pci_1_3() enable kvm_mmu_op and then call pc_init_pci_1_4()
  instead. But that would require changing pc_init_pci_no_kvmclock() and
  pc_init_isa() as well. So to keep the changes simple, I am keeping the
  pattern used when pc_init_pci_1_3() was introduced, making
  pc_init_pci_1_4() disable kvm_mmu_op and then call pc_init_pci_1_3().
  
  Changes v2:
   - Coding style fix
   - Removed redundant comments above machine init functions
  ---
   hw/pc_piix.c  | 9 -
   target-i386/cpu.c | 9 +
   target-i386/cpu.h | 1 +
   3 files changed, 18 insertions(+), 1 deletion(-)
  
  diff --git a/hw/pc_piix.c b/hw/pc_piix.c
  index 99747a7..a32af6a 100644
  --- a/hw/pc_piix.c
  +++ b/hw/pc_piix.c
  @@ -217,6 +217,7 @@ static void pc_init1(MemoryRegion *system_memory,
   }
   }
   
  +/* machine init function for pc-0.14 - pc-1.2 */
   static void pc_init_pci(QEMUMachineInitArgs *args)
   {
   ram_addr_t ram_size = args-ram_size;
  @@ -238,6 +239,12 @@ static void pc_init_pci_1_3(QEMUMachineInitArgs *args)
   pc_init_pci(args);
   }
   
  +static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
  +{
  +disable_kvm_mmu_op();
  +pc_init_pci_1_3(args);
  +}
  +
   static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs *args)
   {
   ram_addr_t ram_size = args-ram_size;
  @@ -285,7 +292,7 @@ static QEMUMachine pc_machine_v1_4 = {
   .name = pc-1.4,
   .alias = pc,
   .desc = Standard PC,
  -.init = pc_init_pci_1_3,
  +.init = pc_init_pci_1_4,
   .max_cpus = 255,
   .is_default = 1,
   };
  diff --git a/target-i386/cpu.c b/target-i386/cpu.c
  index e6435da..c83a566 100644
  --- a/target-i386/cpu.c
  +++ b/target-i386/cpu.c
  @@ -158,6 +158,15 @@ void enable_kvm_pv_eoi(void)
   #endif
   }
   
  +void disable_kvm_mmu_op(void)
  +{
  +#ifdef CONFIG_KVM
 No need for ifdef here too.

Same case of the previous patch: KVM_FEATURE_MMU_OP is available only if
CONFIG_KVM is set.

-- 
Eduardo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH qom-cpu 02/11] target-i386: Disable kvm_mmu_op by default on pc-1.4

2013-01-06 Thread Gleb Natapov 
On Fri, Jan 04, 2013 at 08:01:03PM -0200, Eduardo Habkost wrote:
 The kvm_mmu_op feature was removed from the kernel since v3.3 (released
 in March 2012), it was marked for removal since January 2011 and it's
 slower than shadow or hardware assisted paging (see kernel commit
 fb92045843). It doesn't make sense to keep it enabled by default.
 
Actually it was effectively removed Oct 1 2009 by a68a6a7282373. After 3
and a half years of not having it I think we can safely drop it without
trying to preserve it in older machine types.

 Also, keeping it enabled by default would cause unnecessary hassle when
 libvirt start using the enforce option.
 
 Signed-off-by: Eduardo Habkost ehabk...@redhat.com
 ---
 Cc: k...@vger.kernel.org
 Cc: Michael S. Tsirkin m...@redhat.com
 Cc: Gleb Natapov g...@redhat.com
 Cc: Marcelo Tosatti mtosa...@redhat.com
 Cc: libvir-list@redhat.com
 Cc: Jiri Denemark jdene...@redhat.com
 
 I was planning to reverse the logic of the compat init functions and
 make pc_init_pci_1_3() enable kvm_mmu_op and then call pc_init_pci_1_4()
 instead. But that would require changing pc_init_pci_no_kvmclock() and
 pc_init_isa() as well. So to keep the changes simple, I am keeping the
 pattern used when pc_init_pci_1_3() was introduced, making
 pc_init_pci_1_4() disable kvm_mmu_op and then call pc_init_pci_1_3().
 
 Changes v2:
  - Coding style fix
  - Removed redundant comments above machine init functions
 ---
  hw/pc_piix.c  | 9 -
  target-i386/cpu.c | 9 +
  target-i386/cpu.h | 1 +
  3 files changed, 18 insertions(+), 1 deletion(-)
 
 diff --git a/hw/pc_piix.c b/hw/pc_piix.c
 index 99747a7..a32af6a 100644
 --- a/hw/pc_piix.c
 +++ b/hw/pc_piix.c
 @@ -217,6 +217,7 @@ static void pc_init1(MemoryRegion *system_memory,
  }
  }
  
 +/* machine init function for pc-0.14 - pc-1.2 */
  static void pc_init_pci(QEMUMachineInitArgs *args)
  {
  ram_addr_t ram_size = args-ram_size;
 @@ -238,6 +239,12 @@ static void pc_init_pci_1_3(QEMUMachineInitArgs *args)
  pc_init_pci(args);
  }
  
 +static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
 +{
 +disable_kvm_mmu_op();
 +pc_init_pci_1_3(args);
 +}
 +
  static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs *args)
  {
  ram_addr_t ram_size = args-ram_size;
 @@ -285,7 +292,7 @@ static QEMUMachine pc_machine_v1_4 = {
  .name = pc-1.4,
  .alias = pc,
  .desc = Standard PC,
 -.init = pc_init_pci_1_3,
 +.init = pc_init_pci_1_4,
  .max_cpus = 255,
  .is_default = 1,
  };
 diff --git a/target-i386/cpu.c b/target-i386/cpu.c
 index e6435da..c83a566 100644
 --- a/target-i386/cpu.c
 +++ b/target-i386/cpu.c
 @@ -158,6 +158,15 @@ void enable_kvm_pv_eoi(void)
  #endif
  }
  
 +void disable_kvm_mmu_op(void)
 +{
 +#ifdef CONFIG_KVM
No need for ifdef here too.

 +if (kvm_enabled()) {
 +kvm_default_features = ~(1UL  KVM_FEATURE_MMU_OP);
clear_bit()

 +}
 +#endif
 +}
 +
  void host_cpuid(uint32_t function, uint32_t count,
  uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx)
  {
 diff --git a/target-i386/cpu.h b/target-i386/cpu.h
 index 1283537..27c8d0c 100644
 --- a/target-i386/cpu.h
 +++ b/target-i386/cpu.h
 @@ -1219,5 +1219,6 @@ void do_smm_enter(CPUX86State *env1);
  void cpu_report_tpr_access(CPUX86State *env, TPRAccess access);
  
  void enable_kvm_pv_eoi(void);
 +void disable_kvm_mmu_op(void);
  
  #endif /* CPU_I386_H */
 -- 
 1.7.11.7

--
Gleb.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH qom-cpu 02/11] target-i386: Disable kvm_mmu_op by default on pc-1.4

2013-01-04 Thread Eduardo Habkost 
The kvm_mmu_op feature was removed from the kernel since v3.3 (released
in March 2012), it was marked for removal since January 2011 and it's
slower than shadow or hardware assisted paging (see kernel commit
fb92045843). It doesn't make sense to keep it enabled by default.

Also, keeping it enabled by default would cause unnecessary hassle when
libvirt start using the enforce option.

Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
Cc: k...@vger.kernel.org
Cc: Michael S. Tsirkin m...@redhat.com
Cc: Gleb Natapov g...@redhat.com
Cc: Marcelo Tosatti mtosa...@redhat.com
Cc: libvir-list@redhat.com
Cc: Jiri Denemark jdene...@redhat.com

I was planning to reverse the logic of the compat init functions and
make pc_init_pci_1_3() enable kvm_mmu_op and then call pc_init_pci_1_4()
instead. But that would require changing pc_init_pci_no_kvmclock() and
pc_init_isa() as well. So to keep the changes simple, I am keeping the
pattern used when pc_init_pci_1_3() was introduced, making
pc_init_pci_1_4() disable kvm_mmu_op and then call pc_init_pci_1_3().

Changes v2:
 - Coding style fix
 - Removed redundant comments above machine init functions
---
 hw/pc_piix.c  | 9 -
 target-i386/cpu.c | 9 +
 target-i386/cpu.h | 1 +
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 99747a7..a32af6a 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -217,6 +217,7 @@ static void pc_init1(MemoryRegion *system_memory,
 }
 }
 
+/* machine init function for pc-0.14 - pc-1.2 */
 static void pc_init_pci(QEMUMachineInitArgs *args)
 {
 ram_addr_t ram_size = args-ram_size;
@@ -238,6 +239,12 @@ static void pc_init_pci_1_3(QEMUMachineInitArgs *args)
 pc_init_pci(args);
 }
 
+static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
+{
+disable_kvm_mmu_op();
+pc_init_pci_1_3(args);
+}
+
 static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs *args)
 {
 ram_addr_t ram_size = args-ram_size;
@@ -285,7 +292,7 @@ static QEMUMachine pc_machine_v1_4 = {
 .name = pc-1.4,
 .alias = pc,
 .desc = Standard PC,
-.init = pc_init_pci_1_3,
+.init = pc_init_pci_1_4,
 .max_cpus = 255,
 .is_default = 1,
 };
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e6435da..c83a566 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -158,6 +158,15 @@ void enable_kvm_pv_eoi(void)
 #endif
 }
 
+void disable_kvm_mmu_op(void)
+{
+#ifdef CONFIG_KVM
+if (kvm_enabled()) {
+kvm_default_features = ~(1UL  KVM_FEATURE_MMU_OP);
+}
+#endif
+}
+
 void host_cpuid(uint32_t function, uint32_t count,
 uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx)
 {
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 1283537..27c8d0c 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1219,5 +1219,6 @@ void do_smm_enter(CPUX86State *env1);
 void cpu_report_tpr_access(CPUX86State *env, TPRAccess access);
 
 void enable_kvm_pv_eoi(void);
+void disable_kvm_mmu_op(void);
 
 #endif /* CPU_I386_H */
-- 
1.7.11.7

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list