Additional virtio-mmio spec changes

Additional virtio-mmio spec changes

[Pawel Moll]

Hi Rusty,

Peter (on Cc) got the qemu implementation up and running (thanks!) and
provided some useful feedback regarding the spec. To make the long story
short, this is what came out of the discussion (feel free to correct me
at any point, Peter ;-):

diff --git a/virtio-spec.lyx b/virtio-spec.lyx
index 6426f8f..79a6498 100644
--- a/virtio-spec.lyx
+++ b/virtio-spec.lyx
@@ -6681,7 +6681,9 @@ s on QueueNum, QueueAlign and QueuePFN apply to.
 
  Writing to this register notifies the Host what size of the queue the Guest
  will use.
- This applies to the queue selected by writing to QueueSel.
+ This applies to the queue selected by writing to QueueSel and is allowed
+ only when QueuePFN is set to zero (0x0), so when the queue is not actively
+ used.
  
 \end_layout
 
@@ -6790,8 +6792,8 @@ This interrupt is asserted when configuration of the device has changed.
 
  Writing to this register notifies the Host that the Guest finished handling
  interrupts.
- Every bit of the value clears corresponding bit of the InterruptStatus
- register.
+ Set bits in the value clear the corresponding bits of the
+ InterruptStatus register.
  
 \end_layout
 
@@ -6975,7 +6977,9 @@ reference "sub:Notifying-The-Device"
 \end_layout
 
 \begin_layout Enumerate
-The memory mapped virtio device is using single, dedicated interrupt signal.
+The memory mapped virtio device is using single, dedicated interrupt signal,
+ which is raised when at least one of the interrupts described in the
+ InterruptStatus register description is asserted.
  After receiving an interrupt, the driver must read the InterruptStatus
  register to check what caused the interrupt (see the register description).
  After the interrupt is handled, the driver must acknowledge it by writing



Of course those changes are not critical so can easily wait till the
next release. Peter also mentioned that he didn't like the "Num" in
"QueueNum" register name and would rather see it called "QueueSize". I
have no strong opinions either way and can provide patches to both the
spec and the driver (and header) to change it. Any thoughts?

Cheers!

Paweł

Re: Additional virtio-mmio spec changes

[Rusty Russell]

On Tue, 01 Nov 2011 13:01:39 +0000, Pawel Moll <pawel.moll@arm.com> wrote:
> Hi Rusty,
> 
> Peter (on Cc) got the qemu implementation up and running (thanks!) and
> provided some useful feedback regarding the spec. To make the long story
> short, this is what came out of the discussion (feel free to correct me
> at any point, Peter ;-):

OK, I applied them manually.  One of the pains of using LyX is that you
need to turn change tracking on before making changes (I really like to
produce change-tracking versions of new spec releases).

> Of course those changes are not critical so can easily wait till the
> next release. Peter also mentioned that he didn't like the "Num" in
> "QueueNum" register name and would rather see it called "QueueSize". I
> have no strong opinions either way and can provide patches to both the
> spec and the driver (and header) to change it. Any thoughts?

Num isn't great, but Size might be misleading.  I'd leave it...

Thanks,
Rusty.

Re: Additional virtio-mmio spec changes

[Peter Maydell]

Rusty wrote:
>Pawel Moll <pawel.moll@arm.com> wrote:
>> Peter also mentioned that he didn't like the "Num" in
>> "QueueNum" register name and would rather see it called "QueueSize". I
>> have no strong opinions either way and can provide patches to both the
>> spec and the driver (and header) to change it. Any thoughts?
>
> Num isn't great, but Size might be misleading.  I'd leave it...

I'd note that calling it QueueNum  is inconsistent with the main
virtio spec, which calls the equivalent virtio header field
"Queue Size". Also if the docs say

"QueueNum
 Virtual queue size.
 Queue size is a number of elements in the queue [...]"

this rather suggests to me that we've picked the wrong name for
the register...

-- PMM