On Thu, Jan 30, 2014 at 12:38:38PM +0200, Michael S. Tsirkin wrote:
> On Wed, Jan 29, 2014 at 03:04:50PM +1030, Rusty Russell wrote:
> > Hi Laura,
> >
> > Thanks for the feedback!
> >
> > Laura Novich <> writes:
> > > If you can't open the document here are my comments
> >
> > Hmm, attachment got stripped?
> >
> > > INTRODUCTION
> > > ===============
> > > 1. Remove The entire first paragraph with the exveption of the first sentence.
> > > 2. Replace with - A virtual device in its truest sense is not any different from a physical device. The virtual machine sees a virtual device just as a physical machine would see a physical one. As such, this document will treat virtual devices as physical devices.
> >
> > This isn't quite correct. Consider:
> >
> > This document describes the specifications of the “virtio”
> > family of devices. These devices are found in virtual
> > environments
> >
> > vs:
> > This document describes the specifications of the “virtio”
> > family of devices. A virtual device in its truest sense is not
> > any different from a physical device.
> >
> > We need to say that virtio devices are found in virtual environments.
> >
> > Secondly, you are missing historical context: Xen and HyperV treat
> > virtual devices as very different beasts than physical devices. It is
> > not a fundamental feature of these devices that they are no different
> > from a physical device, but rather a conscious decision by the spec
> > authors.
> >
> > > 3. The purpose of ...
> > > 4. replace rather than using boutique to rather than using vendor-specific, per-environment, or per-OS mechanisms.
> >
> > Is "boutique" too obscure? I like it because it casts an image of the
> > alternatives being niche. That may be wishful thinking, but it was a
> > deliberate choice :)
> >
> > > 5. ADD -The categories described above are further defined as follows:
> > > Straightforward: Revise last sentence to read - There aren’t any exotic page-flipping or COW mechanisms and it functions as a normal device.[1]
> >
> > The end of this sentence is not meaningful. Compare:
> >
> > There is no exotic page-flipping or COW mechanism: it's just
> > a normal device.
> >
> > The ": it's just..." emphasizes the former. With your variant:
> >
> >
> > There aren’t any exotic page-flipping or COW mechanisms and it
> > functions as a normal device.
> >
> > It's not clear what "functions as a normal device" means.
> >
> > > 6. Efficient: ADD to input and output (I/O),
> >
> > Again, I disagree. I/O conflates input and output, whereas I wish to
> > call them both out separately. Perhaps we should add the word "both",
> > ie:
> >
> > descriptors for both input and output...
> >
> > > 7. effects from both guest and (not sure but i think you should ADD host) device writing to the same cache
> > > lines.
> >
> > Throughout the document, we explicitly refer to "device" and "driver"
> > rather than host and guest. This reinforces the design decision that
> > they should appear "normal", as well as being more specific.
> >
> > > 8. Standard: CHANGE TO READ: Virtio makes no assumptions about the environment in which
> > > it operates, beyond what is required in order to allow the bus to attach the host device to the driver on the guest. Virtio
> > > devices are implemented over PCI and other buses, and earlier drafts that
> > > have been implemented on other buses not included in this specification.[2]
> >
> > You suggest replacing "beyond supporting the bus attaching the device."
> > with "beyond what is required in order to allow the bus to attach the
> > host device to the driver on the guest."
> >
> > What do these extra words add?
> >
> > There is a missing word "have", but it doesn't need "that".
> >
> > ...and earlier drafts been implemented on other buses not included
> > in this spec.
> >
> > And I'll accept changing "spec" to "specification".
> >
> > So this is the difference I have so far from your feedback:
> >
> > diff --git a/introduction.tex b/introduction.tex
> > index 5d57f78..98e1e0d 100644
> > --- a/introduction.tex
> > +++ b/introduction.tex
> > @@ -13,14 +13,14 @@ inter-guest communication) requires copying.
> > }
> >
> > Efficient: Virtio devices consist of rings of descriptors
> > - for input and output, which are neatly separated to avoid cache
> > + for both input and output, which are neatly separated to avoid cache
> > effects from both driver and device writing to the same cache
> > lines.
>
> missed this one. I agree.
Actually, rereading this, it's still IMHO not very clear what is separated
from what. descriptors?
I think output here means avail and input used?
If yes then let's say so:
+ for both input and output. Input rings are neatly separated from output rings
+ to avoid cache
Alternatively if we want to be more precise:
Really descriptors are not in a ring at all:
they are in the buffer. OTOH used ring is separated from
avail ring, but they are not for input and output:
input and output are in descriptor buffer.
How about
+ Efficient: Virtio devices consist of a memory buffer with descriptors
+ for both input and output as well as memory rings of indices
+ for driver-to-device and device-to-driver communication.
+ device-to-driver rings are neatly separated from driver-to-device
+ rings to avoid cache
Though I think the shorter version is fine for introduction.
> > Standard: Virtio makes no assumptions about the environment in which
> > it operates, beyond supporting the bus attaching the device. Virtio
> > devices are implemented over PCI and other buses, and earlier drafts
> > - been implemented on other buses not included in this spec.
> > + have been implemented on other buses not included in this specification.
> > \footnote{The Linux implementation further separates the PCI virtio code
> > from the specific virtio drivers: these drivers are shared with
> > the non-PCI implementations (currently lguest and S/390).
>
> I tweaked this one a bit more.
> Can you pls tell me whether you agree with the patchset I posted?
> I think it clarifies misunderstandings (Laura might not be the
> only one misreading this text) without losing context.
>
> > Cheers,
> > Rusty.
> >
> >
> > This publicly archived list offers a means to provide input to the
> > OASIS Virtual I/O Device (VIRTIO) TC.
> >
> > In order to verify user consent to the Feedback License terms and
> > to minimize spam in the list archive, subscription is required
> > before posting.
> >
> > Subscribe:
> > Unsubscribe:
> > List help:
> > List archive: https://lists.oasis-open.org/archives/virtio-comment/
> > Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
> > List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
> > Committee: https://www.oasis-open.org/committees/virtio/
> > Join OASIS: https://www.oasis-open.org/join/
>
> This publicly archived list offers a means to provide input to the
> OASIS Virtual I/O Device (VIRTIO) TC.
>
> In order to verify user consent to the Feedback License terms and
> to minimize spam in the list archive, subscription is required
> before posting.
>
> Subscribe:
> Unsubscribe:
> List help:
> List archive: https://lists.oasis-open.org/archives/virtio-comment/
> Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
> List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
> Committee: https://www.oasis-open.org/committees/virtio/
> Join OASIS: https://www.oasis-open.org/join/
