On Wed, Dec 12, 2018 at 05:31:39PM +0800, jiangyiwen wrote: > +static struct virtio_vsock_pkt *receive_mergeable(struct virtqueue *vq, > + struct virtio_vsock *vsock, unsigned int *total_len) > +{ > + struct virtio_vsock_pkt *pkt; > + u16 num_buf; > + void *buf; > + unsigned int len; > + size_t vsock_hlen = sizeof(struct virtio_vsock_pkt); > + > + buf = virtqueue_get_buf(vq, &len); > + if (!buf) > + return NULL; > + > + *total_len = len; > + vsock->rx_buf_nr--; > + > + if (unlikely(len < vsock_hlen)) { > + put_page(virt_to_head_page(buf)); > + return NULL; > + } > + > + pkt = buf; > + num_buf = le16_to_cpu(pkt->mrg_rxbuf_hdr.num_buffers); > + if (!num_buf || num_buf > VIRTIO_VSOCK_MAX_VEC_NUM) { > + put_page(virt_to_head_page(buf)); > + return NULL; > + } > + > + /* Initialize pkt residual structure */ > + memset(&pkt->work, 0, vsock_hlen - sizeof(struct virtio_vsock_hdr) - > + sizeof(struct virtio_vsock_mrg_rxbuf_hdr));
struct virtio_vsock_pkt is an internal driver state structure. It must be able to change without breaking the host<->guest device interface. Exposing struct virtio_vsock_pkt across the device interface makes this impossible. One way to solve this is by placing a header length field at the beginning of the mergeable buffer. That way the device knows at which offset the payload should be placed and the driver can continue to use the allocation scheme you've chosen (where a single page contains the virtio_vsock_pkt and payload). Another issue is that virtio requires exclusive read OR write descriptors. You cannot have a descriptor that is both read and write. So there's not a huge advantage to combining the driver state struct and payload, except maybe the driver can save a memory allocation.
signature.asc
Description: PGP signature
_______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization