Re: [patch 3/5] KVM: hypercall batching

[Avi Kivity <avi@qumranet.com>]

Marcelo Tosatti wrote:
> Batch pte updates and tlb flushes in lazy MMU mode.
>
>   

I am slightly uneasy about generic hypercall batching.  An alternative 
way to do it would be to define a kvm_mmu_op structure (with an embedded 
opcode) and have a single hypercall execute a number of these (and not 
have any hypercall for executing just one).

> +static int kvm_hypercall_multicall(struct kvm_vcpu *vcpu, gpa_t addr, u32 nents)
> +{
> +	int i, result = 0;
> +
> +	++vcpu->stat.multicall;
> +	vcpu->stat.multicall_nr += nents;
> +
> +	for (i = 0; i < nents; i++) {
> +		struct kvm_multicall_entry mc;
> +		int ret;
> +
> +		down_read(&vcpu->kvm->slots_lock);
> +		ret = kvm_read_guest(vcpu->kvm, addr, &mc, sizeof(mc));
> +		up_read(&vcpu->kvm->slots_lock);
> +		if (ret)
> +			return -KVM_EFAULT;
> +
> +		ret = dispatch_hypercall(vcpu, mc.nr, mc.a0, mc.a1, mc.a2,
> +					    mc.a3);
> +		if (ret)
> +			result = ret;
> +		addr += sizeof(mc);
> +	}
> +	if (result < 0)
> +		return -KVM_EINVAL;
> +	return result;
> +}
> +

The return code of the hypercalls (if positive) is lost.  In the case 
that one hypercall failed, which one exactly is also lost.

For mmu ops, it doesn't matter (since if any fails, the guest is 
toast).  For a generic framework, it may matter.  If we keep it generic, 
I suggest adding a field for the return code of each hypercall, and have 
the multicall return the number of executed hypercalls.


-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/