Re: [virtio-dev] packed ring layout proposal v3

[Jens Freimann <jfreimann@redhat.com>]

On Sun, Oct 01, 2017 at 04:08:29AM +0000, Michael S. Tsirkin wrote:
>On Thu, Sep 28, 2017 at 09:44:35AM +0000, Liang, Cunming wrote:
>>
>> Get it now. Please correct me if I missing something.
>>
>>
>> Flags status hints,
>>
>> - DESC_DRIVER only: driver owns the descriptor w/o available info ready for device to use
>>
>> - DESC_DRIVER | DESC_WRAP: driver has prepared an available descriptor, device hasn't used it yet
>>
>> - None: device has used the descriptor, and write descriptor out
>>
>> - DESC_WRAP only: shall not happen, device make sure to clear it
>>
>>
>> Polling behavior is,
>>
>> - Device monitor DESC_WRAP bit set or not; If set, go to use descriptor and clear DESC_DRIVER bit in the end (note: always need to clear DESC_WRAP)
>>
>> - Driver monitor DESC_DRIVER bit cleared or not; If cleared, reclaim descriptor(set DESC_DRIVER) and set DESC_WRAP once new available descriptor get ready to go
>>
>>
>> --
>> Steve
>
>
>Hmm no, not what I had in mind.
>
>DESC_DRIVER: used by driver to poll. Driver sets it when writing a
>descriptor.  Device clears it when overwriting a descriptor.
>Thus driver uses DESC_DRIVER to detect that device data in
>descriptor is valid.

Basically DESC_HW from v2 split in two?

>
>DESC_WRAP: used by device to poll. Driver sets it to a *different*
>value every time it overwrites a descriptor. 
>How to achieve it?
>since descriptors are written out in ring order,
>simply maintain the current value internally (start value 1) and flip it
>every time you overwrite the first descriptor.
>Device leaves it intact when overwriting a descriptor.

This is confusing me a bit.

My understanding is: 
1. the internally kept wrap value only flipped when the first
descriptor is overwritten

2. the moment the first descriptor is written the internal wrap value
is flipped 0->1 or 1->0 and this value is written to every descriptor
DESC_WRAP until we reach the first descriptor again

>
>After writing down this explanation, I think the names aren't
>great.
>
>Let me try an alternative explanation.
>
>---------------
>A two-bit field, DRIVER_OWNER, signals the buffer ownership.
>It has 4 possible values:
>values 0x1, 0x11 are written by driver
>values 0x0, 0x10 are written by device

The 0x prefix might add to the confusion here. It is really just two
bits, no?

>each time driver writes out a descriptor, it must make sure
>that the high bit in OWNER changes.
>
>each time device writes out a descriptor, it must make sure
>that the high bit in OWNER does not change.
>
>this is exactly the same functionally, DRIVER is high bit and
>WRAP is the low bit.  Does this make things clearer?

So far it makes sense to me.
>---------------
>
>
>
>Maybe the difference between device and driver
>is confusing. We can fix that by changing the values.
>Here is an alternative. Let me know if you like it better -
>I need to think a bit more to make sure it works,
>but to give you an idea:
>
>
>---------------
>A two-bit field, DRIVER_OWNER, signals the buffer ownership.
>It has 4 possible values:
>values 0x1, 0x10 are written by driver
>values 0x0, 0x11 are written by device
>
>each time driver writes out a descriptor, it must make sure
>that the high bit in OWNER changes. Thus first time
>it writes 0x10, next time 0x1, then 0x10 again.
>
>each time device writes out a descriptor, it must make sure
>that the low bit in OWNER changes.  Thus first time
>it writes 0x11, next time 0x0, then 0x11 again.

DESC_WRAP is changed by the device now, so this would work differently
than in the scenario from above. 
This would mean we don't need the internally kept wrap value, right?


regards,
Jens