[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 12/19] apic: move MAX_APICS check to 'apic' c
From: |
Bandan Das |
Subject: |
Re: [Qemu-devel] [PATCH v3 12/19] apic: move MAX_APICS check to 'apic' class |
Date: |
Wed, 13 Jul 2016 20:10:04 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eduardo Habkost <address@hidden> writes:
> On Wed, Jul 13, 2016 at 06:47:20PM -0400, Bandan Das wrote:
>> Igor Mammedov <address@hidden> writes:
>>
>> > 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 <address@hidden>
>> > ---
>> > hw/intc/apic.c | 10 ++++++++++
>> > hw/intc/apic_common.c | 8 --------
>> > include/hw/i386/apic_internal.h | 4 +---
>> > 3 files changed, 11 insertions(+), 11 deletions(-)
>> >
>> > 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;
>>
>> Can this be a global ? I understand there are no other users but
>> maybe it fits alongside local_apics.
>
> The variable is removed by the next patch. :)
>
>>
>> > + if (apic_no >= MAX_APICS) {
>> > + error_setg(errp, "%s initialization failed.",
>> > + object_get_typename(OBJECT(dev)));
>> > + return;
>> > + }
>> > + s->idx = apic_no++;
>>
>> Is there a possibility of a race here with the apic removal path ?
>
> If there's that possibility: 1) it was already in
> apic_common_realize(); 2) it is removed by patch 13/19.
Ugh! Stupid me :) Yep, thanks for pointing that out.
>>
>> Bandan
>>
>> > 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);
>> > diff --git a/include/hw/i386/apic_internal.h
>> > b/include/hw/i386/apic_internal.h
>> > index 74fe935..5d3be9a 100644
>> > --- a/include/hw/i386/apic_internal.h
>> > +++ b/include/hw/i386/apic_internal.h
>> > @@ -120,8 +120,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"
>> > @@ -175,7 +173,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;
- [Qemu-devel] [PATCH v3 10/19] pc: register created initial and hotpluged CPUs in one place pc_cpu_plug(), (continued)
[Qemu-devel] [PATCH v3 11/19] pc: cpu: allow device_add to be used with x86 cpu, Igor Mammedov, 2016/07/06
[Qemu-devel] [PATCH v3 12/19] apic: move MAX_APICS check to 'apic' class, Igor Mammedov, 2016/07/06
[Qemu-devel] [PATCH v3 13/19] apic: drop APICCommonState.idx and use APIC ID as index in local_apics[], Igor Mammedov, 2016/07/06
[Qemu-devel] [PATCH v3 14/19] (kvm)apic: add unrealize callbacks, Igor Mammedov, 2016/07/06
[Qemu-devel] [PATCH v3 15/19] apic: use apic_id as apic's migration instance_id, Igor Mammedov, 2016/07/06
[Qemu-devel] [PATCH v3 16/19] target-i386: cpu: do not ignore error and fix apic parent, Igor Mammedov, 2016/07/06
[Qemu-devel] [PATCH v3 17/19] target-i386: fix apic object leak when CPU is deleted, Igor Mammedov, 2016/07/06