diff mbox series

[2/8] hw/arm/exynos4210: Explicit number of GIC external IRQs

Message ID 20250130182441.40480-3-philmd@linaro.org
State New
Headers show
Series hw/arm: Explicit number of GIC external IRQs for Cortex A9/A15 MPCore | expand

Commit Message

Philippe Mathieu-Daudé Jan. 30, 2025, 6:24 p.m. UTC
When not specified, Cortex-A9MP configures its GIC with 64 external
IRQs (see commit a32134aad89 "arm:make the number of GIC interrupts
configurable"). Add the GIC_EXT_IRQS definition (with a comment)
to make that explicit.

Except explicitly setting a property value to its same implicit
value, there is no logical change intended.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/exynos4210.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Peter Maydell Jan. 30, 2025, 6:30 p.m. UTC | #1
On Thu, 30 Jan 2025 at 18:25, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> When not specified, Cortex-A9MP configures its GIC with 64 external
> IRQs (see commit a32134aad89 "arm:make the number of GIC interrupts
> configurable"). Add the GIC_EXT_IRQS definition (with a comment)
> to make that explicit.
>
> Except explicitly setting a property value to its same implicit
> value, there is no logical change intended.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/arm/exynos4210.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
> index 99b05a175d6..75d6e4d1ab9 100644
> --- a/hw/arm/exynos4210.c
> +++ b/hw/arm/exynos4210.c
> @@ -103,6 +103,14 @@
>  #define EXYNOS4210_PL330_BASE1_ADDR         0x12690000
>  #define EXYNOS4210_PL330_BASE2_ADDR         0x12850000
>
> +/*
> + * The Cortex-A9MP may have anything from 0 to 224 external interrupt
> + * IRQ lines (with another 32 internal). We default to 64+32, which
> + * is the number provided by the Cortex-A9MP test chip in the
> + * Realview PBX-A9 and Versatile Express A9 development boards.
> + */

This isn't the A9MP test chip or the vexpress or realview
board, though. We should be setting this to whatever the
exynos4210's GIC is actually configured with, or else saying
we don't know what the right value is but this is the
one QEMU has always used (i.e. probably a TODO comment). Those
Arm devboards are irrelevant either way.

> +#define GIC_EXT_IRQS 64
> +
>  enum ExtGicId {
>      EXT_GIC_ID_MDMA_LCD0 = 66,
>      EXT_GIC_ID_PDMA0,
> @@ -588,6 +596,8 @@ static void exynos4210_realize(DeviceState *socdev, Error **errp)
>
>      /* Private memory region and Internal GIC */
>      qdev_prop_set_uint32(DEVICE(&s->a9mpcore), "num-cpu", EXYNOS4210_NCPUS);
> +    qdev_prop_set_uint32(DEVICE(&s->a9mpcore), "num-irq",
> +                         GIC_EXT_IRQS + GIC_INTERNAL);
>      busdev = SYS_BUS_DEVICE(&s->a9mpcore);
>      sysbus_realize(busdev, &error_fatal);
>      sysbus_mmio_map(busdev, 0, EXYNOS4210_SMP_PRIVATE_BASE_ADDR);

thanks
-- PMM
Philippe Mathieu-Daudé Jan. 30, 2025, 7:39 p.m. UTC | #2
On 30/1/25 19:30, Peter Maydell wrote:
> On Thu, 30 Jan 2025 at 18:25, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> When not specified, Cortex-A9MP configures its GIC with 64 external
>> IRQs (see commit a32134aad89 "arm:make the number of GIC interrupts
>> configurable"). Add the GIC_EXT_IRQS definition (with a comment)
>> to make that explicit.
>>
>> Except explicitly setting a property value to its same implicit
>> value, there is no logical change intended.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/arm/exynos4210.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
>> index 99b05a175d6..75d6e4d1ab9 100644
>> --- a/hw/arm/exynos4210.c
>> +++ b/hw/arm/exynos4210.c
>> @@ -103,6 +103,14 @@
>>   #define EXYNOS4210_PL330_BASE1_ADDR         0x12690000
>>   #define EXYNOS4210_PL330_BASE2_ADDR         0x12850000
>>
>> +/*
>> + * The Cortex-A9MP may have anything from 0 to 224 external interrupt
>> + * IRQ lines (with another 32 internal). We default to 64+32, which
>> + * is the number provided by the Cortex-A9MP test chip in the
>> + * Realview PBX-A9 and Versatile Express A9 development boards.
>> + */
> 
> This isn't the A9MP test chip or the vexpress or realview
> board, though. We should be setting this to whatever the
> exynos4210's GIC is actually configured with, or else saying
> we don't know what the right value is but this is the
> one QEMU has always used (i.e. probably a TODO comment). Those
> Arm devboards are irrelevant either way.

Historically this is how this board ended using this value though.

I'll reword the comment appropriately.

Regards,

Phil.

> 
>> +#define GIC_EXT_IRQS 64
diff mbox series

Patch

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 99b05a175d6..75d6e4d1ab9 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -103,6 +103,14 @@ 
 #define EXYNOS4210_PL330_BASE1_ADDR         0x12690000
 #define EXYNOS4210_PL330_BASE2_ADDR         0x12850000
 
+/*
+ * The Cortex-A9MP may have anything from 0 to 224 external interrupt
+ * IRQ lines (with another 32 internal). We default to 64+32, which
+ * is the number provided by the Cortex-A9MP test chip in the
+ * Realview PBX-A9 and Versatile Express A9 development boards.
+ */
+#define GIC_EXT_IRQS 64
+
 enum ExtGicId {
     EXT_GIC_ID_MDMA_LCD0 = 66,
     EXT_GIC_ID_PDMA0,
@@ -588,6 +596,8 @@  static void exynos4210_realize(DeviceState *socdev, Error **errp)
 
     /* Private memory region and Internal GIC */
     qdev_prop_set_uint32(DEVICE(&s->a9mpcore), "num-cpu", EXYNOS4210_NCPUS);
+    qdev_prop_set_uint32(DEVICE(&s->a9mpcore), "num-irq",
+                         GIC_EXT_IRQS + GIC_INTERNAL);
     busdev = SYS_BUS_DEVICE(&s->a9mpcore);
     sysbus_realize(busdev, &error_fatal);
     sysbus_mmio_map(busdev, 0, EXYNOS4210_SMP_PRIVATE_BASE_ADDR);