Re: [Qemu-devel] [PATCH v4 08/16] apic: move MAX_APICS check to 'apic' class

["Radim Krčmář" <rkrcmar@redhat.com>]

2016-07-14 18:54+0200, Igor Mammedov:
> MAX_APICS is only used by child 'apic' class and not
> by its parent TYPE_APIC_COMMON or any other derived
> class.
> Move check into end user 'apic' class so it won't
> get in the way of other APIC implementations
> if they support more then MAX_APICS.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---

The other two APIC implementations, for KVM a Xen, don't seem to need
this extra check.

Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>

> CC: Radim Krčmář <rkrcmar@redhat.com>
> CC: pbonzini@redhat.com
> ---
>  include/hw/i386/apic_internal.h |  4 +---
>  hw/intc/apic.c                  | 10 ++++++++++
>  hw/intc/apic_common.c           |  8 --------
>  3 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
> index 73ce716..67348e9 100644
> --- a/include/hw/i386/apic_internal.h
> +++ b/include/hw/i386/apic_internal.h
> @@ -121,8 +121,6 @@
>  #define VAPIC_ENABLE_BIT                0
>  #define VAPIC_ENABLE_MASK               (1 << VAPIC_ENABLE_BIT)
>  
> -#define MAX_APICS 255
> -
>  typedef struct APICCommonState APICCommonState;
>  
>  #define TYPE_APIC_COMMON "apic-common"
> @@ -176,7 +174,7 @@ struct APICCommonState {
>      uint32_t initial_count;
>      int64_t initial_count_load_time;
>      int64_t next_time;
> -    int idx;
> +    int idx; /* not actually common, used only by 'apic' derived class */
>      QEMUTimer *timer;
>      int64_t timer_expiry;
>      int sipi_vector;
> diff --git a/hw/intc/apic.c b/hw/intc/apic.c
> index e1ab935..b0d237b 100644
> --- a/hw/intc/apic.c
> +++ b/hw/intc/apic.c
> @@ -28,7 +28,9 @@
>  #include "trace.h"
>  #include "hw/i386/pc.h"
>  #include "hw/i386/apic-msidef.h"
> +#include "qapi/error.h"
>  
> +#define MAX_APICS 255
>  #define MAX_APIC_WORDS 8
>  
>  #define SYNC_FROM_VAPIC                 0x1
> @@ -869,6 +871,14 @@ static const MemoryRegionOps apic_io_ops = {
>  static void apic_realize(DeviceState *dev, Error **errp)
>  {
>      APICCommonState *s = APIC_COMMON(dev);
> +    static int apic_no;
> +
> +    if (apic_no >= MAX_APICS) {
> +        error_setg(errp, "%s initialization failed.",
> +                   object_get_typename(OBJECT(dev)));
> +        return;
> +    }
> +    s->idx = apic_no++;
>  
>      memory_region_init_io(&s->io_memory, OBJECT(s), &apic_io_ops, s, "apic-msi",
>                            APIC_SPACE_SIZE);
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index e6eb694..fd425d1 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -299,14 +299,6 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
>      APICCommonState *s = APIC_COMMON(dev);
>      APICCommonClass *info;
>      static DeviceState *vapic;
> -    static int apic_no;
> -
> -    if (apic_no >= MAX_APICS) {
> -        error_setg(errp, "%s initialization failed.",
> -                   object_get_typename(OBJECT(dev)));
> -        return;
> -    }
> -    s->idx = apic_no++;
>  
>      info = APIC_COMMON_GET_CLASS(s);
>      info->realize(dev, errp);
> -- 
> 2.7.4
>