Virtualization - Mar 2013 - [PATCH] virtio-spec: add field for scsi command size

Add field for guest to specify command size for virtio-blk.

Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
---
 virtio-spec.lyx | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 78 insertions(+), 5 deletions(-)

diff --git a/virtio-spec.lyx b/virtio-spec.lyx
index a8ce3f9..fea97ed 100644
--- a/virtio-spec.lyx
+++ b/virtio-spec.lyx
@@ -5826,6 +5826,16 @@ VIRTIO_BLK_F_TOPOLOGY (10) Device exports information on
optimal I/O alignment.
 \change_inserted 1531152142 1341302349
 VIRTIO_BLK_F_CONFIG_WCE (11) Device can toggle its cache between writeback
  and writethrough modes.
+\change_inserted 1986246365 1363257418
+
+\end_layout
+
+\begin_layout Description
+
+\change_inserted 1986246365 1363258629
+VIRTIO_BLK_F_CMD_SIZE (12) cmd_size field is valid.
+\change_inserted 1531152142 1341302349
+
 \end_layout
 
 \end_deeper
@@ -5994,6 +6004,30 @@ struct virtio_blk_config {
 \change_inserted 1531152142 1341301918
 
     u8 writeback;
+\change_inserted 1986246365 1363257385
+
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1363258610
+
+    u8 reserved[3];
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1363259147
+
+    // buffer length for cmd field in requests.
+ reset value 0.
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1363257810
+
+    u32 cmd_size;
 \change_unchanged
 
 \end_layout
@@ -6085,6 +6119,21 @@ Until version 1.1, QEMU remained in writeback mode even
after a guest announced
 \end_inset
 
 .
+\change_inserted 1986246365 1363258989
+
+\end_layout
+
+\begin_layout Enumerate
+
+\change_inserted 1986246365 1363259237
+If VIRTIO_BLK_F_SCSI_CMD_SIZE is negotiated, Guest should set the desired
+ size for scsi commands to the 
+\emph on
+cmd_size 
+\emph default
+field.
+\change_unchanged
+
 \end_layout
 
 \begin_layout Section*
@@ -6369,8 +6418,33 @@ cmd
 \emph default
  field is only present for scsi packet command requests, and indicates the
  command to perform.
- This field must reside in a single, separate read-only buffer; command
- length can be derived from the length of this buffer.
+ 
+\change_inserted 1986246365 1363258881
+If VIRTIO_BLK_F_SCSI_CMD_SIZE feature bit is negotiated, Guest must set
+ the command length by writing it to the cmd_size field in configuration
+ space
+\begin_inset Foot
+status open
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1363258931
+Guest should always set cmd_size during initialization.
+\change_unchanged
+
+\end_layout
+
+\end_inset
+
+.
+ If VIRTIO_BLK_F_SCSI_CMD_SIZE is not negotiated, 
+\change_deleted 1986246365 1363258703
+T
+\change_inserted 1986246365 1363258703
+t
+\change_unchanged
+his field must reside in a single, separate read-only buffer; command length
+ can be derived from the length of this buffer.
  
 \end_layout
 
-- 
MST

Il 14/03/2013 12:10, Michael S. Tsirkin ha scritto:
> Add field for guest to specify command size for virtio-blk.
> 
> Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
> ---
>  virtio-spec.lyx | 83
+++++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 78 insertions(+), 5 deletions(-)
> 
> diff --git a/virtio-spec.lyx b/virtio-spec.lyx
> index a8ce3f9..fea97ed 100644
> --- a/virtio-spec.lyx
> +++ b/virtio-spec.lyx
> @@ -5826,6 +5826,16 @@ VIRTIO_BLK_F_TOPOLOGY (10) Device exports
information on optimal I/O alignment.
>  \change_inserted 1531152142 1341302349
>  VIRTIO_BLK_F_CONFIG_WCE (11) Device can toggle its cache between writeback
>   and writethrough modes.
> +\change_inserted 1986246365 1363257418
> +
> +\end_layout
> +
> +\begin_layout Description
> +
> +\change_inserted 1986246365 1363258629
> +VIRTIO_BLK_F_CMD_SIZE (12) cmd_size field is valid.
SCSI_CMD_SIZE and scsi_cmd_size please.
> +\change_inserted 1531152142 1341302349
> +
>  \end_layout
>  
>  \end_deeper
> @@ -5994,6 +6004,30 @@ struct virtio_blk_config {
>  \change_inserted 1531152142 1341301918
>  
>      u8 writeback;
> +\change_inserted 1986246365 1363257385
> +
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1363258610
> +
> +    u8 reserved[3];
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1363259147
> +
> +    // buffer length for cmd field in requests.
size of the cmd field in VIRTIO_BLK_T_SCSI_CMD requests.
> + reset value 0.
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1363257810
> +
> +    u32 cmd_size;
scsi_cmd_size
>  \change_unchanged
>  
>  \end_layout
> @@ -6085,6 +6119,21 @@ Until version 1.1, QEMU remained in writeback mode
even after a guest announced
>  \end_inset
>  
>  .
> +\change_inserted 1986246365 1363258989
> +
> +\end_layout
> +
> +\begin_layout Enumerate
> +
> +\change_inserted 1986246365 1363259237
> +If VIRTIO_BLK_F_SCSI_CMD_SIZE is negotiated, Guest should set the desired
> + size for scsi commands to the 
If both VIRTIO_BLK_F_SCSI and VIRTIO_BLK_F_SCSI_CMD_SIZE are negotiated,
the driver should write a non-zero value to the scsi_cmd_size field
before sending a VIRTIO_BLK_T_SCSI_CMD request.
> +\emph on
> +cmd_size 
> +\emph default
> +field.
> +\change_unchanged
> +
>  \end_layout
>  
>  \begin_layout Section*
> @@ -6369,8 +6418,33 @@ cmd
>  \emph default
>   field is only present for scsi packet command requests, and indicates the
>   command to perform.
> - This field must reside in a single, separate read-only buffer; command
> - length can be derived from the length of this buffer.
> + 
> +\change_inserted 1986246365 1363258881
> +If VIRTIO_BLK_F_SCSI_CMD_SIZE feature bit is negotiated, Guest must set
> + the command length by writing it to the cmd_size field in configuration
> + space
If the VIRTIO_BLK_F_SCSI_CMD_SIZE feature bit is negotiated, the driver
should write the length of the cmd field to the scsi_cmd_size field of
the configuration space.

(please make sure bold and emphasis are consistent)
> +\begin_inset Foot
> +status open
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1363258931
> +Guest should always set cmd_size during initialization.
scsi_cmd_size
> +\change_unchanged
> +
> +\end_layout
> +
> +\end_inset
> +
> +.
> + If VIRTIO_BLK_F_SCSI_CMD_SIZE is not negotiated, 
> +\change_deleted 1986246365 1363258703
> +T
> +\change_inserted 1986246365 1363258703
> +t
> +\change_unchanged
> +his field must reside in a single, separate read-only buffer; command
length
> + can be derived from the length of this buffer.
the command length...
>   
>  \end_layout
>  
> 
Thanks,

Paolo

On Thu, Mar 14, 2013 at 04:15:28PM +0100, Paolo Bonzini
wrote:
> Il 14/03/2013 12:10, Michael S. Tsirkin ha scritto:
> > Add field for guest to specify command size for virtio-blk.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
OK, but Rusty usually tweaks wording anyway.
Rusty want to apply and make changes Paolo suggested yourself
or want me to?
> > ---
> >  virtio-spec.lyx | 83
+++++++++++++++++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 78 insertions(+), 5 deletions(-)
> > 
> > diff --git a/virtio-spec.lyx b/virtio-spec.lyx
> > index a8ce3f9..fea97ed 100644
> > --- a/virtio-spec.lyx
> > +++ b/virtio-spec.lyx
> > @@ -5826,6 +5826,16 @@ VIRTIO_BLK_F_TOPOLOGY (10) Device exports
information on optimal I/O alignment.
> >  \change_inserted 1531152142 1341302349
> >  VIRTIO_BLK_F_CONFIG_WCE (11) Device can toggle its cache between
writeback
> >   and writethrough modes.
> > +\change_inserted 1986246365 1363257418
> > +
> > +\end_layout
> > +
> > +\begin_layout Description
> > +
> > +\change_inserted 1986246365 1363258629
> > +VIRTIO_BLK_F_CMD_SIZE (12) cmd_size field is valid.
> 
> SCSI_CMD_SIZE and scsi_cmd_size please.
> 
> > +\change_inserted 1531152142 1341302349
> > +
> >  \end_layout
> >  
> >  \end_deeper
> > @@ -5994,6 +6004,30 @@ struct virtio_blk_config {
> >  \change_inserted 1531152142 1341301918
> >  
> >      u8 writeback;
> > +\change_inserted 1986246365 1363257385
> > +
> > +\end_layout
> > +
> > +\begin_layout Plain Layout
> > +
> > +\change_inserted 1986246365 1363258610
> > +
> > +    u8 reserved[3];
> > +\end_layout
> > +
> > +\begin_layout Plain Layout
> > +
> > +\change_inserted 1986246365 1363259147
> > +
> > +    // buffer length for cmd field in requests.
> 
> size of the cmd field in VIRTIO_BLK_T_SCSI_CMD requests.
> 
> > + reset value 0.
> > +\end_layout
> > +
> > +\begin_layout Plain Layout
> > +
> > +\change_inserted 1986246365 1363257810
> > +
> > +    u32 cmd_size;
> 
> scsi_cmd_size
> 
> >  \change_unchanged
> >  
> >  \end_layout
> > @@ -6085,6 +6119,21 @@ Until version 1.1, QEMU remained in writeback
mode even after a guest announced
> >  \end_inset
> >  
> >  .
> > +\change_inserted 1986246365 1363258989
> > +
> > +\end_layout
> > +
> > +\begin_layout Enumerate
> > +
> > +\change_inserted 1986246365 1363259237
> > +If VIRTIO_BLK_F_SCSI_CMD_SIZE is negotiated, Guest should set the
desired
> > + size for scsi commands to the 
> 
> If both VIRTIO_BLK_F_SCSI and VIRTIO_BLK_F_SCSI_CMD_SIZE are negotiated,
> the driver should write a non-zero value to the scsi_cmd_size field
> before sending a VIRTIO_BLK_T_SCSI_CMD request.
> 
> > +\emph on
> > +cmd_size 
> > +\emph default
> > +field.
> > +\change_unchanged
> > +
> >  \end_layout
> >  
> >  \begin_layout Section*
> > @@ -6369,8 +6418,33 @@ cmd
> >  \emph default
> >   field is only present for scsi packet command requests, and
indicates the
> >   command to perform.
> > - This field must reside in a single, separate read-only buffer;
command
> > - length can be derived from the length of this buffer.
> > + 
> > +\change_inserted 1986246365 1363258881
> > +If VIRTIO_BLK_F_SCSI_CMD_SIZE feature bit is negotiated, Guest must
set
> > + the command length by writing it to the cmd_size field in
configuration
> > + space
> 
> If the VIRTIO_BLK_F_SCSI_CMD_SIZE feature bit is negotiated, the driver
> should write the length of the cmd field to the scsi_cmd_size field of
> the configuration space.
> 
> (please make sure bold and emphasis are consistent)
> 
> > +\begin_inset Foot
> > +status open
> > +
> > +\begin_layout Plain Layout
> > +
> > +\change_inserted 1986246365 1363258931
> > +Guest should always set cmd_size during initialization.
> 
> scsi_cmd_size
> 
> > +\change_unchanged
> > +
> > +\end_layout
> > +
> > +\end_inset
> > +
> > +.
> > + If VIRTIO_BLK_F_SCSI_CMD_SIZE is not negotiated, 
> > +\change_deleted 1986246365 1363258703
> > +T
> > +\change_inserted 1986246365 1363258703
> > +t
> > +\change_unchanged
> > +his field must reside in a single, separate read-only buffer; command
length
> > + can be derived from the length of this buffer.
> 
> the command length...
> 
> >   
> >  \end_layout
> >  
> > 
> 
> Thanks,
> 
> Paolo

On Thu, Mar 14, 2013 at 07:39:18PM +0200, Michael S. Tsirkin
wrote:
> On Thu, Mar 14, 2013 at 04:15:28PM +0100, Paolo Bonzini wrote:
> > Il 14/03/2013 12:10, Michael S. Tsirkin ha scritto:
> > > Add field for guest to specify command size for virtio-blk.
> > > 
> > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
> 
> OK, but Rusty usually tweaks wording anyway.
> Rusty want to apply and make changes Paolo suggested yourself
> or want me to?
Ping. Rusty could you indicate your preference please?
> > > ---
> > >  virtio-spec.lyx | 83
+++++++++++++++++++++++++++++++++++++++++++++++++++++----
> > >  1 file changed, 78 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/virtio-spec.lyx b/virtio-spec.lyx
> > > index a8ce3f9..fea97ed 100644
> > > --- a/virtio-spec.lyx
> > > +++ b/virtio-spec.lyx
> > > @@ -5826,6 +5826,16 @@ VIRTIO_BLK_F_TOPOLOGY (10) Device exports
information on optimal I/O alignment.
> > >  \change_inserted 1531152142 1341302349
> > >  VIRTIO_BLK_F_CONFIG_WCE (11) Device can toggle its cache between
writeback
> > >   and writethrough modes.
> > > +\change_inserted 1986246365 1363257418
> > > +
> > > +\end_layout
> > > +
> > > +\begin_layout Description
> > > +
> > > +\change_inserted 1986246365 1363258629
> > > +VIRTIO_BLK_F_CMD_SIZE (12) cmd_size field is valid.
> > 
> > SCSI_CMD_SIZE and scsi_cmd_size please.
> > 
> > > +\change_inserted 1531152142 1341302349
> > > +
> > >  \end_layout
> > >  
> > >  \end_deeper
> > > @@ -5994,6 +6004,30 @@ struct virtio_blk_config {
> > >  \change_inserted 1531152142 1341301918
> > >  
> > >      u8 writeback;
> > > +\change_inserted 1986246365 1363257385
> > > +
> > > +\end_layout
> > > +
> > > +\begin_layout Plain Layout
> > > +
> > > +\change_inserted 1986246365 1363258610
> > > +
> > > +    u8 reserved[3];
> > > +\end_layout
> > > +
> > > +\begin_layout Plain Layout
> > > +
> > > +\change_inserted 1986246365 1363259147
> > > +
> > > +    // buffer length for cmd field in requests.
> > 
> > size of the cmd field in VIRTIO_BLK_T_SCSI_CMD requests.
> > 
> > > + reset value 0.
> > > +\end_layout
> > > +
> > > +\begin_layout Plain Layout
> > > +
> > > +\change_inserted 1986246365 1363257810
> > > +
> > > +    u32 cmd_size;
> > 
> > scsi_cmd_size
> > 
> > >  \change_unchanged
> > >  
> > >  \end_layout
> > > @@ -6085,6 +6119,21 @@ Until version 1.1, QEMU remained in
writeback mode even after a guest announced
> > >  \end_inset
> > >  
> > >  .
> > > +\change_inserted 1986246365 1363258989
> > > +
> > > +\end_layout
> > > +
> > > +\begin_layout Enumerate
> > > +
> > > +\change_inserted 1986246365 1363259237
> > > +If VIRTIO_BLK_F_SCSI_CMD_SIZE is negotiated, Guest should set
the desired
> > > + size for scsi commands to the 
> > 
> > If both VIRTIO_BLK_F_SCSI and VIRTIO_BLK_F_SCSI_CMD_SIZE are
negotiated,
> > the driver should write a non-zero value to the scsi_cmd_size field
> > before sending a VIRTIO_BLK_T_SCSI_CMD request.
> > 
> > > +\emph on
> > > +cmd_size 
> > > +\emph default
> > > +field.
> > > +\change_unchanged
> > > +
> > >  \end_layout
> > >  
> > >  \begin_layout Section*
> > > @@ -6369,8 +6418,33 @@ cmd
> > >  \emph default
> > >   field is only present for scsi packet command requests, and
indicates the
> > >   command to perform.
> > > - This field must reside in a single, separate read-only buffer;
command
> > > - length can be derived from the length of this buffer.
> > > + 
> > > +\change_inserted 1986246365 1363258881
> > > +If VIRTIO_BLK_F_SCSI_CMD_SIZE feature bit is negotiated, Guest
must set
> > > + the command length by writing it to the cmd_size field in
configuration
> > > + space
> > 
> > If the VIRTIO_BLK_F_SCSI_CMD_SIZE feature bit is negotiated, the
driver
> > should write the length of the cmd field to the scsi_cmd_size field of
> > the configuration space.
> > 
> > (please make sure bold and emphasis are consistent)
> > 
> > > +\begin_inset Foot
> > > +status open
> > > +
> > > +\begin_layout Plain Layout
> > > +
> > > +\change_inserted 1986246365 1363258931
> > > +Guest should always set cmd_size during initialization.
> > 
> > scsi_cmd_size
> > 
> > > +\change_unchanged
> > > +
> > > +\end_layout
> > > +
> > > +\end_inset
> > > +
> > > +.
> > > + If VIRTIO_BLK_F_SCSI_CMD_SIZE is not negotiated, 
> > > +\change_deleted 1986246365 1363258703
> > > +T
> > > +\change_inserted 1986246365 1363258703
> > > +t
> > > +\change_unchanged
> > > +his field must reside in a single, separate read-only buffer;
command length
> > > + can be derived from the length of this buffer.
> > 
> > the command length...
> > 
> > >   
> > >  \end_layout
> > >  
> > > 
> > 
> > Thanks,
> > 
> > Paolo

On Thu, Mar 14, 2013 at 01:10:20PM +0200, Michael S. Tsirkin
wrote:
> Add field for guest to specify command size for virtio-blk.
> 
> Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
There's one concern here: are we going to add a
feature flag for flexible layout? If yes this feature
could thinkably just reuse it.

It allows a nice optimization in virtio net that is very easy
with latest qemu.

Rusty, what is our plan wrt this?  Are you fine with a per-device
feature bit?  Or do we need a transport feature bit (there's the small
issue that we almost run out of them for virtio-pci, do we use the last
one?).

As an aside, the new virtio pci config proposal seems to be stuck too,
are you looking into it or want me to?
-- 
MST