Re: [PATCH net-next 8/8] vhost: event suppression for packed ring

From: Jason Wang <jasowang@redhat.com>
To: Wei Xu <wexu@redhat.com>
Cc: kvm@vger.kernel.org, mst@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	maxime.coquelin@redhat.com
Subject: Re: [PATCH net-next 8/8] vhost: event suppression for packed ring
Date: Wed, 4 Jul 2018 13:23:18 +0800	[thread overview]
Message-ID: <493c3a3e-e088-d6fb-1da4-cda8bfb34400@redhat.com> (raw)
In-Reply-To: <20180704041352.GA9287@wei-ubt>

On 2018年07月04日 12:13, Wei Xu wrote:
> On Tue, Jul 03, 2018 at 01:38:04PM +0800, Jason Wang wrote:
>> This patch introduces support for event suppression. This is done by
>> have a two areas: device area and driver area. One side could then try
>> to disable or enable (delayed) notification from other side by using a
>> boolean hint or event index interface in the areas.
>>
>> For more information, please refer Virtio spec.
>>
>> Signed-off-by: Jason Wang<jasowang@redhat.com>
>> ---
>>   drivers/vhost/vhost.c | 191 ++++++++++++++++++++++++++++++++++++++++++++++----
>>   drivers/vhost/vhost.h |  10 ++-
>>   2 files changed, 185 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>> index 0f3f07c..cccbc82 100644
>> --- a/drivers/vhost/vhost.c
>> +++ b/drivers/vhost/vhost.c
>> @@ -1115,10 +1115,15 @@ static int vq_access_ok_packed(struct vhost_virtqueue *vq, unsigned int num,
>>   			       struct vring_used __user *used)
>>   {
>>   	struct vring_desc_packed *packed = (struct vring_desc_packed *)desc;
>> +	struct vring_packed_desc_event *driver_event =
>> +		(struct vring_packed_desc_event *)avail;
>> +	struct vring_packed_desc_event *device_event =
>> +		(struct vring_packed_desc_event *)used;
>>   
>> -	/* TODO: check device area and driver area */
>>   	return access_ok(VERIFY_READ, packed, num * sizeof(*packed)) &&
>> -	       access_ok(VERIFY_WRITE, packed, num * sizeof(*packed));
>> +	       access_ok(VERIFY_WRITE, packed, num * sizeof(*packed)) &&
> R/W parameter doesn't make sense to most architectures and the comment in x86
> says WRITE is a superset of READ, is it possible to converge them here?
>
> /**
>   * access_ok: - Checks if a user space pointer is valid
>   * @type: Type of access: %VERIFY_READ or %VERIFY_WRITE.  Note that
>   *        %VERIFY_WRITE is a superset of %VERIFY_READ - if it is safe
>   *        to write to a block, it is always safe to read from it.
>   * @addr: User space pointer to start of block to check
>   * @size: Size of block to check
>   *
>   * Context: User context only. This function may sleep if pagefaults are
>   *          enabled.
>   *
>   * Checks if a pointer to a block of memory in user space is valid.
>   *
>   * Returns true (nonzero) if the memory block may be valid, false (zero)
>   * if it is definitely invalid.
>   *
>   * Note that, depending on architecture, this function probably just
>   * checks that the pointer is in the user space range - after calling
>   * this function, memory access functions may still return -EFAULT.
>   */
> #define access_ok(type, addr, size)
> ......
>
> Thanks,
> Wei
>

Well, this is a question that beyond the scope of this patch.

My understanding is we should keep it unless type was meaningless on all 
archs.

Thanks

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization