Hi Michael,
> > -struct vring_used_elem *vring_add_used_user(struct vring_host *vh,
> > - unsigned int head, int len)
> > +
> > +static inline struct vring_used_elem *_vring_add_used(struct vring_host
> *vh,
> > + u32 head, u32 len,
> > + bool (*cpy)(void *dst,
> > + void *src,
> > + size_t s),
> > + void (*wbarrier)(void))
> > {
> > struct vring_used_elem *used;
> > + u16 last_used;
> >
> > /* The virtqueue contains a ring of used buffers. Get a pointer to the
> > * next entry in that used ring. */
> > - used = &vh->vr.used->ring[vh->last_used_idx % vh->vr.num];
> > - if (__put_user(head, &used->id)) {
> > - pr_debug("Failed to write used id");
> > + used = &vh->vr.used->ring[vh->last_used_idx & (vh->vr.num - 1)];
> > + if (!cpy(&used->id, &head, sizeof(used->id)) ||
> > + !cpy(&used->len, &len, sizeof(used->len)))
> > return NULL;
> > - }
> > - if (__put_user(len, &used->len)) {
> > - pr_debug("Failed to write used len");
> > + wbarrier();
> > + last_used = vh->last_used_idx + 1;
> > + if (!cpy(&vh->vr.used->idx, &last_used, sizeof(vh->vr.used->idx)))
> > return NULL;
>
> I think this is broken: we need a 16 bit access, this is
> doing a memcpy which is byte by byte.
I have played around with gcc and -O2 option, and it seems to me
that GCC is smart enough to optimize the use of sizeof and memcpy
into MOV operations. But my assembly knowledge is very rusty,
so a second opinion on this would be good.
> > - }
> > - /* Make sure buffer is written before we update index. */
> > - smp_wmb();
> > - if (__put_user(vh->last_used_idx + 1, &vh->vr.used->idx)) {
> > - pr_debug("Failed to increment used idx");
> > - return NULL;
> > - }
> > - vh->last_used_idx++;
> > + vh->last_used_idx = last_used;
> > return used;
> > }
> > -EXPORT_SYMBOL(vring_add_used_user);
Thanks,
Sjur
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
