All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cornelia Huck <cohuck@redhat.com>
To: Halil Pasic <pasic@linux.ibm.com>
Cc: Christian Schoenebeck <qemu_oss@crudebyte.com>,
	virtio-comment@lists.oasis-open.org,
	Stefan Hajnoczi <stefanha@redhat.com>, Greg Kurz <groug@kaod.org>,
	Dominique Martinet <asmadeus@codewreck.org>,
	Halil Pasic <pasic@linux.ibm.com>
Subject: Re: [virtio-comment] Re: [PATCH v3 4/4] Add CCW configuration field "indirect_num"
Date: Tue, 22 Mar 2022 10:57:00 +0100	[thread overview]
Message-ID: <87k0cmqphv.fsf@redhat.com> (raw)
In-Reply-To: <20220322025617.3f9df5c1.pasic@linux.ibm.com>

On Tue, Mar 22 2022, Halil Pasic <pasic@linux.ibm.com> wrote:

> On Mon, 21 Mar 2022 17:36:26 +0100
> Cornelia Huck <cohuck@redhat.com> wrote:
>
>> On Sat, Mar 19 2022, Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
>> 
>> > On Freitag, 18. März 2022 17:06:25 CET Halil Pasic wrote:  
>> 
>> >> I agree that the "including" is important, but I'm not sure about the
>> >> "its contents are undefined". I don't really understand why should we use
>> >> a plural here. What speaks against specifying that in SHOULD be stored
>> >> as 0 by the device, and MUST be ignored by the driver?  
>> >
>> > Both solutions would be viable. Personally I would just use something like 
>> > "Should be zero" if there is a value in recommending that, but I don't see a 
>> > value in recommending to set something to zero and at the same time requiring 
>> > to not access it in the first place.
>> >  
>> >> Currently we say that \field{max_indirect_num} exists like a be32 field
>> >> even if VIRTIO_RING_F_INDIRECT_SIZE is not negotiated. Which kind of
>> >> implies that at least type invariants should hold. Of course, there is
>> >> none here (i.e. every bits value is also a be32 value), but for something
>> >> like an enum interesting corner cases can pop up.  
>> >
>> > I can't follow you on that one. What has that do with enums in this case?
>> >
>> > Anyway, I won't persist on my suggestion to use the (IMO more compact form) 
>> > "undefined". If you guys prefer the more specific solution "SHOULD be 0 and 
>> > MUST not be accessed" then I will go that way.  
>> 
>> I'm not sure what mandating 0 and non-access would buy us here... the
>> driver can of course read the field (e.g. when copying the structure
>> wholesale); it just can't make use of the contents when it did not
>> negotiate the feature (but why would it do so in that case anyway?)
>
> My train of thought was that making the device give us a well defined
> 0 could benefit robustness. The idea was, that even if the driver was
> buggy, and used the value we would still end up with some sane behavior.

I'm not sure a 0 would lead to sane behaviour in an already buggy
driver... operating with a limit of 0 would imply that the driver cannot
really do anything, and I'm not sure a driver buggy enough to access the
field would heed that. There's nothing wrong with a device using 0 if
the feature had not been negotiated, but I don't think it will help much
with already buggy drivers.

>
>> 
>> Also, I think junk remains junk, whether it is a be32 field or
>> interpreted as an enum. It is simply not valid, even if it might by
>> accident end up to be a defined enum value.
>
> What I had in mind is the difference between "trap representation" and
> "unspecified value" in terms of the C standard. Using a "trap
> representation" is undefined behavior, while using an "unspecified value"
> is far less serious. As far as I remember, there are no trap
> representations for enumerated types in C, so the example ain't perfect.
> But if some code was to assume that all it can see it the values defined
> in the enum, strange stuff may happen.

While the struct definitions look suspiciously like C, they are not in
fact C :) I don't think the spec defines anything of the above, and I
don't think it should.

>
>
>> 
>> So I think "undefined" should be fine.
>> 
>
> BTW the C standard uses the term "indeterminate value" in this situation.

"Indeterminate value" is a bit of a mouthful, though; "undefined" or
"unpredictable" from the driver's point of view should already capture
it, as the driver is not supposed to do anything with the value anyway.


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/


  reply	other threads:[~2022-03-22  9:57 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-16 13:44 [PATCH v3 0/4] Add VIRTIO_RING_F_INDIRECT_SIZE and queue_indirect_size Christian Schoenebeck
2022-03-16 13:47 ` [PATCH v3 1/4] Add VIRTIO_RING_F_INDIRECT_SIZE Christian Schoenebeck
2022-03-17 13:40   ` [virtio-comment] " Cornelia Huck
2022-03-18 10:45     ` Christian Schoenebeck
2022-03-18 16:03       ` [virtio-comment] " Cornelia Huck
2022-03-19  9:33   ` [virtio-comment] " Michael S. Tsirkin
2022-03-19 12:00     ` Christian Schoenebeck
2022-03-20 12:31       ` Michael S. Tsirkin
2022-03-20 13:32         ` Christian Schoenebeck
2022-03-20 13:55           ` Michael S. Tsirkin
2022-03-20 15:17             ` Christian Schoenebeck
2022-03-20 16:06               ` Michael S. Tsirkin
2022-03-20 16:07                 ` Michael S. Tsirkin
2022-03-20 17:43                 ` Christian Schoenebeck
2022-03-20 21:52                   ` Michael S. Tsirkin
2022-03-21  9:23                     ` Christian Schoenebeck
2022-03-21 22:13                       ` Michael S. Tsirkin
2022-03-23 10:20                         ` Christian Schoenebeck
2022-03-23 12:35                           ` Michael S. Tsirkin
2022-03-24  9:16                             ` Christian Schoenebeck
2022-03-24 10:36                               ` Michael S. Tsirkin
2022-03-24 11:11                                 ` Christian Schoenebeck
2022-03-24 11:16                                   ` Michael S. Tsirkin
2022-03-24 11:52                                     ` Christian Schoenebeck
2022-03-16 13:50 ` [PATCH v3 2/4] Add PCI configuration field "queue_indirect_size" Christian Schoenebeck
2022-03-16 14:41   ` [virtio-comment] " Christian Schoenebeck
2022-03-16 13:52 ` [PATCH v3 3/4] Add MMIO configuration register "QueueIndirectNum" Christian Schoenebeck
2022-03-16 13:55 ` [PATCH v3 4/4] Add CCW configuration field "indirect_num" Christian Schoenebeck
2022-03-17 14:12   ` [virtio-comment] " Cornelia Huck
2022-03-18 11:02     ` Christian Schoenebeck
2022-03-18 16:06       ` Halil Pasic
2022-03-19 10:12         ` Christian Schoenebeck
2022-03-21 16:36           ` Cornelia Huck
2022-03-22  1:56             ` Halil Pasic
2022-03-22  9:57               ` Cornelia Huck [this message]
2022-03-22 11:21                 ` Halil Pasic
2022-03-18 16:10       ` Cornelia Huck
2022-03-19 10:23         ` Christian Schoenebeck
2022-03-21 16:25           ` Cornelia Huck

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=87k0cmqphv.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.