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

[Jiri Pirko <jiri@nvidia.com>]

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
...

>
>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?


>
>
>-- 
>MST
>