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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2012-11-30 at 14:02 +0000, Stefano Stabellini wrote:
> On Fri, 30 Nov 2012, Stefano Stabellini wrote:
> > On Fri, 30 Nov 2012, Ian Campbell wrote:
> > > On Fri, 2012-11-30 at 12:25 +0000, Stefano Stabellini wrote:
> > > > 
> > > > 
> > > > > > +    if ( interfaces < 4 )
> > > > > > +    {
> > > > > > +        early_printk("fdt: node `%s': invalid `reg' property\n",
> > > > name);
> > > > > > +        return;
> > > > > > +    }
> > > > > > +    device_tree_get_reg(&cell, address_cells, size_cells, &start,
> > > > &size);
> > > > > > +    early_info.gic.gic_dist_addr = start;
> > > > > > +    device_tree_get_reg(&cell, address_cells, size_cells, &start,
> > > > &size);
> > > > > > +    early_info.gic.gic_cpu_addr = start;
> > > > > > +    device_tree_get_reg(&cell, address_cells, size_cells, &start,
> > > > &size);
> > > > > > +    early_info.gic.gic_hyp_addr = start;
> > > > > > +    device_tree_get_reg(&cell, address_cells, size_cells, &start,
> > > > &size);
> > > > > > +    early_info.gic.gic_vcpu_addr = start;
> > > > > 
> > > > > Is the GIC driver still hardcoding the register region sizes?  Or
> > > > does
> > > > > it not need the size?
> > > > 
> > > > Yes, it is. However we do know the size of all the GIC interfaces
> > > > because they are specified in the GIC docs.
> > > 
> > > Perhaps we ought to check that the size given meets (either == or >=)
> > > our expectations?
> > 
> > OK, in that case I am going to merge the patch with the checks with the
> > original
> 
> Actually I found an issue in the GIC DT specs:
> 
> - reg : Specifies base physical address(s) and size of the GIC registers. The
>   first region is the GIC distributor register base and size. The 2nd region 
> is
>   the GIC cpu interface register base and size.
> 
> however this is what I get for vexpress:
> 
> reg = <0 0x2c001000 0 0x1000>,
>       <0 0x2c002000 0 0x1000>,
>       <0 0x2c004000 0 0x2000>,
>       <0 0x2c006000 0 0x2000>;
> 
> Leaving aside the last two regions that are due to the virtualization
> extensions, the size of the cpu interface is one page according to DT.
> However the GIC specs state that the GICC_DIR register is at offset
> 0x1000, so it is certainly slightly bigger than one page.

I think, and I'm not at all sure about this, that 0x1000 is GIC v1 and
the second page of GICC stuff was introduced with GIC v2.

I wonder what version the cortex a9 implemented. This could well be a
meaningful difference between compat = arm,cortex-a9-gic and
arm,cortex-a15-gic which isn't current expressed in the DT?

> I think that nobody noticed this in the Linux world because they don't
> seem to be using the GICC_DIR register at all so they can get away with
> a cpu interface of just one page.
> 
> So I think we should not check for sizes :)



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