From: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
To: sashiko-reviews@lists.linux.dev
Cc: kvm@vger.kernel.org
Subject: Re: [RFC v2 3/5] KVM: PPC: selftests: add support for powerpc
In-Reply-To: <20260523123023.64A321F000E9@smtp.kernel.org>
Date: Wed, 27 May 2026 16:46:04 +0530
Message-ID: <se7d6r2j.ritesh.list@gmail.com>
References: <6e0c74f1273a4baf652d84820ae83696e77263d9.1779524962.git.ritesh.list@gmail.com> <20260523123023.64A321F000E9@smtp.kernel.org>
Precedence: bulk

sashiko-bot@kernel.org writes:

>> diff --git a/tools/testing/selftests/kvm/lib/powerpc/handlers.S b/tools/testing/selftests/kvm/lib/powerpc/handlers.S
>> new file mode 100644
>> --- /dev/null
>> +++ b/tools/testing/selftests/kvm/lib/powerpc/handlers.S
> [ ... ]
>> +virt_handle_interrupt:
>> +	stdu	%r1,-INT_FRAME_SIZE(%r1)
>> +	mr	%r3,%r31
>> +	bl	route_interrupt
>> +	ld	%r4,(32*8)(%r31) /* NIA */
>> +	ld	%r5,(33*8)(%r31) /* MSR */
>> +	ld	%r6,(35*8)(%r31) /* LR */
>> +	ld	%r7,(36*8)(%r31) /* CTR */
>> +	ld	%r8,(37*8)(%r31) /* XER */
>> +	lwz	%r9,(38*8)(%r31) /* CR */
>> +	mtspr	SPR_SRR0,%r4
>> +	mtspr	SPR_SRR1,%r5
>> +	mtlr	%r6
>> +	mtctr	%r7
>> +	mtxer	%r8
>> +	mtcr	%r9
>> +reg=4
>> +	ld	%r0,(0*8)(%r31)
>> +	ld	%r3,(3*8)(%r31)
>> +.rept 28
>> +	ld	reg,(reg*8)(%r31)
>> +	reg=reg+1
>> +.endr
>
> [Severity: Medium]
> Are r1 (stack pointer) and r2 (TOC pointer) missing from the restore sequence?
>
> Since they aren't restored here, tests cannot modify the stack pointer during
> exception handling.
>

The guest exception handling carefully create and destroy the stack
frame during exception handling for running C code.
Also LIBKVM is directly linked in as object files directly into each test
binary, so the call to rout_interrupt() is the same TOC call.

So it's not missing, but we really don't need save/restore of r1 & r2 here.

>
> [Severity: Medium]
> Is it possible for tests to map memory without write or execute permissions
> (e.g., read-only or non-executable pages)?
>
> If so, this assertion will falsely trigger and abort the test during virtual
> to physical address translation. Should this check only verify PTE_VALID and
> PTE_LEAF to support testing diverse page permissions?
>

Yup, that is something which can be relaxed I guess. Will do that in v3.

-ritesh