[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: VIRTIO_RING_F_INDIRECT_SIZE status
On Mon, Mar 06, 2023 at 03:51:13PM +0100, Christian Schoenebeck wrote: > On Wednesday, March 1, 2023 2:57:57 PM CET Stefan Hajnoczi wrote: > > On Wed, Mar 01, 2023 at 01:55:14PM +0100, Christian Schoenebeck wrote: > > > On Tuesday, February 28, 2023 1:05:21 PM CET Michael S. Tsirkin wrote: > > > > On Tue, Feb 28, 2023 at 12:49:00PM +0100, Christian Schoenebeck wrote: > > > > > On Monday, February 27, 2023 6:41:12 PM CET Michael S. Tsirkin wrote: > > > > > > On Mon, Feb 27, 2023 at 05:13:12PM +0100, Christian Schoenebeck wrote: > > > > > > > On Monday, February 27, 2023 4:45:45 PM CET Stefan Hajnoczi wrote: > > > > > > > > On Mon, Feb 27, 2023 at 02:53:55PM +0000, Afsa, Baptiste wrote: > > > > > > > > > The issue that we have now, is that this limitation does not seem to be enforced > > > > > > > > > in Linux virtio drivers today. I came across: > > > > > > > > > > > > > > > > > > https://github.com/oasis-tcs/virtio-spec/issues/122 > > > > > > > > > > > > > > > > > > which looks like a good base for us to build upon, but I'm not sure what is the > > > > > > > > > status with this issue. Do you know whether there is any plan regarding this? > > > > > > > > > > > > > > > > CCing Christian regarding extending queue size limits. > > > > > > > > > > > > > > Status on this issue: it was suspended in May last year. AFAICR Michael > > > > > > > expressed the need to give some more thought about it, and a new virtio spec > > > > > > > release was just ahead at that time. > > > > > > > > > > > > > > Michael, suggestions how to bring this forward? > > > > > > > > > > > > > > Best regards, > > > > > > > Christian Schoenebeck > > > > > > > > > > > > > > > > > > > How about a summary letter listing various options, pros and cons? > > > > > > > > > > Actually I had submitted a draft for this feature (latest version linked > > > > > above), and AFAICR Michael was the only person who expressed concerns from > > > > > design perspective. Comments by other people were already just aboout precise > > > > > wording, but not about the design itself. > > > > > > > > > > We stopped the discussion at the point where Michael expressed the need to > > > > > think more about it, but as his expressed concerns were a bit vague, I still > > > > > don't see how I could bring this issue forward. > > > > > > > > > > Best regards, > > > > > Christian Schoenebeck > > > > > > > > > > > > > > > > > So the last time there were two things: > > > > > > > > > > > > 1. bugs introduced during the packed ring work. For example: > > > > > > > > > > > > > > I don't think so: > > > > > > > > > > 2.6.5.3.1 Driver Requirements: Indirect Descriptors > > > > > .. > > > > > "A driver MUST NOT set both VIRTQ_DESC_F_INDIRECT and VIRTQ_DESC_F_NEXT in > > > > > flags." > > > > > So as far as I can see it, the amount of direct descriptors is currently > > > > > always exactly one if an indirect table is used. > > > > > > > > > > Best regards, > > > > > Christian Schoenebeck > > > > > > > > > > > > > Oh. You are right. Weirdly this text is not in packed-ring.tex - I > > > > think this and a bunch of other cases like this are an oversight, > > > > however we need to fix them first before adding features that assume > > > > they are fixed ... > > > > > > > > we need to get them fixed before we poke at core ring otherwise it's a > > > > mess - maybe the TC will accept just fixing this > > > > without a feature bit, or maybe people will feel a feature bit > > > > is required. > > > > > > It's been a while. Looking at v1.2 it says: > > > > > > 2.8 Packed Virtqueues > > > ... > > > 2.8.5 Scatter-Gather Support [1] > > > ... > > > While unusual (most implementations either create all lists solely using > > > non-indirect descriptors, or always use a single indirect element), if both > > > features have been negotiated, mixing indirect and non-indirect descriptors > > > in a ring is valid, as long as each list only contains descriptors of a > > > given type. > > > > > > [1] https://docs.oasis-open.org/virtio/virtio/v1.2/cs01/virtio-v1.2-cs01.html#x1-770005 > > > > > > To avoid misapprehensions: the way I understand it, same restrictions apply to > > > packed queues as split queues, in the sense that you may neither chain several > > > tables in a single message, nor multi-level nest tables, nor mix a list of > > > direct descriptors and indirect descriptors on the same level within one > > > message. So the explicit exception described here, only means you may use > > > *one* indirect table in one message, while using chained direct descriptors in > > > another message. But that's it, right? > > > > Hi Christian, > > Yes, for packed virtqueues it is forbidden to mix normal and indirect > > descriptors in a single buffer. Further support for this interpretation > > is in 2.8.19 Driver Requirements: Indirect Descriptors: > > > > A driver MUST NOT write direct descriptors with VIRTQ_DESC_F_INDIRECT > > set in a scatter-gather list linked by VIRTQ_DESC_F_NEXT. flags. > > > > VIRTQ_DESC_F_INDIRECT cannot be set if the descriptor chain uses > > VIRTQ_DESC_F_NEXT, therefore it's not possible to mix normal and > > indirect descriptors in a single buffer. > > > > However, the Split Virtqueues section documents an exception where > > mixing normal and indirect descriptors in a single buffer is allowed, > > but only in a specific order. 2.7.5.3.2 Device Requirements: Indirect > > Descriptors says: > > > > The device MUST handle the case of zero or more normal chained > > descriptors followed by a single descriptor with > > flags&VIRTQ_DESC_F_INDIRECT. Note: While unusual (most implementations > > either create a chain solely using non-indirect descriptors, or use a > > single indirect element), such a layout is valid. > > > > So what degree of mixing normal and indirect descriptors is allowed > > depends on the virtqueue format. > > Oh, good catch! I was not aware about this device requirement for split > queues. I thought 2.7.5.3.1 "A driver MUST NOT set both VIRTQ_DESC_F_INDIRECT > and VIRTQ_DESC_F_NEXT in flags" would rule that mixed case out, but right, if > the indirect descriptor is last in the buffer then 2.7.5.3.1 is not violated. > > Which makes Michael's suggestion even mandatory. > > Out of pure curiosity: is this mixed case actively used by somebody? I would love to know as well. :) Stefan
Attachment:
signature.asc
Description: PGP signature
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]