| Message ID | c4b8b2d3190387567e580ec0ac0449246e433328.1346095866.git.mst@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Aug 28, 2012 at 1:22 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > In preparation for adding PV EOI support, disable PV EOI by default for > 1.1 and older machine types, to avoid CPUID changing during migration. > > PV EOI can still be enabled/disabled by specifying it explicitly. > Enable for 1.1 > -M pc-1.1 -cpu kvm64,+kvm_pv_eoi > Disable for 1.2 > -M pc-1.2 -cpu kvm64,-kvm_pv_eoi > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > hw/Makefile.objs | 2 +- > hw/cpu_flags.c | 32 ++++++++++++++++++++++++++++++++ > hw/cpu_flags.h | 9 +++++++++ > hw/pc_piix.c | 2 ++ > target-i386/cpu.c | 8 ++++++++ > 5 files changed, 52 insertions(+), 1 deletion(-) > create mode 100644 hw/cpu_flags.c > create mode 100644 hw/cpu_flags.h > > diff --git a/hw/Makefile.objs b/hw/Makefile.objs > index 850b87b..3f2532a 100644 > --- a/hw/Makefile.objs > +++ b/hw/Makefile.objs > @@ -1,5 +1,5 @@ > hw-obj-y = usb/ ide/ > -hw-obj-y += loader.o > +hw-obj-y += loader.o cpu_flags.o > hw-obj-$(CONFIG_VIRTIO) += virtio-console.o > hw-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o > hw-obj-y += fw_cfg.o > diff --git a/hw/cpu_flags.c b/hw/cpu_flags.c > new file mode 100644 > index 0000000..d821d8c > --- /dev/null > +++ b/hw/cpu_flags.c > @@ -0,0 +1,32 @@ > +/* > + * CPU compatibility flags. > + * > + * Copyright (c) 2012 Red Hat Inc. > + * Author: Michael S. Tsirkin. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License along > + * with this program; if not, see <http://www.gnu.org/licenses/>. > + */ > +#include "hw/cpu_flags.h" > + > +static bool _kvm_pv_eoi_disabled; NACK. I find your lack of compliance disturbing. > + > +void disable_kvm_pv_eoi(void) > +{ > + _kvm_pv_eoi_disabled = true; > +} > + > +bool kvm_pv_eoi_disabled(void) > +{ > + return _kvm_pv_eoi_disabled; > +} > diff --git a/hw/cpu_flags.h b/hw/cpu_flags.h > new file mode 100644 > index 0000000..05777b6 > --- /dev/null > +++ b/hw/cpu_flags.h > @@ -0,0 +1,9 @@ > +#ifndef HW_CPU_FLAGS_H > +#define HW_CPU_FLAGS_H > + > +#include <stdbool.h> > + > +void disable_kvm_pv_eoi(void); > +bool kvm_pv_eoi_disabled(void); > + > +#endif > diff --git a/hw/pc_piix.c b/hw/pc_piix.c > index 008d42f..bdbceda 100644 > --- a/hw/pc_piix.c > +++ b/hw/pc_piix.c > @@ -46,6 +46,7 @@ > #ifdef CONFIG_XEN > # include <xen/hvm/hvm_info_table.h> > #endif > +#include "cpu_flags.h" > > #define MAX_IDE_BUS 2 > > @@ -371,6 +372,7 @@ static QEMUMachine pc_machine_v1_2 = { > > static void pc_machine_v1_1_compat(void) > { > + disable_kvm_pv_eoi(); > } > > static void pc_init_pci_v1_1(ram_addr_t ram_size, > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 120a2e3..0d02fd1 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -23,6 +23,7 @@ > > #include "cpu.h" > #include "kvm.h" > +#include "asm/kvm_para.h" > > #include "qemu-option.h" > #include "qemu-config.h" > @@ -33,6 +34,7 @@ > #include "hyperv.h" > > #include "hw/hw.h" > +#include "hw/cpu_flags.h" > > /* feature flags taken from "Intel Processor Identification and the CPUID > * Instruction" and AMD's "CPUID Specification". In cases of disagreement > @@ -889,6 +891,12 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model) > > plus_kvm_features = ~0; /* not supported bits will be filtered out later */ > > + /* Disable PV EOI for old machine types. > + * Feature flags can still override. */ > + if (kvm_pv_eoi_disabled()) { > + plus_kvm_features &= ~(0x1 << KVM_FEATURE_PV_EOI); > + } > + > add_flagname_to_bitmaps("hypervisor", &plus_features, > &plus_ext_features, &plus_ext2_features, &plus_ext3_features, > &plus_kvm_features, &plus_svm_features); > -- > MST > >
On Tue, Aug 28, 2012 at 05:05:25PM +0000, Blue Swirl wrote: > > +static bool _kvm_pv_eoi_disabled; > > NACK. I find your lack of compliance disturbing. Compliance with what? Could you please add some motivation for the NACK?
On Tue, Aug 28, 2012 at 5:22 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > On Tue, Aug 28, 2012 at 05:05:25PM +0000, Blue Swirl wrote: >> > +static bool _kvm_pv_eoi_disabled; >> >> NACK. I find your lack of compliance disturbing. > > Compliance with what? Could you please add some > motivation for the NACK? You did not respect my review comments. > > -- > MST
On Tue, Aug 28, 2012 at 05:28:22PM +0000, Blue Swirl wrote: > On Tue, Aug 28, 2012 at 5:22 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Tue, Aug 28, 2012 at 05:05:25PM +0000, Blue Swirl wrote: > >> > +static bool _kvm_pv_eoi_disabled; > >> > >> NACK. I find your lack of compliance disturbing. > > > > Compliance with what? Could you please add some > > motivation for the NACK? > > You did not respect my review comments. I merely asking for motivation for the comments. Anyway, malc explained it to me so I got it now. Thanks for the review will post soon. > > > > -- > > MST
diff --git a/hw/Makefile.objs b/hw/Makefile.objs index 850b87b..3f2532a 100644 --- a/hw/Makefile.objs +++ b/hw/Makefile.objs @@ -1,5 +1,5 @@ hw-obj-y = usb/ ide/ -hw-obj-y += loader.o +hw-obj-y += loader.o cpu_flags.o hw-obj-$(CONFIG_VIRTIO) += virtio-console.o hw-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o hw-obj-y += fw_cfg.o diff --git a/hw/cpu_flags.c b/hw/cpu_flags.c new file mode 100644 index 0000000..d821d8c --- /dev/null +++ b/hw/cpu_flags.c @@ -0,0 +1,32 @@ +/* + * CPU compatibility flags. + * + * Copyright (c) 2012 Red Hat Inc. + * Author: Michael S. Tsirkin. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, see <http://www.gnu.org/licenses/>. + */ +#include "hw/cpu_flags.h" + +static bool _kvm_pv_eoi_disabled; + +void disable_kvm_pv_eoi(void) +{ + _kvm_pv_eoi_disabled = true; +} + +bool kvm_pv_eoi_disabled(void) +{ + return _kvm_pv_eoi_disabled; +} diff --git a/hw/cpu_flags.h b/hw/cpu_flags.h new file mode 100644 index 0000000..05777b6 --- /dev/null +++ b/hw/cpu_flags.h @@ -0,0 +1,9 @@ +#ifndef HW_CPU_FLAGS_H +#define HW_CPU_FLAGS_H + +#include <stdbool.h> + +void disable_kvm_pv_eoi(void); +bool kvm_pv_eoi_disabled(void); + +#endif diff --git a/hw/pc_piix.c b/hw/pc_piix.c index 008d42f..bdbceda 100644 --- a/hw/pc_piix.c +++ b/hw/pc_piix.c @@ -46,6 +46,7 @@ #ifdef CONFIG_XEN # include <xen/hvm/hvm_info_table.h> #endif +#include "cpu_flags.h" #define MAX_IDE_BUS 2 @@ -371,6 +372,7 @@ static QEMUMachine pc_machine_v1_2 = { static void pc_machine_v1_1_compat(void) { + disable_kvm_pv_eoi(); } static void pc_init_pci_v1_1(ram_addr_t ram_size, diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 120a2e3..0d02fd1 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -23,6 +23,7 @@ #include "cpu.h" #include "kvm.h" +#include "asm/kvm_para.h" #include "qemu-option.h" #include "qemu-config.h" @@ -33,6 +34,7 @@ #include "hyperv.h" #include "hw/hw.h" +#include "hw/cpu_flags.h" /* feature flags taken from "Intel Processor Identification and the CPUID * Instruction" and AMD's "CPUID Specification". In cases of disagreement @@ -889,6 +891,12 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model) plus_kvm_features = ~0; /* not supported bits will be filtered out later */ + /* Disable PV EOI for old machine types. + * Feature flags can still override. */ + if (kvm_pv_eoi_disabled()) { + plus_kvm_features &= ~(0x1 << KVM_FEATURE_PV_EOI); + } + add_flagname_to_bitmaps("hypervisor", &plus_features, &plus_ext_features, &plus_ext2_features, &plus_ext3_features, &plus_kvm_features, &plus_svm_features);
In preparation for adding PV EOI support, disable PV EOI by default for 1.1 and older machine types, to avoid CPUID changing during migration. PV EOI can still be enabled/disabled by specifying it explicitly. Enable for 1.1 -M pc-1.1 -cpu kvm64,+kvm_pv_eoi Disable for 1.2 -M pc-1.2 -cpu kvm64,-kvm_pv_eoi Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- hw/Makefile.objs | 2 +- hw/cpu_flags.c | 32 ++++++++++++++++++++++++++++++++ hw/cpu_flags.h | 9 +++++++++ hw/pc_piix.c | 2 ++ target-i386/cpu.c | 8 ++++++++ 5 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 hw/cpu_flags.c create mode 100644 hw/cpu_flags.h