Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 05 Jan 2023 10:59:06 +0000
Message-ID: <86k0216ydh.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Shanker Donthineni <sdonthineni@nvidia.com>
Cc:     Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>,
        James Morse <james.morse@arm.com>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] arm64: gic: increase the number of IRQ descriptors
In-Reply-To: <2a0116a8-fbd0-d866-ada0-ed50f0523f1d@nvidia.com>
References: <20230104023738.1258925-1-sdonthineni@nvidia.com>
        <86sfgq7jb3.wl-maz@kernel.org>
        <2a0116a8-fbd0-d866-ada0-ed50f0523f1d@nvidia.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/28.2
 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Wed, 04 Jan 2023 13:47:03 +0000,
Shanker Donthineni <sdonthineni@nvidia.com> wrote:
> 
> Hi Marc,
> 
> On 1/4/23 03:14, Marc Zyngier wrote:
> > External email: Use caution opening links or attachments
> > 
> > 
> > On Wed, 04 Jan 2023 02:37:38 +0000,
> > Shanker Donthineni <sdonthineni@nvidia.com> wrote:
> >> 
> >> The default value of NR_IRQS is not sufficient to support GICv4.1
> >> features and ~56K LPIs. This parameter would be too small for certain
> >> server platforms where it has many IO devices and is capable of
> >> direct injection of vSGI and vLPI features.
> >> 
> >> Currently, maximum of 64 + 8192 (IRQ_BITMAP_BITS) IRQ descriptors
> >> are allowed. The vCPU creation fails after reaching count ~400 with
> >> kvm-arm.vgic_v4_enable=1.
> >> 
> >> This patch increases NR_IRQS to 1^19 to cover 56K LPIs and 262144
> >> vSGIs (16K vPEs x 16).
> >> 
> >> Signed-off-by: Shanker Donthineni <sdonthineni@nvidia.com>
> >> ---
> >> Changes since v1:
> >>   -create from v6.2-rc1 and edit commit text
> >> 
> >>   arch/arm64/include/asm/irq.h | 4 ++++
> >>   1 file changed, 4 insertions(+)
> >> 
> >> diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
> >> index fac08e18bcd5..3fffc0b8b704 100644
> >> --- a/arch/arm64/include/asm/irq.h
> >> +++ b/arch/arm64/include/asm/irq.h
> >> @@ -4,6 +4,10 @@
> >> 
> >>   #ifndef __ASSEMBLER__
> >> 
> >> +#if defined(CONFIG_ARM_GIC_V3_ITS)
> >> +#define  NR_IRQS  (1 << 19)
> >> +#endif
> >> +
> >>   #include <asm-generic/irq.h>
> >> 
> >>   struct pt_regs;
> > 
> > Sorry, but I don't think this is an acceptable change. This is a large
> > overhead that affects *everyone*, and that will eventually be too
> > small anyway with larger systems and larger interrupt spaces.
> > 
> > A better way to address this would be to move to a more dynamic
> > allocation, converting the irqdesc rb-tree into an xarray, getting rid
> > of the bitmaps (the allocation bitmap and the resend one), and track
> > everything in the xarray.
> 
> The actual memory allocation for IRQ descriptors is still dynamic for ARM64.
> This change increases static memory for variable 'allocated_irqs' by 64KB,
> feel not a noticeable overhead.

64kB for each bitmap, so that's already 128kB (you missed the
irqs_resend bitmap). And that's for a number of IRQs that is still way
below what the GIC architecture supports today.

The architecture supports 32bit INTIDs, and that's 1GB worth of
bitmaps, only for the physical side. Add the virtual stuff for which
we create host-side descriptors, and we can go way beyond that.

So what happens next, once you exceed the arbitrary limit that only
satisfies your own use case? We will bump it up again, and again,
bloating the kernel with useless static data that *nobody* needs.
Specially not the VMs that you plan to run.

So I'm putting my foot down right now, and saying that it needs to be
fixed once and for all. The current scheme was OK for small interrupt
spaces, but it isn't fit for purpose anymore, certainly not with
things like the GICv4 architecture.

I'm happy to help with it, but I'm certainly not willing to accept any
sort of new compile-time limit.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.