Re: [PATCH 1/7] libkvm: delayed MMIO support (common part)

[Avi Kivity <avi@qumranet.com>]

Laurent Vivier wrote:
> This patch introduces in libkvm the common part of MMIO batching interface.
>
> It checks the MMIO batching availability with ioctl(KVM_CHECK_EXTENSION).
> If KVM_CAP_DELAYED_MMIO is available, it processes the MMIO ring buffer
> at the return of ioctl(KVM_RUN).
> It defines kvm_register_delayed_mmio() to register a delayed MMIO zone, and 
> kvm_unregister_delayed_mmio() to unregister it.
>
>  
>  void init_slots(void);
> diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c
> index d1e95a4..718f4c6 100644
> --- a/libkvm/libkvm.c
> +++ b/libkvm/libkvm.c
> @@ -879,6 +879,20 @@ again:
>  
>  	post_kvm_run(kvm, vcpu);
>  
> +#if defined(KVM_CAP_DELAYED_MMIO)
> +	if (kvm->delayed_mmio) {
> +	        struct kvm_batch *batch = (void *)run +
> +					  kvm->delayed_mmio * PAGE_SIZE;
> +		while (batch->first != batch->last) {
> +			kvm->callbacks->mmio_write(kvm->opaque,
> +					    batch->mmio[batch->first].phys_addr,
> +					   &batch->mmio[batch->first].data[0],
> +					    batch->mmio[batch->first].len);
> +			batch->first = (batch->first + 1) % KVM_MAX_BATCH;
>   

There needs to be a write barrier after this (smp_wmb()), to avoid 
compiler and cpu reordering.  Especially important for non-x86.  Also 
need a read barrier before reading batch->first.

Or maybe not, likely the pthread_mutex_lock() acts as the correct 
barrier.  But we do need the equivalent barriers in the kernel code 
(missed it on that review).

> +		}
> +	}
> +#endif
> +
>  	if (r == -1) {
>  		r = handle_io_window(kvm);
>  		goto more;
>   

> +
> +int kvm_register_delayed_mmio(kvm_context_t kvm, uint64_t addr, uint32_t size)
> +{
> +#ifdef KVM_CAP_DELAYED_MMIO
> +	struct kvm_delayed_mmio_zone zone;
> +	int r;
> +
> +	if (kvm->delayed_mmio) {
> +
> +		zone.addr = addr;
> +		zone.size = size;
> +
> +		r = ioctl(kvm->vm_fd, KVM_REGISTER_DELAYED_MMIO, &zone);
> +		if (r == -1) {
> +			perror("kvm_register_delayed_mmio_zone");
> +			return -errno;
> +		}
> +		return 0;
> +	}
> +#endif
> +	return -1;
>   

-ENOSYS, or something.  -1 is -ENOPERM.

> +}
> +
> +int kvm_unregister_delayed_mmio(kvm_context_t kvm, uint64_t addr, uint32_t size)
> +{
> +#ifdef KVM_CAP_DELAYED_MMIO
> +	struct kvm_delayed_mmio_zone zone;
> +	int r;
> +
> +	if (kvm->delayed_mmio) {
> +
> +		zone.addr = addr;
> +		zone.size = size;
> +
> +		r = ioctl(kvm->vm_fd, KVM_UNREGISTER_DELAYED_MMIO, &zone);
> +		if (r == -1) {
> +			perror("kvm_unregister_delayed_mmio_zone");
> +			return -errno;
> +		}
> +		return 0;
> +	}
> +#endif
> +	return -1;
>   

ditto.

-- 
error compiling committee.c: too many arguments to function