Re: [Xen-devel] [PATCH] xen: get GIC addresses from DT

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2012-11-23 at 15:21 +0000, Stefano Stabellini wrote:
> Get the address of the GIC distributor, cpu, virtual and virtual cpu
> interfaces registers from device tree.
> 
> 
> Note: I couldn't completely get rid of GIC_BASE_ADDRESS, GIC_DR_OFFSET
> and friends because we are using them from mode_switch.S, that is
> executed before device tree has been parsed. But at least mode_switch.S
> is known to contain vexpress specific code anyway.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>

I've a few comments on mostly pre-existing things which I happened to
notice while reviewing this:

> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 0c6fab9..8efbeb3 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -34,9 +35,9 @@
>  /* Access to the GIC Distributor registers through the fixmap */
>  #define GICD ((volatile uint32_t *) FIXMAP_ADDR(FIXMAP_GICD))

There's an implicit assumption here that the GICD is correctly aligned
-- I suspect it really ought to be handled like the others... but...

>  #define GICC ((volatile uint32_t *) (FIXMAP_ADDR(FIXMAP_GICC1)  \
> -                                     + (GIC_CR_OFFSET & 0xfff)))
> +                                     + ((uint32_t) gic.cbase & 0xfff)))
>  #define GICH ((volatile uint32_t *) (FIXMAP_ADDR(FIXMAP_GICH)  \
> -                                     + (GIC_HR_OFFSET & 0xfff)))
> +                                     + ((uint32_t) gic.hbase & 0xfff)))

... is there actually any chance of these being non-page aligned?

Also now that these are dynamic perhaps an actual variable initialised
at start of day would be better than a #define?

Ian.


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