On Fri, Sep 30, 2016 at 06:10:06PM +0200, Radim Krčmář wrote: > 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. > > This patch will break compilation if some code uses apic_get_class() > with CONFIG_USER_ONLY. > > Suggested-by: Eduardo Habkost <ehabk...@redhat.com> > Signed-off-by: Radim Krčmář <rkrc...@redhat.com> > --- > v2: assert() instead of error_report() and exit() [Peter] > v3: completely rewrite the mechanism [Eduardo] > > 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(-) > > 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 > <http://www.gnu.org/licenses/> > */ > #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_internal.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 index) > return !!(tab[i] & mask); > } > > +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) > } > > #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 = "apic"; > > if (kvm_apic_in_kernel()) { > @@ -2856,7 +2855,16 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error > **errp) > apic_type = "xen-apic"; > } > > - cpu->apic_state = 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 = OBJECT_CLASS(apic_get_class()); > + > + assert(apic_object_class); > + cpu->apic_state = DEVICE(object_new_with_type(apic_object_class->type));
ObjectClass::type is private. I believe the common idiom is object_new(object_class_get_name(c)). Except for that, I believe the interface is OK and matches the existing logic. We can always make it better later, if appropriate. (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). > > object_property_add_child(OBJECT(cpu), "lapic", > OBJECT(cpu->apic_state), &error_abort); > -- > 2.10.0 > -- Eduardo
