From: Cornelia Huck <cohuck@redhat.com>
To: Christian Schoenebeck <qemu_oss@crudebyte.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>,
virtio-comment@lists.oasis-open.org, Greg Kurz <groug@kaod.org>,
Dominique Martinet <asmadeus@codewreck.org>,
Halil Pasic <pasic@linux.ibm.com>
Subject: [virtio-comment] Re: [PATCH v2 4/4] Add CCW configuration field "indirect_num" to vq_info_block
Date: Fri, 11 Mar 2022 10:21:54 +0100 [thread overview]
Message-ID: <87o82chmj1.fsf@redhat.com> (raw)
In-Reply-To: <2478424.v7pibGEp8r@silver>
On Fri, Mar 11 2022, Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> On Donnerstag, 10. März 2022 17:09:25 CET Cornelia Huck wrote:
>> On Thu, Mar 10 2022, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>> > On Mon, Feb 21, 2022 at 06:01:41PM +0100, Christian Schoenebeck wrote:
>> >> This new CCW configuration field allows to negotiate a more fine
>> >> graded maximum lenght of indirect descriptor chains.
>> >>
>> >> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/122
>> >> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com
>> >> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
>> >> ---
>> >>
>> >> content.tex | 5 +++++
>> >> 1 file changed, 5 insertions(+)
>> >>
>> >> diff --git a/content.tex b/content.tex
>> >> index a3baf4d..d400ea7 100644
>> >> --- a/content.tex
>> >> +++ b/content.tex
>> >> @@ -2599,6 +2599,7 @@ \subsubsection{Configuring a
>> >> Virtqueue}\label{sec:Virtio Transport Options / Vir>>
>> >> be16 num;
>> >> be64 driver;
>> >> be64 device;
>> >>
>> >> + be32 indirect_num;
>> >>
>> >> };
>> >> \end{lstlisting}
>> >>
>> >> @@ -2607,6 +2608,10 @@ \subsubsection{Configuring a
>> >> Virtqueue}\label{sec:Virtio Transport Options / Vir>>
>> >> available area and used area for queue \field{index}, respectively. The
>> >> actual virtqueue size (number of allocated buffers) is transmitted in
>> >> \field{num}.>>
>> >> +If VIRTIO_RING_F_INDIRECT_SIZE has been negotiated then
>> >> \field{indirect_num} +reflects the maximum length of indirect descriptor
>> >> tables for queue +\field{index}.
>> >
>> > I think the transfer direction of CCW_CMD_SET_VQ struct vq_info_block is
>> > driver-to-device. So it allows the driver to set the Queue Indirect
>> > Size, but how does the driver query the device's maximum Queue Indirect
>> > Size value?
>>
>> [cc:ing Halil in case he has any further comments]
>>
>> You're right, CCW_CMD_SET_VQ + vq_info_block is driver-to-device. The
>> driver will obtain information about a queue via CCW_CMD_READ_VQ_CONF +
>> vq_config_block, so a max_indirect_num field needs to be added there as
>> well, I think.
>>
>> Moreover, we're changing the length of the ccw payload. Extending at the
>> end is generally fine, but the device and the driver need to agree on
>> what the expected payload is. We basically have two options here:
>>
>> * Make it depend on the feature bit being negotiated. This works because
>> virtqueue discovery needs to be done only after feature negotiation
>> has completed. However, this will get a bit awkward if we need to add
>> another field depending on a new feature bit: negotiating that
>> hypothetical feature would imply that the indirect num fields would be
>> present, but not valid, if the indirect feature had not been
>> negotiated. Not a showstopper, but looks a bit odd.
>> * Tie it to a new ccw revision (3) and make offering the feature bit
>> dependant upon revision 3 or later being negotiated. This has the
>> advantage that ccw revisions always build on each other (so no
>> awkwardness for future extension) and the drawback of introducing
>> another transport-specific prereq.
>>
>> If we can live with the possible awkwardness of future extensions, tying
>> the size of the structures to feature bits might be the preferable way.
>
> Really? My intuitive pick would rather be option 2 (CCW revision). But I'll go
> for whatever the common opinion is on this CCW issue.
Either would work; that's why I'd like to have Halil's opinion as well.
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: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
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/
next prev parent reply other threads:[~2022-03-11 9:22 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-21 16:22 [virtio-comment] [PATCH v2 0/4] Add VIRTIO_RING_F_INDIRECT_SIZE and queue_indirect_size Christian Schoenebeck
2022-02-21 16:25 ` [virtio-comment] [PATCH v2 1/4] Add VIRTIO_RING_F_INDIRECT_SIZE Christian Schoenebeck
2022-03-10 14:57 ` Stefan Hajnoczi
2022-03-10 16:20 ` Christian Schoenebeck
2022-02-21 16:27 ` [PATCH v2 2/4] Add PCI configuration field "queue_indirect_size" Christian Schoenebeck
2022-03-10 15:04 ` [virtio-comment] " Stefan Hajnoczi
2022-02-21 16:58 ` [PATCH v2 3/4] Add MMIO configuration register "QueueIndirectNum" Christian Schoenebeck
2022-03-10 15:23 ` Stefan Hajnoczi
2022-02-21 17:01 ` [PATCH v2 4/4] Add CCW configuration field "indirect_num" to vq_info_block Christian Schoenebeck
2022-03-10 15:26 ` Stefan Hajnoczi
2022-03-10 16:09 ` [virtio-comment] " Cornelia Huck
2022-03-11 8:53 ` Christian Schoenebeck
2022-03-11 9:21 ` Cornelia Huck [this message]
2022-03-11 11:15 ` Halil Pasic
2022-03-11 12:09 ` [virtio-comment] " Christian Schoenebeck
2022-03-10 16:09 ` Christian Schoenebeck
2022-03-09 14:58 ` [virtio-comment] [PATCH v2 0/4] Add VIRTIO_RING_F_INDIRECT_SIZE and queue_indirect_size Christian Schoenebeck
2022-03-10 15:35 ` Stefan Hajnoczi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87o82chmj1.fsf@redhat.com \
--to=cohuck@redhat.com \
--cc=asmadeus@codewreck.org \
--cc=groug@kaod.org \
--cc=pasic@linux.ibm.com \
--cc=qemu_oss@crudebyte.com \
--cc=stefanha@redhat.com \
--cc=virtio-comment@lists.oasis-open.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox