diff mbox

suggesting wording fixes for virtio-spec 0.9.5

Message ID 87r4i1gadv.fsf@rustcorp.com.au (mailing list archive)
State New, archived
Headers show

Commit Message

Rusty Russell April 23, 2013, 4:05 a.m. UTC
Laszlo Ersek <lersek@redhat.com> writes:
> Hi,
>
> (I'm not subscribed to either list,)
>
> using the word "descriptor" is misleading in the following sections:

Yes, I like the use of 'descriptor chains'.  This is a definite
improvement.

Here's the diff I ended up with (massaged to minimize it).

Thanks!
Rusty.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Laszlo Ersek April 23, 2013, 9:39 a.m. UTC | #1
On 04/23/13 06:05, Rusty Russell wrote:
> Laszlo Ersek <lersek@redhat.com> writes:
>> Hi,
>>
>> (I'm not subscribed to either list,)
>>
>> using the word "descriptor" is misleading in the following sections:
> 
> Yes, I like the use of 'descriptor chains'.  This is a definite
> improvement.
> 
> Here's the diff I ended up with (massaged to minimize it).
> 
> Thanks!
> Rusty.
> 
> --- virtio-spec.txt-old	2013-04-23 13:22:21.339158214 +0930
> +++ virtio-spec.txt	2013-04-23 13:34:14.055176464 +0930
> @@ -482,10 +482,10 @@
>  
>  2.3.4 Available Ring
>  
> -The available ring refers to what descriptors we are offering the 
> -device: it refers to the head of a descriptor chain. The “flags” 
> +The available ring refers to what descriptor chains we are offering the
> +device: each entry refers to the head of a descriptor chain. The “flags”
>  field is currently 0 or 1: 1 indicating that we do not need an 
> -interrupt when the device consumes a descriptor from the 
> +interrupt when the device consumes a descriptor chain from the
>  available ring. Alternatively, the guest can ask the device to 
>  delay interrupts until an entry with an index specified by the “
>  used_event” field is written in the used ring (equivalently, 
> @@ -671,16 +671,16 @@
>  
>  avail->ring[avail->idx % qsz] = head;
>  
> -However, in general we can add many descriptors before we update 
> -the “idx” field (at which point they become visible to the 
> -device), so we keep a counter of how many we've added:
> +However, in general we can add many separate descriptor chains before we update
> +the “idx” field (at which point they become visible to the device),
> +so we keep a counter of how many we've added:
>  
>  avail->ring[(avail->idx + added++) % qsz] = head;
>  
>  2.4.1.3 Updating The Index Field
>  
>  Once the idx field of the virtqueue is updated, the device will 
> -be able to access the descriptor entries we've created and the 
> +be able to access the descriptor chains we've created and the 
>  memory they refer to. This is why a memory barrier is generally 
>  used before the idx update, to ensure it sees the most up-to-date 
>  copy.
> 

Not sure if it's customary here or if you need it / want it, but anyway

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

(Also I've fixed the OVMF driver; just reposting the patch today with a
better commit message.)

Thanks much!
Laszlo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rusty Russell April 29, 2013, 12:45 a.m. UTC | #2
Laszlo Ersek <lersek@redhat.com> writes:
> On 04/23/13 06:05, Rusty Russell wrote:
>> Laszlo Ersek <lersek@redhat.com> writes:
>>> Hi,
>>>
>>> (I'm not subscribed to either list,)
>>>
>>> using the word "descriptor" is misleading in the following sections:
>> 
>> Yes, I like the use of 'descriptor chains'.  This is a definite
>> improvement.
...
> Not sure if it's customary here or if you need it / want it, but anyway
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Not needed, but always useful to ahve confirmation that I didn't
introduce another mistake while fixing my old ones!

> (Also I've fixed the OVMF driver; just reposting the patch today with a
> better commit message.)
>
> Thanks much!
> Laszlo

Cheers,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

--- virtio-spec.txt-old	2013-04-23 13:22:21.339158214 +0930
+++ virtio-spec.txt	2013-04-23 13:34:14.055176464 +0930
@@ -482,10 +482,10 @@ 
 
 2.3.4 Available Ring
 
-The available ring refers to what descriptors we are offering the 
-device: it refers to the head of a descriptor chain. The “flags” 
+The available ring refers to what descriptor chains we are offering the
+device: each entry refers to the head of a descriptor chain. The “flags”
 field is currently 0 or 1: 1 indicating that we do not need an 
-interrupt when the device consumes a descriptor from the 
+interrupt when the device consumes a descriptor chain from the
 available ring. Alternatively, the guest can ask the device to 
 delay interrupts until an entry with an index specified by the “
 used_event” field is written in the used ring (equivalently, 
@@ -671,16 +671,16 @@ 
 
 avail->ring[avail->idx % qsz] = head;
 
-However, in general we can add many descriptors before we update 
-the “idx” field (at which point they become visible to the 
-device), so we keep a counter of how many we've added:
+However, in general we can add many separate descriptor chains before we update
+the “idx” field (at which point they become visible to the device),
+so we keep a counter of how many we've added:
 
 avail->ring[(avail->idx + added++) % qsz] = head;
 
 2.4.1.3 Updating The Index Field
 
 Once the idx field of the virtqueue is updated, the device will 
-be able to access the descriptor entries we've created and the 
+be able to access the descriptor chains we've created and the 
 memory they refer to. This is why a memory barrier is generally 
 used before the idx update, to ensure it sees the most up-to-date 
 copy.