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

[Andre Przywara <andre.przywara@xxxxxxx>]

Hi,

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.

Cheers,
Andre.

>> +    spin_lock(&lpi_data.host_lpis_lock);
>> +
>> +    for ( i = 0; i < LPI_BLOCK; i++ )
>> +        write_u64_atomic(&hlpi[i].data, empty_lpi.data);
>> +
>> +    /*
>> +     * Make sure the next allocation can reuse this block, as we do only
>> +     * forward scanning when finding an unused block.
>> +     */
>> +    if ( lpi_data.next_free_lpi > first_lpi )
>> +        lpi_data.next_free_lpi = first_lpi;
>> +
>> +    spin_unlock(&lpi_data.host_lpis_lock);
>> +
>> +    return;
>> +}
>> +

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