On Thu, Feb 11, 2016 at 07:19:45PM +0100, David Herrmann wrote: > Hi > > On Thu, Feb 11, 2016 at 7:08 PM, Ville Syrjälä > <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > > On Thu, Feb 11, 2016 at 03:54:38PM -0200, Tiago Vignatti wrote: > >> > >> Thanks for reviewing, David. Please take a look in my comments in-line. > >> > >> > >> On 02/09/2016 07:26 AM, David Herrmann wrote: > >> > > >> > On Tue, Dec 22, 2015 at 10:36 PM, Tiago Vignatti > >> > <tiago.vignatti@xxxxxxxxx> wrote: > >> >> From: Daniel Vetter <daniel.vetter@xxxxxxxx> > > <snip> > >> >> + > >> >> +#define DMA_BUF_SYNC_READ (1 << 0) > >> >> +#define DMA_BUF_SYNC_WRITE (2 << 0) > >> >> +#define DMA_BUF_SYNC_RW (DMA_BUF_SYNC_READ | DMA_BUF_SYNC_WRITE) > >> >> +#define DMA_BUF_SYNC_START (0 << 2) > >> >> +#define DMA_BUF_SYNC_END (1 << 2) > >> >> +#define DMA_BUF_SYNC_VALID_FLAGS_MASK \ > >> >> + (DMA_BUF_SYNC_RW | DMA_BUF_SYNC_END) > >> >> + > >> >> +#define DMA_BUF_BASE 'b' > >> >> +#define DMA_BUF_IOCTL_SYNC _IOW(DMA_BUF_BASE, 0, struct dma_buf_sync) > >> > > >> > Why _IOW? A read-only ioctl should be able to call DMA_BUF_SYNC_READ, right? > >> > >> yup. I've changed it to _IOWR now. > > > > AFAICS the ioctl only does copy_from_user() so _IOW seemed perfectly > > correct to me. > > AFAIK, the _IOC_DIR() arguments in ioctls specify the mode of the > operation, not the way the arguments are used. So if read(2) was an > ioctl, it would be annotated as _IOC_READ (even though it _writes_ > into the passed buffer). write(2) would be annotated as _IOC_WRITE > (even though it _reads_ the buffer). As such, they correspond to the > file-access mode, whether you opened it readable and/or writable. > > Anyway, turns out VFS does not verify those. As such, you can specify > whatever you want. I just checked with different existing ioctls > throughout the kernel (`git grep _IOC_DIR`), and they seem to match > what I describe. Not sure which ones you checked because I don't think I've ever seen that interpretation in the kernel. Though I suppose often it sort of matches, eg. when the ioctl just gets/sets some value. Any relationship to some hardware operation is mostly coincidental (well, except when the hardware really just does some kind of read/write operation). And for lost ioctls there is no hardware interaction whatsoever. As far as checking the file access mode goes, well lots of ioctls totally ignore it. About the direction you're right. _IOW means userspace writes to the kernel, and _IOR means userspace reads from the kernel. Which is exactly what the code did. Anyway ioctl-numbers.txt says this: _IO an ioctl with no parameters _IOW an ioctl with write parameters (copy_from_user) _IOR an ioctl with read parameters (copy_to_user) _IOWR an ioctl with both write and read parameters. so I win ;) -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
