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

[Avi Kivity <avi@qumranet.com>]

Hollis Blanchard wrote:
> On Sat, 2008-02-16 at 17:09 -0500, Marcelo Tosatti wrote:
>   
>> plain text document attachment (kvm-multicall)
>> Batch pte updates and tlb flushes in lazy MMU mode.
>>
>> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>> Cc: Anthony Liguori <aliguori@us.ibm.com>
>>
>> Index: kvm.paravirt/arch/x86/kernel/kvm.c
>> ===================================================================
>> --- kvm.paravirt.orig/arch/x86/kernel/kvm.c
>> +++ kvm.paravirt/arch/x86/kernel/kvm.c
>> @@ -25,6 +25,74 @@
>>  #include <linux/kvm_para.h>
>>  #include <linux/cpu.h>
>>  #include <linux/mm.h>
>> +#include <linux/hardirq.h>
>> +
>> +#define MAX_MULTICALL_NR (PAGE_SIZE / sizeof(struct kvm_multicall_entry))
>> +
>> +struct kvm_para_state {
>> +	struct kvm_multicall_entry queue[MAX_MULTICALL_NR];
>> +	int queue_index;
>> +	enum paravirt_lazy_mode mode;
>> +};
>> +
>> +static DEFINE_PER_CPU(struct kvm_para_state, para_state);
>>     
>
> AFAICS there is no guarantee about page-alignment here...
>
>   

Right.

>> +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;
>> +}
>>     
>
> ... but here you're assuming that 'queue' is physically contiguous,
> which is not necessarily true one you cross a page boundary.
>   

Kernel data is physically contiguous (true for per-cpu data as well?), 
so no there's issue here.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


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