[SLOF] [PATCH v1 10/27] virtio: make virtio_vring_desc 1.0 aware

[Alexey Kardashevskiy]

On 01/14/2016 10:04 PM, Nikunj A Dadhania wrote:
> Alexey Kardashevskiy <aik at ozlabs.ru> writes:
>
>> On 01/13/2016 10:16 PM, Nikunj A Dadhania wrote:
>>> Signed-off-by: Nikunj A Dadhania <nikunj at linux.vnet.ibm.com>
>>> ---
>>>    lib/libvirtio/virtio.c | 14 +++++++++++---
>>>    1 file changed, 11 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/lib/libvirtio/virtio.c b/lib/libvirtio/virtio.c
>>> index aa4a45b..f95e86b 100644
>>> --- a/lib/libvirtio/virtio.c
>>> +++ b/lib/libvirtio/virtio.c
>>> @@ -235,14 +235,22 @@ struct vring_desc *virtio_get_vring_desc(struct virtio_device *dev, int queue)
>>>    {
>>>    	struct vring_desc *desc = 0;
>>>
>>> -	if (dev->type == VIRTIO_TYPE_PCI) {
>>> +	if (dev->type != VIRTIO_TYPE_PCI)
>>> +		return NULL;


[1]


>>> +	if (dev->is_modern) {
>>> +		void *q_sel = dev->common.addr + offset_of(struct virtio_dev_common, q_select);
>>> +		void *q_desc = dev->common.addr + offset_of(struct virtio_dev_common, q_desc);
>>> +
>>> +		ci_write_16(q_sel, cpu_to_le16(queue));
>>> +		eieio();
>>
>>
>> Here and in next patches I see this new eieio(), please merge similar
>> changes to one patch and add a commit log about this "make xxx 1.0 aware"
>> change.
>
> These are two different code path now:
>
> if (modern) {
>     select_queue_virtio_1_0()
>     eieio()
>     write_virtio_1_0()
> }
> else {
>    select_queue()
>    eieio()
>    write()
> }
>
> So the change should come with this patch and not separately. Am i
> missing something ?


Ah, I missed that old code did have eieio() already. Never mind.



>
>>
>>
>>> +		desc = (void *)(virtio_read64(q_desc));
>>> +	} else {
>>>    		ci_write_16(dev->base+VIRTIOHDR_QUEUE_SELECT,
>>>    			    cpu_to_le16(queue));
>>>    		eieio();
>>>    		desc = (void*)(4096L *
>>> -		   le32_to_cpu(ci_read_32(dev->base+VIRTIOHDR_QUEUE_ADDRESS)));
>>> +			       le32_to_cpu(ci_read_32(dev->base+VIRTIOHDR_QUEUE_ADDRESS)));
>>
>> Unrelared change.
>
> Tab changes which wasnt correct.


Separate patch, please. You can merge all these code design fixes into one 
huge patch. [1] could go there too. Easier to read now and bisect later if 
functional changes are separated from cosmetic ones.


>
>>
>>
>>>    	}
>>> -
>>
>> Here too.
>>
>>
>>>    	return desc;
>>>    }
>>>
>>>
>>
>>
>> --
>> Alexey
>


-- 
Alexey