Re: [Xen-devel] [PATCH v7 1/5] xen/arm: Add support for GIC v3

[Vijay Kilari <vijay.kilari@xxxxxxxxx>]

On Wed, Jul 16, 2014 at 5:11 PM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> On Fri, 2014-07-11 at 18:21 +0530, vijay.kilari@xxxxxxxxx wrote:
>> +static inline void gicv3_restore_lrs(int nr_lrs, const struct vcpu *v)
>> +static u16 gicv3_compute_target_list(int *base_cpu, const struct cpumask 
>> *mask,
>> +                                     uint64_t cluster_id)
>> +{
>> +    int cpu = *base_cpu;
>> +    uint64_t mpidr = cpu_logical_map(cpu);
>> +    u16 tlist = 0;
>> +
>> +    while ( cpu < nr_cpu_ids )
>> +    {
>> +        /*
>> +         * If we ever get a cluster of more than 16 CPUs, just
>> +         * scream and skip that CPU.
>> +         */
>> +        if ( (mpidr & 0xff) >= 16 )
>
> MPIPDR_EFF0_MASK if that's what this is, and at least once more in this
> function.
>
>> +        {
>> +            dprintk(XENLOG_WARNING, "GICv3:Cluster with more than 16's 
>> cpus\n");
>> +            goto out;
>
> Please validate this and complain in gicv3_init not every time we send
> an SGI.
>

   We can move this check to gicv3_populate_rdist() which is called
for each cpu.
However we will miss check on mpidr value. But being MPIDR value which does
not change it should be ok to remove this check

>> +/*
>> + * Additional registers defined in GIC v3.
>> + * Common GICD registers are defined in gic.h
>> + */
>> +
>> +#define GICD_STATUSR                 (0x010)
>> [...][
>> +#define GICV3_GICD_PIDR0             (0x92)
>
> What is the distinction between variables with GIC[DR]_ prefixes and
> those with GICV3_GIC[DR]_ ones?

GICV3 is prefixed for indicating that there are values not the addresses.
In anycase I will remove GICV3 prefixes and postfix _VAL

Regards
Vijay

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel