On Fri, Apr 29, 2016 at 01:18:34AM +0000, Jianfeng Tan wrote:
> +static void
> +vdev_read_dev_config(struct virtio_hw *hw, uint64_t offset,
> + void *dst, int length)
> +{
> + int i;
> + struct virtio_user_hw *uhw = (struct virtio_user_hw *)hw->vdev_private;
Unnecessary cast.
> +static int
> +vdev_setup_queue(struct virtio_hw *hw __rte_unused, struct virtqueue *vq)
> +{
> + /* Changed to use virtual addr */
> + vq->vq_ring_mem = (phys_addr_t)vq->mz->addr;
> + if (vq->virtio_net_hdr_mz) {
> + vq->virtio_net_hdr_mem =
> + (phys_addr_t)vq->virtio_net_hdr_mz->addr;
> + /* Do it one more time after we reset virtio_net_hdr_mem */
> + vring_hdr_desc_init(vq);
> + }
> + vq->offset = offsetof(struct rte_mbuf, buf_addr);
> + return 0;
Here as last email said, you should not mix vq stuff. What's more,
why do you invoke vring_hdr_desc_init() here? If it needs a special
handling, do it in driver.
The "setup_queue" method is actually for telling the device where desc,
avail and used vring are located. Hence, the implementation could be simple:
just log them.
> +
> +const struct virtio_pci_ops vdev_ops = {
Note that this is the interface for the driver to talk to the device,
we should put this file into upper layer then, in the driver.
And let me make a summary, trying to make it clear:
- We should not use any structures/functions from the virtio driver
here, unless it's really a must.
- It's allowed for driver to make *few* special handling for the virtio
user device. And that's what the driver supposed to do: to handle
different device variants.
So, I think it's okay to export the virtio_user_device struct to
driver and do all those kind of "fake pci" configration there.
--yliu
> + .read_dev_cfg = vdev_read_dev_config,
> + .write_dev_cfg = vdev_write_dev_config,
> + .reset = vdev_reset,
> + .get_status = vdev_get_status,
> + .set_status = vdev_set_status,
> + .get_features = vdev_get_features,
> + .set_features = vdev_set_features,
> + .get_isr = vdev_get_isr,
> + .set_config_irq = vdev_set_config_irq,
> + .get_queue_num = vdev_get_queue_num,
> + .setup_queue = vdev_setup_queue,
> + .del_queue = vdev_del_queue,
> + .notify_queue = vdev_notify_queue,
> +};
> --
> 2.1.4
