On Mon, Dec 18 2023, Alex Bennée alex.bennee@linaro.org wrote:
Cornelia Huck cohuck@redhat.com writes:
On Tue, Dec 05 2023, Alex Bennée alex.bennee@linaro.org wrote:
Cornelia Huck cohuck@redhat.com writes:
On Tue, Dec 05 2023, Viresh Kumar viresh.kumar@linaro.org wrote:
+The device MUST present each event, in a transport defined way, from the +moment it takes place until the driver acknowledges the event.
<snip> >>>> + >>>> +\drivernormative{\subsection}{Virtio Transport Requirements}{Virtio Transport Options} >>>> + >>>> +The driver MUST NOT access guest memory locations outside what's made >>>> +available by the device to the driver. >>> >>> I don't think that makes sense -- I'd assume most guest memory locations >>> do not have anything to do with virtio, and we should try to avoid >>> host/guest terminology. >> >> I agree guest memory isn't the right terminology here. However there are >> discussions about how to implement secure buffers for VirtIO - so for >> example a buffer mediated by some sort of secure layer. In those cases >> the driver may not have access to it outside of the transactions. > > Yes, I think we need to limit the scope of "guest memory" here. I think > we are basically wanting to deal with any memory used by virtio (device > type including memory access controlled by it, transport, and the > protocol itself). We would be talking about memory made available to the > device by the driver for explicit usage to implement the virtio spec. I > think this would cover mediation by a secure layer as well (with the > driver calling into that secure layer?) Or does the (host) device end up > donating memory to the (guest) driver, and we need to make sure it > doesn't scribble over it?
I'm not sure if we have example of the host donating memory apart from the sort of static partitioning we see with guests on start-up where a region is defined as shared. The Xen grant model leaves the guest to grant access to its own pages to the backend.
I guess for firmware mediated sharing this would still be driven by the guest rather than the host?
Yes, I think the guest telling the firmware which parts of its memory the host may access is the usual pattern, or at least what I've seen so far.
+The driver MUST NOT access virtqueue contents before the device notifies +about the readiness of the same.
+The driver MUST NOT access buffers, after it has added them to the +virtqueue and notified the device about their availability. The driver +MAY access them after the device has processed them and notified the +driver of their availability, in a transport defined way.
+The driver MAY ask the device to reset the virtqueues if, for example, +the driver times out waiting for a notification from the device for a +previously queued request.
Again, I believe this has already been covered in the generic sections -- do we instead need to specify that a transport MUST provide a method to do xy? (or SHOULD, MAY, as applicable -- it would be good to list explicitly what is mandatory for a transport to implement, and what is optional.)
Yes I think so. The s390x channel transport gets referenced because it has a nice enumerated list of operations. It would be good to codify which operations are mandatory for all transports and which are optional.
The problem with the ccw transport is that while it has a nice list of operations, (a) it only covers guest-initiated actions,
What examples of host initiated actions are there (aside from an IPI indicating a receive VirtQueue has buffers waiting)?
Also notifications for configuration changes; but I think we can put all of this under the device->driver notification header. (Hmm, we probably should change all those host/guest references for ccw some day...)
(b) probably not all of them shold be mandatory (and some of them are more of an artifact of how channel I/O works),
These ones?
#define CCW_CMD_SET_IND 0x43 #define CCW_CMD_SET_CONF_IND 0x53 #define CCW_CMD_SET_IND_ADAPTER 0x73
Yes, and also CCW_CMD_SET_VIRTIO_REV (I think there used to be some interest to implement something like that for mmio, but I don't think anything ever ended up being specified?)
and (c) it only implements a subset of the defined operations (which makes the not-implemented ones de facto optional, of course :) But yes, we could use it as a starting point.
Got to start somewhere :-)
Indeed :)