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

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

On Wed, 20 Nov 2024 17:33:48 +0100
Janosch Frank <frankja@linux.ibm.com> wrote:

> 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.

I don't remember :D
I'll have a closer look

> 
> 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.
> 
> 
>