linux-kernel - Re: [PATCH RFC 2/2] vhost: support urgent descriptors

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 19 Sep 2014 15:10:33 +0800
From:	Jason Wang <jasowang@...hat.com>
To:	"Michael S. Tsirkin" <mst@...hat.com>, linux-kernel@...r.kernel.org
CC:	Rusty Russell <rusty@...tcorp.com.au>,
	virtualization@...ts.linux-foundation.org, kvm@...r.kernel.org
Subject: Re: [PATCH RFC 2/2] vhost: support urgent descriptors

On 07/01/2014 06:49 PM, Michael S. Tsirkin wrote:
> Signed-off-by: Michael S. Tsirkin <mst@...hat.com>
> ---
>  drivers/vhost/vhost.h | 19 +++++++++++++------
>  drivers/vhost/net.c   | 30 +++++++++++++++++++++---------
>  drivers/vhost/scsi.c  | 23 +++++++++++++++--------
>  drivers/vhost/test.c  |  5 +++--
>  drivers/vhost/vhost.c | 23 ++++++++++++++++-------
>  5 files changed, 68 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 3eda654..61ca542 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
[...]
>  EXPORT_SYMBOL_GPL(vhost_add_used_n);
> @@ -1433,12 +1439,13 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
>  	    unlikely(vq->avail_idx == vq->last_avail_idx))
>  		return true;
>  
> -	if (!vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX)) {
> +	if (vq->urgent || !vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX)) {

So the urgent descriptor only work when event index was not enabled?
This seems suboptimal, we may still want to benefit from event index
even if urgent descriptor is used. Looks like we need return true here
when vq->urgent is true?

Another question is whether or not we need to do this a new flag.
Technically we can do it purely in guest side through event index, this
can help to eliminate both the changes in host and a new feature bit.
>  		__u16 flags;
>  		if (__get_user(flags, &vq->avail->flags)) {
>  			vq_err(vq, "Failed to get flags");
>  			return true;
>  		}
> +		vq->urgent = false;
>  		return !(flags & VRING_AVAIL_F_NO_INTERRUPT);
>  	}
>  	old = vq->signalled_used;
> @@ -1468,9 +1475,10 @@ EXPORT_SYMBOL_GPL(vhost_signal);
>  /* And here's the combo meal deal.  Supersize me! */
>  void vhost_add_used_and_signal(struct vhost_dev *dev,
>  			       struct vhost_virtqueue *vq,
> +			       bool urgent,
>  			       unsigned int head, int len)
>  {
> -	vhost_add_used(vq, head, len);
> +	vhost_add_used(vq, urgent, head, len);
>  	vhost_signal(dev, vq);
>  }
>  EXPORT_SYMBOL_GPL(vhost_add_used_and_signal);
> @@ -1478,9 +1486,10 @@ EXPORT_SYMBOL_GPL(vhost_add_used_and_signal);
>  /* multi-buffer version of vhost_add_used_and_signal */
>  void vhost_add_used_and_signal_n(struct vhost_dev *dev,
>  				 struct vhost_virtqueue *vq,
> +				 bool urgent,
>  				 struct vring_used_elem *heads, unsigned count)
>  {
> -	vhost_add_used_n(vq, heads, count);
> +	vhost_add_used_n(vq, urgent, heads, count);
>  	vhost_signal(dev, vq);
>  }
>  EXPORT_SYMBOL_GPL(vhost_add_used_and_signal_n);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/