Re: [Xen-devel] [PATCH v2 04/27] ARM: GICv3 ITS: map ITS command buffer

[Julien Grall <julien.grall@xxxxxxx>]

Hi Andre,

On 22/03/17 16:31, André Przywara wrote:
> On 22/03/17 15:23, Julien Grall wrote:
> > On 16/03/17 11:20, Andre Przywara wrote:
> > > +
> > >  LIST_HEAD(host_its_list);
> > >
> > >  bool gicv3_its_host_has_its(void)
> > > @@ -56,6 +59,55 @@ static uint64_t encode_propbaser_phys_addr(paddr_t
> > > addr, unsigned int page_bits)
> > >      return ret | ((addr & GENMASK(51, 48)) >> (48 - 12));
> > >  }
> > >
> > > +static void *its_map_cbaser(struct host_its *its)
> > > +{
> > > +    void __iomem *cbasereg = its->its_base + GITS_CBASER;
> > > +    uint64_t reg;
> > > +    void *buffer;
> > > +    unsigned int order;
> > > +
> > > +    reg  = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT;
> > > +    reg |= GIC_BASER_CACHE_SameAsInner <<
> > > GITS_BASER_OUTER_CACHEABILITY_SHIFT;
> > > +    reg |= GIC_BASER_CACHE_RaWaWb <<
> > > GITS_BASER_INNER_CACHEABILITY_SHIFT;
> > > +
> > > +    /* The ITS command buffer needs to be 64K aligned. */
> > Looking at the spec, the command buffer does not need to be 64K aligned.
> > On the previous version, you made it 4K aligned. So why this restriction?
> As you have already learnt, the GIC is more subtle sometimes ;-) Read
> the description at "Physical_Address, bits [51:12]" in the CBASER
> paragraph and tell me what you take from it. I decided to read it as
> "has to be 64K aligned".
> Happy to correct this otherwise.
Hmmm. I think you are right. This is a little bit weird, but fine. Lets 
keep like that.
> > > +    order = max(get_order_from_bytes(ITS_CMD_QUEUE_SZ), 16U -
> > > PAGE_SHIFT);
> > > +    buffer = alloc_xenheap_pages(order, 0);
> > I am not sure to understand why you move from _zalloc to
> > alloc_xenheap_*. The resulting behavior will be exactly the same but the
> > former result to a simpler code.
>                                                   ^^^^^^^^^^^^^^^^
> Really?
> I checked and found that alloc_xenheap_pages gives me physically
> contiguous allocation in the given order, where *alloc just guarantees
> virtually contiguous pages.
Yes. Give a look at my answer on patch #3.

Regards,

--
Julien Grall

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