[dpdk-dev] [PATCH 2/6] virtio: introduce struct virtio_pci_ops
Tan, Jianfeng
jianfeng.tan at intel.com
Tue Dec 29 12:31:35 CET 2015
> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Yuanhan Liu
> Sent: Thursday, December 10, 2015 11:54 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH 2/6] virtio: introduce struct virtio_pci_ops
>
> Introduce struct virtio_pci_ops, to let legacy virtio (v0.95) and
> modern virtio (1.0) have different implementation regarding to a
> specific pci action, such as read host status.
>
...
> +static void
> +legacy_reset(struct virtio_hw *hw)
> +{
> + /*
> + * Setting the status to RESET sets the host device to
> + * the original, uninitialized state.
> + */
> + legacy_set_status(hw, VIRTIO_CONFIG_STATUS_RESET);
> + legacy_get_status(hw);
May need a comment to explain why here we need to get_status().
> +}
> +
> +static uint8_t
> +legacy_get_isr(struct virtio_hw *hw)
> +{
Please delete the following blank line.
> +
> + return VIRTIO_READ_REG_1(hw, VIRTIO_PCI_ISR);
> +}
> +
> +/* Enable one vector (0) for Link State Intrerrupt */
> +static uint16_t
> +legacy_set_irq(struct virtio_hw *hw, uint16_t vec)
> +{
> + VIRTIO_WRITE_REG_2(hw, VIRTIO_MSI_CONFIG_VECTOR, vec);
> + return VIRTIO_READ_REG_2(hw, VIRTIO_MSI_CONFIG_VECTOR);
> +}
> +
...
> +
> +struct virtio_pci_ops {
> + void (*read_dev_cfg)(struct virtio_hw *hw, uint64_t offset,
> + void *dst, int len);
> + void (*write_dev_cfg)(struct virtio_hw *hw, uint64_t offset,
> + void *src, int len);
> + void (*reset)(struct virtio_hw *hw);
> +
> + uint8_t (*get_status)(struct virtio_hw *hw);
> + void (*set_status)(struct virtio_hw *hw, uint8_t status);
> +
> + uint32_t (*get_features)(struct virtio_hw *hw);
> + void (*set_features)(struct virtio_hw *hw, uint32_t features);
> +
> + uint8_t (*get_isr)(struct virtio_hw *hw);
> +
> + uint16_t (*set_irq)(struct virtio_hw *hw, uint16_t vec);
> +
> + uint16_t (*get_queue_num)(struct virtio_hw *hw, uint16_t
> queue_id);
How about changing the name into get_queue_size? From my side, queue_num is very confusing.
Thanks,
Jianfeng
> + void (*setup_queue)(struct virtio_hw *hw, struct virtqueue *vq);
> + void (*del_queue)(struct virtio_hw *hw, struct virtqueue *vq);
> + void (*notify_queue)(struct virtio_hw *hw, struct virtqueue *vq);
> +};
> +
...
> #ifdef RTE_LIBRTE_VIRTIO_DEBUG_DUMP
> --
> 1.9.0
More information about the dev
mailing list