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