Re: [RFC V2 PATCH 4/8] selftests: kvm: priv_memfd_test: Add shared access test

[Shuah Khan <skhan@linuxfoundation.org>]

On 5/10/22 6:08 PM, Vishal Annapurve wrote:
> Add a test to access private memory in shared fashion
> which should exercise implicit memory conversion path
> using KVM_EXIT_MEMORY_ERROR.
> 

This comment applies all patches in this series. Keep commit log
line length around 76 for readability in "git log" display.


Also same comment about combining lines of code when it isn't
necessary to split them, align the lines with parenthesis to
make it easier to read, and run checkpatch.

> Signed-off-by: Vishal Annapurve <vannapurve@google.com>
> ---
>   tools/testing/selftests/kvm/priv_memfd_test.c | 69 +++++++++++++++++++
>   1 file changed, 69 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/priv_memfd_test.c b/tools/testing/selftests/kvm/priv_memfd_test.c
> index 55e24c893b07..48bc4343e7b5 100644
> --- a/tools/testing/selftests/kvm/priv_memfd_test.c
> +++ b/tools/testing/selftests/kvm/priv_memfd_test.c
> @@ -147,12 +147,81 @@ static void pmpat_guest_code(void)
>   	GUEST_DONE();
>   }
>   
> +/* Test to verify guest shared accesses on private memory with following steps:
> + * 1) Upon entry, guest signals VMM that it has started.
> + * 2) VMM populates the shared memory with known pattern and continues guest
> + *    execution.
> + * 3) Guest reads private gpa range in a shared fashion and verifies that it
> + *    reads what VMM has written in step2.
> + * 3) Guest writes a different pattern on the shared memory and signals VMM
> + *      that it has updated the shared memory.
> + * 4) VMM verifies shared memory contents to be same as the data populated
> + *      in step 3 and continues guest execution.
> + */
> +#define PMSAT_ID				1
> +#define PMSAT_DESC				"PrivateMemorySharedAccessTest"
> +
> +/* Guest code execution stages for private mem access test */
> +#define PMSAT_GUEST_STARTED			0ULL
> +#define PMSAT_GUEST_TEST_MEM_UPDATED		1ULL
> +
> +static bool pmsat_handle_vm_stage(struct kvm_vm *vm,
> +			void *test_info,
> +			uint64_t stage)
> +{
> +	void *shared_mem = ((struct test_run_helper *)test_info)->shared_mem;
> +
> +	switch (stage) {
> +	case PMSAT_GUEST_STARTED: {
> +		/* Initialize the contents of shared memory */
> +		TEST_ASSERT(do_mem_op(SET_PAT, shared_mem,
> +			TEST_MEM_DATA_PAT1, TEST_MEM_SIZE),
> +			"Shared memory update failed");
> +		VM_STAGE_PROCESSED(PMSAT_GUEST_STARTED);
> +		break;
> +	}
> +	case PMSAT_GUEST_TEST_MEM_UPDATED: {
> +		/* verify data to be same as what guest wrote */
> +		TEST_ASSERT(do_mem_op(VERIFY_PAT, shared_mem,
> +			TEST_MEM_DATA_PAT2, TEST_MEM_SIZE),
> +			"Shared memory view mismatch");
> +		VM_STAGE_PROCESSED(PMSAT_GUEST_TEST_MEM_UPDATED);
> +		break;
> +	}
> +	default:
> +		printf("Unhandled VM stage %ld\n", stage);

Is this a test failure? Add more information to use why it isn't handled.

> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +static void pmsat_guest_code(void)
> +{
> +	void *shared_mem = (void *)TEST_MEM_GPA;
> +
> +	GUEST_SYNC(PMSAT_GUEST_STARTED);
> +	GUEST_ASSERT(do_mem_op(VERIFY_PAT, shared_mem,
> +			TEST_MEM_DATA_PAT1, TEST_MEM_SIZE));
> +
> +	GUEST_ASSERT(do_mem_op(SET_PAT, shared_mem,
> +			TEST_MEM_DATA_PAT2, TEST_MEM_SIZE));
> +	GUEST_SYNC(PMSAT_GUEST_TEST_MEM_UPDATED);
> +
> +	GUEST_DONE();
> +}
> +
>   static struct test_run_helper priv_memfd_testsuite[] = {
>   	[PMPAT_ID] = {
>   		.test_desc = PMPAT_DESC,
>   		.vmst_handler = pmpat_handle_vm_stage,
>   		.guest_fn = pmpat_guest_code,
>   	},
> +	[PMSAT_ID] = {
> +		.test_desc = PMSAT_DESC,
> +		.vmst_handler = pmsat_handle_vm_stage,
> +		.guest_fn = pmsat_guest_code,
> +	},
>   };
>   
>   static void handle_vm_exit_hypercall(struct kvm_run *run,
> 

thanks,
-- Shuah