On 2018/12/14 0:20, Stefan Hajnoczi wrote:
> 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.
>
Right, I will separate pkt and payload in different pages, as you mentioned
in another patch.
Thanks,
Yiwen.
