From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34041) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bpciS-0007EH-Do for qemu-devel@nongnu.org; Thu, 29 Sep 2016 10:53:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bpciP-0004qn-5O for qemu-devel@nongnu.org; Thu, 29 Sep 2016 10:53:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54826) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bpciO-0004qT-SU for qemu-devel@nongnu.org; Thu, 29 Sep 2016 10:53:37 -0400 Date: Thu, 29 Sep 2016 11:53:34 -0300 From: Eduardo Habkost Message-ID: <20160929145334.GR3877@thinpad.lan.raisama.net> References: <20160929112329.2408-1-rkrcmar@redhat.com> <20160929112329.2408-2-rkrcmar@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160929112329.2408-2-rkrcmar@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 1/7] apic: add global apic_get_class() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Radim =?utf-8?B?S3LEjW3DocWZ?= Cc: qemu-devel@nongnu.org, Peter Xu , Igor Mammedov , Paolo Bonzini , Richard Henderson , "Michael S. Tsirkin" On Thu, Sep 29, 2016 at 01:23:23PM +0200, Radim Kr=C4=8Dm=C3=A1=C5=99 wro= te: > Every configuration has only up to one APIC class and we'll be extendin= g > the class with a function that can be called without an instanced > object, so a direct access to the class is convenient. >=20 > Signed-off-by: Radim Kr=C4=8Dm=C3=A1=C5=99 > --- > v2: assert() instead of error_report() and exit() [Peter] > --- > hw/intc/apic_common.c | 11 +++++++++++ > include/hw/i386/apic_internal.h | 3 +++ > 2 files changed, 14 insertions(+) >=20 > diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c > index 14ac43c18666..a766f0efc805 100644 > --- a/hw/intc/apic_common.c > +++ b/hw/intc/apic_common.c > @@ -18,6 +18,7 @@ > * License along with this library; if not, see > */ > #include "qemu/osdep.h" > +#include "qemu/error-report.h" > #include "qapi/error.h" > #include "qemu-common.h" > #include "cpu.h" > @@ -296,6 +297,13 @@ static int apic_load_old(QEMUFile *f, void *opaque= , int version_id) > =20 > static const VMStateDescription vmstate_apic_common; > =20 > +APICCommonClass *apic_class; > + > +APICCommonClass *apic_get_class(void) > +{ > + return apic_class; > +} > + This will break in case we need to look at the APIC class before realizing the CPUs for any reason. Instead of adding a global variable, what about moving the class name logic that's already inside x86_cpu_apic_create() to the apic_get_class() function? Then x86_cpu_apic_create() can simply call apic_get_class() too. > static void apic_common_realize(DeviceState *dev, Error **errp) > { > APICCommonState *s =3D APIC_COMMON(dev); > @@ -306,6 +314,9 @@ static void apic_common_realize(DeviceState *dev, E= rror **errp) > info =3D APIC_COMMON_GET_CLASS(s); > info->realize(dev, errp); > =20 > + assert(apic_class =3D=3D info || !apic_class); > + apic_class =3D info; > + > /* Note: We need at least 1M to map the VAPIC option ROM */ > if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK && > ram_size >=3D 1024 * 1024) { > diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_int= ernal.h > index 06c4e9f6f95b..9ba8a5c87f90 100644 > --- a/include/hw/i386/apic_internal.h > +++ b/include/hw/i386/apic_internal.h > @@ -222,4 +222,7 @@ static inline int apic_get_bit(uint32_t *tab, int i= ndex) > return !!(tab[i] & mask); > } > =20 > + > +APICCommonClass *apic_get_class(void); > + > #endif /* QEMU_APIC_INTERNAL_H */ > --=20 > 2.10.0 >=20 --=20 Eduardo