Subject: Re: kern/32342: OpenBSD firmware loading framework
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Jason Thorpe <thorpej@shagadelic.org>
List: netbsd-bugs
Date: 12/19/2005 22:25:02
The following reply was made to PR kern/32342; it has been noted by GNATS.

From: Jason Thorpe <thorpej@shagadelic.org>
To: gnats-bugs@netbsd.org
Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org,
	netbsd-bugs@netbsd.org
Subject: Re: kern/32342: OpenBSD firmware loading framework
Date: Mon, 19 Dec 2005 14:20:53 -0800

 On Dec 19, 2005, at 2:10 PM, plunky@rya-online.net wrote:
 
 > 	I want to load driver firmware file from inside the kernel and there
 > 	is no way currently to do that. Christos Zoulas mentioned on tech- 
 > kern
 > 	that OpenBSD had a framwork to handle this so I took a look. This is
 > 	the result, please find below a shar archive containing two new  
 > files:
 >
 > 		sys/dev/firmload.c
 > 		share/man/man9/load_firmware.9
 
 I'm not sure I like this:
 
 - Tries to read it all in at once (into a wired kernel buffer  
 allocated from kmem_map).  It would be better to allow a driver to  
 read in pieces at a time, potentially into pageable memory.
 
 - It should be possible to set the directory search path via sysctl,  
 rather than hard-coding "/etc/firmware".
 
 >
 > 	which are exactly as in OpenBSD. Following this are patches for these
 > 	two files to make them work for NetBSD. Following that is a cvs diff
 > 	against the following files:
 >
 > 		sys/conf/files
 > 		sys/sys/device.h
 > 		share/man/man9/Makefile
 >
 > 	to integrate this patch with the -current version of NetBSD
 >
 >> How-To-Repeat:
 > 	
 >> Fix:
 >
 > # This is a shell archive.  Save it in a file, remove anything before
 > # this line, and then unpack it by entering "sh file".  Note, it may
 > # create directories; files and directories will be owned by you and
 > # have default permissions.
 > #
 > # This archive contains:
 > #
 > #	sys/dev/firmload.c
 > #	share/man/man9/load_firmware.9
 > #
 > echo x - sys/dev/firmload.c
 > sed 's/^X//' >sys/dev/firmload.c << 'END-of-sys/dev/firmload.c'
 > X/*	$OpenBSD: firmload.c,v 1.5 2005/08/01 08:15:02 deraadt Exp $	*/
 > X
 > X/*
 > X * Copyright (c) 2004 Theo de Raadt <deraadt@openbsd.org>
 > X *
 > X * Permission to use, copy, modify, and distribute this software  
 > for any
 > X * purpose with or without fee is hereby granted, provided that  
 > the above
 > X * copyright notice and this permission notice appear in all copies.
 > X *
 > X * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL  
 > WARRANTIES
 > X * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
 > X * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE  
 > LIABLE FOR
 > X * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY  
 > DAMAGES
 > X * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER  
 > IN AN
 > X * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,  
 > ARISING OUT OF
 > X * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 > X */
 > X
 > X#include <sys/param.h>
 > X#include <sys/systm.h>
 > X#include <sys/syslimits.h>
 > X#include <sys/time.h>
 > X#include <sys/namei.h>
 > X#include <sys/vnode.h>
 > X#include <sys/mount.h>
 > X#include <sys/errno.h>
 > X#include <sys/malloc.h>
 > X#include <sys/proc.h>
 > X#include <sys/device.h>
 > X
 > Xint
 > Xloadfirmware(const char *name, u_char **bufp, size_t *buflen)
 > X{
 > X	struct proc *p = curproc;
 > X	struct nameidata nid;
 > X	char *path, *ptr;
 > X	struct iovec iov;
 > X	struct uio uio;
 > X	struct vattr va;
 > X	int error;
 > X
 > X	if (!rootvp || !vcount(rootvp))
 > X		return (EIO);
 > X
 > X	path = malloc(MAXPATHLEN, M_TEMP, M_NOWAIT);
 > X	if (path == NULL)
 > X		return (ENOMEM);
 > X		
 > X	snprintf(path, MAXPATHLEN, "/etc/firmware/%s", name);
 > X
 > X	NDINIT(&nid, LOOKUP, NOFOLLOW|LOCKLEAF, UIO_SYSSPACE, path, p);
 > X	error = namei(&nid);
 > X	if (error)
 > X		goto err;
 > X	error = VOP_GETATTR(nid.ni_vp, &va, p->p_ucred, p);
 > X	if (error)
 > X		goto fail;
 > X	if (va.va_size == 0) {
 > X		error = EINVAL;
 > X		goto fail;
 > X	}
 > X	if (va.va_size > FIRMWARE_MAX) {
 > X		error = E2BIG;
 > X		goto fail;
 > X	}
 > X	ptr = malloc(va.va_size, M_DEVBUF, M_NOWAIT);
 > X	if (ptr == NULL) {
 > X		error = ENOMEM;
 > X		goto fail;
 > X	}
 > X
 > X	iov.iov_base = ptr;
 > X	iov.iov_len = va.va_size;
 > X	uio.uio_iov = &iov;
 > X	uio.uio_iovcnt = 1;
 > X	uio.uio_offset = 0;
 > X	uio.uio_resid = va.va_size;
 > X	uio.uio_segflg = UIO_SYSSPACE;
 > X	uio.uio_rw = UIO_READ;
 > X	uio.uio_procp = p;
 > X
 > X	error = VOP_READ(nid.ni_vp, &uio, 0, NOCRED);
 > X
 > X	if (error == 0) {
 > X		*bufp = ptr;
 > X		*buflen = va.va_size;
 > X	} else
 > X		free(ptr, M_DEVBUF);
 > X
 > Xfail:
 > X	vput(nid.ni_vp);
 > Xerr:
 > X	if (path)
 > X		free(path, M_TEMP);
 > X	return (error);
 > X}
 > END-of-sys/dev/firmload.c
 > echo x - share/man/man9/load_firmware.9
 > sed 's/^X//' >share/man/man9/load_firmware.9 << 'END-of-share/man/ 
 > man9/load_firmware.9'
 > X.\"	$OpenBSD: loadfirmware.9,v 1.2 2004/11/22 16:41:44 jmc Exp $
 > X.\"
 > X.\" Copyright (c) 2004 Theo de Raadt
 > X.\" All rights reserved.
 > X.\"
 > X.\" Permission to use, copy, modify, and distribute this software  
 > for any
 > X.\" purpose with or without fee is hereby granted, provided that  
 > the above
 > X.\" copyright notice and this permission notice appear in all copies.
 > X.\"
 > X.\" THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL  
 > WARRANTIES
 > X.\" WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
 > X.\" MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE  
 > LIABLE FOR
 > X.\" ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY  
 > DAMAGES
 > X.\" WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS,  
 > WHETHER IN AN
 > X.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,  
 > ARISING OUT OF
 > X.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 > X.\"
 > X.Dd November 21, 2004
 > X.Dt LOADFIRMWARE 9
 > X.Os
 > X.Sh NAME
 > X.Nm loadfirmware
 > X.Nd load a firmware file from the filesystem
 > X.Sh SYNOPSIS
 > X.Fd #include <sys/device.h>
 > X.Ft int
 > X.Fn loadfirmware "const char *filename" "u_char **buf" "size_t  
 > *buflen"
 > X.Sh DESCRIPTION
 > XThe
 > X.Fn loadfirmware
 > Xfunction loads a firmware from the file specified by
 > X.Ar filename
 > Xin the directory
 > X.Pa /etc/firmware .
 > XMemory for the firmware is allocated using
 > X.Xr malloc 9
 > Xwith type
 > X.Va M_DEVBUF
 > Xas need be, within a reasonable size limit.
 > X.Pp
 > XIf no longer needed, the firmware buffer
 > X.Va buf
 > Xcan be freed using
 > X.Xr free 9
 > Xwith type
 > X.Va M_DEVBUF .
 > X.Sh RETURN VALUES
 > XIf successful,
 > X.Ar buf
 > Xis set to point to the allocation and
 > X.Ar buflen
 > Xis set to the size of the firmware.
 > XThen
 > X.Fn loadfirmware
 > Xreturns 0.
 > XOtherwise, it returns an
 > X.Va errno
 > Xstyle error.
 > END-of-share/man/man9/load_firmware.9
 > exit
 >
 > --- sys/dev/firmload.c	2005-08-01 09:15:02.000000000 +0100
 > +++ sys/dev/firmload.c	2005-12-19 21:46:02.000000000 +0000
 > @@ -1,3 +1,4 @@
 > +/*	$NetBSD: firmload.c$	*/
 >  /*	$OpenBSD: firmload.c,v 1.5 2005/08/01 08:15:02 deraadt Exp $	*/
 >
 >  /*
 > @@ -29,11 +30,12 @@
 >  #include <sys/device.h>
 >
 >  int
 > -loadfirmware(const char *name, u_char **bufp, size_t *buflen)
 > +load_firmware(const char *name, uint8_t **bufp, size_t *buflen)
 >  {
 > -	struct proc *p = curproc;
 > +	struct lwp *l = curlwp;
 >  	struct nameidata nid;
 > -	char *path, *ptr;
 > +	char *path;
 > +	uint8_t *ptr;
 >  	struct iovec iov;
 >  	struct uio uio;
 >  	struct vattr va;
 > @@ -48,11 +50,11 @@
 >  		
 >  	snprintf(path, MAXPATHLEN, "/etc/firmware/%s", name);
 >
 > -	NDINIT(&nid, LOOKUP, NOFOLLOW|LOCKLEAF, UIO_SYSSPACE, path, p);
 > +	NDINIT(&nid, LOOKUP, NOFOLLOW|LOCKLEAF, UIO_SYSSPACE, path, l);
 >  	error = namei(&nid);
 >  	if (error)
 >  		goto err;
 > -	error = VOP_GETATTR(nid.ni_vp, &va, p->p_ucred, p);
 > +	error = VOP_GETATTR(nid.ni_vp, &va, l->l_proc->p_ucred, l);
 >  	if (error)
 >  		goto fail;
 >  	if (va.va_size == 0) {
 > @@ -77,7 +79,7 @@
 >  	uio.uio_resid = va.va_size;
 >  	uio.uio_segflg = UIO_SYSSPACE;
 >  	uio.uio_rw = UIO_READ;
 > -	uio.uio_procp = p;
 > +	uio.uio_lwp = l;
 >
 >  	error = VOP_READ(nid.ni_vp, &uio, 0, NOCRED);
 >
 > --- share/man/man9/load_firmware.9	2004-11-22 16:41:44.000000000 +0000
 > +++ share/man/man9/load_firmware.9	2005-12-19 21:56:48.000000000 +0000
 > @@ -1,3 +1,4 @@
 > +.\"	$NetBSD: load_firmware.9$
 >  .\"	$OpenBSD: loadfirmware.9,v 1.2 2004/11/22 16:41:44 jmc Exp $
 >  .\"
 >  .\" Copyright (c) 2004 Theo de Raadt
 > @@ -16,18 +17,18 @@
 >  .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 >  .\"
 >  .Dd November 21, 2004
 > -.Dt LOADFIRMWARE 9
 > +.Dt LOAD_FIRMWARE 9
 >  .Os
 >  .Sh NAME
 > -.Nm loadfirmware
 > +.Nm load_firmware
 >  .Nd load a firmware file from the filesystem
 >  .Sh SYNOPSIS
 >  .Fd #include <sys/device.h>
 >  .Ft int
 > -.Fn loadfirmware "const char *filename" "u_char **buf" "size_t  
 > *buflen"
 > +.Fn load_firmware "const char *filename" "u_char **buf" "size_t  
 > *buflen"
 >  .Sh DESCRIPTION
 >  The
 > -.Fn loadfirmware
 > +.Fn load_firmware
 >  function loads a firmware from the file specified by
 >  .Ar filename
 >  in the directory
 > @@ -36,9 +37,10 @@
 >  .Xr malloc 9
 >  with type
 >  .Va M_DEVBUF
 > -as need be, within a reasonable size limit.
 > +as need be, within a reasonable size limit defined by
 > +.Va FIRMWARE_MAX .
 >  .Pp
 > -If no longer needed, the firmware buffer
 > +When no longer needed, the firmware buffer
 >  .Va buf
 >  can be freed using
 >  .Xr free 9
 > @@ -51,7 +53,7 @@
 >  .Ar buflen
 >  is set to the size of the firmware.
 >  Then
 > -.Fn loadfirmware
 > +.Fn load_firmware
 >  returns 0.
 >  Otherwise, it returns an
 >  .Va errno
 >
 > Index: sys/conf/files
 > ===================================================================
 > RCS file: /cvsroot/src/sys/conf/files,v
 > retrieving revision 1.746
 > diff -u -r1.746 files
 > --- sys/conf/files	7 Dec 2005 00:42:03 -0000	1.746
 > +++ sys/conf/files	19 Dec 2005 21:54:52 -0000
 > @@ -234,6 +235,9 @@
 >  define 	pckbport	{[slot = -1]}
 >  define	pckbport_machdep_cnattach
 >
 > +# filesystem firmware loading attribute
 > +define firmload
 > +
 >  # audio device attributes
 >  #
 >  define	mulaw
 > @@ -1169,6 +1170,7 @@
 >  file	dev/dkwedge/dkwedge_bsdlabel.c	dkwedge_method_bsdlabel
 >  file	dev/dkwedge/dkwedge_gpt.c	dkwedge_method_gpt
 >  file	dev/dkwedge/dkwedge_mbr.c	dkwedge_method_mbr
 > +file	dev/firmload.c			firmload
 >  file	dev/fss.c			fss			needs-count
 >  file	dev/md.c			md			needs-count
 >  file	dev/midi.c			midi | midibus		needs-flag
 > Index: sys/sys/device.h
 > ===================================================================
 > RCS file: /cvsroot/src/sys/sys/device.h,v
 > retrieving revision 1.81
 > diff -u -r1.81 device.h
 > --- sys/sys/device.h	11 Dec 2005 12:25:20 -0000	1.81
 > +++ sys/sys/device.h	19 Dec 2005 21:54:54 -0000
 > @@ -417,6 +417,9 @@
 >  #define	device_lookup(cfd, unit)					\
 >  	(((unit) < (cfd)->cd_ndevs) ? (cfd)->cd_devs[(unit)] : NULL)
 >
 > +int	load_firmware(const char *, uint8_t **, size_t *);
 > +#define FIRMWARE_MAX	(5*1024*1024)
 > +
 >  #ifdef DDB
 >  void event_print(int, void (*)(const char *, ...));
 >  #endif
 > Index: share/man/man9/Makefile
 > ===================================================================
 > RCS file: /cvsroot/src/share/man/man9/Makefile,v
 > retrieving revision 1.183
 > diff -u -r1.183 Makefile
 > --- share/man/man9/Makefile	24 Nov 2005 08:20:51 -0000	1.183
 > +++ share/man/man9/Makefile	19 Dec 2005 21:54:56 -0000
 > @@ -21,8 +21,8 @@
 >  	ieee80211_radiotap.9 \
 >  	in4_cksum.9 inittodr.9 intro.9 ioasic.9 ioctl.9 ipkdb.9 isa.9 \
 >  	isapnp.9 itimerfix.9 kcopy.9 kcont.9 \
 > -	kfilter_register.9 knote.9 \
 > -	kprintf.9 kthread.9 linedisc.9 lock.9 log.9 ltsleep.9 \
 > +	kfilter_register.9 knote.9 kprintf.9 kthread.9 linedisc.9 \
 > +	load_firmware.9 lock.9 log.9 ltsleep.9 \
 >  	malloc.9 mbuf.9 mca.9 memcmp.9 memcpy.9 memmove.9 memset.9 \
 >  	microtime.9 mstohz.9 m_tag.9 namecache.9 namei.9 need_resched.9 \
 >  	opencrypto.9 \
 >
 >> Unformatted:
 >  	
 >  	
 
 -- thorpej