Re: [PATCH v2 5/7] bcm-vk: add bcm_vk UAPI

[Arnd Bergmann]

On Fri, Feb 21, 2020 at 2:16 AM Scott Branden
<scott.branden@xxxxxxxxxxxx> wrote:


> >> +struct vk_access {
> >> +    __u8 barno;     /* BAR number to use */
> >> +    __u8 type;      /* Type of access */
> >> +#define VK_ACCESS_READ 0
> >> +#define VK_ACCESS_WRITE 1
> >> +    __u32 len;      /* length of data */
> > Horrible padding issues, are you sure this all works properly?
> Haven't had any issues.
> >
> >> +    __u64 offset;   /* offset in BAR */
> >> +    __u32 *data;    /* where to read/write data to */
> > Are you _SURE_ you want a pointer here?  How do you handle the compat
> > issues with 32/64 user/kernel space?
> Don't care about 32-bit user space for this driver.
> I don't think there isn't even enough memory in such systems for the
> number of streams of video buffers needed for transcoding.
> This driver is only used in high end 64-bit x86 servers.

Please see Documentation/core-api/ioctl.rst

All ioctl interfaces should be written in a portable way that works with
compat user space and avoids all padding in order to not leak kernel
data into user space.

If the driver is passing video buffers for transcoding, shouldn't the driver
use the existing drivers/media interfaces for that? If it needs features
that are not present there, they can probably be added.

        Arnd