lkml.org 
[lkml]   [2018]   [Mar]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] irqchip: gic-v3-its: ensure nr_ites >= nr_lpis
From
Date
On 06/03/18 15:51, Ard Biesheuvel wrote:
> When struct its_device instances are created, the nr_ites member
> will be set to a power of 2 that equals or exceeds the requested
> number of MSIs passed to the msi_prepare() callback. At the same
> time, the LPI map is allocated to be some multiple of 32 in size,
> where the allocated size may be less than the requested size
> depending on whether a contiguous range of sufficient size is
> available in the global LPI bitmap.
>
> This may result in the situation where the nr_ites < nr_lpis, and
> since nr_ites is what we program into the hardware when we map the
> device, the additional LPIs will be non-functional.
>
> For bog standard hardware, this does not really matter. However,
> in cases where ITS device IDs are shared between different PCIe
> devices, we may end up allocating these additional LPIs without
> taking into account that they don't actually work.
>
> So let's make nr_ites at least 32. This ensures that all allocated
> LPIs are 'live', and that its_alloc_device_irq() will fail when
> attempts are made to allocate MSIs beyond what was allocated in
> the first place.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> drivers/irqchip/irq-gic-v3-its.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 1d3056f53747..ee488c468400 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -1412,7 +1412,7 @@ static struct irq_chip its_irq_chip = {
> * This gives us (((1UL << id_bits) - 8192) >> 5) possible allocations.
> */
> #define IRQS_PER_CHUNK_SHIFT 5
> -#define IRQS_PER_CHUNK (1 << IRQS_PER_CHUNK_SHIFT)
> +#define IRQS_PER_CHUNK (1UL << IRQS_PER_CHUNK_SHIFT)
> #define ITS_MAX_LPI_NRBITS 16 /* 64K LPIs */
>
> static unsigned long *lpi_bitmap;
> @@ -2123,7 +2123,7 @@ static struct its_device *its_create_device(struct its_node *its, u32 dev_id,
> * of two entries. No, the architecture doesn't let you
> * express an ITT with a single entry.

This comment feels slightly out of place now.

> */
> - nr_ites = max(2UL, roundup_pow_of_two(nvecs));
> + nr_ites = max(IRQS_PER_CHUNK, roundup_pow_of_two(nvecs));
> sz = nr_ites * its->ite_size;
> sz = max(sz, ITS_ITT_ALIGN) + ITS_ITT_ALIGN - 1;
> itt = kzalloc(sz, GFP_KERNEL);
>

Otherwise, this looks good to me. I'll amend the comment when applying
the patch, no need to respin.

Thanks,

M.
--
Jazz is not dead. It just smells funny...

\
 
 \ /
  Last update: 2018-03-06 17:02    [W:0.032 / U:0.600 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site