[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v6a 14/17] xen/arm: calculate vgic irq rank based on register size

On 06/26/2014 06:34 AM, vijay.kilari@xxxxxxxxx wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx>
> vGIC irq rank was computed assuming the register offset is byte
> size.Use the HSR abort address size in calculating register size.
> So, with this patch following are acheived


>    (1)  In the code 'dabt.size != number' this number is always
>         BYTE/HALF_WORD/WORD/DOUBLE defined by hsr registers.
>         Instead of checking for hard coded values use HSR abort
>         address size values.
>    (2) The vgic_irq_rank also depends on the same HSR defined


>        values to calculate irq rank.
> This make vgic_irq_rank generic as it takes register

Same here.

> size as parameter to calculate irq rank instead of hard coding to
> value 2 in previous patches
> Also, output of REG_RANK_INDEX macro is modulo by 32 to make
> sure register index is always within irq rank

I though a bit more about this patch. You are using the value of
DABT_{WORD,...} for multiple things that have non-sense together.

While it's perfectly fine to use it to check dabt.size, the
vgic_rank_offset is using this value for shift. Assuming that DABT_*
will always contains the right shift is completely wrong.

You have either to explain it somewhere or define a new set of defines.

> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
> index ebc683d..22a1998 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -251,6 +251,14 @@ extern struct cpuinfo_arm cpu_data[];
>  extern u32 __cpu_logical_map[];
>  #define cpu_logical_map(cpu) __cpu_logical_map[cpu]
> +/* HSR data abort size definition */
> +enum dabt_size {
> +    DABT_BYTE        = 0,
> +    DABT_HALF_WORD   = 1,
> +    DABT_WORD        = 2,
> +    DABT_DOUBLE_WORD = 3,
> +};
> +

The enum looks like pointless here as you never use the defined type...


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.