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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

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 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