diff mbox

[Xen-devel] xen/arm: gic: GICv2 & GICv3 only supports 1020 physical interrupts

Message ID 1425400555-14725-1-git-send-email-julien.grall@linaro.org
State New
Headers show

Commit Message

Julien Grall March 3, 2015, 4:35 p.m. UTC 
GICD_TYPER.ITLinesNumber can encode up to 1024 interrupts. Although,
IRQ 1020-1023 are reserved for special purpose.

This helps to check when an IRQ is valid or not.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/gic-v2.c | 16 ++++++++++------
 xen/arch/arm/gic-v3.c | 16 ++++++++++------
 2 files changed, 20 insertions(+), 12 deletions(-)

Comments

Julien Grall March 9, 2015, noon UTC | #1 
Hi Ian,

On 05/03/2015 19:00, Ian Campbell wrote:
> On Tue, 2015-03-03 at 16:35 +0000, Julien Grall wrote:
>> +    gicv3_info.nr_lines = min((unsigned)1020, nr_lines);
>
> "1020U" is the correct way to write (unsigned)1020 I think (in both
> places).

I gave a look on several usage of min in arch/arm and (unsigned) was used.

Anyway, I guess 1020U is fine. I will use it.

>
> Otherwise looks ok, although I had to look twice to figure out that the
> register initialisation was the same afterwards.
>
> Where does this value get used? Can you spell it out in the commit log
> please.

This value is used to check if we can route the IRQ to Xen/a Guest.

For instance, using IRQ 1023-1024 in the LRs is unpredictable. So we 
have to catch it earlier. Even though, the hardware domain should not 
have the permission to control it.

> In particular are you sure that there are no usages which assume this is
> a multiple of 32?

The only place where we require a multiple of 32 is the vGIC emulation. 
Although, I have a patch with use DIV_ROUND_UP but I forgot to send with 
this patch.

I will resend both together.

Regards,
Julien Grall March 10, 2015, 11:46 a.m. UTC | #2 
Hi Ian,

On 09/03/15 16:06, Ian Campbell wrote:
> On Mon, 2015-03-09 at 14:00 +0200, Julien Grall wrote:
>> Hi Ian,
>>
>> On 05/03/2015 19:00, Ian Campbell wrote:
>>> On Tue, 2015-03-03 at 16:35 +0000, Julien Grall wrote:
>>>> +    gicv3_info.nr_lines = min((unsigned)1020, nr_lines);
>>>
>>> "1020U" is the correct way to write (unsigned)1020 I think (in both
>>> places).
>>
>> I gave a look on several usage of min in arch/arm and (unsigned) was used.
> 
> I don't see any with a literal number, which is the main point.
> (unsigned)PAGE_SIZE is fine, because you can't write PAGE_SIZEU very
> easily. (Leaving aside whether PAGE_SIZE should be unsigned in its
> definition.

Hmmm ... right. I will update the patch.

Regards,
diff mbox

Patch

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 20cdbc9..826a457 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -256,6 +256,7 @@  static void __init gicv2_dist_init(void)
     uint32_t type;
     uint32_t cpumask;
     uint32_t gic_cpus;
+    unsigned int nr_lines;
     int i;
 
     cpumask = readl_gicd(GICD_ITARGETSR) & 0xff;
@@ -266,31 +267,34 @@  static void __init gicv2_dist_init(void)
     writel_gicd(0, GICD_CTLR);
 
     type = readl_gicd(GICD_TYPER);
-    gicv2_info.nr_lines = 32 * ((type & GICD_TYPE_LINES) + 1);
+    nr_lines = 32 * ((type & GICD_TYPE_LINES) + 1);
     gic_cpus = 1 + ((type & GICD_TYPE_CPUS) >> 5);
     printk("GICv2: %d lines, %d cpu%s%s (IID %8.8x).\n",
-           gicv2_info.nr_lines, gic_cpus, (gic_cpus == 1) ? "" : "s",
+           nr_lines, gic_cpus, (gic_cpus == 1) ? "" : "s",
            (type & GICD_TYPE_SEC) ? ", secure" : "",
            readl_gicd(GICD_IIDR));
 
     /* Default all global IRQs to level, active low */
-    for ( i = 32; i < gicv2_info.nr_lines; i += 16 )
+    for ( i = 32; i < nr_lines; i += 16 )
         writel_gicd(0x0, GICD_ICFGR + (i / 16) * 4);
 
     /* Route all global IRQs to this CPU */
-    for ( i = 32; i < gicv2_info.nr_lines; i += 4 )
+    for ( i = 32; i < nr_lines; i += 4 )
         writel_gicd(cpumask, GICD_ITARGETSR + (i / 4) * 4);
 
     /* Default priority for global interrupts */
-    for ( i = 32; i < gicv2_info.nr_lines; i += 4 )
+    for ( i = 32; i < nr_lines; i += 4 )
         writel_gicd(GIC_PRI_IRQ << 24 | GIC_PRI_IRQ << 16 |
                     GIC_PRI_IRQ << 8 | GIC_PRI_IRQ,
                     GICD_IPRIORITYR + (i / 4) * 4);
 
     /* Disable all global interrupts */
-    for ( i = 32; i < gicv2_info.nr_lines; i += 32 )
+    for ( i = 32; i < nr_lines; i += 32 )
         writel_gicd(~0x0, GICD_ICENABLER + (i / 32) * 4);
 
+    /* Only 1020 interrupts are supported */
+    gicv2_info.nr_lines = min((unsigned)1020, nr_lines);
+
     /* Turn on the distributor */
     writel_gicd(GICD_CTL_ENABLE, GICD_CTLR);
 }
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index ab80670..99a3b9a 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -528,23 +528,24 @@  static void __init gicv3_dist_init(void)
     uint32_t type;
     uint32_t priority;
     uint64_t affinity;
+    unsigned int nr_lines;
     int i;
 
     /* Disable the distributor */
     writel_relaxed(0, GICD + GICD_CTLR);
 
     type = readl_relaxed(GICD + GICD_TYPER);
-    gicv3_info.nr_lines = 32 * ((type & GICD_TYPE_LINES) + 1);
+    nr_lines = 32 * ((type & GICD_TYPE_LINES) + 1);
 
     printk("GICv3: %d lines, (IID %8.8x).\n",
-           gicv3_info.nr_lines, readl_relaxed(GICD + GICD_IIDR));
+           nr_lines, readl_relaxed(GICD + GICD_IIDR));
 
     /* Default all global IRQs to level, active low */
-    for ( i = NR_GIC_LOCAL_IRQS; i < gicv3_info.nr_lines; i += 16 )
+    for ( i = NR_GIC_LOCAL_IRQS; i < nr_lines; i += 16 )
         writel_relaxed(0, GICD + GICD_ICFGR + (i / 16) * 4);
 
     /* Default priority for global interrupts */
-    for ( i = NR_GIC_LOCAL_IRQS; i < gicv3_info.nr_lines; i += 4 )
+    for ( i = NR_GIC_LOCAL_IRQS; i < nr_lines; i += 4 )
     {
         priority = (GIC_PRI_IRQ << 24 | GIC_PRI_IRQ << 16 |
                     GIC_PRI_IRQ << 8 | GIC_PRI_IRQ);
@@ -552,7 +553,7 @@  static void __init gicv3_dist_init(void)
     }
 
     /* Disable all global interrupts */
-    for ( i = NR_GIC_LOCAL_IRQS; i < gicv3_info.nr_lines; i += 32 )
+    for ( i = NR_GIC_LOCAL_IRQS; i < nr_lines; i += 32 )
         writel_relaxed(0xffffffff, GICD + GICD_ICENABLER + (i / 32) * 4);
 
     gicv3_dist_wait_for_rwp();
@@ -566,8 +567,11 @@  static void __init gicv3_dist_init(void)
     /* Make sure we don't broadcast the interrupt */
     affinity &= ~GICD_IROUTER_SPI_MODE_ANY;
 
-    for ( i = NR_GIC_LOCAL_IRQS; i < gicv3_info.nr_lines; i++ )
+    for ( i = NR_GIC_LOCAL_IRQS; i < nr_lines; i++ )
         writeq_relaxed(affinity, GICD + GICD_IROUTER + i * 8);
+
+    /* Only 1020 interrupts are supported */
+    gicv3_info.nr_lines = min((unsigned)1020, nr_lines);
 }
 
 static int gicv3_enable_redist(void)