[virtio-dev] Re: [PATCH v1 2/2] virtio-mmio: add features for virtio-mmio specification version 3

2020-01-08 Thread Liu, Jing2



On 1/5/2020 7:04 PM, Michael S. Tsirkin wrote:

On Wed, Dec 25, 2019 at 10:50:23AM +0800, Zha Bin wrote:

From: Liu Jiang

Userspace VMMs (e.g. Qemu microvm, Firecracker) take advantage of using
virtio over mmio devices as a lightweight machine model for modern
cloud. The standard virtio over MMIO transport layer only supports one
legacy interrupt, which is much heavier than virtio over PCI transport
layer using MSI. Legacy interrupt has long work path and causes specific
VMExits in following cases, which would considerably slow down the
performance:

1) read interrupt status register
2) update interrupt status register
3) write IOAPIC EOI register

We proposed to update virtio over MMIO to version 3[1] to add the
following new features and enhance the performance.

1) Support Message Signaled Interrupt(MSI), which increases the
interrupt performance for virtio multi-queue devices
2) Support per-queue doorbell, so the guest kernel may directly write
to the doorbells provided by virtio devices.

Do we need to come up with new "doorbell" terminology?
virtio spec calls these available event notifications,
let's stick to this.


Yes, let's keep virtio words, which just calls notifications.


The following is the network tcp_rr performance testing report, tested
with virtio-pci device, vanilla virtio-mmio device and patched
virtio-mmio device (run test 3 times for each case):

netperf -t TCP_RR -H 192.168.1.36 -l 30 -- -r 32,1024

Virtio-PCIVirtio-MMIO   Virtio-MMIO(MSI)
trans/s 953669399500
trans/s 973470299749
trans/s 989470959318

[1]https://lkml.org/lkml/2019/12/20/113

Signed-off-by: Liu Jiang
Signed-off-by: Zha Bin
Signed-off-by: Chao Peng
Signed-off-by: Jing Liu

Do we need a new version though? What is wrong with
a feature bit? This way we can create compatible devices
and drivers.


We considered using 1 feature bit of 24~37 to specify MSI capability, but

this feature bit only means for mmio transport layer, but not representing

comment feature negotiation of the virtio device. So we're not sure if 
this is a good choice.



[...]
  
+static void mmio_write_msi_msg(struct msi_desc *desc, struct msi_msg *msg)

+{
+   struct device *dev = desc->dev;
+   struct virtio_device *vdev = dev_to_virtio(dev);
+   struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
+   void __iomem *pos = vm_dev->base;
+   uint16_t cmd = VIRTIO_MMIO_MSI_CMD(VIRTIO_MMIO_MSI_CMD_UPDATE,
+   desc->platform.msi_index);
+
+   writel(msg->address_lo, pos + VIRTIO_MMIO_MSI_ADDRESS_LOW);
+   writel(msg->address_hi, pos + VIRTIO_MMIO_MSI_ADDRESS_HIGH);
+   writel(msg->data, pos + VIRTIO_MMIO_MSI_DATA);
+   writew(cmd, pos + VIRTIO_MMIO_MSI_COMMAND);
+}

All this can happen when IRQ affinity changes while device
is sending interrupts. An interrupt sent between the writel
operations will then be directed incorrectly.


When investigating kernel MSI behavior, I found in most case there's no 
action during IRQ affinity changes to avoid the interrupt coming.


For example, when migrate_one_irq, it masks the irq before 
irq_do_set_affinity. But for others, like user setting any irq affinity


via /proc/, it only holds desc->lock instead of disable/mask irq. In 
such case, how can it ensure the interrupt sending between writel ops?




[...]
+
+/* RO: MSI feature enabled mask */
+#define VIRTIO_MMIO_MSI_ENABLE_MASK0x8000

I don't understand the comment. Is this a way for
a version 3 device to say "I want/do not want MSI"?
Why not just use a feature bit? We are not short on these.


This is just used for current MSI enabled/disabled status, after all MSI 
configurations setup finished.


Not for showing MSI capability.

In other words, since the concern of feature bit, we choose to update 
the virtio mmio


version that devices with v3 have MSI capability and notifications.


Thanks,

Jing


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



Re: [virtio-dev][PATCH v1 1/2] virtio-mmio: Add MSI and different notification address support

2020-01-08 Thread Liu, Jing2



On 1/7/2020 12:18 AM, Stefan Hajnoczi wrote:

On Fri, Dec 20, 2019 at 11:25:03PM +0800, Jing Liu wrote:

Upgrade virtio-mmio to version 3, with the abilities to support
MSI interrupt and notification features.

The details of version 2 will be appended as part of legacy interface
in next patch.

Cool, MSI is useful.  Not a full review, but some comments below...


Hi Stefan,

Thanks for reviewing patches! Glad to see that MSI is welcome.


Signed-off-by: Jing Liu
Signed-off-by: Chao Peng
Signed-off-by: Zha Bin
Signed-off-by: Liu Jiang
---
  content.tex  | 191 ---
  msi-status.c |   5 ++
  2 files changed, 163 insertions(+), 33 deletions(-)
  create mode 100644 msi-status.c

diff --git a/content.tex b/content.tex
index d68cfaf..eaaffec 100644
--- a/content.tex
+++ b/content.tex
@@ -1597,9 +1597,9 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio 
Transport Options / Vi
}
\hline
\mmioreg{Version}{Device version number}{0x004}{R}{%
-0x2.
+0x3.
  \begin{note}
-  Legacy devices (see \ref{sec:Virtio Transport Options / Virtio Over MMIO 
/ Legacy interface}~\nameref{sec:Virtio Transport Options / Virtio Over MMIO / 
Legacy interface}) used 0x1.
+  Legacy devices (see \ref{sec:Virtio Transport Options / Virtio Over MMIO 
/ Legacy interface}~\nameref{sec:Virtio Transport Options / Virtio Over MMIO / 
Legacy interface}) used 0x1 or 0x2.

"Legacy devices" refers to pre-VIRTIO 1.0 devices.  0x2 is VIRTIO 1.0
and therefore not "Legacy".  I suggest the following wording:

   Legacy devices (see \ref{sec:Virtio Transport Options / Virtio Over MMIO 
/ Legacy interface}~\nameref{sec:Virtio Transport Options / Virtio Over MMIO / 
Legacy interface}) used 0x1.
+ VIRTIO 1.0 and 1.1 used 0x2.

Thanks for the guide.

Did you consider using a transport feature bit instead of changing the
device version number?  Feature bits allow more graceful compatibility:
old drivers will continue to work with new devices and new drivers will
continue to work with old devices.


Yes, we considered using a feature bit from 24~37 or above, while a 
concern is that,


the device which uses such bit only represents the behavior of mmio 
transport layer but not common behavior


of virtio device. Or am I missing some "transport" feature bit range?



  \end{note}
}
\hline
@@ -1671,25 +1671,23 @@ \subsection{MMIO Device Register 
Layout}\label{sec:Virtio Transport Options / Vi
  accesses apply to the queue selected by writing to \field{QueueSel}.
}
\hline
-  \mmioreg{QueueNotify}{Queue notifier}{0x050}{W}{%
-Writing a value to this register notifies the device that
-there are new buffers to process in a queue.
+  \mmioreg{QueueNotify}{Queue notifier}{0x050}{RW}{%
+Reading from the register returns the virtqueue notification configuration.
  
-When VIRTIO_F_NOTIFICATION_DATA has not been negotiated,

-the value written is the queue index.
+See \ref{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific 
Initialization And Device Operation / Notification Address}
+for the configuration format.
  
-When VIRTIO_F_NOTIFICATION_DATA has been negotiated,

-the \field{Notification data} value has the following format:
+Writing when the notification address calculated by the notification 
configuration
+is just located at this register.

I don't understand this sentence.  What happens when the driver writes
to this register?


We're trying to define the notification mechanism that, driver MUST read 
0x50 to get the notification configuration


and calculate the notify address. The writing case here is that, the 
notify address is just located here e.g. notify_base=0x50, notify_mul=0.



  
-\lstinputlisting{notifications-le.c}

-
-See \ref{sec:Virtqueues / Driver notifications}~\nameref{sec:Virtqueues / 
Driver notifications}
-for the definition of the components.
+See \ref{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific 
Initialization And Device Operation / Available Buffer Notifications}
+to see the notification data format.
}
\hline
\mmioreg{InterruptStatus}{Interrupt status}{0x60}{R}{%
  Reading from this register returns a bit mask of events that
-caused the device interrupt to be asserted.
+caused the device interrupt to be asserted. This is only used
+when MSI is not enabled.
  The following events are possible:
  \begin{description}
\item[Used Buffer Notification] - bit 0 - the interrupt was asserted
@@ -1703,7 +1701,7 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio 
Transport Options / Vi
\mmioreg{InterruptACK}{Interrupt acknowledge}{0x064}{W}{%
  Writing a value with bits set as defined in \field{InterruptStatus}
  to this register notifies the device that events causing
-the interrupt have been handled.
+the interrupt have been handled. This is only us

Re: [virtio-dev] Re: [PATCH v2 1/1] virtio-video: Add virtio video device specification

2020-01-08 Thread Keiichi Watanabe
Hi Gerd,

On Thu, Dec 19, 2019 at 10:12 PM Gerd Hoffmann  wrote:
>
>   Hi,
>
> > > However that still doesn't let the driver know which buffers will be
> > > dequeued when. A simple example of this scenario is when the guest is
> > > done displaying a frame and requeues the buffer back to the decoder.
> > > Then the decoder will not choose it for decoding next frames into as
> > > long as the frame in that buffer is still used as a reference frame,
> > > even if one sends the drain request.
> > It might be that I'm getting your point wrong, but do you mean some hardware
> > can mark a buffer as ready to be displayed yet still using the underlying
> > memory to decode other frames?
>
> Yes, this is how I understand Tomasz Figa.
>
> > This means, if you occasionally/intentionally
> > write to the buffer you mess up the whole decoding pipeline.
>
> And to avoid this the buffer handling aspect must be clarified in the
> specification.  Is the device allowed to continue using the buffer after
> finishing decoding and completing the queue request?  If so, how do we
> hand over buffer ownership back to the driver so it can free the pages?
> drain request?  How do we handle re-using buffers?  Can the driver
> simply re-queue them and expect the device figures by itself whenever it
> can use the buffer or whenever it is still needed as reference frame?

The device shouldn't be able to access buffers after it completes a
queue request.
The device can touch buffer contents from when a queue request is sent
until the device responds it.
In contrast, the driver must not modify buffer contents in that period.

Regarding re-using, the driver can simply re-queue buffers returned by
the device. If the device needs a buffer as reference frame, it must
not return the buffer.
I'll describe this rule in the next version of the patch.

Best regards,
Keiichi

>
> cheers,
>   Gerd
>

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



Re: [virtio-dev] Re: [PATCH v2 1/1] virtio-video: Add virtio video device specification

2020-01-08 Thread Keiichi Watanabe
Hi Gerd,
Thank you so much for the review. I'm sorry for not replying earlier.

On Thu, Dec 19, 2019 at 10:02 PM Gerd Hoffmann  wrote:
>
>   Hi,
>
> > > Not clearly defined in the spec:  When is the decoder supposed to send
> > > the response for a queue request?  When it finished decoding (i.e. frame
> > > is ready for playback), or when it doesn't need the buffer any more for
> > > decoding (i.e. buffer can be re-queued or pages can be released)?

The answer is "when it doesn't need the buffer any more for decoding".
The device can access buffer contents from when a queue request is
sent until the device responds it. So, the device must not responds a
queue request before finishing all process that requires the buffer
content.

Actually, the first one "When it finished decoding (i.e. frame is
ready for playback)" doesn't make much sense, as it's not necessary to
have a one-to-one correspondence between an input bitstream buffer and
a decoded frame.
It's okay to decode one input buffer contains bitstream data for two
frames. Also, a user can pass bitstream for one frame as two input
buffers.

I'll document it in the spec.

Best regards,
Keiichi

> > In my eyes the both statements mean almost the same and both are valid.
>
> Well, no.  When the device decoded a P-Frame it can notify the device,
> saying "here is your decoded frame".  But the device might still need
> the buffer with the decoded frame to properly decode the following B/I
> Frames which reference the P-Frame.
>
> cheers,
>   Gerd
>

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



Re: [virtio-dev] Re: [PATCH v2 1/1] virtio-video: Add virtio video device specification

2020-01-08 Thread Keiichi Watanabe
On Wed, Jan 8, 2020 at 10:11 PM Dmitry Sepp  wrote:
>
> Hi Tomasz, Keiichi,
>
> On Mittwoch, 8. Januar 2020 13:46:25 CET Tomasz Figa wrote:
> > On Wed, Jan 8, 2020 at 9:15 PM Keiichi Watanabe 
> wrote:
> > > Hi Dmitry,
> > >
> > > On Wed, Jan 8, 2020 at 7:00 PM Dmitry Sepp 
> wrote:
> > > > Hi Keiichi,
> > > >
> > > > On Mittwoch, 8. Januar 2020 07:59:22 CET Keiichi Watanabe wrote:
> > > > > Hi Dmitry,
> > > > >
> > > > > On Wed, Jan 8, 2020 at 1:50 AM Dmitry Sepp
> > > > > 
> > > >
> > > > wrote:
> > > > > > Hi Keiichi,
> > > > > >
> > > > > > thanks for the updates, please see my comments below.
> > > > > >
> > > > > > On Dienstag, 7. Januar 2020 14:24:31 CET Keiichi Watanabe wrote:
> > > > > > > Hi Dmitry,
> > > > > > >
> > > > > > > On Mon, Jan 6, 2020 at 11:59 PM Dmitry Sepp
> > > > > > > 
> > > > > >
> > > > > > wrote:
> > > > > > > > Hi,
> > > > > > > >
> > > > > > > > a couple of new comments:
> > > > > > > >
> > > > > > > > On Mittwoch, 18. Dezember 2019 14:02:14 CET Keiichi Watanabe
> wrote:
> > > > > > > > > From: Dmitry Sepp 
> > > > > > > > >
> > > > > > > > > The virtio video encoder device and decoder device provide
> > > > > > > > > functionalities
> > > > > > > > > to encode and decode video stream respectively.
> > > > > > > > > Though video encoder and decoder are provided as different
> > > > > > > > > devices,
> > > > > > > > > they
> > > > > > > > > use a same protocol.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Dmitry Sepp 
> > > > > > > > > Signed-off-by: Keiichi Watanabe 
> > > > > > > > > ---
> > > > > > > > >
> > > > > > > > >  content.tex  |   1 +
> > > > > > > > >  virtio-video.tex | 579
> > > > > > > > >  +++
> > > > > > > > >  2 files changed, 580 insertions(+)
> > > > > > > > >  create mode 100644 virtio-video.tex
> > > > > > > > >
> > > > > > > > > diff --git a/content.tex b/content.tex
> > > > > > > > > index 556b373..9e56839 100644
> > > > > > > > > --- a/content.tex
> > > > > > > > > +++ b/content.tex
> > > > > > > > > @@ -5743,6 +5743,7 @@ \subsubsection{Legacy Interface: Framing
> > > > > > > > > Requirements}\label{sec:Device \input{virtio-vsock.tex}
> > > > > > > > >
> > > > > > > > >  \input{virtio-fs.tex}
> > > > > > > > >  \input{virtio-rpmb.tex}
> > > > > > > > >
> > > > > > > > > +\input{virtio-video.tex}
> > > > > > > > >
> > > > > > > > >  \chapter{Reserved Feature Bits}\label{sec:Reserved Feature
> > > > > > > > >  Bits}
> > > > > > > > >
> > > > > > > > > diff --git a/virtio-video.tex b/virtio-video.tex
> > > > > > > > > new file mode 100644
> > > > > > > > > index 000..30e728d
> > > > > > > > > --- /dev/null
> > > > > > > > > +++ b/virtio-video.tex
> > > > > > > > > @@ -0,0 +1,579 @@
> > > > > > > > > +\section{Video Device}\label{sec:Device Types / Video Device}
> > > > > > > > > +
> > > > > > > > > +The virtio video encoder device and decoder device are
> > > > > > > > > virtual
> > > > > > > > > devices
> > > > > > > > > that +supports encoding and decoding respectively. Though the
> > > > > > > > > encoder
> > > > > > > > > and the decoder +are different devices, they use the same
> > > > > > > > > protocol.
> > > > > > > > > +
> > > > > > > > > +\subsection{Device ID}\label{sec:Device Types / Video Device
> > > > > > > > > /
> > > > > > > > > Device
> > > > > > > > > ID}
> > > > > > > > > +
> > > > > > > > > +\begin{description}
> > > > > > > > > +\item[30] encoder device
> > > > > > > > > +\item[31] decoder device
> > > > > > > > > +\end{description}
> > > > > > > > > +
> > > > > > > > > +\subsection{Virtqueues}\label{sec:Device Types / Video Device
> > > > > > > > > /
> > > > > > > > > Virtqueues} +
> > > > > > > > > +\begin{description}
> > > > > > > > > +\item[0] controlq - queue for sending control commands.
> > > > > > > > > +\item[1] eventq - queue for sending events happened in the
> > > > > > > > > device.
> > > > > > > > > +\end{description}
> > > > > > > > > +
> > > > > > > > > +\subsection{Feature bits}\label{sec:Device Types / Video
> > > > > > > > > Device /
> > > > > > > > > Feature
> > > > > > > > > bits} +
> > > > > > > > > +\begin{description}
> > > > > > > > > +\item[VIRTIO_VIDEO_F_RESOURCE_GUEST_PAGES (0)] Guest pages
> > > > > > > > > can be
> > > > > > > > > used
> > > > > > > > > for
> > > > > > > > > video +  buffers.
> > > > > > > > > +\end{description}
> > > > > > > > > +
> > > > > > > > > +\devicenormative{\subsubsection}{Feature bits}{Device Types /
> > > > > > > > > Video
> > > > > > > > > Device
> > > > > > > > > / Feature bits} +
> > > > > > > > > +The device MUST offer at least one of feature bits.
> > > > > > > > > +
> > > > > > > > > +\subsection{Device configuration layout}\label{sec:Device
> > > > > > > > > Types /
> > > > > > > > > Video
> > > > > > > > > Device / Device configuration layout} +
> > > > > > > > > +Video device configuration uses the following layout
> > > > > > > > > structure:
> > > > > > > > > +
> > > > > > > > > +\begin{lstlisting}
> > > 

[virtio-dev] Re: [PATCH v2 1/1] virtio-video: Add virtio video device specification

2020-01-08 Thread Dmitry Sepp
Hi Tomasz, Keiichi,

On Mittwoch, 8. Januar 2020 13:46:25 CET Tomasz Figa wrote:
> On Wed, Jan 8, 2020 at 9:15 PM Keiichi Watanabe  
wrote:
> > Hi Dmitry,
> > 
> > On Wed, Jan 8, 2020 at 7:00 PM Dmitry Sepp  
wrote:
> > > Hi Keiichi,
> > > 
> > > On Mittwoch, 8. Januar 2020 07:59:22 CET Keiichi Watanabe wrote:
> > > > Hi Dmitry,
> > > > 
> > > > On Wed, Jan 8, 2020 at 1:50 AM Dmitry Sepp
> > > > 
> > > 
> > > wrote:
> > > > > Hi Keiichi,
> > > > > 
> > > > > thanks for the updates, please see my comments below.
> > > > > 
> > > > > On Dienstag, 7. Januar 2020 14:24:31 CET Keiichi Watanabe wrote:
> > > > > > Hi Dmitry,
> > > > > > 
> > > > > > On Mon, Jan 6, 2020 at 11:59 PM Dmitry Sepp
> > > > > > 
> > > > > 
> > > > > wrote:
> > > > > > > Hi,
> > > > > > > 
> > > > > > > a couple of new comments:
> > > > > > > 
> > > > > > > On Mittwoch, 18. Dezember 2019 14:02:14 CET Keiichi Watanabe 
wrote:
> > > > > > > > From: Dmitry Sepp 
> > > > > > > > 
> > > > > > > > The virtio video encoder device and decoder device provide
> > > > > > > > functionalities
> > > > > > > > to encode and decode video stream respectively.
> > > > > > > > Though video encoder and decoder are provided as different
> > > > > > > > devices,
> > > > > > > > they
> > > > > > > > use a same protocol.
> > > > > > > > 
> > > > > > > > Signed-off-by: Dmitry Sepp 
> > > > > > > > Signed-off-by: Keiichi Watanabe 
> > > > > > > > ---
> > > > > > > > 
> > > > > > > >  content.tex  |   1 +
> > > > > > > >  virtio-video.tex | 579
> > > > > > > >  +++
> > > > > > > >  2 files changed, 580 insertions(+)
> > > > > > > >  create mode 100644 virtio-video.tex
> > > > > > > > 
> > > > > > > > diff --git a/content.tex b/content.tex
> > > > > > > > index 556b373..9e56839 100644
> > > > > > > > --- a/content.tex
> > > > > > > > +++ b/content.tex
> > > > > > > > @@ -5743,6 +5743,7 @@ \subsubsection{Legacy Interface: Framing
> > > > > > > > Requirements}\label{sec:Device \input{virtio-vsock.tex}
> > > > > > > > 
> > > > > > > >  \input{virtio-fs.tex}
> > > > > > > >  \input{virtio-rpmb.tex}
> > > > > > > > 
> > > > > > > > +\input{virtio-video.tex}
> > > > > > > > 
> > > > > > > >  \chapter{Reserved Feature Bits}\label{sec:Reserved Feature
> > > > > > > >  Bits}
> > > > > > > > 
> > > > > > > > diff --git a/virtio-video.tex b/virtio-video.tex
> > > > > > > > new file mode 100644
> > > > > > > > index 000..30e728d
> > > > > > > > --- /dev/null
> > > > > > > > +++ b/virtio-video.tex
> > > > > > > > @@ -0,0 +1,579 @@
> > > > > > > > +\section{Video Device}\label{sec:Device Types / Video Device}
> > > > > > > > +
> > > > > > > > +The virtio video encoder device and decoder device are
> > > > > > > > virtual
> > > > > > > > devices
> > > > > > > > that +supports encoding and decoding respectively. Though the
> > > > > > > > encoder
> > > > > > > > and the decoder +are different devices, they use the same
> > > > > > > > protocol.
> > > > > > > > +
> > > > > > > > +\subsection{Device ID}\label{sec:Device Types / Video Device
> > > > > > > > /
> > > > > > > > Device
> > > > > > > > ID}
> > > > > > > > +
> > > > > > > > +\begin{description}
> > > > > > > > +\item[30] encoder device
> > > > > > > > +\item[31] decoder device
> > > > > > > > +\end{description}
> > > > > > > > +
> > > > > > > > +\subsection{Virtqueues}\label{sec:Device Types / Video Device
> > > > > > > > /
> > > > > > > > Virtqueues} +
> > > > > > > > +\begin{description}
> > > > > > > > +\item[0] controlq - queue for sending control commands.
> > > > > > > > +\item[1] eventq - queue for sending events happened in the
> > > > > > > > device.
> > > > > > > > +\end{description}
> > > > > > > > +
> > > > > > > > +\subsection{Feature bits}\label{sec:Device Types / Video
> > > > > > > > Device /
> > > > > > > > Feature
> > > > > > > > bits} +
> > > > > > > > +\begin{description}
> > > > > > > > +\item[VIRTIO_VIDEO_F_RESOURCE_GUEST_PAGES (0)] Guest pages
> > > > > > > > can be
> > > > > > > > used
> > > > > > > > for
> > > > > > > > video +  buffers.
> > > > > > > > +\end{description}
> > > > > > > > +
> > > > > > > > +\devicenormative{\subsubsection}{Feature bits}{Device Types /
> > > > > > > > Video
> > > > > > > > Device
> > > > > > > > / Feature bits} +
> > > > > > > > +The device MUST offer at least one of feature bits.
> > > > > > > > +
> > > > > > > > +\subsection{Device configuration layout}\label{sec:Device
> > > > > > > > Types /
> > > > > > > > Video
> > > > > > > > Device / Device configuration layout} +
> > > > > > > > +Video device configuration uses the following layout
> > > > > > > > structure:
> > > > > > > > +
> > > > > > > > +\begin{lstlisting}
> > > > > > > > +struct virtio_video_config {
> > > > > > > > +le32 max_cap_len;
> > > > > > > > +};
> > > > > > > > +\end{lstlisting}
> > > > > > > > +
> > > > > > > > +\begin{description}
> > > > > > > > +\item[\field{max_cap_len}] defines the maximum length of a
> > > > > > > > descriptor

[virtio-dev] Re: [PATCH v2 1/1] virtio-video: Add virtio video device specification

2020-01-08 Thread Keiichi Watanabe
On Wed, Jan 8, 2020 at 9:46 PM Tomasz Figa  wrote:
>
> On Wed, Jan 8, 2020 at 9:15 PM Keiichi Watanabe  wrote:
> >
> > Hi Dmitry,
> >
> > On Wed, Jan 8, 2020 at 7:00 PM Dmitry Sepp  
> > wrote:
> > >
> > > Hi Keiichi,
> > >
> > > On Mittwoch, 8. Januar 2020 07:59:22 CET Keiichi Watanabe wrote:
> > > > Hi Dmitry,
> > > >
> > > > On Wed, Jan 8, 2020 at 1:50 AM Dmitry Sepp 
> > > wrote:
> > > > > Hi Keiichi,
> > > > >
> > > > > thanks for the updates, please see my comments below.
> > > > >
> > > > > On Dienstag, 7. Januar 2020 14:24:31 CET Keiichi Watanabe wrote:
> > > > > > Hi Dmitry,
> > > > > >
> > > > > > On Mon, Jan 6, 2020 at 11:59 PM Dmitry Sepp
> > > > > > 
> > > > >
> > > > > wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > a couple of new comments:
> > > > > > >
> > > > > > > On Mittwoch, 18. Dezember 2019 14:02:14 CET Keiichi Watanabe 
> > > > > > > wrote:
> > > > > > > > From: Dmitry Sepp 
> > > > > > > >
> > > > > > > > The virtio video encoder device and decoder device provide
> > > > > > > > functionalities
> > > > > > > > to encode and decode video stream respectively.
> > > > > > > > Though video encoder and decoder are provided as different 
> > > > > > > > devices,
> > > > > > > > they
> > > > > > > > use a same protocol.
> > > > > > > >
> > > > > > > > Signed-off-by: Dmitry Sepp 
> > > > > > > > Signed-off-by: Keiichi Watanabe 
> > > > > > > > ---
> > > > > > > >
> > > > > > > >  content.tex  |   1 +
> > > > > > > >  virtio-video.tex | 579
> > > > > > > >  +++
> > > > > > > >  2 files changed, 580 insertions(+)
> > > > > > > >  create mode 100644 virtio-video.tex
> > > > > > > >
> > > > > > > > diff --git a/content.tex b/content.tex
> > > > > > > > index 556b373..9e56839 100644
> > > > > > > > --- a/content.tex
> > > > > > > > +++ b/content.tex
> > > > > > > > @@ -5743,6 +5743,7 @@ \subsubsection{Legacy Interface: Framing
> > > > > > > > Requirements}\label{sec:Device \input{virtio-vsock.tex}
> > > > > > > >
> > > > > > > >  \input{virtio-fs.tex}
> > > > > > > >  \input{virtio-rpmb.tex}
> > > > > > > >
> > > > > > > > +\input{virtio-video.tex}
> > > > > > > >
> > > > > > > >  \chapter{Reserved Feature Bits}\label{sec:Reserved Feature 
> > > > > > > > Bits}
> > > > > > > >
> > > > > > > > diff --git a/virtio-video.tex b/virtio-video.tex
> > > > > > > > new file mode 100644
> > > > > > > > index 000..30e728d
> > > > > > > > --- /dev/null
> > > > > > > > +++ b/virtio-video.tex
> > > > > > > > @@ -0,0 +1,579 @@
> > > > > > > > +\section{Video Device}\label{sec:Device Types / Video Device}
> > > > > > > > +
> > > > > > > > +The virtio video encoder device and decoder device are virtual
> > > > > > > > devices
> > > > > > > > that +supports encoding and decoding respectively. Though the
> > > > > > > > encoder
> > > > > > > > and the decoder +are different devices, they use the same 
> > > > > > > > protocol.
> > > > > > > > +
> > > > > > > > +\subsection{Device ID}\label{sec:Device Types / Video Device /
> > > > > > > > Device
> > > > > > > > ID}
> > > > > > > > +
> > > > > > > > +\begin{description}
> > > > > > > > +\item[30] encoder device
> > > > > > > > +\item[31] decoder device
> > > > > > > > +\end{description}
> > > > > > > > +
> > > > > > > > +\subsection{Virtqueues}\label{sec:Device Types / Video Device /
> > > > > > > > Virtqueues} +
> > > > > > > > +\begin{description}
> > > > > > > > +\item[0] controlq - queue for sending control commands.
> > > > > > > > +\item[1] eventq - queue for sending events happened in the 
> > > > > > > > device.
> > > > > > > > +\end{description}
> > > > > > > > +
> > > > > > > > +\subsection{Feature bits}\label{sec:Device Types / Video 
> > > > > > > > Device /
> > > > > > > > Feature
> > > > > > > > bits} +
> > > > > > > > +\begin{description}
> > > > > > > > +\item[VIRTIO_VIDEO_F_RESOURCE_GUEST_PAGES (0)] Guest pages can 
> > > > > > > > be
> > > > > > > > used
> > > > > > > > for
> > > > > > > > video +  buffers.
> > > > > > > > +\end{description}
> > > > > > > > +
> > > > > > > > +\devicenormative{\subsubsection}{Feature bits}{Device Types / 
> > > > > > > > Video
> > > > > > > > Device
> > > > > > > > / Feature bits} +
> > > > > > > > +The device MUST offer at least one of feature bits.
> > > > > > > > +
> > > > > > > > +\subsection{Device configuration layout}\label{sec:Device 
> > > > > > > > Types /
> > > > > > > > Video
> > > > > > > > Device / Device configuration layout} +
> > > > > > > > +Video device configuration uses the following layout structure:
> > > > > > > > +
> > > > > > > > +\begin{lstlisting}
> > > > > > > > +struct virtio_video_config {
> > > > > > > > +le32 max_cap_len;
> > > > > > > > +};
> > > > > > > > +\end{lstlisting}
> > > > > > > > +
> > > > > > > > +\begin{description}
> > > > > > > > +\item[\field{max_cap_len}] defines the maximum length of a
> > > > > > > > descriptor
> > > > > > > > +  required to call VIRTIO_VIDEO_GET_CAPABILITY in bytes. The 
> > > > > 

[virtio-dev] Re: [PATCH v2 1/1] virtio-video: Add virtio video device specification

2020-01-08 Thread Tomasz Figa
On Wed, Jan 8, 2020 at 9:15 PM Keiichi Watanabe  wrote:
>
> Hi Dmitry,
>
> On Wed, Jan 8, 2020 at 7:00 PM Dmitry Sepp  
> wrote:
> >
> > Hi Keiichi,
> >
> > On Mittwoch, 8. Januar 2020 07:59:22 CET Keiichi Watanabe wrote:
> > > Hi Dmitry,
> > >
> > > On Wed, Jan 8, 2020 at 1:50 AM Dmitry Sepp 
> > wrote:
> > > > Hi Keiichi,
> > > >
> > > > thanks for the updates, please see my comments below.
> > > >
> > > > On Dienstag, 7. Januar 2020 14:24:31 CET Keiichi Watanabe wrote:
> > > > > Hi Dmitry,
> > > > >
> > > > > On Mon, Jan 6, 2020 at 11:59 PM Dmitry Sepp
> > > > > 
> > > >
> > > > wrote:
> > > > > > Hi,
> > > > > >
> > > > > > a couple of new comments:
> > > > > >
> > > > > > On Mittwoch, 18. Dezember 2019 14:02:14 CET Keiichi Watanabe wrote:
> > > > > > > From: Dmitry Sepp 
> > > > > > >
> > > > > > > The virtio video encoder device and decoder device provide
> > > > > > > functionalities
> > > > > > > to encode and decode video stream respectively.
> > > > > > > Though video encoder and decoder are provided as different 
> > > > > > > devices,
> > > > > > > they
> > > > > > > use a same protocol.
> > > > > > >
> > > > > > > Signed-off-by: Dmitry Sepp 
> > > > > > > Signed-off-by: Keiichi Watanabe 
> > > > > > > ---
> > > > > > >
> > > > > > >  content.tex  |   1 +
> > > > > > >  virtio-video.tex | 579
> > > > > > >  +++
> > > > > > >  2 files changed, 580 insertions(+)
> > > > > > >  create mode 100644 virtio-video.tex
> > > > > > >
> > > > > > > diff --git a/content.tex b/content.tex
> > > > > > > index 556b373..9e56839 100644
> > > > > > > --- a/content.tex
> > > > > > > +++ b/content.tex
> > > > > > > @@ -5743,6 +5743,7 @@ \subsubsection{Legacy Interface: Framing
> > > > > > > Requirements}\label{sec:Device \input{virtio-vsock.tex}
> > > > > > >
> > > > > > >  \input{virtio-fs.tex}
> > > > > > >  \input{virtio-rpmb.tex}
> > > > > > >
> > > > > > > +\input{virtio-video.tex}
> > > > > > >
> > > > > > >  \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> > > > > > >
> > > > > > > diff --git a/virtio-video.tex b/virtio-video.tex
> > > > > > > new file mode 100644
> > > > > > > index 000..30e728d
> > > > > > > --- /dev/null
> > > > > > > +++ b/virtio-video.tex
> > > > > > > @@ -0,0 +1,579 @@
> > > > > > > +\section{Video Device}\label{sec:Device Types / Video Device}
> > > > > > > +
> > > > > > > +The virtio video encoder device and decoder device are virtual
> > > > > > > devices
> > > > > > > that +supports encoding and decoding respectively. Though the
> > > > > > > encoder
> > > > > > > and the decoder +are different devices, they use the same 
> > > > > > > protocol.
> > > > > > > +
> > > > > > > +\subsection{Device ID}\label{sec:Device Types / Video Device /
> > > > > > > Device
> > > > > > > ID}
> > > > > > > +
> > > > > > > +\begin{description}
> > > > > > > +\item[30] encoder device
> > > > > > > +\item[31] decoder device
> > > > > > > +\end{description}
> > > > > > > +
> > > > > > > +\subsection{Virtqueues}\label{sec:Device Types / Video Device /
> > > > > > > Virtqueues} +
> > > > > > > +\begin{description}
> > > > > > > +\item[0] controlq - queue for sending control commands.
> > > > > > > +\item[1] eventq - queue for sending events happened in the 
> > > > > > > device.
> > > > > > > +\end{description}
> > > > > > > +
> > > > > > > +\subsection{Feature bits}\label{sec:Device Types / Video Device /
> > > > > > > Feature
> > > > > > > bits} +
> > > > > > > +\begin{description}
> > > > > > > +\item[VIRTIO_VIDEO_F_RESOURCE_GUEST_PAGES (0)] Guest pages can be
> > > > > > > used
> > > > > > > for
> > > > > > > video +  buffers.
> > > > > > > +\end{description}
> > > > > > > +
> > > > > > > +\devicenormative{\subsubsection}{Feature bits}{Device Types / 
> > > > > > > Video
> > > > > > > Device
> > > > > > > / Feature bits} +
> > > > > > > +The device MUST offer at least one of feature bits.
> > > > > > > +
> > > > > > > +\subsection{Device configuration layout}\label{sec:Device Types /
> > > > > > > Video
> > > > > > > Device / Device configuration layout} +
> > > > > > > +Video device configuration uses the following layout structure:
> > > > > > > +
> > > > > > > +\begin{lstlisting}
> > > > > > > +struct virtio_video_config {
> > > > > > > +le32 max_cap_len;
> > > > > > > +};
> > > > > > > +\end{lstlisting}
> > > > > > > +
> > > > > > > +\begin{description}
> > > > > > > +\item[\field{max_cap_len}] defines the maximum length of a
> > > > > > > descriptor
> > > > > > > +  required to call VIRTIO_VIDEO_GET_CAPABILITY in bytes. The 
> > > > > > > device
> > > > > > > +  MUST set this value.
> > > > > > > +\end{description}
> > > > > > > +
> > > > > > > +\subsection{Device Initialization}\label{sec:Device Types / Video
> > > > > > > Device /
> > > > > > > Device Initialization} +
> > > > > > > +\devicenormative{\subsubsection}{Device Initialization}{Device
> > > > > > > Types /
> > > > > > > Video Device / Device Initializatio

[virtio-dev] Re: [PATCH v2 1/1] virtio-video: Add virtio video device specification

2020-01-08 Thread Keiichi Watanabe
Hi Dmitry,

On Wed, Dec 18, 2019 at 10:02 PM Keiichi Watanabe  wrote:
>
> From: Dmitry Sepp 
>
> The virtio video encoder device and decoder device provide functionalities to
> encode and decode video stream respectively.
> Though video encoder and decoder are provided as different devices, they use a
> same protocol.
>
> Signed-off-by: Dmitry Sepp 
> Signed-off-by: Keiichi Watanabe 
> ---
>  content.tex  |   1 +
>  virtio-video.tex | 579 +++
>  2 files changed, 580 insertions(+)
>  create mode 100644 virtio-video.tex
>
> diff --git a/content.tex b/content.tex
> index 556b373..9e56839 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -5743,6 +5743,7 @@ \subsubsection{Legacy Interface: Framing 
> Requirements}\label{sec:Device
>  \input{virtio-vsock.tex}
>  \input{virtio-fs.tex}
>  \input{virtio-rpmb.tex}
> +\input{virtio-video.tex}
>
>  \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>
> diff --git a/virtio-video.tex b/virtio-video.tex
> new file mode 100644
> index 000..30e728d
> --- /dev/null
> +++ b/virtio-video.tex
> @@ -0,0 +1,579 @@
> +\section{Video Device}\label{sec:Device Types / Video Device}
> +
> +The virtio video encoder device and decoder device are virtual devices that
> +supports encoding and decoding respectively. Though the encoder and the 
> decoder
> +are different devices, they use the same protocol.
> +
> +\subsection{Device ID}\label{sec:Device Types / Video Device / Device ID}
> +
> +\begin{description}
> +\item[30] encoder device
> +\item[31] decoder device
> +\end{description}
> +
> +\subsection{Virtqueues}\label{sec:Device Types / Video Device / Virtqueues}
> +
> +\begin{description}
> +\item[0] controlq - queue for sending control commands.
> +\item[1] eventq - queue for sending events happened in the device.
> +\end{description}
> +
> +\subsection{Feature bits}\label{sec:Device Types / Video Device / Feature 
> bits}
> +
> +\begin{description}
> +\item[VIRTIO_VIDEO_F_RESOURCE_GUEST_PAGES (0)] Guest pages can be used for 
> video
> +  buffers.
> +\end{description}
> +
> +\devicenormative{\subsubsection}{Feature bits}{Device Types / Video Device / 
> Feature bits}
> +
> +The device MUST offer at least one of feature bits.
> +
> +\subsection{Device configuration layout}\label{sec:Device Types / Video 
> Device / Device configuration layout}
> +
> +Video device configuration uses the following layout structure:
> +
> +\begin{lstlisting}
> +struct virtio_video_config {
> +le32 max_cap_len;
> +};
> +\end{lstlisting}
> +
> +\begin{description}
> +\item[\field{max_cap_len}] defines the maximum length of a descriptor
> +  required to call VIRTIO_VIDEO_GET_CAPABILITY in bytes. The device
> +  MUST set this value.
> +\end{description}
> +
> +\subsection{Device Initialization}\label{sec:Device Types / Video Device / 
> Device Initialization}
> +
> +\devicenormative{\subsubsection}{Device Initialization}{Device Types / Video 
> Device / Device Initialization}
> +
> +The driver SHOULD query device capability by using the
> +VIRTIO_VIDEO_T_GET_CAPABILITY and use that information for the initial
> +setup.
> +
> +\subsection{Device Operation}\label{sec:Device Types / Video Device / Device 
> Operation}
> +
> +The driver allocates input and output buffers and queues the buffers
> +to the device. The device performs operations on the buffers according
> +to the function in question.
> +
> +\subsubsection{Device Operation: Create stream}
> +
> +To process buffers, the device needs to associate them with a certain
> +video stream (essentially, a context). Streams are created by
> +VIRTIO_VIDEO_T_STREAM_CREATE with a default set of parameters
> +determined by the device.
> +
> +\subsubsection{Device Operation: Create buffers}
> +
> +Buffers are used to store the actual data as well as the relevant
> +metadata. Scatter lists are supported, so the buffer doesn't need to
> +be contiguous in guest physical memory.
> +
> +\begin{itemize*}
> +\item Use VIRTIO_VIDEO_T_RESOURCE_CREATE to create a virtio video
> +  resource that is backed by a buffer allocated from the driver's
> +  memory.
> +\item Use VIRTIO_VIDEO_T_RESOURCE_DESTROY to destroy a resource that
> +  is no longer needed.
> +\end{itemize*}
> +
> +\subsubsection{Device Operation: Stream parameter control}
> +
> +\begin{itemize*}
> +\item Use VIRTIO_VIDEO_T_GET_PARAMS to get the current stream parameters for
> +  input and output streams from the device.
> +\item Use VIRTIO_VIDEO_T_SET_PARAMS to provide new stream parameters to the
> +  device.
> +\item After setting stream parameters, the driver may issue
> +  VIRTIO_VIDEO_T_GET_PARAMS as some parameters of both input and output can 
> be
> +  changed implicitly by the device during the set operation.
> +\end{itemize*}
> +
> +\subsubsection{Device Operation: Process buffers}
> +
> +\begin{itemize*}
> +\item If the function and the buffer type require so, write data to
> +the buffer memory.
> +\item Use VIRTIO_VIDEO_T

[virtio-dev] Re: [PATCH v2 1/1] virtio-video: Add virtio video device specification

2020-01-08 Thread Keiichi Watanabe
Hi Dmitry,

On Wed, Jan 8, 2020 at 7:00 PM Dmitry Sepp  wrote:
>
> Hi Keiichi,
>
> On Mittwoch, 8. Januar 2020 07:59:22 CET Keiichi Watanabe wrote:
> > Hi Dmitry,
> >
> > On Wed, Jan 8, 2020 at 1:50 AM Dmitry Sepp 
> wrote:
> > > Hi Keiichi,
> > >
> > > thanks for the updates, please see my comments below.
> > >
> > > On Dienstag, 7. Januar 2020 14:24:31 CET Keiichi Watanabe wrote:
> > > > Hi Dmitry,
> > > >
> > > > On Mon, Jan 6, 2020 at 11:59 PM Dmitry Sepp
> > > > 
> > >
> > > wrote:
> > > > > Hi,
> > > > >
> > > > > a couple of new comments:
> > > > >
> > > > > On Mittwoch, 18. Dezember 2019 14:02:14 CET Keiichi Watanabe wrote:
> > > > > > From: Dmitry Sepp 
> > > > > >
> > > > > > The virtio video encoder device and decoder device provide
> > > > > > functionalities
> > > > > > to encode and decode video stream respectively.
> > > > > > Though video encoder and decoder are provided as different devices,
> > > > > > they
> > > > > > use a same protocol.
> > > > > >
> > > > > > Signed-off-by: Dmitry Sepp 
> > > > > > Signed-off-by: Keiichi Watanabe 
> > > > > > ---
> > > > > >
> > > > > >  content.tex  |   1 +
> > > > > >  virtio-video.tex | 579
> > > > > >  +++
> > > > > >  2 files changed, 580 insertions(+)
> > > > > >  create mode 100644 virtio-video.tex
> > > > > >
> > > > > > diff --git a/content.tex b/content.tex
> > > > > > index 556b373..9e56839 100644
> > > > > > --- a/content.tex
> > > > > > +++ b/content.tex
> > > > > > @@ -5743,6 +5743,7 @@ \subsubsection{Legacy Interface: Framing
> > > > > > Requirements}\label{sec:Device \input{virtio-vsock.tex}
> > > > > >
> > > > > >  \input{virtio-fs.tex}
> > > > > >  \input{virtio-rpmb.tex}
> > > > > >
> > > > > > +\input{virtio-video.tex}
> > > > > >
> > > > > >  \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> > > > > >
> > > > > > diff --git a/virtio-video.tex b/virtio-video.tex
> > > > > > new file mode 100644
> > > > > > index 000..30e728d
> > > > > > --- /dev/null
> > > > > > +++ b/virtio-video.tex
> > > > > > @@ -0,0 +1,579 @@
> > > > > > +\section{Video Device}\label{sec:Device Types / Video Device}
> > > > > > +
> > > > > > +The virtio video encoder device and decoder device are virtual
> > > > > > devices
> > > > > > that +supports encoding and decoding respectively. Though the
> > > > > > encoder
> > > > > > and the decoder +are different devices, they use the same protocol.
> > > > > > +
> > > > > > +\subsection{Device ID}\label{sec:Device Types / Video Device /
> > > > > > Device
> > > > > > ID}
> > > > > > +
> > > > > > +\begin{description}
> > > > > > +\item[30] encoder device
> > > > > > +\item[31] decoder device
> > > > > > +\end{description}
> > > > > > +
> > > > > > +\subsection{Virtqueues}\label{sec:Device Types / Video Device /
> > > > > > Virtqueues} +
> > > > > > +\begin{description}
> > > > > > +\item[0] controlq - queue for sending control commands.
> > > > > > +\item[1] eventq - queue for sending events happened in the device.
> > > > > > +\end{description}
> > > > > > +
> > > > > > +\subsection{Feature bits}\label{sec:Device Types / Video Device /
> > > > > > Feature
> > > > > > bits} +
> > > > > > +\begin{description}
> > > > > > +\item[VIRTIO_VIDEO_F_RESOURCE_GUEST_PAGES (0)] Guest pages can be
> > > > > > used
> > > > > > for
> > > > > > video +  buffers.
> > > > > > +\end{description}
> > > > > > +
> > > > > > +\devicenormative{\subsubsection}{Feature bits}{Device Types / Video
> > > > > > Device
> > > > > > / Feature bits} +
> > > > > > +The device MUST offer at least one of feature bits.
> > > > > > +
> > > > > > +\subsection{Device configuration layout}\label{sec:Device Types /
> > > > > > Video
> > > > > > Device / Device configuration layout} +
> > > > > > +Video device configuration uses the following layout structure:
> > > > > > +
> > > > > > +\begin{lstlisting}
> > > > > > +struct virtio_video_config {
> > > > > > +le32 max_cap_len;
> > > > > > +};
> > > > > > +\end{lstlisting}
> > > > > > +
> > > > > > +\begin{description}
> > > > > > +\item[\field{max_cap_len}] defines the maximum length of a
> > > > > > descriptor
> > > > > > +  required to call VIRTIO_VIDEO_GET_CAPABILITY in bytes. The device
> > > > > > +  MUST set this value.
> > > > > > +\end{description}
> > > > > > +
> > > > > > +\subsection{Device Initialization}\label{sec:Device Types / Video
> > > > > > Device /
> > > > > > Device Initialization} +
> > > > > > +\devicenormative{\subsubsection}{Device Initialization}{Device
> > > > > > Types /
> > > > > > Video Device / Device Initialization} +
> > > > > > +The driver SHOULD query device capability by using the
> > > > > > +VIRTIO_VIDEO_T_GET_CAPABILITY and use that information for the
> > > > > > initial
> > > > > > +setup.
> > > > > > +
> > > > > > +\subsection{Device Operation}\label{sec:Device Types / Video Device
> > > > > > /
> > > > > > Device Operation} +
> > > > > > +The driver allocates input and output buffers and 

Re: [virtio-dev][RFC PATCH v1 2/2] virtio-gpu: add the ability to export resources

2020-01-08 Thread David Stevens
> Is there a specific reason why you want the host pick the uuid?  I would
> let the guest define the uuid, i.e. move the uuid fields to
> virtio_gpu_export_resource and scratch virtio_gpu_resp_export_resource.

Sending the uuid in the original request doesn't really buy us
anything, at least in terms of asynchronicity. The guest would still
need to wait for the response to arrive before it could safely pass
the uuid to any other virtio devices, to prevent a race where the
import fails because it is processed before virtio-gpu processes the
export. Perhaps this wouldn't be the case if we supported sharing
fences between virtio devices, but even then, fences are more of a
thing for the operation of a pipeline, not for the setup of a
pipeline.

At that point, I think it's just a matter of aesthetics. I lean
slightly towards returning the uuid from the host, since that rules
out any implementation with the aforementioned race. That being said,
if there are any specific reasons or preferences to assigning the uuid
from the guest, I can switch to that direction.

-David

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



[virtio-dev] Re: [PATCH v4] virtio-snd: add virtio sound device specification

2020-01-08 Thread Anton Yakovlev

On 08.01.2020 11:34, Gerd Hoffmann wrote:

On Tue, Jan 07, 2020 at 01:50:49PM +0100, Anton Yakovlev wrote:

This patch proposes virtio specification for a new virtio sound device,
that may be useful in case when having audio is required but a device
passthrough or emulation is not an option.


Looks pretty good overall.  Some small nits:


+/* a common header */
+struct virtio_snd_hdr {
+le32 code;
+};
+
+/* an event notification */
+struct virtio_snd_event {
+le32 type;


Use "struct virtio_snd_hdr hdr" instead of "type", for consistency with
the other structs?


Yes, good idea.



+\begin{description}
+\item[\field{code}] (device-readable) specifies a device request type
+(VIRTIO_SND_R_*).
+\end{description}
+
+A response part has, or consists of, a common header containing the following
+field:
+
+\begin{description}
+\item[\field{code}] (device-writable) specifies a device request status
+(VIRTIO_SND_S_*).
+\end{description}


Repeating "device-readable" or "device-writable" for each struct field
looks a bit odd because this applies to the whole struct.  Not so much
for these structs with a single field only, but there are structs with
more fields further down the spec ...


Well, I'm not sure here. Previously, I was told to mark explicitly every
field.



+\begin{description}
+\item[\field{type}] (device-writable) specifies an event type 
(VIRTIO_SND_EVT_*).
+\item[\field{data}] (device-writable) specifies an optional event data.
+\end{description}


... here for example.


+/* a response containing PCM stream capabilities */
+struct virtio_snd_pcm_caps {
+struct virtio_snd_hdr hdr; /* VIRTIO_SND_S_XXX */
+u8 directions; /* 1 << VIRTIO_SND_PCM_T_XXX */
+u8 channels_min;
+u8 channels_max;
+u8 features; /* 1 << VIRTIO_SND_PCM_F_XXX */
+le64 formats; /* 1 << VIRTIO_SND_PCM_FMT_XXX */
+le32 rates; /* 1 << VIRTIO_SND_PCM_RATE_XXX */


Use le64 for rates too, just in case?

For features a few more bits don't hurt too, we already have 5 defined,
so having only 8 total available isn't that much for possible
extensions ...


Okey, then I will rearrange the structure.



+\paragraph{VIRTIO_SND_R_PCM_PAUSE}
+\paragraph{VIRTIO_SND_R_PCM_UNPAUSE}


Still here.

So there are good reasons to keep them instead of just using stop/start?


We can get rid of pause/unpause at the moment. Later, they can be added as a 
stream feature, if necessary.


Also, regarding requirement for populating tx/rx queues with period size 
buffers. Maybe it's better to replace MUST with SHOULD? It would give more 
flexibility for the driver. What do you think?



--
Anton Yakovlev
Senior Software Engineer

OpenSynergy GmbH
Rotherstr. 20, 10245 Berlin

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



Re: [virtio-dev][RFC PATCH v1 1/2] content: define what exporting a resource is

2020-01-08 Thread David Stevens
>
> Hmm, I'd suggest to move the whole thing into the virtio-gpu section.
> There is no such thing as a "resource" in general virtio context ...
>

If this is moved into the virtio-gpu section, then any device type that
imports resources will have to refer to something defined by the GPU device
type. This would make the GPU device type a sort of special device type
that isn't just a leaf node of the spec. I think it's better to define
'resource' as a top level concept for virtio devices, even if the specifics
of what a 'resource' is are defined by individual device types.

-David


Re: [virtio-dev][RFC PATCH v1 2/2] virtio-gpu: add the ability to export resources

2020-01-08 Thread Gerd Hoffmann
> +\begin{lstlisting}
> +struct virtio_gpu_export_resource {
> +struct virtio_gpu_ctrl_hdr hdr;
> +le32 resource_id;
> +le32 padding;
> +};
> +
> +struct virtio_gpu_resp_export_resource {
> +struct virtio_gpu_ctrl_hdr hdr;
> +le64 uuid_low;
> +le64 uuid_high;
> +};
> +\end{lstlisting}

Is there a specific reason why you want the host pick the uuid?  I would
let the guest define the uuid, i.e. move the uuid fields to
virtio_gpu_export_resource and scratch virtio_gpu_resp_export_resource.

Also I'd siggest to name the command (and struct) RESOURCE_ASSIGN_UUID.

cheers,
  Gerd


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



Re: [virtio-dev][RFC PATCH v1 1/2] content: define what exporting a resource is

2020-01-08 Thread Gerd Hoffmann
On Wed, Jan 08, 2020 at 06:01:58PM +0900, David Stevens wrote:
> Define a mechanism for sharing resources between different virtio
> devices.
> 
> Signed-off-by: David Stevens 
> ---
>  content.tex | 18 ++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/content.tex b/content.tex
> index b1ea9b9..73bd28e 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -373,6 +373,24 @@ \section{Driver Notifications}
> \label{sec:Virtqueues / Driver notifications}
> 
>  \input{shared-mem.tex}
> 
> +\section{Exporting Resources}\label{sec:Basic Facilities of a Virtio
> Device / Exporting Resources}
> +
> +When a resource created by one virtio device needs to be
> +shared with a seperate virtio device, the first device can
> +export the resource by generating a \field{uuid} which the
> +guest can pass to the second device to identify the resource.
> +
> +What constitutes a resource, how to export resources, and
> +how to import resources are defined by the individual device
> +types. The generation method of a \field{uuid} is dependent
> +upon the implementation of the exporting device.
> +
> +Whether a particular exported resource can be imported into
> +a device is dependent upon the implementations of the exporting
> +and importing devices. Generally speaking, the guest should
> +have some knowledge of the host configuration before trying to
> +use exported resources.

Hmm, I'd suggest to move the whole thing into the virtio-gpu section.
There is no such thing as a "resource" in general virtio context ...

cheers,
  Gerd


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



[virtio-dev] Re: [PATCH v4] virtio-snd: add virtio sound device specification

2020-01-08 Thread Gerd Hoffmann
On Tue, Jan 07, 2020 at 01:50:49PM +0100, Anton Yakovlev wrote:
> This patch proposes virtio specification for a new virtio sound device,
> that may be useful in case when having audio is required but a device
> passthrough or emulation is not an option.

Looks pretty good overall.  Some small nits:

> +/* a common header */
> +struct virtio_snd_hdr {
> +le32 code;
> +};
> +
> +/* an event notification */
> +struct virtio_snd_event {
> +le32 type;

Use "struct virtio_snd_hdr hdr" instead of "type", for consistency with
the other structs?

> +\begin{description}
> +\item[\field{code}] (device-readable) specifies a device request type
> +(VIRTIO_SND_R_*).
> +\end{description}
> +
> +A response part has, or consists of, a common header containing the following
> +field:
> +
> +\begin{description}
> +\item[\field{code}] (device-writable) specifies a device request status
> +(VIRTIO_SND_S_*).
> +\end{description}

Repeating "device-readable" or "device-writable" for each struct field
looks a bit odd because this applies to the whole struct.  Not so much
for these structs with a single field only, but there are structs with
more fields further down the spec ...

> +\begin{description}
> +\item[\field{type}] (device-writable) specifies an event type 
> (VIRTIO_SND_EVT_*).
> +\item[\field{data}] (device-writable) specifies an optional event data.
> +\end{description}

... here for example.

> +/* a response containing PCM stream capabilities */
> +struct virtio_snd_pcm_caps {
> +struct virtio_snd_hdr hdr; /* VIRTIO_SND_S_XXX */
> +u8 directions; /* 1 << VIRTIO_SND_PCM_T_XXX */
> +u8 channels_min;
> +u8 channels_max;
> +u8 features; /* 1 << VIRTIO_SND_PCM_F_XXX */
> +le64 formats; /* 1 << VIRTIO_SND_PCM_FMT_XXX */
> +le32 rates; /* 1 << VIRTIO_SND_PCM_RATE_XXX */

Use le64 for rates too, just in case?

For features a few more bits don't hurt too, we already have 5 defined,
so having only 8 total available isn't that much for possible
extensions ...

> +\paragraph{VIRTIO_SND_R_PCM_PAUSE}
> +\paragraph{VIRTIO_SND_R_PCM_UNPAUSE}

Still here.

So there are good reasons to keep them instead of just using stop/start?

cheers,
  Gerd


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



[virtio-dev] Re: [PATCH v2 1/1] virtio-video: Add virtio video device specification

2020-01-08 Thread Dmitry Sepp
Hi Keiichi,

On Mittwoch, 8. Januar 2020 07:59:22 CET Keiichi Watanabe wrote:
> Hi Dmitry,
> 
> On Wed, Jan 8, 2020 at 1:50 AM Dmitry Sepp  
wrote:
> > Hi Keiichi,
> > 
> > thanks for the updates, please see my comments below.
> > 
> > On Dienstag, 7. Januar 2020 14:24:31 CET Keiichi Watanabe wrote:
> > > Hi Dmitry,
> > > 
> > > On Mon, Jan 6, 2020 at 11:59 PM Dmitry Sepp
> > > 
> > 
> > wrote:
> > > > Hi,
> > > > 
> > > > a couple of new comments:
> > > > 
> > > > On Mittwoch, 18. Dezember 2019 14:02:14 CET Keiichi Watanabe wrote:
> > > > > From: Dmitry Sepp 
> > > > > 
> > > > > The virtio video encoder device and decoder device provide
> > > > > functionalities
> > > > > to encode and decode video stream respectively.
> > > > > Though video encoder and decoder are provided as different devices,
> > > > > they
> > > > > use a same protocol.
> > > > > 
> > > > > Signed-off-by: Dmitry Sepp 
> > > > > Signed-off-by: Keiichi Watanabe 
> > > > > ---
> > > > > 
> > > > >  content.tex  |   1 +
> > > > >  virtio-video.tex | 579
> > > > >  +++
> > > > >  2 files changed, 580 insertions(+)
> > > > >  create mode 100644 virtio-video.tex
> > > > > 
> > > > > diff --git a/content.tex b/content.tex
> > > > > index 556b373..9e56839 100644
> > > > > --- a/content.tex
> > > > > +++ b/content.tex
> > > > > @@ -5743,6 +5743,7 @@ \subsubsection{Legacy Interface: Framing
> > > > > Requirements}\label{sec:Device \input{virtio-vsock.tex}
> > > > > 
> > > > >  \input{virtio-fs.tex}
> > > > >  \input{virtio-rpmb.tex}
> > > > > 
> > > > > +\input{virtio-video.tex}
> > > > > 
> > > > >  \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> > > > > 
> > > > > diff --git a/virtio-video.tex b/virtio-video.tex
> > > > > new file mode 100644
> > > > > index 000..30e728d
> > > > > --- /dev/null
> > > > > +++ b/virtio-video.tex
> > > > > @@ -0,0 +1,579 @@
> > > > > +\section{Video Device}\label{sec:Device Types / Video Device}
> > > > > +
> > > > > +The virtio video encoder device and decoder device are virtual
> > > > > devices
> > > > > that +supports encoding and decoding respectively. Though the
> > > > > encoder
> > > > > and the decoder +are different devices, they use the same protocol.
> > > > > +
> > > > > +\subsection{Device ID}\label{sec:Device Types / Video Device /
> > > > > Device
> > > > > ID}
> > > > > +
> > > > > +\begin{description}
> > > > > +\item[30] encoder device
> > > > > +\item[31] decoder device
> > > > > +\end{description}
> > > > > +
> > > > > +\subsection{Virtqueues}\label{sec:Device Types / Video Device /
> > > > > Virtqueues} +
> > > > > +\begin{description}
> > > > > +\item[0] controlq - queue for sending control commands.
> > > > > +\item[1] eventq - queue for sending events happened in the device.
> > > > > +\end{description}
> > > > > +
> > > > > +\subsection{Feature bits}\label{sec:Device Types / Video Device /
> > > > > Feature
> > > > > bits} +
> > > > > +\begin{description}
> > > > > +\item[VIRTIO_VIDEO_F_RESOURCE_GUEST_PAGES (0)] Guest pages can be
> > > > > used
> > > > > for
> > > > > video +  buffers.
> > > > > +\end{description}
> > > > > +
> > > > > +\devicenormative{\subsubsection}{Feature bits}{Device Types / Video
> > > > > Device
> > > > > / Feature bits} +
> > > > > +The device MUST offer at least one of feature bits.
> > > > > +
> > > > > +\subsection{Device configuration layout}\label{sec:Device Types /
> > > > > Video
> > > > > Device / Device configuration layout} +
> > > > > +Video device configuration uses the following layout structure:
> > > > > +
> > > > > +\begin{lstlisting}
> > > > > +struct virtio_video_config {
> > > > > +le32 max_cap_len;
> > > > > +};
> > > > > +\end{lstlisting}
> > > > > +
> > > > > +\begin{description}
> > > > > +\item[\field{max_cap_len}] defines the maximum length of a
> > > > > descriptor
> > > > > +  required to call VIRTIO_VIDEO_GET_CAPABILITY in bytes. The device
> > > > > +  MUST set this value.
> > > > > +\end{description}
> > > > > +
> > > > > +\subsection{Device Initialization}\label{sec:Device Types / Video
> > > > > Device /
> > > > > Device Initialization} +
> > > > > +\devicenormative{\subsubsection}{Device Initialization}{Device
> > > > > Types /
> > > > > Video Device / Device Initialization} +
> > > > > +The driver SHOULD query device capability by using the
> > > > > +VIRTIO_VIDEO_T_GET_CAPABILITY and use that information for the
> > > > > initial
> > > > > +setup.
> > > > > +
> > > > > +\subsection{Device Operation}\label{sec:Device Types / Video Device
> > > > > /
> > > > > Device Operation} +
> > > > > +The driver allocates input and output buffers and queues the
> > > > > buffers
> > > > > +to the device. The device performs operations on the buffers
> > > > > according
> > > > > +to the function in question.
> > > > > +
> > > > > +\subsubsection{Device Operation: Create stream}
> > > > > +
> > > > > +To process buffers, the device needs to associate them with a

[virtio-dev][RFC PATCH v1 1/2] content: define what exporting a resource is

2020-01-08 Thread David Stevens
Define a mechanism for sharing resources between different virtio
devices.

Signed-off-by: David Stevens 
---
 content.tex | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/content.tex b/content.tex
index b1ea9b9..73bd28e 100644
--- a/content.tex
+++ b/content.tex
@@ -373,6 +373,24 @@ \section{Driver Notifications}
\label{sec:Virtqueues / Driver notifications}

 \input{shared-mem.tex}

+\section{Exporting Resources}\label{sec:Basic Facilities of a Virtio
Device / Exporting Resources}
+
+When a resource created by one virtio device needs to be
+shared with a seperate virtio device, the first device can
+export the resource by generating a \field{uuid} which the
+guest can pass to the second device to identify the resource.
+
+What constitutes a resource, how to export resources, and
+how to import resources are defined by the individual device
+types. The generation method of a \field{uuid} is dependent
+upon the implementation of the exporting device.
+
+Whether a particular exported resource can be imported into
+a device is dependent upon the implementations of the exporting
+and importing devices. Generally speaking, the guest should
+have some knowledge of the host configuration before trying to
+use exported resources.
+
 \chapter{General Initialization And Device
Operation}\label{sec:General Initialization And Device Operation}

 We start with an overview of device initialization, then expand on the
-- 
2.24.1.735.g03f4e72817-goog

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



[virtio-dev][RFC PATCH v1 2/2] virtio-gpu: add the ability to export resources

2020-01-08 Thread David Stevens
Signed-off-by: David Stevens 
---
 virtio-gpu.tex | 29 +
 1 file changed, 29 insertions(+)

diff --git a/virtio-gpu.tex b/virtio-gpu.tex
index af4ca61..522f478 100644
--- a/virtio-gpu.tex
+++ b/virtio-gpu.tex
@@ -186,12 +186,16 @@ \subsubsection{Device Operation: Request
header}\label{sec:Device Types / GPU De
 VIRTIO_GPU_CMD_UPDATE_CURSOR = 0x0300,
 VIRTIO_GPU_CMD_MOVE_CURSOR,

+/* misc commands */
+VIRTIO_GPU_CMD_EXPORT_RESOURCE = 0x0400,
+
 /* success responses */
 VIRTIO_GPU_RESP_OK_NODATA = 0x1100,
 VIRTIO_GPU_RESP_OK_DISPLAY_INFO,
 VIRTIO_GPU_RESP_OK_CAPSET_INFO,
 VIRTIO_GPU_RESP_OK_CAPSET,
 VIRTIO_GPU_RESP_OK_EDID,
+VIRTIO_GPU_RESP_OK_EXPORT_RESOURCE,

 /* error responses */
 VIRTIO_GPU_RESP_ERR_UNSPEC = 0x1200,
@@ -454,6 +458,31 @@ \subsubsection{Device Operation:
controlq}\label{sec:Device Types / GPU Device /
 This detaches any backing pages from a resource, to be used in case of
 guest swapping or object destruction.

+\item[VIRTIO_GPU_CMD_EXPORT_RESOURCE] Exports a resource for use by other
+  virtio devices. Request data is \field{struct
+virtio_gpu_export_resource}.  Response type is
+  VIRTIO_GPU_RESP_OK_EXPORT_RESOURCE, response data is \field{struct
+virtio_gpu_resp_export_resource}.
+
+\begin{lstlisting}
+struct virtio_gpu_export_resource {
+struct virtio_gpu_ctrl_hdr hdr;
+le32 resource_id;
+le32 padding;
+};
+
+struct virtio_gpu_resp_export_resource {
+struct virtio_gpu_ctrl_hdr hdr;
+le64 uuid_low;
+le64 uuid_high;
+};
+\end{lstlisting}
+
+The response contains a uuid which identifies the host private resource to
+other virtio devices. Note that if the resource has an attached backing,
+modifications made to an exported resource by other devices are not visible
+in the attached backing until they are transferred into the backing.
+
 \end{description}

 \subsubsection{Device Operation: cursorq}\label{sec:Device Types /
GPU Device / Device Operation / Device Operation: cursorq}
-- 
2.24.1.735.g03f4e72817-goog

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



[virtio-dev][RFC PATCH v1 0/2] Cross-device resource sharing

2020-01-08 Thread David Stevens
This RFC comes from the recent discussion on buffer sharing [1],
specifically about the need to share resources between different
virtio devices. For a concrete use case, this can be used to share
virtio-gpu allocated buffers with the recently proposed virtio video
device [2], without the need to memcpy decoded frames through the
guest.

[1] https://markmail.org/thread/jeh5xjjxvylyrbur
[2] https://markmail.org/thread/yb25fim2dqfuktgf

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org