Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 8 Feb 2021 12:09:18 +0100
From:   Andrew Jones <drjones@redhat.com>
To:     "Maciej S. Szmigiero" <mail@maciej.szmigiero.name>
Cc:     Paolo Bonzini <pbonzini@redhat.com>, Shuah Khan <shuah@kernel.org>,
        Sean Christopherson <seanjc@google.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Igor Mammedov <imammedo@redhat.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>, kvm@vger.kernel.org,
        linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] KVM: selftests: add a memslot-related performance
 benchmark
Message-ID: <20210208110918.2rzt24qv2g67jtqo@kamzik.brq.redhat.com>
References: <5e5d83b305077e3e65b130dbb31c131bfb831170.1612139762.git.maciej.szmigiero@oracle.com>
 <ec67677c725d9f52d4abef3cfef07eac266c15cd.1612139762.git.maciej.szmigiero@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ec67677c725d9f52d4abef3cfef07eac266c15cd.1612139762.git.maciej.szmigiero@oracle.com>
Precedence: bulk

On Mon, Feb 01, 2021 at 09:10:57AM +0100, Maciej S. Szmigiero wrote:
[...]
> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
> index ce8f4ad39684..059a655053ca 100644
> --- a/tools/testing/selftests/kvm/.gitignore
> +++ b/tools/testing/selftests/kvm/.gitignore
> @@ -31,3 +31,4 @@
>  /kvm_create_max_vcpus
>  /set_memory_region_test
>  /steal_time
> +/memslot_perf_test
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index e7c6237d7383..2abc9e182c30 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -65,6 +65,7 @@ TEST_GEN_PROGS_x86_64 += dirty_log_perf_test
>  TEST_GEN_PROGS_x86_64 += kvm_create_max_vcpus
>  TEST_GEN_PROGS_x86_64 += set_memory_region_test
>  TEST_GEN_PROGS_x86_64 += steal_time
> +TEST_GEN_PROGS_x86_64 += memslot_perf_test
> 
>  TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list
>  TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list-sve

We've been trying keep the lists in .gitignore and Makefile in alphabetic
order. It's not really important, but seems like we should keep it now
that we've got it. Well, except I see dirty_log_perf_test and
dirty_log_test are out of order already...

[...]
> +static bool prepare_vm(struct vm_data *data, int nslots, uint64_t *maxslots,
> +		       void *guest_code, uint64_t mempages,
> +		       struct timespec *slot_runtime)
> +{
> +	uint32_t max_mem_slots;
> +	uint64_t rempages;
> +	uint64_t guest_addr;
> +	uint32_t slot;
> +	struct timespec tstart;
> +	struct sync_area *sync;
> +
> +	max_mem_slots = kvm_check_cap(KVM_CAP_NR_MEMSLOTS);
> +	TEST_ASSERT(max_mem_slots > 1,
> +		    "KVM_CAP_NR_MEMSLOTS should be greater than 1");
> +	TEST_ASSERT(nslots > 1 || nslots == -1,
> +		    "Slot count cap should be greater than 1");
> +	if (nslots != -1)
> +		max_mem_slots = min(max_mem_slots, (uint32_t)nslots);
> +	pr_info_v("Allowed number of memory slots: %"PRIu32"\n", max_mem_slots);
> +
> +	TEST_ASSERT(mempages > 1,
> +		    "Can't test without any memory");
> +
> +	data->npages = mempages;
> +	data->nslots = max_mem_slots - 1;
> +	data->pages_per_slot = mempages / data->nslots;
> +	if (!data->pages_per_slot) {
> +		*maxslots = mempages + 1;
> +		return false;
> +	}
> +
> +	rempages = mempages % data->nslots;
> +	data->hva_slots = malloc(sizeof(*data->hva_slots) * data->nslots);
> +	TEST_ASSERT(data->hva_slots, "malloc() fail");
> +
> +	data->vm = vm_create_default(VCPU_ID, mempages, guest_code);
> +
> +	vcpu_set_cpuid(data->vm, VCPU_ID, kvm_get_supported_cpuid());

This vcpu_set_cpuid() call, which causes problems for non-x86 builds,
is now embedded in vm_create_default() and therefore redundant here.


Otherwise this looks good to me. I'll try to find some time to test
it on an AArch64 machine configured to use 4k pages on the host.

Reviewed-by: Andrew Jones <drjones@redhat.com>

Thanks,
drew