Re: [PATCH v4 19/19] KVM: selftests: Add test for KVM_SET_USER_MEMORY_REGION

[Christian Borntraeger]

On 18.12.19 17:39, Sean Christopherson wrote:
> On Wed, Dec 18, 2019 at 12:39:43PM +0100, Christian Borntraeger wrote:
>>
>> On 17.12.19 21:40, Sean Christopherson wrote:
>>> Add a KVM selftest to test moving the base gfn of a userspace memory
>>> region.  The test is primarily targeted at x86 to verify its memslot
>>> metadata is correctly updated, but also provides basic functionality
>>> coverage on other architectures.
>>> +static void *vcpu_worker(void *data)
>>> +{
>>> +	struct kvm_vm *vm = data;
>>> +	struct kvm_run *run;
>>> +	struct ucall uc;
>>> +	uint64_t cmd;
>>> +
>>> +	/*
>>> +	 * Loop until the guest is done.  Re-enter the guest on all MMIO exits,
>>> +	 * which will occur if the guest attempts to access a memslot while it
>>> +	 * is being moved.
>>> +	 */
>>> +	run = vcpu_state(vm, VCPU_ID);
>>> +	do {
>>> +		vcpu_run(vm, VCPU_ID);
>>> +	} while (run->exit_reason == KVM_EXIT_MMIO);
>>> +
>>> +	TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
>>> +		    "Unexpected exit reason = %d", run->exit_reason);
>>
>>
>> This will also not work for s390. Maybe just make this test x86 specific for now?
> 
> Doh, that's obvious in hindsight.  I think the basic premise is also
> broken on arm64 as it returns -EFAULT on is_error_noslot_pfn(pfn).  So
> yeah, x86 only for now :-(

I started looking into this what it would cost to implement this on s390.
s390 is also returning EFAULT if no memory slot is available.

According to the doc this is not documented at all. So this part of the test
        vm = vm_create(VM_MODE_DEFAULT, 0, O_RDWR);
        vm_vcpu_add(vm, VCPU_ID);
        /* Fails with ENOSPC because the MMU can't create pages (no slots). */
        TEST_ASSERT(_vcpu_run(vm, VCPU_ID) == -1 && errno == ENOSPC,
                    "Unexpected error code = %d", errno);
        kvm_vm_free(vm);

is actually just testing that the implementation for x86 does not change the error
from ENOSPC to something else.

The question is: do we want to document the error for the "no memslot" case and then
change all existing platforms?