[SLOF] [PATCH v1 08/27] virtio: make virtio_get_qsize 1.0 aware

[Alexey Kardashevskiy]

On 01/14/2016 09:57 PM, Nikunj A Dadhania wrote:
> Alexey Kardashevskiy <aik at ozlabs.ru> writes:
>
>> On 01/13/2016 10:16 PM, Nikunj A Dadhania wrote:
>>> Also return unsigned int
>>
>> Why?
>
> Size is not negative :-)


The size equal to 0xffffffff makes as much sense as a negative one ;)
Are sizes in other places all unsigned already? If yes, then add this note 
to the commit log, if not, then change it in other places in a separate patch.


btw (out of curiosity) while looking at the kernel tree about 
VIRTIOHDR_QUEUE_SIZE, I could not spot its counterpart there, I thought it 
is VIRTIO_PCI_COMMON_Q_SIZE but it is not, what is it?


>
>>
>>
>>>
>>> Signed-off-by: Nikunj A Dadhania <nikunj at linux.vnet.ibm.com>
>>> ---
>>>    lib/libvirtio/virtio.c | 15 +++++++++++----
>>>    lib/libvirtio/virtio.h |  2 +-
>>>    2 files changed, 12 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/lib/libvirtio/virtio.c b/lib/libvirtio/virtio.c
>>> index bcf050b..77676c7 100644
>>> --- a/lib/libvirtio/virtio.c
>>> +++ b/lib/libvirtio/virtio.c
>>> @@ -185,17 +185,24 @@ unsigned long virtio_vring_size(unsigned int qsize)
>>>     * @param   queue virtio queue number
>>>     * @return  number of elements
>>>     */
>>> -int virtio_get_qsize(struct virtio_device *dev, int queue)
>>> +unsigned int virtio_get_qsize(struct virtio_device *dev, int queue)
>>>    {
>>> -	int size = 0;
>>> +	unsigned int size = 0;
>>>
>>> -	if (dev->type == VIRTIO_TYPE_PCI) {
>>> +	if (dev->type != VIRTIO_TYPE_PCI)
>>> +		return 0;
>>> +	if (dev->is_modern) {
>>> +		void *addr = dev->common.addr + offset_of(struct virtio_dev_common, q_select);
>>> +		ci_write_16(addr, cpu_to_le16(queue));
>>> +		eieio();
>>> +		size = le16_to_cpu(ci_read_16(addr + 2));
>>
>>
>> Please add commit log explaining this magic.
>
> Sure.
>
>>
>>
>>> +	}
>>> +	else {
>>>    		ci_write_16(dev->base+VIRTIOHDR_QUEUE_SELECT,
>>>    			    cpu_to_le16(queue));
>>>    		eieio();
>>>    		size = le16_to_cpu(ci_read_16(dev->base+VIRTIOHDR_QUEUE_SIZE));
>>>    	}
>>> -
>>
>> Unrelated change.
>
> Yep.
>
>>
>>
>>>    	return size;
>>>    }
>>>
>>> diff --git a/lib/libvirtio/virtio.h b/lib/libvirtio/virtio.h
>>> index eee3144..537862b 100644
>>> --- a/lib/libvirtio/virtio.h
>>> +++ b/lib/libvirtio/virtio.h
>>> @@ -107,7 +107,7 @@ struct vqs {
>>>    #define VQ_ALIGN(addr)	(((addr) + 0xfff) & ~0xfff)
>>>
>>>    extern unsigned long virtio_vring_size(unsigned int qsize);
>>> -extern int virtio_get_qsize(struct virtio_device *dev, int queue);
>>> +extern unsigned int virtio_get_qsize(struct virtio_device *dev, int queue);
>>>    extern struct vring_desc *virtio_get_vring_desc(struct virtio_device *dev, int queue);
>>>    extern struct vring_avail *virtio_get_vring_avail(struct virtio_device *dev, int queue);
>>>    extern struct vring_used *virtio_get_vring_used(struct virtio_device *dev, int queue);
>>>
>>
>>
>> --
>> Alexey
>


-- 
Alexey