[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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 <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>
> > Cc: libvir-list redhat com
> > Cc: Jiri Denemark <jdenemar 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]