On 1/18/2016 9:44 PM, Yuanhan Liu wrote:
> 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.
yes, that is how ops abstraction works.
>
>> +
>> + 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
>
