Re: [PATCH] arm64: kvm: Make nvhe stack size configurable

[Kalesh Singh]

On Sun, Nov 10, 2024 at 3:06 AM Marc Zyngier <maz@xxxxxxxxxx> wrote:
>
> [that's an impressive Cc list...]
>
> On Fri, 08 Nov 2024 21:14:00 +0000,
> Kalesh Singh <kaleshsingh@xxxxxxxxxx> wrote:
> >
> > In order to make the nVHE stack size easily configurable,
> > introduce NVHE_STACK_SHIFT which must be >= PAGE_SHIFT.
> >
> > The default stack size remains 1 page (no functional change)
> >
> > Downstream vendor features which require a larger stack
> > can configure it by setting CONFIG_NVHE_STACK_SHIFT.
>
> We don't let make the stack size configurable for the rest of the
> kernel, do we? Why should a tiny portion be treated differently?
>
> Making this configurable means that testing is harder, and bugs harder
> to reproduce. It also seems specially designed to allow badly written
> code to run in hypervisor context. And once you allow two pages to be
> used (up to 128kB), what prevents the "downstream vendor" to push this
> to 4 or 8?
>
> If anything, I would prefer to see this (obviously out of tree) code
> isolated with its own stack instead of growing EL2's private stack. At
> least this would make the problems attributable to the guilty party.

Hi Marc,

Thanks for the comments.

I tried to be conservative with the configuration only allowing only
up to 2 pages. Your points are fair concerns, I can see how this still
may not be ideal for upstream. However, I think introducing
NVHE_STACK_SIZE as a refactor still has value, similar to how
OVERFLOW_STACK_SIZE is handled. If there are no objections, I’ll
resend a v2 of the patch, dropping the configuration parts and
focusing on the refactor to introduce NVHE_STACK_SIZE.

--Kalesh
>
> Thanks,
>
>         M.
>
> --
> Without deviation from the norm, progress is not possible.