On Sun, Aug 28, 2005 at 02:25:17AM +0200, Reinoud Zandijk wrote:
> Dear folks,
> 
> i'm trying to call a SCIOCCOMMAND ioctl using VOP_IOCTL() on a device node 
> in-kernel. To enable kernel calls i thought the following patch would be 
> sufficient :
> 
> ---------------------
> --- scsipi_ioctl.c      1 Feb 2005 00:19:34 -0000       1.52
> +++ scsipi_ioctl.c      27 Aug 2005 23:54:43 -0000
> @@ -348,7 +348,7 @@
>                         si->si_uio.uio_iovcnt = 1;
>                         si->si_uio.uio_resid = len;
>                         si->si_uio.uio_offset = 0;
> -                       si->si_uio.uio_segflg = UIO_USERSPACE;
> +                       si->si_uio.uio_segflg = (flag & FKIOCTL) ? UIO_SYSSPACE : UIO_USERSPACE;
>                         si->si_uio.uio_rw =
>                             (screq->flags & SCCMD_READ) ? UIO_READ : 
> UIO_WRITE;
>                         si->si_uio.uio_procp = p;
> 
> ----------------------
> 
> only to find out that it uses :
> 			error = physio(scsistrategy, &si->si_bp, dev,
> 			    (screq->flags & SCCMD_READ) ? B_READ : B_WRITE,
> 			    periph->periph_channel->chan_adapter->adapt_minphys,
> 			    &si->si_uio);
> 
> to execute the command. Now this shouldn't be be problem as it passes the 
> userland/kernel space flag in its uio.
> 
> Looking at physio it just bluntly ignores the uio_segflg calling the 
> scsistrategy bluntly with the address provided in the uio_iov. Thus 
> destroying knowledge of if it happened to be a userland of kernel space 
> address.
> 
> Now in kern/kern_physio.c line 197:
>                        /*
>                          * [mark the buffer busy for physical I/O]
>                          * (i.e. set B_PHYS (because it's an I/O to user
>                          * memory, and B_RAW, because B_RAW is to be
>                          * "Set by physio for raw transfers.", in addition
>                          * to the "busy" and read/write flag.)
>                          */
>                         bp->b_flags = B_BUSY | B_PHYS | B_RAW | flags;
> 
> it aparently allways sets B_PHYS. I know these parts of the the kernel are 
> deemed sacret but shouldn't this be dependent on the uio_segflg?
> 
> Why was it choosen to use physio() in the first place when its first 
> getting a struct scsi_ioctl with accompanied buf, filling it in, calling 
> physio that calls scsistrategy() wich lookups the struct scsi_ioctl from 
> the buf and then proceeds...
> 
> Shouldn't scsistrategy() either honour the B_PHYS flag or the 
> si->si_uio.uio_segflg and pass it over to scsipi_command by 
> setting/clearing of the XS_CTL_USERCMD flag?
> 
> If this is not changed i'm forced to use scsipi_command() myself like
> 	return (scsipi_command(cd->sc_periph,
> 	    (void *)&cmd, sizeof(cmd), (void *)data, len, CDRETRIES,
> 	    30000, NULL, flags | XS_CTL_DATA_IN));
> 
> but then i'd have to figure out the struct scsipi_periph wich is a big NONO 
> IMHO since it breaks abstraction and i'd be cleaner to use VOP_IOCTL(). I 
> could offcource encode all nessisary SCSI commands to CDIOCTL's but 
> wouldn't that explode the number of IOCTL's unnessisary?

BTW, what command do you need to send from the kernel ? Maybe we need to
define a new ioctl to retrieve this information ? I would expect your
kernel code to also work for non-scsi devices.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--