Re: [Xen-devel] [PATCH v2 02/27] ARM: GICv3: allocate LPI pending and property table

[Julien Grall <julien.grall@xxxxxxx>]

On 23/03/17 10:50, Andre Przywara wrote:
> Hi,
Hi Andre,

> On 21/03/17 23:27, Stefano Stabellini wrote:
> > On Tue, 21 Mar 2017, André Przywara wrote:
> > > On 21/03/17 22:57, Stefano Stabellini wrote:
> > > > On Thu, 16 Mar 2017, Andre Przywara wrote:
> > > > > The ARM GICv3 provides a new kind of interrupt called LPIs.
> > > > > The pending bits and the configuration data (priority, enable bits) for
> > > > > those LPIs are stored in tables in normal memory, which software has to
> > > > > provide to the hardware.
> > > > > Allocate the required memory, initialize it and hand it over to each
> > > > > redistributor. The maximum number of LPIs to be used can be adjusted with
> > > > > the command line option "max_lpi_bits", which defaults to a compile time
> > > > > constant exposed in Kconfig.
> > > > >
> > > > > Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> > > ....
> > >
> > > > > diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c
> > > > > new file mode 100644
> > > > > index 0000000..4f8414b
> > > > > --- /dev/null
> > > > > +++ b/xen/arch/arm/gic-v3-lpi.c
> > > ....
> > >
> > > > > +/*
> > > > > + * Tell a redistributor about the (shared) property table, allocating one
> > > > > + * if not already done.
> > > > > + */
> > > > > +static int gicv3_lpi_set_proptable(void __iomem * rdist_base)
> > > > > +{
> > > > > +    uint64_t reg;
> > > > > +
> > > > > +    reg  = GIC_BASER_CACHE_RaWaWb << GICR_PROPBASER_INNER_CACHEABILITY_SHIFT;
> > > > > +    reg |= GIC_BASER_CACHE_SameAsInner << 
> > > > > GICR_PROPBASER_OUTER_CACHEABILITY_SHIFT;
> > > > > +    reg |= GIC_BASER_InnerShareable << GICR_PROPBASER_SHAREABILITY_SHIFT;
> > > > > +
> > > > > +    /*
> > > > > +     * The property table is shared across all redistributors, so allocate
> > > > > +     * this only once, but return the same value on subsequent calls.
> > > > > +     */
> > > > > +    if ( !lpi_data.lpi_property )
> > > > > +    {
> > > > > +        /* The property table holds one byte per LPI. */
> > > > > +        unsigned int order = get_order_from_bytes(lpi_data.nr_host_lpis);
> > > > > +        void *table = alloc_xenheap_pages(order, 0);
> > > > you investigated the possibility of using _xmalloc to 64K align it?
> > > Yes, _xmalloc gives me the alignment, but AFAICT not the physical
> > > contiguous memory guarantee the table needs. If there are other ways to
> > > satisfy those requirements, I am all ears. Otherwise I'd just add a
> > > comment to motivate this.
As already said numerous times, _x*alloc will guarantee contiguous 
physical memory. The same as kzalloc.
> > OK, but the problem is that I am not sure whether alloc_xenheap_pages
> > makes any guarantees on the alignment of the allocated pages. Is it
> > possible to allocate 1<<4 pages that are not 64K aligned with it?
> My understanding of the code is that it naturally aligns the allocation
> to the order you requested: So calling it with (4, 0) would give you (4K
> << 4) = 64K alignment (because it's taken from the 64K bucket).
>
> But anyway the property table needs only to be 4K aligned, isn't it?
>
> I know that the GICv3 is amazing in this aspect: every table has
> slightly different requirements when it comes to alignment, support of
> physical address space, page sizes and so on...
alloc_xenheap_pages will guarantee allocation aligned to the (1 << 
order) * PAGE_SIZE.
Cheers,

--
Julien Grall

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