On Mon, Jan 18, 2016 at 06:13:09PM +0900, Tetsuya Mukawa wrote:
> +struct virtio_pci_access_ops {
> + uint8_t (*legacy_read8)(struct virtio_hw *hw, uint8_t *addr);
> + uint16_t (*legacy_read16)(struct virtio_hw *hw, uint16_t *addr);
> + uint32_t (*legacy_read32)(struct virtio_hw *hw, uint32_t *addr);
> + void (*legacy_write8)(struct virtio_hw *hw,
> + uint8_t *addr, uint8_t val);
> + void (*legacy_write16)(struct virtio_hw *hw,
> + uint16_t *addr, uint16_t val);
> + void (*legacy_write32)(struct virtio_hw *hw,
> + uint32_t *addr, uint32_t val);
> +
> + uint8_t (*modern_read8)(struct virtio_hw *hw, uint8_t *addr);
> + uint16_t (*modern_read16)(struct virtio_hw *hw, uint16_t *addr);
> + uint32_t (*modern_read32)(struct virtio_hw *hw, uint32_t *addr);
> + void (*modern_write8)(struct virtio_hw *hw,
> + uint8_t *addr, uint8_t val);
> + void (*modern_write16)(struct virtio_hw *hw,
> + uint16_t *addr, uint16_t val);
> + void (*modern_write32)(struct virtio_hw *hw,
> + uint32_t *addr, uint32_t val);
One thing about abstraction is that you need define one set of operations,
instead of two similar sets. Thus, you need define following operations
only:
- read8
- read16
- read32
- write8
- write16
- write32
And make a proper assignment after the modern/legacy detection.
> +
> + int (*map_pci_cfg)(struct virtio_hw *hw);
> + void (*unmap_pci_cfg)(struct virtio_hw *hw);
> + void *(*get_cfg_addr)(struct virtio_hw *hw,
> + struct virtio_pci_cap *cap);
> + int (*read_pci_cfg)(struct virtio_hw *hw,
> + void *buf, size_t len, off_t offset);
It'd be good if you can post the patches that use above abstract
operations, so that people can tell if they are properly defined.
--yliu
