[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCHv2 3/4] cpuid: disable pv eoi for 1.1 and older c
From: |
Blue Swirl |
Subject: |
Re: [Qemu-devel] [PATCHv2 3/4] cpuid: disable pv eoi for 1.1 and older compat types |
Date: |
Mon, 27 Aug 2012 19:40:56 +0000 |
On Mon, Aug 27, 2012 at 7:24 PM, Michael S. Tsirkin <address@hidden> 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 <address@hidden> 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 <address@hidden>
>> >> 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 <address@hidden>
>> >> > ---
>> >> > 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
- [Qemu-devel] [PATCHv2 2/4] pc: refactor compat code, (continued)
[Qemu-devel] [PATCHv2 3/4] cpuid: disable pv eoi for 1.1 and older compat types, Michael S. Tsirkin, 2012/08/27
- Re: [Qemu-devel] [PATCHv2 3/4] cpuid: disable pv eoi for 1.1 and older compat types, Blue Swirl, 2012/08/27
- Re: [Qemu-devel] [PATCHv2 3/4] cpuid: disable pv eoi for 1.1 and older compat types, Michael S. Tsirkin, 2012/08/27
- Re: [Qemu-devel] [PATCHv2 3/4] cpuid: disable pv eoi for 1.1 and older compat types, Blue Swirl, 2012/08/27
- Re: [Qemu-devel] [PATCHv2 3/4] cpuid: disable pv eoi for 1.1 and older compat types, Michael S. Tsirkin, 2012/08/27
- Re: [Qemu-devel] [PATCHv2 3/4] cpuid: disable pv eoi for 1.1 and older compat types,
Blue Swirl <=
- Re: [Qemu-devel] [PATCHv2 3/4] cpuid: disable pv eoi for 1.1 and older compat types, Michael S. Tsirkin, 2012/08/28
- Re: [Qemu-devel] [PATCHv2 3/4] cpuid: disable pv eoi for 1.1 and older compat types, malc, 2012/08/28
- Re: [Qemu-devel] [PATCHv2 3/4] cpuid: disable pv eoi for 1.1 and older compat types, Michael S. Tsirkin, 2012/08/28
Re: [Qemu-devel] [PATCHv2 3/4] cpuid: disable pv eoi for 1.1 and older compat types, Anthony Liguori, 2012/08/28
Re: [Qemu-devel] [PATCHv2 3/4] cpuid: disable pv eoi for 1.1 and older compat types, Michael S. Tsirkin, 2012/08/28
[Qemu-devel] [PATCHv2 4/4] kvm: get/set PV EOI MSR, Michael S. Tsirkin, 2012/08/27