On Mon, Aug 27, 2012 at 7:06 PM, Michael S. Tsirkin <mst@xxxxxxxxxx> wrote: > On Mon, Aug 27, 2012 at 06:58:29PM +0000, Blue Swirl wrote: >> On Mon, Aug 27, 2012 at 12:20 PM, Michael S. Tsirkin <mst@xxxxxxxxxx> 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@xxxxxxxxxx> >> > --- >> > 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..2422d20 >> > --- /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; >> >> Don't use identifiers with leading underscores. > > C99 spec says " > Any other predefined macro names > shall begin with a leading underscore followed by an uppercase letter or > a second underscore. > " > > what are chances of compiler predefining macro __kvm_pv_eoi_disabled? Why do you even consider that since it's trivially easy to use something else? If a standard (and HACKING in our case) specifies something, why do you want to fight it? > > But OK, will rename _kvm_pv_eoi_disabled. > _ + lower case is guaranteed OK. No, just use kvm_pv_eoi_disabled, the underscore is useless. > > >> > + >> > +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 >> > >> > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
