On 11/24/25 2:30 PM, Michael S. Tsirkin wrote:
> On Wed, Nov 19, 2025 at 01:15:14PM -0600, Daniel Jurgens wrote:
>> Currently querying and setting capabilities is restricted to a single
>> capability and contained within the virtio PCI driver. However, each
>> device type has generic and device specific capabilities, that may be
>> queried and set. In subsequent patches virtio_net will query and set
>> flow filter capabilities.
>>
>> This changes the size of virtio_admin_cmd_query_cap_id_result. It's safe
>> to do because this data is written by DMA, so a newer controller can't
>> overrun the size on an older kernel.
>>
>> Signed-off-by: Daniel Jurgens <[email protected]>
>> Reviewed-by: Parav Pandit <[email protected]>
>> Reviewed-by: Xuan Zhuo <[email protected]>
>>
>> ---
>> v4: Moved this logic from virtio_pci_modern to new file
>> virtio_admin_commands.
>>
>> v12:
>> - Removed uapi virtio_pci include in virtio_admin.h. MST
>> - Added virtio_pci uapi include to virtio_admin_commands.c
>> - Put () around cap in macro. MST
>> - Removed nonsense comment above VIRTIO_ADMIN_MAX_CAP. MST
>> - +1 VIRTIO_ADMIN_MAX_CAP when calculating array size. MST
>> - Updated commit message
>> ---
>> drivers/virtio/Makefile | 2 +-
>> drivers/virtio/virtio_admin_commands.c | 91 ++++++++++++++++++++++++++
>> include/linux/virtio_admin.h | 80 ++++++++++++++++++++++
>> include/uapi/linux/virtio_pci.h | 6 +-
>> 4 files changed, 176 insertions(+), 3 deletions(-)
>> create mode 100644 drivers/virtio/virtio_admin_commands.c
>> create mode 100644 include/linux/virtio_admin.h
>>
>> diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
>> index eefcfe90d6b8..2b4a204dde33 100644
>> --- a/drivers/virtio/Makefile
>> +++ b/drivers/virtio/Makefile
>> @@ -1,5 +1,5 @@
>> # SPDX-License-Identifier: GPL-2.0
>> -obj-$(CONFIG_VIRTIO) += virtio.o virtio_ring.o
>> +obj-$(CONFIG_VIRTIO) += virtio.o virtio_ring.o virtio_admin_commands.o
>> obj-$(CONFIG_VIRTIO_ANCHOR) += virtio_anchor.o
>> obj-$(CONFIG_VIRTIO_PCI_LIB) += virtio_pci_modern_dev.o
>> obj-$(CONFIG_VIRTIO_PCI_LIB_LEGACY) += virtio_pci_legacy_dev.o
>> diff --git a/drivers/virtio/virtio_admin_commands.c
>> b/drivers/virtio/virtio_admin_commands.c
>> new file mode 100644
>> index 000000000000..a2254e71e8dc
>> --- /dev/null
>> +++ b/drivers/virtio/virtio_admin_commands.c
>> @@ -0,0 +1,91 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +
>> +#include <linux/virtio.h>
>> +#include <linux/virtio_config.h>
>> +#include <linux/virtio_admin.h>
>> +#include <uapi/linux/virtio_pci.h>
>> +
>> +int virtio_admin_cap_id_list_query(struct virtio_device *vdev,
>> + struct virtio_admin_cmd_query_cap_id_result
>> *data)
>> +{
>> + struct virtio_admin_cmd cmd = {};
>> + struct scatterlist result_sg;
>> +
>> + if (!vdev->config->admin_cmd_exec)
>> + return -EOPNOTSUPP;
>> +
>> + sg_init_one(&result_sg, data, sizeof(*data));
>> + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_CAP_ID_LIST_QUERY);
>> + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SELF);
>> + cmd.result_sg = &result_sg;
>> +
>> + return vdev->config->admin_cmd_exec(vdev, &cmd);
>> +}
>> +EXPORT_SYMBOL_GPL(virtio_admin_cap_id_list_query);
>> +
>> +int virtio_admin_cap_get(struct virtio_device *vdev,
>> + u16 id,
>> + void *caps,
>> + size_t cap_size)
>
>
> I still don't get why cap_size needs to be as large as size_t.
>
> if you don't care what's it size is, just say "unsigned".
> or u8 as a hint to users it's a small value.
The size is small for net flow filters, but this is supposed to be a
generic interface for future uses as well. Why limit it?
>
>> +{
>> + struct virtio_admin_cmd_cap_get_data *data;
>> + struct virtio_admin_cmd cmd = {};
>> + struct scatterlist result_sg;
>> + struct scatterlist data_sg;
>> + int err;
>> +
>> + if (!vdev->config->admin_cmd_exec)
>> + return -EOPNOTSUPP;
>> +
>> + data = kzalloc(sizeof(*data), GFP_KERNEL);
>
> uses kzalloc without including linux/slab.h
>
>
>
>> + if (!data)
>> + return -ENOMEM;
>> +
