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

[Nikunj A Dadhania]

Alexey Kardashevskiy <aik at ozlabs.ru> writes:

> 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.

Sure, that is fine, will do that.

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