Re: [PATCH v2 2/6] KVM: Add KVM_CREATE_COALESCED_MMIO_BUFFER ioctl

[Sean Christopherson <seanjc@google.com>]

On Thu, Jul 18, 2024, Ilias Stamatis wrote:
> @@ -40,17 +42,14 @@ static int coalesced_mmio_in_range(struct kvm_coalesced_mmio_dev *dev,
>  	return 1;
>  }
>  
> -static int coalesced_mmio_has_room(struct kvm_coalesced_mmio_dev *dev, u32 last)
> +static int coalesced_mmio_has_room(struct kvm_coalesced_mmio_ring *ring, u32 last)
>  {
> -	struct kvm_coalesced_mmio_ring *ring;
> -
>  	/* Are we able to batch it ? */
>  
>  	/* last is the first free entry
>  	 * check if we don't meet the first used entry
>  	 * there is always one unused entry in the buffer
>  	 */
> -	ring = dev->kvm->coalesced_mmio_ring;
>  	if ((last + 1) % KVM_COALESCED_MMIO_MAX == READ_ONCE(ring->first)) {
>  		/* full */
>  		return 0;
> @@ -65,17 +64,28 @@ static int coalesced_mmio_write(struct kvm_vcpu *vcpu,
>  {
>  	struct kvm_coalesced_mmio_dev *dev = to_mmio(this);
>  	struct kvm_coalesced_mmio_ring *ring = dev->kvm->coalesced_mmio_ring;
> +	spinlock_t *lock = dev->buffer_dev ?
> +			   &dev->buffer_dev->ring_lock :
> +			   &dev->kvm->ring_lock;

I'd prefer

	spinlock_t *lock = dev->buffer_dev ? &dev->buffer_dev->ring_lock :
					     &dev->kvm->ring_lock;

to make it more obvious which parts the "guts" of the effective if-statement.


> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index d0788d0a72cc..9eb22287384f 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -5246,6 +5246,10 @@ static long kvm_vm_ioctl(struct file *filp,
>  		r = kvm_vm_ioctl_unregister_coalesced_mmio(kvm, &zone);
>  		break;
>  	}
> +	case KVM_CREATE_COALESCED_MMIO_BUFFER: {

Curly braces aren't needed, the above case has 'em because it declares local
variables.

> +		r = kvm_vm_ioctl_create_coalesced_mmio_buffer(kvm);
> +		break;
> +	}
>  #endif
>  	case KVM_IRQFD: {
>  		struct kvm_irqfd data;
> -- 
> 2.34.1
>