| Message ID | 20160929112329.2408-2-rkrcmar@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Sep 29, 2016 at 01:23:23PM +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. > > Signed-off-by: Radim Krčmář <rkrcmar@redhat.com> > --- > 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(+) > > 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 <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" > @@ -296,6 +297,13 @@ static int apic_load_old(QEMUFile *f, void *opaque, int version_id) > > static const VMStateDescription vmstate_apic_common; > > +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 = APIC_COMMON(dev); > @@ -306,6 +314,9 @@ static void apic_common_realize(DeviceState *dev, Error **errp) > info = APIC_COMMON_GET_CLASS(s); > info->realize(dev, errp); > > + assert(apic_class == info || !apic_class); > + apic_class = info; > + > /* Note: We need at least 1M to map the VAPIC option ROM */ > if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK && > ram_size >= 1024 * 1024) { > diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.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 index) > return !!(tab[i] & mask); > } > > + > +APICCommonClass *apic_get_class(void); > + > #endif /* QEMU_APIC_INTERNAL_H */ > -- > 2.10.0 >
2016-09-29 11:53-0300, Eduardo Habkost: > On Thu, Sep 29, 2016 at 01:23:23PM +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. >> >> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com> >> --- >> v2: assert() instead of error_report() and exit() [Peter] >> --- >> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c >> @@ -296,6 +297,13 @@ static int apic_load_old(QEMUFile *f, void *opaque, int version_id) >> >> static const VMStateDescription vmstate_apic_common; >> >> +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. Sure, it'll be nicer, thanks. ia64 seems dead, so no issues with making the code x86 specific.
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 <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" @@ -296,6 +297,13 @@ static int apic_load_old(QEMUFile *f, void *opaque, int version_id) static const VMStateDescription vmstate_apic_common; +APICCommonClass *apic_class; + +APICCommonClass *apic_get_class(void) +{ + return apic_class; +} + static void apic_common_realize(DeviceState *dev, Error **errp) { APICCommonState *s = APIC_COMMON(dev); @@ -306,6 +314,9 @@ static void apic_common_realize(DeviceState *dev, Error **errp) info = APIC_COMMON_GET_CLASS(s); info->realize(dev, errp); + assert(apic_class == info || !apic_class); + apic_class = info; + /* Note: We need at least 1M to map the VAPIC option ROM */ if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK && ram_size >= 1024 * 1024) { diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.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 index) return !!(tab[i] & mask); } + +APICCommonClass *apic_get_class(void); + #endif /* QEMU_APIC_INTERNAL_H */
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. Signed-off-by: Radim Krčmář <rkrcmar@redhat.com> --- 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(+)