Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 17 Jan 2023 22:51:42 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Vishal Annapurve <vannapurve@google.com>
Cc:     x86@kernel.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-kselftest@vger.kernel.org, pbonzini@redhat.com,
        vkuznets@redhat.com, wanpengli@tencent.com, jmattson@google.com,
        joro@8bytes.org, tglx@linutronix.de, mingo@redhat.com,
        bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com,
        shuah@kernel.org, yang.zhong@intel.com, ricarkol@google.com,
        aaronlewis@google.com, wei.w.wang@intel.com,
        kirill.shutemov@linux.intel.com, corbet@lwn.net, hughd@google.com,
        jlayton@kernel.org, bfields@fieldses.org,
        akpm@linux-foundation.org, chao.p.peng@linux.intel.com,
        yu.c.zhang@linux.intel.com, jun.nakajima@intel.com,
        dave.hansen@intel.com, michael.roth@amd.com, qperret@google.com,
        steven.price@arm.com, ak@linux.intel.com, david@redhat.com,
        luto@kernel.org, vbabka@suse.cz, marcorr@google.com,
        erdemaktas@google.com, pgonda@google.com, nikunj@amd.com,
        diviness@google.com, maz@kernel.org, dmatlack@google.com,
        axelrasmussen@google.com, maciej.szmigiero@oracle.com,
        mizhang@google.com, bgardon@google.com, ackerleytng@google.com
Subject: Re: [V2 PATCH 4/6] KVM: selftests: x86: Add helpers to execute VMs
 with private memory
Message-ID: <Y8cmfjRIRp2EphTa@google.com>
References: <20221205232341.4131240-1-vannapurve@google.com>
 <20221205232341.4131240-5-vannapurve@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221205232341.4131240-5-vannapurve@google.com>
Precedence: bulk

On Mon, Dec 05, 2022, Vishal Annapurve wrote:
> +void vcpu_run_and_handle_mapgpa(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
> +{
> +	/*
> +	 * Loop until the guest exits with any reason other than
> +	 * KVM_HC_MAP_GPA_RANGE hypercall.
> +	 */
> +
> +	while (true) {
> +		vcpu_run(vcpu);
> +
> +		if ((vcpu->run->exit_reason == KVM_EXIT_HYPERCALL) &&
> +			(vcpu->run->hypercall.nr == KVM_HC_MAP_GPA_RANGE)) {

I get what you're trying to do, and I completely agree that we need better helpers
and/or macros to reduce this type of boilerplate, but adding a one-off helper like
this is going to be a net negative overall.  This helper services exactly one use
case, and also obfuscates what a test does.

In other words, this is yet another thing that needs broad, generic support
(_vcpu_run() is a very special case).  E.g. something like this to make it easy
for tests to run a guest and handle ucalls plus specific exits (just a strawman,
I think we can do better for handling ucalls).

#define vcpu_run_loop(vcpu, handlers, ucalls)				\
do {									\
	uint32_t __exit;						\
	int __r = 0;							\
									\
	while (!r)  {							\
		vcpu_run(vcpu);						\
									\
		__exit = vcpu->run->exit_reason;			\
									\
		if (__exit < ARRAY_SIZE(handlers) && handlers[__exit])	\
			__r = handlers[__exit](vcpu);			\	
		else if (__exit == KVM_EXIT_IO && ucalls)		\
			__r = handle_exit_ucall(vcpu, ucalls,		\
						ARRAY_SIZE(ucalls));	\
		else							\
			TEST_FAIL(...)					\
	}								\
} while (0)


For this series, I think it makes sense to just open code yet another test.  It
really doesn't end up being much code, which is partly why we haven't added
helpers :-/