Re: [Xen-devel] [PATCH v5 09/30] ARM: GICv3 ITS: introduce host LPI array

[Julien Grall <julien.grall@xxxxxxx>]

Hi Andre,

On 06/04/17 11:31, Andre Przywara wrote:
> On 06/04/17 00:37, Stefano Stabellini wrote:
> > On Thu, 6 Apr 2017, Andre Przywara wrote:
> > > The number of LPIs on a host can be potentially huge (millions),
> > > although in practise will be mostly reasonable. So prematurely allocating
> > > an array of struct irq_desc's for each LPI is not an option.
> > > However Xen itself does not care about LPIs, as every LPI will be injected
> > > into a guest (Dom0 for now).
> > > Create a dense data structure (8 Bytes) for each LPI which holds just
> > > enough information to determine the virtual IRQ number and the VCPU into
> > > which the LPI needs to be injected.
> > > Also to not artificially limit the number of LPIs, we create a 2-level
> > > table for holding those structures.
> > > This patch introduces functions to initialize these tables and to
> > > create, lookup and destroy entries for a given LPI.
> > > By using the naturally atomic access guarantee the native uint64_t data
> > > type gives us, we allocate and access LPI information in a way that does
> > > not require a lock.
> > >
> > > Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> > > ---
> > >  xen/arch/arm/gic-v3-its.c        |  60 +++++++++++
> > >  xen/arch/arm/gic-v3-lpi.c        | 227 +++++++++++++++++++++++++++++++++++++++
> > >  xen/include/asm-arm/gic_v3_its.h |   6 ++
> > >  xen/include/asm-arm/irq.h        |   8 ++
> > >  4 files changed, 301 insertions(+)
> > >
> > > diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> > > index 1ecd63b..eb47c9d 100644
> > > --- a/xen/arch/arm/gic-v3-its.c
> > > +++ b/xen/arch/arm/gic-v3-its.c
> [ ... ]
>
> > > +
> > > +void gicv3_free_host_lpi_block(uint32_t first_lpi)
> > > +{
> > > +    union host_lpi *hlpi, empty_lpi = { .dom_id = DOMID_INVALID };
> > > +    int i;
> > > +
> > > +    hlpi = gic_get_host_lpi(first_lpi);
> > > +    if ( !hlpi )
> > > +        return;         /* Nothing to free here. */
> > We should check that first_lpi is actually the first lpi in a block
> > before calling gic_get_host_lpi.
> Are you thinking about an:
>         ASSERT((first_lpi % LPI_BLOCK) == 0);
> here? Or even a BUG_ON?
>
> Technically the only two callers of gicv3_free_host_lpi_block() always
> take this value from the host_lpi_blocks array, which should be safe.
Well, maybe today. But without documentation someone could misuse this 
function in the future...
The check would be here for safety

Cheers,

--
Julien Grall

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