From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37212) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brNSE-00058n-KY for qemu-devel@nongnu.org; Tue, 04 Oct 2016 07:00:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1brNS4-0005eg-NE for qemu-devel@nongnu.org; Tue, 04 Oct 2016 07:00:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58804) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brNS4-0005cy-CN for qemu-devel@nongnu.org; Tue, 04 Oct 2016 07:00:00 -0400 Date: Tue, 4 Oct 2016 12:59:52 +0200 From: Igor Mammedov Message-ID: <20161004125952.72f414b3@nial.brq.redhat.com> In-Reply-To: <20161003160333.GD3877@thinpad.lan.raisama.net> References: <20160930161013.9832-1-rkrcmar@redhat.com> <20160930161013.9832-2-rkrcmar@redhat.com> <20161003160333.GD3877@thinpad.lan.raisama.net> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 1/8] apic: add global apic_get_class() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: Radim =?UTF-8?B?S3LEjW3DocWZ?= , "Michael S. Tsirkin" , qemu-devel@nongnu.org, Peter Xu , Paolo Bonzini , Richard Henderson On Mon, 3 Oct 2016 13:03:33 -0300 Eduardo Habkost wrote: > On Fri, Sep 30, 2016 at 06:10:06PM +0200, Radim Kr=C4=8Dm=C3=A1=C5=99 wro= te: > > Every configuration has only up to one APIC class and we'll be extending > > the class with a function that can be called without an instanced > > object, so a direct access to the class is convenient. > >=20 > > This patch will break compilation if some code uses apic_get_class() > > with CONFIG_USER_ONLY. > >=20 > > Suggested-by: Eduardo Habkost > > Signed-off-by: Radim Kr=C4=8Dm=C3=A1=C5=99 > > --- > > v2: assert() instead of error_report() and exit() [Peter] > > v3: completely rewrite the mechanism [Eduardo] > >=20 > > It still looks horrible, so I'll be glad for any advice. > > And what is CONFIG_USER_ONLY? > > --- > > hw/intc/apic_common.c | 1 + > > include/hw/i386/apic_internal.h | 2 ++ > > target-i386/cpu.c | 14 +++++++++++--- > > 3 files changed, 14 insertions(+), 3 deletions(-) > >=20 > > diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c > > index 14ac43c18666..8d01c9c8750e 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" > > diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_int= ernal.h > > index 06c4e9f6f95b..286684857e9f 100644 > > --- a/include/hw/i386/apic_internal.h > > +++ b/include/hw/i386/apic_internal.h > > @@ -222,4 +222,6 @@ 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 */ > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index 333309b9a70e..6acf9c3c2372 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > > @@ -2845,9 +2845,8 @@ static void mce_init(X86CPU *cpu) > > } > > =20 > > #ifndef CONFIG_USER_ONLY > > -static void x86_cpu_apic_create(X86CPU *cpu, Error **errp) > > +APICCommonClass *apic_get_class(void) > > { > > - APICCommonState *apic; > > const char *apic_type =3D "apic"; > > =20 > > if (kvm_apic_in_kernel()) { > > @@ -2856,7 +2855,16 @@ static void x86_cpu_apic_create(X86CPU *cpu, Err= or **errp) > > apic_type =3D "xen-apic"; > > } > > =20 > > - cpu->apic_state =3D DEVICE(object_new(apic_type)); > > + return APIC_COMMON_CLASS(object_class_by_name(apic_type)); > > +} > > + > > +static void x86_cpu_apic_create(X86CPU *cpu, Error **errp) > > +{ > > + APICCommonState *apic; > > + ObjectClass *apic_object_class =3D OBJECT_CLASS(apic_get_class()); > > + > > + assert(apic_object_class); > > + cpu->apic_state =3D DEVICE(object_new_with_type(apic_object_class-= >type)); =20 [...] > (e.g. I wonder if we could have a container object for all APICs > (icc-bus?), and move the send_msi() method and the > apic_get_class() logic to it). That's probably not worth all the boilerplate code it would bring. >=20 > > =20 > > object_property_add_child(OBJECT(cpu), "lapic", > > OBJECT(cpu->apic_state), &error_abort); > > --=20 > > 2.10.0 > > =20 >=20