Re: [virtio-comment] Re: [virtio-dev] [PATCH v10 08/10] admin: command list discovery

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Mar 08, 2023 at 01:41:37PM +0100, Jiri Pirko wrote:
> Wed, Mar 08, 2023 at 12:54:52PM CET, mst@redhat.com wrote:
> >On Mon, Mar 06, 2023 at 01:22:30PM +0100, Jiri Pirko wrote:
> >> Thu, Mar 02, 2023 at 02:05:22PM CET, mst@redhat.com wrote:
> >> >Add commands to find out which commands does each group support,
> >> >as well as enable their use by driver.
> >> >This will be especially useful once we have multiple group types.
> >> >
> >> >An alternative is per-type VQs. This is possible but will
> >> >require more per-transport work. Discovery through the vq
> >> >helps keep things contained.
> >> >
> >> >Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
> >> >Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >> 
> >> [...]
> >> 
> >> 
> >> >+
> >> >+The driver issues the command VIRTIO_ADMIN_CMD_LIST_QUERY to
> >> >+query the list of commands valid for this group and before sending
> >> >+any commands for any member of a group.
> >> >+
> >> >+The driver then enables use of some of the opcodes by sending to
> >> >+the device the command VIRTIO_ADMIN_CMD_LIST_USE with a subset
> >> >+of the list returned by VIRTIO_ADMIN_CMD_LIST_QUERY that is
> >> >+both understood and used by the driver.
> >> 
> >> To my untrained ear, this sounds somewhat similar to the feature
> >> negotiantion mechanism. Why the fact that device/driver supports some
> >> command can't be covered by just another feature? Looks like unnecassary
> >> complexicity to negotiate supported commands like this.
> >
> >Absolutely, it is similar. The issue is that a single device can
> >be an owner for multiple group types.
> >For example, I think an SRIOV PF itself can have subfunctions with SIOV
> >as well as virtual functions with SRIOV.
> >And the set of supported commands might differ.
> >
> >We thus need a command that is kind of like features but per group type.
> 
> I see. But that does not mean it can't be done using existing features:
> 
> _F_SRIOV_CMD1
> _F_SRIOV_CMD2
> ..
> _F_SIOV_CMD1
> _F_SIOV_CMD2
> ...

True. But annoying if each command has to be duplicated.

> >
> >As a small bonus as we are building this from scratch, I added
> >something which was requested for a long time from features
> >which is blocking access to commands not allowed by features.
> >With standard virtio features we trust the driver not to send
> >commands before enabling a feature and this was considered
> >a maintainance/security problem.
> 
> I understand. Is it unfixable for features not to break buggy drivers?
> 

I think it is, feature negotiation is hard to evolve since our
compat strategy is built around feature negotiation.

-- 
MST