[dpdk-dev] [PATCH 3/3] virtio: Add a new layer to abstract pci access method

[Xie, Huawei]

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
>