diff mbox

[2/9] hw/arm_gic: Remove the special casing of NCPU for the NVIC

Message ID 1335978732-32559-3-git-send-email-peter.maydell@linaro.org
State Accepted
Commit c48c6522f550b9b704f7324164b00b5770ec7345
Headers show

Commit Message

Peter Maydell May 2, 2012, 5:12 p.m. UTC
Drop the special casing of NCPU=1 for the NVIC. This slightly
increases the amount of memory used by its state structure,
but removes some ifdeffery and means we can safely move the
GIC state into a common subclass structure.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm_gic.c     |   23 +++--------------------
 hw/armv7m_nvic.c |    5 ++---
 2 files changed, 5 insertions(+), 23 deletions(-)

Comments

Andreas Färber May 18, 2012, 1:01 p.m. UTC | #1
Am 02.05.2012 19:12, schrieb Peter Maydell:
> Drop the special casing of NCPU=1 for the NVIC. This slightly
> increases the amount of memory used by its state structure,
> but removes some ifdeffery and means we can safely move the
> GIC state into a common subclass structure.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm_gic.c     |   23 +++--------------------
>  hw/armv7m_nvic.c |    5 ++---
>  2 files changed, 5 insertions(+), 23 deletions(-)
> 
> diff --git a/hw/arm_gic.c b/hw/arm_gic.c
> index 17b2eba..2d8ceb8 100644
> --- a/hw/arm_gic.c
> +++ b/hw/arm_gic.c
[...]
> @@ -131,11 +123,9 @@ typedef struct gic_state
>  
>  static inline int gic_get_current_cpu(gic_state *s)
>  {
> -#if NCPU > 1
>      if (s->num_cpu > 1) {
>          return cpu_single_env->cpu_index;
>      }
> -#endif
>      return 0;
>  }

Why special-case the num_cpu == 1 case? Is cpu_single_env not available
in all cases?

Otherwise looks good.

/-F
Peter Maydell May 18, 2012, 1:21 p.m. UTC | #2
On 18 May 2012 14:01, Andreas Färber <afaerber@suse.de> wrote:
> Am 02.05.2012 19:12, schrieb Peter Maydell:
>> Drop the special casing of NCPU=1 for the NVIC. This slightly
>> increases the amount of memory used by its state structure,
>> but removes some ifdeffery and means we can safely move the
>> GIC state into a common subclass structure.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  hw/arm_gic.c     |   23 +++--------------------
>>  hw/armv7m_nvic.c |    5 ++---
>>  2 files changed, 5 insertions(+), 23 deletions(-)
>>
>> diff --git a/hw/arm_gic.c b/hw/arm_gic.c
>> index 17b2eba..2d8ceb8 100644
>> --- a/hw/arm_gic.c
>> +++ b/hw/arm_gic.c
> [...]
>> @@ -131,11 +123,9 @@ typedef struct gic_state
>>
>>  static inline int gic_get_current_cpu(gic_state *s)
>>  {
>> -#if NCPU > 1
>>      if (s->num_cpu > 1) {
>>          return cpu_single_env->cpu_index;
>>      }
>> -#endif
>>      return 0;
>>  }
>
> Why special-case the num_cpu == 1 case? Is cpu_single_env not available
> in all cases?

The "realview_gic" device uses this GIC as a board-level
(ie not built into the CPU) GIC. In that case there might
be a multicore CPU (with its own GIC) but the boardlevel
GIC is still single-CPU-interface, so gic_get_current_cpu
needs to return 0 whichever CPU core is reading it.

-- PMM
diff mbox

Patch

diff --git a/hw/arm_gic.c b/hw/arm_gic.c
index 17b2eba..2d8ceb8 100644
--- a/hw/arm_gic.c
+++ b/hw/arm_gic.c
@@ -25,11 +25,7 @@ 
 /* First 32 are private to each CPU (SGIs and PPIs). */
 #define GIC_INTERNAL 32
 /* Maximum number of possible CPU interfaces, determined by GIC architecture */
-#ifdef NVIC
-#define NCPU 1
-#else
 #define NCPU 8
-#endif
 
 //#define DEBUG_GIC
 
@@ -67,11 +63,7 @@  typedef struct gic_irq_state
 } gic_irq_state;
 
 #define ALL_CPU_MASK ((unsigned)(((1 << NCPU) - 1)))
-#if NCPU > 1
 #define NUM_CPU(s) ((s)->num_cpu)
-#else
-#define NUM_CPU(s) 1
-#endif
 
 #define GIC_SET_ENABLED(irq, cm) s->irq_state[irq].enabled |= (cm)
 #define GIC_CLEAR_ENABLED(irq, cm) s->irq_state[irq].enabled &= ~(cm)
@@ -131,11 +123,9 @@  typedef struct gic_state
 
 static inline int gic_get_current_cpu(gic_state *s)
 {
-#if NCPU > 1
     if (s->num_cpu > 1) {
         return cpu_single_env->cpu_index;
     }
-#endif
     return 0;
 }
 
@@ -842,21 +832,14 @@  static int gic_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
-#if NCPU > 1
-static void gic_init(gic_state *s, int num_cpu, int num_irq)
-#else
 static void gic_init(gic_state *s, int num_irq)
-#endif
 {
     int i;
 
-#if NCPU > 1
-    s->num_cpu = num_cpu;
     if (s->num_cpu > NCPU) {
         hw_error("requested %u CPUs exceeds GIC maximum %d\n",
-                 num_cpu, NCPU);
+                 s->num_cpu, NCPU);
     }
-#endif
     s->num_irq = num_irq + GIC_BASE_IRQ;
     if (s->num_irq > GIC_MAXIRQ) {
         hw_error("requested %u interrupt lines exceeds GIC maximum %d\n",
@@ -880,7 +863,7 @@  static void gic_init(gic_state *s, int num_irq)
      *  [N+32..N+63] PPIs for CPU 1
      *   ...
      */
-    i += (GIC_INTERNAL * num_cpu);
+    i += (GIC_INTERNAL * s->num_cpu);
 #endif
     qdev_init_gpio_in(&s->busdev.qdev, gic_set_irq, i);
     for (i = 0; i < NUM_CPU(s); i++) {
@@ -915,7 +898,7 @@  static int arm_gic_init(SysBusDevice *dev)
     /* Device instance init function for the GIC sysbus device */
     int i;
     gic_state *s = FROM_SYSBUS(gic_state, dev);
-    gic_init(s, s->num_cpu, s->num_irq);
+    gic_init(s, s->num_irq);
     /* Distributor */
     sysbus_init_mmio(dev, &s->iomem);
     /* cpu interfaces (one for "current cpu" plus one per cpu) */
diff --git a/hw/armv7m_nvic.c b/hw/armv7m_nvic.c
index 986a6bb..99a87a2 100644
--- a/hw/armv7m_nvic.c
+++ b/hw/armv7m_nvic.c
@@ -389,9 +389,8 @@  static int armv7m_nvic_init(SysBusDevice *dev)
 {
     nvic_state *s= FROM_SYSBUSGIC(nvic_state, dev);
 
-   /* note that for the M profile gic_init() takes the number of external
-    * interrupt lines only.
-    */
+    /* The NVIC always has only one CPU */
+    s->gic.num_cpu = 1;
     gic_init(&s->gic, s->num_irq);
     memory_region_add_subregion(get_system_memory(), 0xe000e000, &s->gic.iomem);
     s->systick.timer = qemu_new_timer_ns(vm_clock, systick_timer_tick, s);