Re: [PATCH v4 1/8] media: virtio: Add protocol

[Brian Daniels]

> > +	u32 __reserved;
> 
> Knowing ~ nothing about virtio with ~ no experience of it I'll offer up 
> a question/statement anyway.
> 
> Aren't your reservations here a bit conservative ? Would you not be well 
> advised to reserve a bit more space per other virtio protocols ?
> 
> ➜   grep reserved ./include/uapi/linux/*.h | grep virtio
> ./include/uapi/linux/virtio_balloon.h: *         __u8 reserved[6];
> ./include/uapi/linux/virtio_balloon.h: * In other words, add explicit 
> reserved fields to align field and
> ./include/uapi/linux/virtio_blk.h:      __u8 reserved[38];
> ./include/uapi/linux/virtio_blk.h:      __u8 reserved[56];
> ./include/uapi/linux/virtio_config.h: * VIRTIO_TRANSPORT_F_END are 
> reserved for the transport
> ./include/uapi/linux/virtio_crypto.h:   __le32 reserved;
> ./include/uapi/linux/virtio_i2c.h: * Copyright (c) 2021 Intel 
> Corporation. All rights reserved.
> ./include/uapi/linux/virtio_input.h:    __u8    reserved[5];
> ./include/uapi/linux/virtio_iommu.h:    __u8 
>         reserved[3];
> ./include/uapi/linux/virtio_iommu.h:    __u8 
>         reserved[3];
> ./include/uapi/linux/virtio_iommu.h:    __u8 
>         reserved[3];
> ./include/uapi/linux/virtio_iommu.h:    __u8 
>         reserved[4];
> ./include/uapi/linux/virtio_iommu.h:    __u8 
>         reserved[8];
> ./include/uapi/linux/virtio_iommu.h:    __u8 
>         reserved[4];
> ./include/uapi/linux/virtio_iommu.h:    __u8 
>         reserved[3];
> ./include/uapi/linux/virtio_iommu.h:    __u8 
>         reserved[64];
> ./include/uapi/linux/virtio_iommu.h:    __u8 
>         reserved[3];
> ./include/uapi/linux/virtio_iommu.h:    __u8 
>         reserved2[4];
> ./include/uapi/linux/virtio_net.h:      __le16 reserved[4];
> ./include/uapi/linux/virtio_net.h:      __le16 reserved;
> ./include/uapi/linux/virtio_net.h:              __le16 reserved[3];
> ./include/uapi/linux/virtio_net.h:      __u8 reserved;
> ./include/uapi/linux/virtio_net.h:      __le16 reserved1;
> ./include/uapi/linux/virtio_pcidev.h: * @VIRTIO_PCIDEV_OP_RESERVED: 
> reserved to catch errors
> ./include/uapi/linux/virtio_pcidev.h: * @reserved: reserved
> ./include/uapi/linux/virtio_pcidev.h:   __u16 reserved;
> ./include/uapi/linux/virtio_pci.h:      * 2-65535 - reserved
> ./include/uapi/linux/virtio_pci.h:      /* Unused, reserved for future 
> extensions. */
> ./include/uapi/linux/virtio_pci.h:      __u8 reserved1[12];
> ./include/uapi/linux/virtio_pci.h:      /* Unused, reserved for future 
> extensions. */
> ./include/uapi/linux/virtio_pci.h:      __u8 reserved2[4];
> ./include/uapi/linux/virtio_pci.h:      __u8 reserved[7];
> ./include/uapi/linux/virtio_pci.h:      __u8 reserved[6];
> ./include/uapi/linux/virtio_pci.h:      __u8 reserved[6];
> ./include/uapi/linux/virtio_pci.h:      __u8 reserved[2];
> ./include/uapi/linux/virtio_pci.h:      __u8 reserved[7];
> ./include/uapi/linux/virtio_pci.h:      __u8 reserved[7];
> ./include/uapi/linux/virtio_pci.h:      __u8 reserved;
> ./include/uapi/linux/virtio_pci.h:                      __le32 reserved;
> ./include/uapi/linux/virtio_pci.h:                      __u8 reserved[6];
> ./include/uapi/linux/virtio_pci.h:                      __le32 reserved;
> ./include/uapi/linux/virtio_pci.h:                      __le32 reserved;
> ./include/uapi/linux/virtio_pci.h:                      __le32 reserved;
> ./include/uapi/linux/virtio_pci.h:      __u8 reserved[7];
> ./include/uapi/linux/virtio_rtc.h: * Copyright (c) 2024 Qualcomm 
> Innovation Center, Inc. All rights reserved.
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[6];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[7];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[6];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[6];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[5];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[6];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[6];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[5];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[5];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[7];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[6];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[7];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[5];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[5];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[6];
> ./include/uapi/linux/virtio_spi.h: * Copyright (C) 2025 Qualcomm 
> Innovation Center, Inc. All rights reserved.
> ./include/uapi/linux/virtio_spi.h: *   other bits are reserved as 0, 
> 1-bit transfer is always supported.
> ./include/uapi/linux/virtio_spi.h: *   other bits are reserved as 0, 
> 1-bit transfer is always supported.
> ./include/uapi/linux/virtio_spi.h: * @reserved: for future use.
> ./include/uapi/linux/virtio_spi.h:      __u8 reserved[3];+      u32 
> __reserved;

I'm not an expert here, but taking a look at those files, the vast majority of
those reserved fields appear to be padding to ensure the struct has 64-bit
alignment, which matches the use here in virtio-media as well.

virtio_pci appears to be the only device that explicitly states the
reserved bytes are for future extensions. Unless there's a good a reason to
expect a future use case where more space is needed, I would prefer to not add
more at this time.