Re: [Qemu-devel] [PATCH v4] i386: Introduce ARAT CPU feature

From: Eduardo Habkost <ehabkost@redhat.com>
To: Jan Kiszka <jan.kiszka@web.de>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	qemu-devel <qemu-devel@nongnu.org>, kvm <kvm@vger.kernel.org>,
	"Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v4] i386: Introduce ARAT CPU feature
Date: Tue, 26 May 2015 14:25:09 -0300	[thread overview]
Message-ID: <20150526172509.GP28075@thinpad.lan.raisama.net> (raw)
In-Reply-To: <20150526171832.GT17796@thinpad.lan.raisama.net>

On Tue, May 26, 2015 at 02:18:32PM -0300, Eduardo Habkost wrote:
> (Fixing subject line)
> 
> On Tue, May 26, 2015 at 06:35:45AM +0200, Jan Kiszka wrote:
> > From: Jan Kiszka <jan.kiszka@siemens.com>
> > 
> > ARAT signals that the APIC timer does not stop in power saving states.
> > As our APICs are emulated, it's fine to expose this feature to guests,
> > at least when asking for KVM host features or with CPU types that
> > include the flag. The exact model number that introduced the feature is
> > not known, but reports can be found that it's at least available since
> > Sandy Bridge.
> > 
> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > ---
> > 
> > Changes in v4:
> >  - followed suggestions by Eduardo, now using PC_COMPAT_2_3 define
> > 
> >  hw/i386/pc_piix.c    |  4 ++++
> >  hw/i386/pc_q35.c     |  4 ++++
> >  include/hw/i386/pc.h |  8 ++++++++
> >  target-i386/cpu.c    | 33 ++++++++++++++++++++++++++++++++-
> >  target-i386/cpu.h    |  3 +++
> >  target-i386/kvm.c    |  2 ++
> >  6 files changed, 53 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index 212e263..b675d2c 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -543,6 +543,10 @@ static QEMUMachine pc_i440fx_machine_v2_3 = {
> >      PC_I440FX_2_3_MACHINE_OPTIONS,
> >      .name = "pc-i440fx-2.3",
> >      .init = pc_init_pci_2_3,
> > +    .compat_props = (GlobalProperty[]) {
> > +        PC_COMPAT_2_3,
> > +        { /* end of list */ }
> > +    },
> >  };
> >  
> >  #define PC_I440FX_2_2_MACHINE_OPTIONS PC_I440FX_2_3_MACHINE_OPTIONS
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index e67f2de..38c3cf2 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -439,6 +439,10 @@ static QEMUMachine pc_q35_machine_v2_3 = {
> >      PC_Q35_2_3_MACHINE_OPTIONS,
> >      .name = "pc-q35-2.3",
> >      .init = pc_q35_init_2_3,
> > +    .compat_props = (GlobalProperty[]) {
> > +        PC_COMPAT_2_3,
> > +        { /* end of list */ }
> > +    },
> >  };
> 
> This breaks pc-*-2.2 and pc-*-2.1, because 2.2 nas no .compat_props set,
> and 2.1 has .compat_props = HW_COMPAT_2_1.
> 
> Those issues are fixed by the series:
>   [PATCH v2 00/13] pc, hw, spapr: Cleanup of {HW, PC, SPAPR}_COMPAT_* macros
> 
> I suggest we base this patch on top of it.

For reference, the PC compat series I have submitted recently are
available on git, at:

  git://github.com/ehabkost/qemu-hacks.git work/pc-compat-cleanup2
  (Subject: pc, hw, spapr: Cleanup of {HW, PC, SPAPR}_COMPAT_* macros)

  git://github.com/ehabkost/qemu-hacks.git work/pc-compat-macros
  (Subject: pc: Don't use QEMUMachine, simplify compat+init code)

-- 
Eduardo