[PATCH 0/2] KVM: s390: fix ESSA addressing exception injection in migration mode

[PATCH 0/2] KVM: s390: fix ESSA addressing exception injection in migration mode

[Gautam Gala]

ESSA emulation in migration mode silently dropped addressing exceptions
when the guest page-table walk failed, so the guest never received the
expected PGM_ADDRESSING interrupt during live migration.

Patch 1 fixes dat_perform_essa() to return -PGM_ADDRESSING on a walk
failure, and __do_essa() to inject the program interrupt for that return
value.

Patch 2 adds a regression test integrated into the existing cmma_test
selftest.

Gautam Gala (2):
  KVM: s390: fix ESSA addressing exception injection in migration mode
  KVM: s390: selftests: add ESSA migration mode PGM_ADDRESSING
    regression test

 arch/s390/kvm/gmap/dat.c                     |  5 +-
 arch/s390/kvm/s390/priv.c                    |  4 +-
 tools/testing/selftests/kvm/s390/cmma_test.c | 79 ++++++++++++++++++++
 3 files changed, 86 insertions(+), 2 deletions(-)

-- 
2.55.0

[PATCH 1/2] KVM: s390: fix ESSA addressing exception injection in migration mode

[Gautam Gala]

__do_essa() treated any negative return from dat_perform_essa() as the
normal "no CBRL entry appended" case and returned 0, so a page-table
walk failure silently dropped the addressing exception.

DAT_WALK_ANY is required in the dat_entry_walk() call so that a GFN
backed by a large page (segment or region entry) does not return an
error: the walk stops early but succeeds, leaving ptep as NULL, which is
already handled by the existing "set exception state, return -1" path.
Without DAT_WALK_ANY such addresses would fall through to the new error
path and incorrectly inject PGM_ADDRESSING.

Return -PGM_ADDRESSING on a walk failure and inject the program
interrupt in __do_essa() for the -PGM_ADDRESSING return value.

Fixes: e38c884df921 ("KVM: s390: Switch to new gmap")
Reported-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Suggested-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Signed-off-by: Gautam Gala <ggala@linux.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@linux.ibm.com>
---
 arch/s390/kvm/gmap/dat.c  | 5 ++++-
 arch/s390/kvm/s390/priv.c | 4 +++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/s390/kvm/gmap/dat.c b/arch/s390/kvm/gmap/dat.c
index ff80d02c9f56..1bc6d2ec5b08 100644
--- a/arch/s390/kvm/gmap/dat.c
+++ b/arch/s390/kvm/gmap/dat.c
@@ -1084,6 +1084,7 @@ int dat_set_prefix_notif_bit(union asce asce, gfn_t gfn)
  * * %1 if the page state has been altered and the page is to be added to the CBRL
  * * %0 if the page state has been altered, but the page is not to be added to the CBRL
  * * %-1 if the page state has not been altered and the page is not to be added to the CBRL
+ * * %-PGM_ADDRESSING if the guest page-table walk failed
  */
 int dat_perform_essa(union asce asce, gfn_t gfn, int orc, union essa_state *state, bool *dirty)
 {
@@ -1092,7 +1093,9 @@ int dat_perform_essa(union asce asce, gfn_t gfn, int orc, union essa_state *stat
 	union pte *ptep;
 	int res = 0;
 
-	if (dat_entry_walk(NULL, gfn, asce, 0, TABLE_TYPE_PAGE_TABLE, &crstep, &ptep)) {
+	if (dat_entry_walk(NULL, gfn, asce, DAT_WALK_ANY, TABLE_TYPE_PAGE_TABLE, &crstep, &ptep))
+		return -PGM_ADDRESSING;
+	if (!ptep) {
 		*state = (union essa_state) { .exception = 1 };
 		return -1;
 	}
diff --git a/arch/s390/kvm/s390/priv.c b/arch/s390/kvm/s390/priv.c
index b3cb2c2c3aa7..2abd1bdfdf32 100644
--- a/arch/s390/kvm/s390/priv.c
+++ b/arch/s390/kvm/s390/priv.c
@@ -1168,8 +1168,10 @@ static inline int __do_essa(struct kvm_vcpu *vcpu, const int orc)
 	entries = (vcpu->arch.sie_block->cbrlo & ~PAGE_MASK) >> 3;
 
 	nappended = dat_perform_essa(vcpu->arch.gmap->asce, gfn, orc, &state, &dirtied);
+	if (nappended == -PGM_ADDRESSING)
+		return kvm_s390_inject_program_int(vcpu, -PGM_ADDRESSING);
 	vcpu->run->s.regs.gprs[r1] = state.val;
-	if (nappended < 0)
+	if (nappended == -1)
 		return 0;
 	/*
 	 * It is possible that all the normal 511 slots were full, in which case
-- 
2.55.0

[PATCH 2/2] KVM: s390: selftests: add ESSA migration mode PGM_ADDRESSING regression test

[Gautam Gala]

Regression test for the bug where ESSA emulation in migration mode
silently dropped addressing exceptions instead of injecting
PGM_ADDRESSING into the guest.

The test is integrated into cmma_test alongside the existing CMMA
migration tests.

Signed-off-by: Gautam Gala <ggala@linux.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@linux.ibm.com>
---
 tools/testing/selftests/kvm/s390/cmma_test.c | 79 ++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/tools/testing/selftests/kvm/s390/cmma_test.c b/tools/testing/selftests/kvm/s390/cmma_test.c
index 15d81b2ed7ad..2b351b4fc894 100644
--- a/tools/testing/selftests/kvm/s390/cmma_test.c
+++ b/tools/testing/selftests/kvm/s390/cmma_test.c
@@ -8,6 +8,7 @@
  *  Nico Boehr <nrb@linux.ibm.com>
  */
 #include <fcntl.h>
+#include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -18,6 +19,12 @@
 #include "kselftest.h"
 #include "ucall_common.h"
 #include "processor.h"
+#include "sie.h"
+
+#define PGM_ADDRESSING		0x05
+#define VCPU_RUN_TIMEOUT_S	2
+#define UNMAPPED_GFN		0xffff000UL
+#define __LC_PGM_NEW_PSW	0x1d0
 
 #define MAIN_PAGE_COUNT 512
 
@@ -651,6 +658,77 @@ static void test_get_skip_holes(void)
 	assert_no_pages_cmma_dirty(vm);
 }
 
+static void pgm_handler_c(void)
+{
+	GUEST_SYNC(PGM_ADDRESSING);
+	for (;;)
+		;
+}
+
+static void guest_essa_unmapped(void)
+{
+	unsigned long addr = UNMAPPED_GFN << PAGE_SHIFT;
+
+	asm volatile(
+		"	lg   2,%[addr]\n"
+		/* ESSA r1,r2,orc=1 */
+		"	.insn rrf,0xb9ab0000,1,2,1,0\n"
+		"0:	j 0b\n"
+		:
+		: [addr] "Q"(addr)
+		: "r1", "r2", "cc", "memory"
+	);
+}
+
+static void handle_alarm(int sig __maybe_unused)
+{
+	ksft_test_result_fail(
+		"ESSA migration mode: vcpu_run timed out - PGM_ADDRESSING was not injected\n");
+	ksft_finished();
+}
+
+static void test_essa_migration_pgm(void)
+{
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+	struct ucall uc;
+	char *lowcore;
+	u64 pgm_psw[2] = {};
+
+	vm = __vm_create(VM_SHAPE_DEFAULT, 1, 0);
+
+	enable_cmma(vm);
+
+	vcpu = vm_vcpu_add(vm, 0, guest_essa_unmapped);
+
+	lowcore = addr_gpa2hva(vm, 0);
+	pgm_psw[0] = vcpu->run->psw_mask;
+	pgm_psw[1] = (u64)pgm_handler_c;
+	memcpy(lowcore + __LC_PGM_NEW_PSW, pgm_psw, 16);
+
+	/* Migration mode requires dirty tracking on all memslots. */
+	vm_mem_region_set_flags(vm, 0, KVM_MEM_LOG_DIRTY_PAGES);
+	enable_migration_mode(vm);
+
+	signal(SIGALRM, handle_alarm);
+	alarm(VCPU_RUN_TIMEOUT_S);
+
+	vcpu_run(vcpu);
+
+	alarm(0);
+
+	TEST_ASSERT_EQ(vcpu->run->exit_reason, KVM_EXIT_S390_SIEIC);
+	TEST_ASSERT_EQ(vcpu->run->s390_sieic.icptcode, ICPT_INST);
+	TEST_ASSERT_EQ(vcpu->run->s390_sieic.ipa & 0xff00, 0x8300);   /* DIAG */
+	TEST_ASSERT_EQ(vcpu->run->s390_sieic.ipb >> 16, 0x501);
+
+	get_ucall(vcpu, &uc);
+	TEST_ASSERT_EQ(uc.cmd, UCALL_SYNC);
+	TEST_ASSERT_EQ(uc.args[1], PGM_ADDRESSING);
+
+	kvm_vm_free(vm);
+}
+
 struct testdef {
 	const char *name;
 	void (*test)(void);
@@ -659,6 +737,7 @@ struct testdef {
 	{ "GET_CMMA_BITS: basic calls", test_get_cmma_basic },
 	{ "GET_CMMA_BITS: all pages are dirty initially", test_get_initial_dirty },
 	{ "GET_CMMA_BITS: holes are skipped", test_get_skip_holes },
+	{ "ESSA migration mode: PGM_ADDRESSING injected on unmapped GFN", test_essa_migration_pgm },
 };
 
 /**
-- 
2.55.0

Re: [PATCH 1/2] KVM: s390: fix ESSA addressing exception injection in migration mode

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] kvm: s390: invalid exception code injected due to negative argument
- [High] s390/kvm: Incorrect exception masking and invalid PIC injection in __do_essa
--

commit 0857e0bc0073ed5469250557e895e66a7a5d870f
Author: Gautam Gala <ggala@linux.ibm.com>

    KVM: s390: fix ESSA addressing exception injection in migration mode
    
    __do_essa() treated any negative return from dat_perform_essa() as the
    normal "no CBRL entry appended" case and returned 0, so a page-table
    walk failure silently dropped the addressing exception.

> diff --git a/arch/s390/kvm/gmap/dat.c b/arch/s390/kvm/gmap/dat.c
> index ff80d02c9f564..1bc6d2ec5b083 100644
> --- a/arch/s390/kvm/gmap/dat.c
> +++ b/arch/s390/kvm/gmap/dat.c

[ ... ]

> @@ -1092,7 +1093,9 @@ int dat_perform_essa(union asce asce, gfn_t gfn, int orc, union essa_state *stat
>  	union pte *ptep;
>  	int res = 0;
>  
> -	if (dat_entry_walk(NULL, gfn, asce, 0, TABLE_TYPE_PAGE_TABLE, &crstep, &ptep)) {
> +	if (dat_entry_walk(NULL, gfn, asce, DAT_WALK_ANY, TABLE_TYPE_PAGE_TABLE, &crstep, &ptep))
> +		return -PGM_ADDRESSING;

[Severity: High]
Would this mask other valid, positive Program Interruption Codes (PICs)
that dat_entry_walk() might return?

For example, if dat_entry_walk() returns PGM_SECURE_STORAGE_ACCESS from
hitting a memory hole, it appears it would be unconditionally mapped to
-PGM_ADDRESSING here.

> +	if (!ptep) {
>  		*state = (union essa_state) { .exception = 1 };
>  		return -1;
>  	}
> diff --git a/arch/s390/kvm/s390/priv.c b/arch/s390/kvm/s390/priv.c
> index b3cb2c2c3aa71..2abd1bdfdf322 100644
> --- a/arch/s390/kvm/s390/priv.c
> +++ b/arch/s390/kvm/s390/priv.c

[ ... ]

> @@ -1168,8 +1168,10 @@ static inline int __do_essa(struct kvm_vcpu *vcpu, const int orc)
>  	entries = (vcpu->arch.sie_block->cbrlo & ~PAGE_MASK) >> 3;
>  
>  	nappended = dat_perform_essa(vcpu->arch.gmap->asce, gfn, orc, &state, &dirtied);
> +	if (nappended == -PGM_ADDRESSING)
> +		return kvm_s390_inject_program_int(vcpu, -PGM_ADDRESSING);

[Severity: High]
Can passing -PGM_ADDRESSING to kvm_s390_inject_program_int() result in an
invalid program interruption code being injected?

Since the code argument of kvm_s390_inject_program_int() is an unsigned u16,
passing -5 causes an integer wraparound to 65531 (0xFFFB). This would inject
a corrupt exception state and violate the architectural ABI instead of
injecting the correct Addressing Exception (0x0005).

Regarding the test_essa_migration_pgm selftest added in a subsequent
patch, it appears it fails to catch this regression because its
exception handler pgm_handler_c() hardcodes a GUEST_SYNC(PGM_ADDRESSING)
call without actually reading the true PIC from the guest's lowcore
(__LC_PGM_INT_CODE).

>  	vcpu->run->s.regs.gprs[r1] = state.val;
> -	if (nappended < 0)
> +	if (nappended == -1)
>  		return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013144225.8885-1-ggala@linux.ibm.com?part=1