Re: [kvm-unit-tests PATCH v2 1/1] s390x: pv: Add test for large host pages backing

[Janosch Frank <frankja@linux.ibm.com>]

On 11/11/24 1:15 PM, Claudio Imbrenda wrote:
> Add a new test to check that the host can use 1M large pages to back
> protected guests when the corresponding feature is present.
> 
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
>   s390x/Makefile               |   2 +
>   lib/s390x/asm/arch_def.h     |   1 +
>   lib/s390x/asm/uv.h           |  18 ++
>   s390x/pv-edat1.c             | 463 +++++++++++++++++++++++++++++++++++
>   s390x/snippets/c/pv-memhog.c |  59 +++++
>   5 files changed, 543 insertions(+)
>   create mode 100644 s390x/pv-edat1.c
>   create mode 100644 s390x/snippets/c/pv-memhog.c
> 
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 23342bd6..c5c6f92c 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -48,6 +48,7 @@ tests += $(TEST_DIR)/sie-dat.elf
>   pv-tests += $(TEST_DIR)/pv-diags.elf
>   pv-tests += $(TEST_DIR)/pv-icptcode.elf
>   pv-tests += $(TEST_DIR)/pv-ipl.elf
> +pv-tests += $(TEST_DIR)/pv-edat1.elf
>   
>   ifneq ($(HOST_KEY_DOCUMENT),)
>   ifneq ($(GEN_SE_HEADER),)
> @@ -137,6 +138,7 @@ $(TEST_DIR)/pv-icptcode.elf: pv-snippets += $(SNIPPET_DIR)/asm/icpt-loop.gbin
>   $(TEST_DIR)/pv-icptcode.elf: pv-snippets += $(SNIPPET_DIR)/asm/loop.gbin
>   $(TEST_DIR)/pv-icptcode.elf: pv-snippets += $(SNIPPET_DIR)/asm/pv-icpt-vir-timing.gbin
>   $(TEST_DIR)/pv-ipl.elf: pv-snippets += $(SNIPPET_DIR)/asm/pv-diag-308.gbin
> +$(TEST_DIR)/pv-edat1.elf: pv-snippets += $(SNIPPET_DIR)/c/pv-memhog.gbin
>   
>   ifneq ($(GEN_SE_HEADER),)
>   snippets += $(pv-snippets)
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index 745a3387..481ede8f 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -249,6 +249,7 @@ extern struct lowcore lowcore;
>   #define PGM_INT_CODE_REGION_FIRST_TRANS		0x39
>   #define PGM_INT_CODE_REGION_SECOND_TRANS	0x3a
>   #define PGM_INT_CODE_REGION_THIRD_TRANS		0x3b
> +#define PGM_INT_CODE_SECURE_PAGE_SIZE		0x3c
>   #define PGM_INT_CODE_SECURE_STOR_ACCESS		0x3d
>   #define PGM_INT_CODE_NON_SECURE_STOR_ACCESS	0x3e
>   #define PGM_INT_CODE_SECURE_STOR_VIOLATION	0x3f
> diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h
> index 611dcd3f..7527be48 100644
> --- a/lib/s390x/asm/uv.h
> +++ b/lib/s390x/asm/uv.h
> @@ -35,6 +35,7 @@
>   #define UVC_CMD_CONV_TO_SEC_STOR	0x0200
>   #define UVC_CMD_CONV_FROM_SEC_STOR	0x0201
>   #define UVC_CMD_DESTR_SEC_STOR		0x0202
> +#define UVC_CMD_VERIFY_LARGE_FRAME	0x0203
>   #define UVC_CMD_SET_SEC_CONF_PARAMS	0x0300
>   #define UVC_CMD_UNPACK_IMG		0x0301
>   #define UVC_CMD_VERIFY_IMG		0x0302
> @@ -74,6 +75,11 @@ enum uv_cmds_inst {
>   	BIT_UVC_CMD_PIN_PAGE_SHARED = 21,
>   	BIT_UVC_CMD_UNPIN_PAGE_SHARED = 22,
>   	BIT_UVC_CMD_ATTESTATION = 28,
> +	BIT_UVC_CMD_VERIFY_LARGE_FRAME = 32,
> +};
> +
> +enum uv_features {
> +	BIT_UV_1M_BACKING = 6,
>   };
>   
>   struct uv_cb_header {
> @@ -312,6 +318,18 @@ static inline int uv_import(uint64_t handle, unsigned long gaddr)
>   	return uv_call(0, (uint64_t)&uvcb);
>   }
>   
> +static inline int uv_merge(uint64_t handle, unsigned long gaddr)
> +{
> +	struct uv_cb_cts uvcb = {
> +		.header.cmd = UVC_CMD_VERIFY_LARGE_FRAME,
> +		.header.len = sizeof(uvcb),
> +		.guest_handle = handle,
> +		.gaddr = gaddr,
> +	};
> +
> +	return uv_call(0, (uint64_t)&uvcb);
> +}

I don't understand why you added this to the lib if you're not using it 
even once since you have your own function that returns more data.

Are you expecting other tests to regularly need this UVC?
The attestation test for instance added the constants but no function 
since the call is basically only used for one test.