diff mbox

[target-arm,v7,04/15] intc: arm_gic: Macroify the MemoryRegion size

Message ID 2e50de7dcc0710ea0f9cc1ab242be4d965585c44.1430952182.git.peter.crosthwaite@xilinx.com
State New
Headers show

Commit Message

Peter Crosthwaite May 6, 2015, 10:50 p.m. UTC
GIC generally uses a 4k memory region for the various subregions, such
as GICC, GICD, GICV and GICH. Macroify this number in the publicly
visible header.

Some machine model code may need to know the individual subregion size
to implement special addresses mappings (such as aliases and
under-decoding logic).

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 hw/intc/arm_gic.c         | 2 +-
 include/hw/intc/arm_gic.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

Comments

Peter Maydell May 7, 2015, 1:59 p.m. UTC | #1
On 6 May 2015 at 23:50, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> GIC generally uses a 4k memory region for the various subregions, such
> as GICC, GICD, GICV and GICH. Macroify this number in the publicly
> visible header.

> +#define ARM_GIC_REGION_SIZE   0x1000

I don't think it makes sense to have a single constant for this.
In GICv2, the GICD distributor region is 4K, but the GICC CPU
region is 8K. We don't currently actually implement the register
in the second page (GICC_DIR), but exporting a constant that's
the wrong size doesn't seem right (especially if it means the
board model will end up repeating the GICC pages at the wrong
points!).

In GICv1, on the other hand, the GICC region was 4K, so we
can't just have a single #define for GICC size either.

-- PMM
Peter Crosthwaite May 8, 2015, 12:15 a.m. UTC | #2
On Thu, May 7, 2015 at 6:59 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 6 May 2015 at 23:50, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>> GIC generally uses a 4k memory region for the various subregions, such
>> as GICC, GICD, GICV and GICH. Macroify this number in the publicly
>> visible header.
>
>> +#define ARM_GIC_REGION_SIZE   0x1000
>
> I don't think it makes sense to have a single constant for this.
> In GICv2, the GICD distributor region is 4K, but the GICC CPU
> region is 8K. We don't currently actually implement the register
> in the second page (GICC_DIR), but exporting a constant that's
> the wrong size doesn't seem right (especially if it means the
> board model will end up repeating the GICC pages at the wrong
> points!).
>
> In GICv1, on the other hand, the GICC region was 4K, so we
> can't just have a single #define for GICC size either.
>

Ok patch dropped.

Regards,
Peter

> -- PMM
>
diff mbox

Patch

diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index a04c822..694b424 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -791,7 +791,7 @@  void gic_init_irqs_and_distributor(GICState *s)
         sysbus_init_irq(sbd, &s->parent_irq[i]);
     }
     memory_region_init_io(&s->iomem, OBJECT(s), &gic_dist_ops, s,
-                          "gic_dist", 0x1000);
+                          "gic_dist", ARM_GIC_REGION_SIZE);
 }
 
 static void arm_gic_realize(DeviceState *dev, Error **errp)
diff --git a/include/hw/intc/arm_gic.h b/include/hw/intc/arm_gic.h
index 0971e37..659be27 100644
--- a/include/hw/intc/arm_gic.h
+++ b/include/hw/intc/arm_gic.h
@@ -31,6 +31,8 @@ 
 #define ARM_GIC_GET_CLASS(obj) \
      OBJECT_GET_CLASS(ARMGICClass, (obj), TYPE_ARM_GIC)
 
+#define ARM_GIC_REGION_SIZE   0x1000
+
 typedef struct ARMGICClass {
     /*< private >*/
     ARMGICCommonClass parent_class;