On Fri Jun 21, 2024 at 12:16 AM AEST, Nina Schoetterl-Glausch wrote:
> sie_is_diag_icpt() checks if the intercept is due to an expected
> diagnose call and is valid.
> It subsumes pv_icptdata_check_diag.
>
> Signed-off-by: Nina Schoetterl-Glausch <nsg@xxxxxxxxxxxxx>
> ---
> lib/s390x/pv_icptdata.h | 42 --------------------------------
> lib/s390x/sie.h | 12 ++++++++++
> lib/s390x/sie.c | 53 +++++++++++++++++++++++++++++++++++++++++
> s390x/pv-diags.c | 8 +++----
> s390x/pv-icptcode.c | 11 ++++-----
> s390x/pv-ipl.c | 7 +++---
> 6 files changed, 76 insertions(+), 57 deletions(-)
> delete mode 100644 lib/s390x/pv_icptdata.h
>
> diff --git a/lib/s390x/pv_icptdata.h b/lib/s390x/pv_icptdata.h
> deleted file mode 100644
> index 4746117e..00000000
> --- a/lib/s390x/pv_icptdata.h
> +++ /dev/null
> @@ -1,42 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -/*
> - * Commonly used checks for PV SIE intercept data
> - *
> - * Copyright IBM Corp. 2023
> - * Author: Janosch Frank <frankja@xxxxxxxxxxxxx>
> - */
> -
> -#ifndef _S390X_PV_ICPTDATA_H_
> -#define _S390X_PV_ICPTDATA_H_
> -
> -#include <sie.h>
> -
> -/*
> - * Checks the diagnose instruction intercept data for consistency with
> - * the constants defined by the PV SIE architecture
> - *
> - * Supports: 0x44, 0x9c, 0x288, 0x308, 0x500
> - */
> -static bool pv_icptdata_check_diag(struct vm *vm, int diag)
> -{
> - int icptcode;
> -
> - switch (diag) {
> - case 0x44:
> - case 0x9c:
> - case 0x288:
> - case 0x308:
> - icptcode = ICPT_PV_NOTIFY;
> - break;
> - case 0x500:
> - icptcode = ICPT_PV_INSTR;
> - break;
> - default:
> - /* If a new diag is introduced add it to the cases above! */
> - assert(0);
> - }
> -
> - return vm->sblk->icptcode == icptcode && vm->sblk->ipa == 0x8302 &&
> - vm->sblk->ipb == 0x50000000 && vm->save_area.guest.grs[5] == diag;
> -}
> -#endif
> diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
> index 53cd767f..6d1a0d6e 100644
> --- a/lib/s390x/sie.h
> +++ b/lib/s390x/sie.h
> @@ -287,6 +287,18 @@ static inline bool sie_is_pv(struct vm *vm)
> return vm->sblk->sdf == 2;
> }
>
> +/**
> + * sie_is_diag_icpt() - Check if intercept is due to diagnose instruction
> + * @vm: the guest
> + * @diag: the expected diagnose code
> + *
> + * Check that the intercept is due to diagnose @diag and valid.
> + * For protected virtualisation, check that the intercept data meets additional
> + * constraints.
> + *
> + * Returns: true if intercept is due to a valid and has matching diagnose code
> + */
> +bool sie_is_diag_icpt(struct vm *vm, unsigned int diag);
> void sie_guest_sca_create(struct vm *vm);
> void sie_guest_create(struct vm *vm, uint64_t guest_mem, uint64_t guest_mem_len);
> void sie_guest_destroy(struct vm *vm);
> diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
> index 0fa915cf..d4ba2a40 100644
> --- a/lib/s390x/sie.c
> +++ b/lib/s390x/sie.c
> @@ -42,6 +42,59 @@ void sie_check_validity(struct vm *vm, uint16_t vir_exp)
> report(vir_exp == vir, "VALIDITY: %x", vir);
> }
>
> +bool sie_is_diag_icpt(struct vm *vm, unsigned int diag)
> +{
> + union {
> + struct {
> + uint64_t : 16;
> + uint64_t ipa : 16;
> + uint64_t ipb : 32;
> + };
> + struct {
> + uint64_t : 16;
> + uint64_t opcode : 8;
> + uint64_t r_1 : 4;
> + uint64_t r_2 : 4;
> + uint64_t r_base : 4;
> + uint64_t displace : 12;
> + uint64_t zero : 16;
> + };
> + } instr = { .ipa = vm->sblk->ipa, .ipb = vm->sblk->ipb };
> + uint8_t icptcode;
> + uint64_t code;
> +
> + switch (diag) {
> + case 0x44:
> + case 0x9c:
> + case 0x288:
> + case 0x308:
> + icptcode = ICPT_PV_NOTIFY;
> + break;
> + case 0x500:
> + icptcode = ICPT_PV_INSTR;
> + break;
> + default:
> + /* If a new diag is introduced add it to the cases above! */
> + assert_msg(false, "unknown diag");
> + }
> +
> + if (sie_is_pv(vm)) {
> + if (instr.r_1 != 0 || instr.r_2 != 2 || instr.r_base != 5)
> + return false;
> + if (instr.displace)
> + return false;
> + } else {
> + icptcode = ICPT_INST;
> + }
> + if (vm->sblk->icptcode != icptcode)
> + return false;
> + if (instr.opcode != 0x83 || instr.zero)
> + return false;
> + code = instr.r_base ? vm->save_area.guest.grs[instr.r_base] : 0;
> + code = (code + instr.displace) & 0xffff;
> + return code == diag;
> +}
It looks like this transformation is equivalent for the PV case. You
could put the switch into the sie_is_pv() branch? Otherwise looks okay.
Reviewed-by: Nicholas Piggin <npiggin@xxxxxxxxx>
> +
> void sie_handle_validity(struct vm *vm)
> {
> if (vm->sblk->icptcode != ICPT_VALIDITY)
> diff --git a/s390x/pv-diags.c b/s390x/pv-diags.c
> index 3193ad99..6ebe469a 100644
> --- a/s390x/pv-diags.c
> +++ b/s390x/pv-diags.c
> @@ -9,7 +9,6 @@
> */
> #include <libcflat.h>
> #include <snippet.h>
> -#include <pv_icptdata.h>
> #include <sie.h>
> #include <sclp.h>
> #include <asm/facility.h>
> @@ -32,8 +31,7 @@ static void test_diag_500(void)
> size_gbin, size_hdr, SNIPPET_UNPACK_OFF);
>
> sie(&vm);
> - report(pv_icptdata_check_diag(&vm, 0x500),
> - "intercept values");
> + report(sie_is_diag_icpt(&vm, 0x500), "intercept values");
> report(vm.save_area.guest.grs[1] == 1 &&
> vm.save_area.guest.grs[2] == 2 &&
> vm.save_area.guest.grs[3] == 3 &&
> @@ -45,7 +43,7 @@ static void test_diag_500(void)
> */
> vm.sblk->iictl = IICTL_CODE_OPERAND;
> sie(&vm);
> - report(pv_icptdata_check_diag(&vm, 0x9c) &&
> + report(sie_is_diag_icpt(&vm, 0x9c) &&
> vm.save_area.guest.grs[0] == PGM_INT_CODE_OPERAND,
> "operand exception");
>
> @@ -57,7 +55,7 @@ static void test_diag_500(void)
> vm.sblk->iictl = IICTL_CODE_SPECIFICATION;
> /* Inject PGM, next exit should be 9c */
> sie(&vm);
> - report(pv_icptdata_check_diag(&vm, 0x9c) &&
> + report(sie_is_diag_icpt(&vm, 0x9c) &&
> vm.save_area.guest.grs[0] == PGM_INT_CODE_SPECIFICATION,
> "specification exception");
>
> diff --git a/s390x/pv-icptcode.c b/s390x/pv-icptcode.c
> index d7c47d6f..bc90df1e 100644
> --- a/s390x/pv-icptcode.c
> +++ b/s390x/pv-icptcode.c
> @@ -13,7 +13,6 @@
> #include <smp.h>
> #include <sclp.h>
> #include <snippet.h>
> -#include <pv_icptdata.h>
> #include <asm/facility.h>
> #include <asm/barrier.h>
> #include <asm/sigp.h>
> @@ -47,7 +46,7 @@ static void test_validity_timing(void)
> size_gbin, size_hdr, SNIPPET_UNPACK_OFF);
>
> sie(&vm);
> - report(pv_icptdata_check_diag(&vm, 0x44), "spt done");
> + report(sie_is_diag_icpt(&vm, 0x44), "spt done");
> stck(&time_exit);
> tmp = vm.sblk->cputm;
> mb();
> @@ -258,7 +257,7 @@ static void test_validity_asce(void)
>
> /* Try if we can still do an entry with the correct asce */
> sie(&vm);
> - report(pv_icptdata_check_diag(&vm, 0x44), "re-entry with valid CR1");
> + report(sie_is_diag_icpt(&vm, 0x44), "re-entry with valid CR1");
> uv_destroy_guest(&vm);
> free_pages(pgd_new);
> report_prefix_pop();
> @@ -294,7 +293,7 @@ static void run_icpt_122_tests_prefix(unsigned long prefix)
>
> sie(&vm);
> /* Guest indicates that it has been setup via the diag 0x44 */
> - assert(pv_icptdata_check_diag(&vm, 0x44));
> + assert(sie_is_diag_icpt(&vm, 0x44));
> /* If the pages have not been shared these writes will cause exceptions */
> ptr = (uint32_t *)prefix;
> WRITE_ONCE(ptr, 0);
> @@ -328,7 +327,7 @@ static void test_icpt_112(void)
>
> /* Setup of the guest's state for 0x0 prefix */
> sie(&vm);
> - assert(pv_icptdata_check_diag(&vm, 0x44));
> + assert(sie_is_diag_icpt(&vm, 0x44));
>
> /* Test on standard 0x0 prefix */
> run_icpt_122_tests_prefix(0);
> @@ -348,7 +347,7 @@ static void test_icpt_112(void)
>
> /* Try a re-entry after everything has been imported again */
> sie(&vm);
> - report(pv_icptdata_check_diag(&vm, 0x9c) &&
> + report(sie_is_diag_icpt(&vm, 0x9c) &&
> vm.save_area.guest.grs[0] == 42,
> "re-entry successful");
> report_prefix_pop();
> diff --git a/s390x/pv-ipl.c b/s390x/pv-ipl.c
> index cc46e7f7..cd49bd95 100644
> --- a/s390x/pv-ipl.c
> +++ b/s390x/pv-ipl.c
> @@ -11,7 +11,6 @@
> #include <sie.h>
> #include <sclp.h>
> #include <snippet.h>
> -#include <pv_icptdata.h>
> #include <asm/facility.h>
> #include <asm/uv.h>
>
> @@ -35,7 +34,7 @@ static void test_diag_308(int subcode)
>
> /* First exit is a diag 0x500 */
> sie(&vm);
> - assert(pv_icptdata_check_diag(&vm, 0x500));
> + assert(sie_is_diag_icpt(&vm, 0x500));
>
> /*
> * The snippet asked us for the subcode and we answer by
> @@ -46,7 +45,7 @@ static void test_diag_308(int subcode)
>
> /* Continue after diag 0x500, next icpt should be the 0x308 */
> sie(&vm);
> - assert(pv_icptdata_check_diag(&vm, 0x308));
> + assert(sie_is_diag_icpt(&vm, 0x308));
> assert(vm.save_area.guest.grs[2] == subcode);
>
> /*
> @@ -118,7 +117,7 @@ static void test_diag_308(int subcode)
> * see a diagnose 0x9c PV instruction notification.
> */
> sie(&vm);
> - report(pv_icptdata_check_diag(&vm, 0x9c) &&
> + report(sie_is_diag_icpt(&vm, 0x9c) &&
> vm.save_area.guest.grs[0] == 42,
> "continue after load");
>
