[kvm-unit-tests PATCH v4 6/8] s390x: uv-host: Properly handle config creation errors

Janosch Frank
2023-06-22T07:53:18Z

If the first bit is set on a error rc, the hypervisor will need to
destroy the config before trying again. Let's properly handle those
cases so we're not using stale data.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
---
lib/s390x/asm/uv.h | 1 +
s390x/uv-host.c | 65 +++++++++++++++++++++++++++++++++++++++-------
2 files changed, 57 insertions(+), 9 deletions(-)

diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h
index 38920461..e9fb19af 100644
--- a/lib/s390x/asm/uv.h
+++ b/lib/s390x/asm/uv.h
@@ -24,6 +24,7 @@
#define UVC_RC_NO_RESUME 0x0007
#define UVC_RC_INV_GHANDLE 0x0020
#define UVC_RC_INV_CHANDLE 0x0021
+#define UVC_RC_DSTR_NEEDED_FLG 0x8000

#define UVC_CMD_QUI 0x0001
#define UVC_CMD_INIT_UV 0x000f
diff --git a/s390x/uv-host.c b/s390x/uv-host.c
index 4112b4b6..65a9c6d3 100644
--- a/s390x/uv-host.c
+++ b/s390x/uv-host.c
@@ -370,6 +370,42 @@ static void test_cpu_create(void)
report_prefix_pop();
}

+/*
+ * If the first bit of the rc is set we need to destroy the
+ * configuration before testing other create config errors.
+ */
+static void cgc_destroy_if_needed(struct uv_cb_cgc *uvcb)
+{
+ uint16_t rc, rrc;
+
+ if (uvcb->header.rc != UVC_RC_EXECUTED &&
+ !(uvcb->header.rc & UVC_RC_DSTR_NEEDED_FLG))
+ return;
+
+ assert(uvcb->guest_handle);
+ assert(!uv_cmd_nodata(uvcb->guest_handle, UVC_CMD_DESTROY_SEC_CONF,
+ &rc, &rrc));
+
+ /* We need to zero it for the next test */
+ uvcb->guest_handle = 0;
+}
+
+static bool cgc_check_data(struct uv_cb_cgc *uvcb, uint16_t rc_expected)
+{
+ /* This function purely checks for error rcs */
+ if (uvcb->header.rc == UVC_RC_EXECUTED)
+ return false;
+
+ /*
+ * We should only receive a handle when the rc is 1 or the
+ * first bit is set.
+ */
+ if (!(uvcb->header.rc & UVC_RC_DSTR_NEEDED_FLG) && uvcb->guest_handle)
+ report_abort("Received a handle when we didn't expect one");
+
+ return (uvcb->header.rc & ~UVC_RC_DSTR_NEEDED_FLG) == rc_expected;
+}
+
static void test_config_create(void)
{
int rc;
@@ -394,58 +430,67 @@ static void test_config_create(void)
rc = uv_call(0, (uint64_t)&uvcb_cgc);
report(uvcb_cgc.header.rc == UVC_RC_INV_LEN && rc == 1 &&
!uvcb_cgc.guest_handle, "hdr invalid length");
+ cgc_destroy_if_needed(&uvcb_cgc);
uvcb_cgc.header.len += 8;

uvcb_cgc.guest_stor_origin = uvcb_qui.max_guest_stor_addr + (1UL << 20) * 2 + 1;
rc = uv_call(0, (uint64_t)&uvcb_cgc);
- report(uvcb_cgc.header.rc == 0x101 && rc == 1,
+ report(cgc_check_data(&uvcb_cgc, 0x101) && rc == 1,
"MSO > max guest addr");
+ cgc_destroy_if_needed(&uvcb_cgc);
uvcb_cgc.guest_stor_origin = 0;

uvcb_cgc.guest_stor_origin = uvcb_qui.max_guest_stor_addr - (1UL << 20);
rc = uv_call(0, (uint64_t)&uvcb_cgc);
- report(uvcb_cgc.header.rc == 0x102 && rc == 1,
+ report(cgc_check_data(&uvcb_cgc, 0x102) && rc == 1,
"MSO + MSL > max guest addr");
+ cgc_destroy_if_needed(&uvcb_cgc);
uvcb_cgc.guest_stor_origin = 0;

uvcb_cgc.guest_asce &= ~ASCE_P;
rc = uv_call(0, (uint64_t)&uvcb_cgc);
- report(uvcb_cgc.header.rc == 0x105 && rc == 1,
+ report(cgc_check_data(&uvcb_cgc, 0x105) && rc == 1,
"ASCE private bit missing");
+ cgc_destroy_if_needed(&uvcb_cgc);
uvcb_cgc.guest_asce |= ASCE_P;

uvcb_cgc.guest_asce |= 0x20;
rc = uv_call(0, (uint64_t)&uvcb_cgc);
- report(uvcb_cgc.header.rc == 0x105 && rc == 1,
+ report(cgc_check_data(&uvcb_cgc, 0x105) && rc == 1,
"ASCE bit 58 set");
+ cgc_destroy_if_needed(&uvcb_cgc);
uvcb_cgc.guest_asce &= ~0x20;

tmp = uvcb_cgc.conf_base_stor_origin;
uvcb_cgc.conf_base_stor_origin = get_max_ram_size() + 8;
rc = uv_call(0, (uint64_t)&uvcb_cgc);
- report(uvcb_cgc.header.rc == 0x108 && rc == 1,
+ report(cgc_check_data(&uvcb_cgc, 0x108) && rc == 1,
"base storage origin > available memory");
+ cgc_destroy_if_needed(&uvcb_cgc);
uvcb_cgc.conf_base_stor_origin = tmp;

tmp = uvcb_cgc.conf_base_stor_origin;
uvcb_cgc.conf_base_stor_origin = 0x1000;
rc = uv_call(0, (uint64_t)&uvcb_cgc);
- report(uvcb_cgc.header.rc == 0x109 && rc == 1,
- "base storage origin contains lowcore");
+ report(cgc_check_data(&uvcb_cgc, 0x109) && rc == 1,
+ "base storage origin contains lowcore %x", uvcb_cgc.header.rc);
+ cgc_destroy_if_needed(&uvcb_cgc);
uvcb_cgc.conf_base_stor_origin = tmp;

tmp = uvcb_cgc.guest_sca;
uvcb_cgc.guest_sca = 0;
rc = uv_call(0, (uint64_t)&uvcb_cgc);
- report(uvcb_cgc.header.rc == 0x10c && rc == 1,
+ report(cgc_check_data(&uvcb_cgc, 0x10c) && rc == 1,
"sca == 0");
+ cgc_destroy_if_needed(&uvcb_cgc);
uvcb_cgc.guest_sca = tmp;

tmp = uvcb_cgc.guest_sca;
uvcb_cgc.guest_sca = get_max_ram_size() + PAGE_SIZE * 4;
rc = uv_call(0, (uint64_t)&uvcb_cgc);
- report(uvcb_cgc.header.rc == 0x10d && rc == 1,
+ report(cgc_check_data(&uvcb_cgc, 0x10d) && rc == 1,
"sca inaccessible");
+ cgc_destroy_if_needed(&uvcb_cgc);
uvcb_cgc.guest_sca = tmp;

rc = uv_call(0, (uint64_t)&uvcb_cgc);
@@ -464,6 +509,7 @@ static void test_config_create(void)
uvcb_cgc.guest_handle = 0;
rc = uv_call(0, (uint64_t)&uvcb_cgc);
report(uvcb_cgc.header.rc >= 0x100 && rc == 1, "reuse uvcb");
+ cgc_destroy_if_needed(&uvcb_cgc);
uvcb_cgc.guest_handle = tmp;

/* Copy over most data from uvcb_cgc, so we have the ASCE that was used. */
@@ -481,6 +527,7 @@ static void test_config_create(void)
rc = uv_call(0, (uint64_t)&uvcb);
report(uvcb.header.rc >= 0x104 && rc == 1 && !uvcb.guest_handle,
"reuse ASCE");
+ cgc_destroy_if_needed(&uvcb);
free((void *)uvcb.conf_base_stor_origin);
free((void *)uvcb.conf_var_stor_origin);

--
2.34.1