On Mon, 3 Oct 2016 13:03:33 -0300 Eduardo Habkost <ehabk...@redhat.com> wrote:
> 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)); [...] > (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. > > > > > object_property_add_child(OBJECT(cpu), "lapic", > > OBJECT(cpu->apic_state), &error_abort); > > -- > > 2.10.0 > > >