diff mbox

[RFC,5/7] target-arm/cpu: Add apic_id property for ARMCPU

Message ID 1424167806-8372-6-git-send-email-zhaoshenglong@huawei.com
State New
Headers show

Commit Message

Shannon Zhao Feb. 17, 2015, 10:10 a.m. UTC 
Add apic_id property for ARMCPU. It can be used for cpu hotplug.

Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
---
 target-arm/cpu-qom.h |    1 +
 target-arm/cpu.c     |   77 ++++++++++++++++++++++++++++++++++++++++++++++++++
 target-arm/cpu.h     |    2 +
 3 files changed, 80 insertions(+), 0 deletions(-)

Comments

Andreas Färber Feb. 18, 2015, 5:45 p.m. UTC | #1 
Hi,

Am 17.02.2015 um 11:10 schrieb Shannon Zhao:
> Add apic_id property for ARMCPU. It can be used for cpu hotplug.
> 
> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> ---
>  target-arm/cpu-qom.h |    1 +
>  target-arm/cpu.c     |   77 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  target-arm/cpu.h     |    2 +
>  3 files changed, 80 insertions(+), 0 deletions(-)
> 
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index ed5a644..d4560e2 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -59,6 +59,7 @@ typedef struct ARMCPU {
>      /*< public >*/
>  
>      CPUARMState env;
> +    uint32_t apic_id;

Can you add a matching @apic_id: documentation entry above the struct?

>  
>      /* Coprocessor information */
>      GHashTable *cp_regs;
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 285947f..9202b07 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
[...]
> @@ -343,6 +407,11 @@ static void arm_cpu_initfn(Object *obj)
>      cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
>                                           g_free, g_free);
>  
> +    object_property_add(obj, "apic-id", "int",
> +                    arm_cpuid_get_apic_id,
> +                    arm_cpuid_set_apic_id, NULL, NULL, NULL);

The property is correctly called apic-id. Please update the commit
message, which has it as apic_id (2x).

Regards,
Andreas

> +
> +     cpu->apic_id = arm_cpu_apic_id_from_index(cs->cpu_index);
>  #ifndef CONFIG_USER_ONLY
>      /* Our inbound IRQ and FIQ lines */
>      if (kvm_enabled()) {
[snip]
Igor Mammedov Feb. 18, 2015, 7:51 p.m. UTC | #2 
On Wed, 18 Feb 2015 18:45:51 +0100
Andreas Färber <afaerber@suse.de> wrote:

> Hi,
> 
> Am 17.02.2015 um 11:10 schrieb Shannon Zhao:
> > Add apic_id property for ARMCPU. It can be used for cpu hotplug.
> > 
> > Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> > ---
> >  target-arm/cpu-qom.h |    1 +
> >  target-arm/cpu.c     |   77
> > ++++++++++++++++++++++++++++++++++++++++++++++++++
> > target-arm/cpu.h     |    2 + 3 files changed, 80 insertions(+), 0
> > deletions(-)
> > 
> > diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> > index ed5a644..d4560e2 100644
> > --- a/target-arm/cpu-qom.h
> > +++ b/target-arm/cpu-qom.h
> > @@ -59,6 +59,7 @@ typedef struct ARMCPU {
> >      /*< public >*/
> >  
> >      CPUARMState env;
> > +    uint32_t apic_id;
> 
> Can you add a matching @apic_id: documentation entry above the struct?
> 
> >  
> >      /* Coprocessor information */
> >      GHashTable *cp_regs;
> > diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> > index 285947f..9202b07 100644
> > --- a/target-arm/cpu.c
> > +++ b/target-arm/cpu.c
> [...]
> > @@ -343,6 +407,11 @@ static void arm_cpu_initfn(Object *obj)
> >      cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
> >                                           g_free, g_free);
> >  
> > +    object_property_add(obj, "apic-id", "int",
> > +                    arm_cpuid_get_apic_id,
> > +                    arm_cpuid_set_apic_id, NULL, NULL, NULL);
> 
> The property is correctly called apic-id. Please update the commit
> message, which has it as apic_id (2x).
Is there such thing as apic-id on ARM?


> 
> Regards,
> Andreas
> 
> > +
> > +     cpu->apic_id = arm_cpu_apic_id_from_index(cs->cpu_index);
> >  #ifndef CONFIG_USER_ONLY
> >      /* Our inbound IRQ and FIQ lines */
> >      if (kvm_enabled()) {
> [snip]
>
Andreas Färber Feb. 18, 2015, 7:57 p.m. UTC | #3 
Am 18.02.2015 um 20:51 schrieb Igor Mammedov:
> On Wed, 18 Feb 2015 18:45:51 +0100
> Andreas Färber <afaerber@suse.de> wrote:
> 
>> Hi,
>>
>> Am 17.02.2015 um 11:10 schrieb Shannon Zhao:
>>> Add apic_id property for ARMCPU. It can be used for cpu hotplug.
>>>
>>> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
>>> ---
>>>  target-arm/cpu-qom.h |    1 +
>>>  target-arm/cpu.c     |   77
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>> target-arm/cpu.h     |    2 + 3 files changed, 80 insertions(+), 0
>>> deletions(-)
>>>
>>> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
>>> index ed5a644..d4560e2 100644
>>> --- a/target-arm/cpu-qom.h
>>> +++ b/target-arm/cpu-qom.h
>>> @@ -59,6 +59,7 @@ typedef struct ARMCPU {
>>>      /*< public >*/
>>>  
>>>      CPUARMState env;
>>> +    uint32_t apic_id;
>>
>> Can you add a matching @apic_id: documentation entry above the struct?
>>
>>>  
>>>      /* Coprocessor information */
>>>      GHashTable *cp_regs;
>>> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
>>> index 285947f..9202b07 100644
>>> --- a/target-arm/cpu.c
>>> +++ b/target-arm/cpu.c
>> [...]
>>> @@ -343,6 +407,11 @@ static void arm_cpu_initfn(Object *obj)
>>>      cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
>>>                                           g_free, g_free);
>>>  
>>> +    object_property_add(obj, "apic-id", "int",
>>> +                    arm_cpuid_get_apic_id,
>>> +                    arm_cpuid_set_apic_id, NULL, NULL, NULL);
>>
>> The property is correctly called apic-id. Please update the commit
>> message, which has it as apic_id (2x).
> Is there such thing as apic-id on ARM?

Oops, confused it with ACPI... On ARM there's the GIC (or NVIC). There
are some IDs that are being represented in the device trees, with jumps
between clusters, i.e. 0..3 and 100..103 or so.

Andreas
Hanjun Guo Feb. 19, 2015, 8:22 a.m. UTC | #4 
On 2015年02月19日 03:51, Igor Mammedov wrote:
> On Wed, 18 Feb 2015 18:45:51 +0100
> Andreas Färber <afaerber@suse.de> wrote:
>
>> Hi,
>>
>> Am 17.02.2015 um 11:10 schrieb Shannon Zhao:
>>> Add apic_id property for ARMCPU. It can be used for cpu hotplug.
>>>
>>> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
>>> ---
>>>   target-arm/cpu-qom.h |    1 +
>>>   target-arm/cpu.c     |   77
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>> target-arm/cpu.h     |    2 + 3 files changed, 80 insertions(+), 0
>>> deletions(-)
>>>
>>> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
>>> index ed5a644..d4560e2 100644
>>> --- a/target-arm/cpu-qom.h
>>> +++ b/target-arm/cpu-qom.h
>>> @@ -59,6 +59,7 @@ typedef struct ARMCPU {
>>>       /*< public >*/
>>>
>>>       CPUARMState env;
>>> +    uint32_t apic_id;
>>
>> Can you add a matching @apic_id: documentation entry above the struct?
>>
>>>
>>>       /* Coprocessor information */
>>>       GHashTable *cp_regs;
>>> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
>>> index 285947f..9202b07 100644
>>> --- a/target-arm/cpu.c
>>> +++ b/target-arm/cpu.c
>> [...]
>>> @@ -343,6 +407,11 @@ static void arm_cpu_initfn(Object *obj)
>>>       cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
>>>                                            g_free, g_free);
>>>
>>> +    object_property_add(obj, "apic-id", "int",
>>> +                    arm_cpuid_get_apic_id,
>>> +                    arm_cpuid_set_apic_id, NULL, NULL, NULL);
>>
>> The property is correctly called apic-id. Please update the commit
>> message, which has it as apic_id (2x).
> Is there such thing as apic-id on ARM?

Not apic-id, apic-id is x86 and ia64 dependent. On ARM, mpidr is
similar as apic-id to represent the CPU hardware ID, so I think
mpidr will be the one used in this patch, and another thing need
to consider that mpidr on ARM64 is 64 bits, not uint32_t.
Thanks
Hanjun
diff mbox

Patch

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index ed5a644..d4560e2 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -59,6 +59,7 @@  typedef struct ARMCPU {
     /*< public >*/
 
     CPUARMState env;
+    uint32_t apic_id;
 
     /* Coprocessor information */
     GHashTable *cp_regs;
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 285947f..9202b07 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -23,12 +23,17 @@ 
 #include "qemu-common.h"
 #include "hw/qdev-properties.h"
 #include "qapi/qmp/qerror.h"
+#include "qapi-visit.h"
+#include "qapi/visitor.h"
+#include "hw/acpi/topology.h"
+
 #if !defined(CONFIG_USER_ONLY)
 #include "hw/loader.h"
 #endif
 #include "hw/arm/arm.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/kvm.h"
+#include "sysemu/cpus.h"
 #include "kvm_arm.h"
 
 static void arm_cpu_set_pc(CPUState *cs, vaddr value)
@@ -332,6 +337,65 @@  static inline void unset_feature(CPUARMState *env, int feature)
     env->features &= ~(1ULL << feature);
 }
 
+static void arm_cpuid_get_apic_id(Object *obj, Visitor *v, void *opaque,
+                                  const char *name, Error **errp)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+    int64_t value = cpu->apic_id;
+
+    visit_type_int(v, &value, name, errp);
+}
+
+static void arm_cpuid_set_apic_id(Object *obj, Visitor *v, void *opaque,
+                                  const char *name, Error **errp)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+    DeviceState *dev = DEVICE(obj);
+    const int64_t min = 0;
+    const int64_t max = UINT32_MAX;
+    Error *error = NULL;
+    int64_t value;
+
+    if (dev->realized) {
+        error_setg(errp, "Attempt to set property '%s' on '%s' after "
+                   "it was realized", name, object_get_typename(obj));
+        return;
+    }
+
+    visit_type_int(v, &value, name, &error);
+    if (error) {
+        error_propagate(errp, error);
+        return;
+    }
+    if (value < min || value > max) {
+        error_setg(errp, "Property %s.%s doesn't take value %" PRId64
+                   " (minimum: %" PRId64 ", maximum: %" PRId64 ")" ,
+                   object_get_typename(obj), name, value, min, max);
+        return;
+    }
+
+    if ((value != cpu->apic_id) && cpu_exists(value)) {
+        error_setg(errp, "CPU with APIC ID %" PRIi64 " exists", value);
+        return;
+    }
+    cpu->apic_id = value;
+}
+
+/* Calculates initial APIC ID for a specific CPU index
+ *
+ * Currently we need to be able to calculate the APIC ID from the CPU index
+ * alone (without requiring a CPU object), as the QEMU<->Seabios interfaces have
+ * no concept of "CPU index", and the NUMA tables on fw_cfg need the APIC ID of
+ * all CPUs up to max_cpus.
+ */
+uint32_t arm_cpu_apic_id_from_index(unsigned int cpu_index)
+{
+    uint32_t correct_id;
+
+    correct_id = apicid_from_cpu_idx(smp_cores, smp_threads, cpu_index);
+    return correct_id;
+}
+
 static void arm_cpu_initfn(Object *obj)
 {
     CPUState *cs = CPU(obj);
@@ -343,6 +407,11 @@  static void arm_cpu_initfn(Object *obj)
     cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
                                          g_free, g_free);
 
+    object_property_add(obj, "apic-id", "int",
+                    arm_cpuid_get_apic_id,
+                    arm_cpuid_set_apic_id, NULL, NULL, NULL);
+
+     cpu->apic_id = arm_cpu_apic_id_from_index(cs->cpu_index);
 #ifndef CONFIG_USER_ONLY
     /* Our inbound IRQ and FIQ lines */
     if (kvm_enabled()) {
@@ -379,6 +448,13 @@  static void arm_cpu_initfn(Object *obj)
     }
 }
 
+static int64_t arm_cpu_get_arch_id(CPUState *cs)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+
+    return cpu->apic_id;
+}
+
 static Property arm_cpu_reset_cbar_property =
             DEFINE_PROP_UINT64("reset-cbar", ARMCPU, reset_cbar, 0);
 
@@ -1183,6 +1259,7 @@  static void arm_cpu_class_init(ObjectClass *oc, void *data)
     cc->set_pc = arm_cpu_set_pc;
     cc->gdb_read_register = arm_cpu_gdb_read_register;
     cc->gdb_write_register = arm_cpu_gdb_write_register;
+    cc->get_arch_id = arm_cpu_get_arch_id;
 #ifdef CONFIG_USER_ONLY
     cc->handle_mmu_fault = arm_cpu_handle_mmu_fault;
 #else
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index cd7a9e8..9e60972 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1819,4 +1819,6 @@  enum {
     QEMU_PSCI_CONDUIT_HVC = 2,
 };
 
+uint32_t arm_cpu_apic_id_from_index(unsigned int cpu_index);
+
 #endif