Re: [PATCH v3 13/16] xen/arm: Add support for GIC v3

[Julien Grall <julien.grall@linaro.org>]

On 04/23/2014 06:01 PM, Ian Campbell wrote:
>> +    } while ( count-- );
>> +
>> +    if ( !count )
>> +        dprintk(XENLOG_WARNING, "RWP timeout\n");
> 
> Shouldn't we panic here?

I don't think panic is the solution here. This might be use on secondary
CPU.

IHMO, we should return an error code here an handle here upper in the
call stack.

>> +static void gicv3_restore_state(struct vcpu *v)
>> +{
>> +    int i;
>> +
>> +    for ( i = 0; i < nr_lrs; i++)
>> +        gich_write_lr(i, v->arch.gic.v3.lr[i]);
> 
> I wonder if the compiler could do a better job of this using the same
> switch and fallthrough method you used for aprn regs?

Marc has written a very nice solution for this stuff. It as written an
assembly function where all registers is save decreasing the number.

When the function, it will calculate the number of regs we don't need of
LRs save. This will avoid lots of jumps.

See code below (see http://permalink.gmane.org
/gmane.linux.ports.arm.kernel/310855 for the whole patch):

+	sub	w23, w23, w22	// How many regs we have to skip
+
+	adr	x24, 1f
+	add	x24, x24, x23, lsl #2
+	br	x24
+
+1:
+	mrs	x20, ICH_LR15_EL2
+	mrs	x19, ICH_LR14_EL2
+	mrs	x18, ICH_LR13_EL2
+	mrs	x17, ICH_LR12_EL2
+	mrs	x16, ICH_LR11_EL2
+	mrs	x15, ICH_LR10_EL2
+	mrs	x14, ICH_LR9_EL2
+	mrs	x13, ICH_LR8_EL2
+	mrs	x12, ICH_LR7_EL2
+	mrs	x11, ICH_LR6_EL2
+	mrs	x10, ICH_LR5_EL2
+	mrs	x9, ICH_LR4_EL2
+	mrs	x8, ICH_LR3_EL2
+	mrs	x7, ICH_LR2_EL2
+	mrs	x6, ICH_LR1_EL2
+	mrs	x5, ICH_LR0_EL2
+

[..]

>> +                return 0;
> 
> So basically we only know the number of redistributors from device tree
> and not the mapping from RDIST to CPU, so we have to try every one?
> 
> Do you have a reference to the GIC v3 Device Tree bindings? I don't see
> them in linux/Documentation/devicetree/bindings/.

GICv3 is not upstream for now. Marc as sent the V3 recently:

http://comments.gmane.org/gmane.linux.ports.arm.kernel/316902

-- 
Julien Grall