Re: [PATCH 09/12] KVM: arm64: Add helper for swapping guest descriptor

[Oliver Upton <oupton@kernel.org>]

On Mon, Nov 17, 2025 at 02:14:00PM +0000, Marc Zyngier wrote:
> On Wed, 12 Nov 2025 18:34:03 +0000,
> Oliver Upton <oupton@kernel.org> wrote:
> > +static int __llsc_swap_desc(u64 __user *ptep, u64 old, u64 new)
> > +{
> > +	unsigned int loops = 128;
> > +	u64 tmp;
> > +	int ret;
> > +
> > +	uaccess_enable_privileged();
> > +
> > +	asm volatile("prfm	pstl1strm, %[addr]\n"
> > +		     "1: ldxr	%[tmp], %[addr]\n"
> > +		     "sub	%[tmp], %[tmp], %[old]\n"
> > +		     "cbnz	%[tmp], 3f\n"
> > +		     "2: stlxr	%w[ret], %[new], %[addr]\n"
> > +		     "cbz	%w[ret], 4f\n"
> > +		     "sub	%w[loops], %w[loops], #1\n"
> > +		     "cbnz	%w[loops], 1b\n"
> > +		     "3: mov	%w[ret], %w[eagain]\n"
> > +		     "4:\n"
> > +		     : [ret] "=r" (ret), [addr] "+Q" (*ptep), [tmp] "=&r" (tmp),
> > +		       [loops] "+r" (loops)
> > +		     : [old] "r" (old), [new] "r" (new), [eagain] "Ir" (-EAGAIN)
> > +		     : "memory");
> 
> Why doesn't this need an exception table as well? I'd expect it to
> fault just as much as the LSE version (and ret cannot report -EFAULT,
> for example).

I just threw in ${something} to satisfy the LL/SC case, so yes, this
needs exception fixup as well.

> I'm also on the fence about the bounded loop. Yes, forward progress is
> a problem, but it should only affect large systems which can readily
> use the atomic instructions. I'd rather we get rid of it until proven
> that we really need something like it.

From a letter of the architecture POV I'm not sure we even need the
LL/SC implementation. As of now the only non-nested use case is the SEA
TTW walker which doesn't set the access flag (nor should it) and any
implementation that has FEAT_NV must also have FEAT_LSE.

I guess that just leaves the recursive NV case where a VMM explicitly
de-features FEAT_LSE. So I guess having a trivial, non-looping
implementation would be enough to cover this pointless issue.

Thanks,
Oliver