On Mon, Aug 27, 2012 at 7:24 PM, Michael S. Tsirkin <mst@xxxxxxxxxx> wrote: > On Mon, Aug 27, 2012 at 07:12:27PM +0000, Blue Swirl wrote: >> 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? > > I missed this in HACKING, you are right: > > 2.4. Reserved namespaces in C and POSIX > Underscore capital, double underscore, and underscore 't' suffixes > should be avoided. > > so _kvm_pv_eoi_disabled is ok __kvm_pv_eoi_disabled is not. > Will fix. No leading underscores. They are not used in QEMU. > >> > >> > But OK, will rename _kvm_pv_eoi_disabled. >> > _ + lower case is guaranteed OK. >> >> No, just use kvm_pv_eoi_disabled, the underscore is useless. > > It isn't useless, this avoids conflict with function name. > _ says it's an internal variable used to implement kvm_pv_eoi_disabled > in a very clear way. Sure, but there are infinite number of ways of making the identifiers unique. Using leading underscores is a way to ever conflict with compiler, linker, libc, POSIX etc. Don't do it. Where's your imagination, can't you invent any other prefix or suffix? > > -- > 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
