Re: [V2 PATCH 4/6] KVM: selftests: x86: Add helpers to execute VMs with private memory

Sean Christopherson
2023-01-17T23:46:11Z

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 :-/