Re: Using shift on external keyboards in softraid passphrases from efiboot
On Fri, Aug 24, 2018 at 11:50:51AM +0900, YASUOKA Masahiko wrote:
> Hi,
>
> I think the diff should be brought to arm64 as well. ok?
ok. But shouldn't armv7 also be kept in sync?
>
> On Thu, 23 Aug 2018 11:21:57 +0900 (JST)
> YASUOKA Masahiko wrote:
> > On Mon, 20 Aug 2018 13:50:13 +0200
> > Theo Buehler wrote:
> >> On Thu, Aug 16, 2018 at 09:51:32PM +0200, Frank Groeneveld wrote:
> >>> I haven't been able to type the passphrase of my softraid device on
> >>> boot when using an external keyboard on my Thinkpad X260. Finally I
> >>> had some time to debug this problem and this is what I discovered.
> >>>
> >>> On a different laptop with EFI, the ReadKeyStroke call will not return
> >>> a packet when shift is pressed on the external keyboard. On the
> >>> Thinkpad however, a packet is returned with UnicodeChar == 0, which
> >>> results in a wrong passphrase being used.
> >>>
> >>> This seems like a bug in the firmware to me, because according to some
> >>> EFI specifications I found online, this should not return a packet.
> >>> I've attached a simple patch that fixes this, but I'm not sure whether
> >>> this might break things on different systems.
> >>
> >> I can't comment on the technical side of this patch but I can confirm
> >> that it allows me to enter the password from an external keyboard with
> >> my x280.
> >
> > In the spec,
> >
> > | The UnicodeChar is the actual printable character or is zero if the
> > | key does not represent a printable character (control key, function
> > | key, etc.).
> >
> > It seems that UnicodeChar can be zero. So I think the diff is OK even
> > on the spec.
> >
> > If there is no futher comment I'll commit it. Thanks.
>
>
> Index: sys/arch/arm64/stand/efiboot/efiboot.c
> ===
> RCS file: /cvs/src/sys/arch/arm64/stand/efiboot/efiboot.c,v
> retrieving revision 1.20
> diff -u -p -r1.20 efiboot.c
> --- sys/arch/arm64/stand/efiboot/efiboot.c23 Aug 2018 15:31:12 -
> 1.20
> +++ sys/arch/arm64/stand/efiboot/efiboot.c24 Aug 2018 02:44:30 -
> @@ -129,7 +129,7 @@ efi_cons_getc(dev_t dev)
> }
>
> status = conin->ReadKeyStroke(conin, &key);
> - while (status == EFI_NOT_READY) {
> + while (status == EFI_NOT_READY || key.UnicodeChar == 0) {
> if (dev & 0x80)
> return (0);
> /*
Re: [PATCH] Pluggable disk formats for vmd (qcow2 preparation)
On Tue, 21 Aug 2018 23:03:36 -0700, Carlos Cardenas
wrote:
> On Sun, Aug 19, 2018 at 11:46:12PM -0700, Ori Bernstein wrote:
> > One more minor update to this patch:
> >
> > - Remove unused enum
> > - Remove unused function prototype
> > - Move some qcow2-specific headers into the qcow2 patch.
>
> Ori, it's nice seeing good progress on this. I have a couple of
> questions inline below.
>
> Why are we making sz represent the number of 512 byte sectors? This may
> be true for hd disks but it needs to be *bytes* for ISO/cdrom.
Oops, was focused on hard disks, and misread what CDs did. Moved this division
out to the hard disk initialization. Fixed in both this and the qcow2, but
will see if you have any more comments on the qcow2 patch before I spam the
list with yet another minor revision.
> This has to be the number of bytes shifted to get the number of 2K
> blocks. As the patch is, this yields the incorrect number of blocks an
> ISO has. This breaks ISO/cdrom support.
Solved, see above -- we can now boot from and mount CD correctly.
> [snip]
> > +struct virtio_backing {
> > + void *p;
> > + char *path;
> > + ssize_t (*pread)(void *p, char *buf, size_t len, off_t off);
> > + ssize_t (*pwrite)(void *p, char *buf, size_t len, off_t off);
> > + void (*close)(void *p);
> > +};
>
> What is path used for? I don't see it used anywhere in this patch or in
> the qcow2 patch.
The intent was to use it for better error messages, especially since with
qcow2 snapshots you can smear the disk image across multiple files. I never
hooked it into any errors, and it's probably not worth the trouble.
Updated patch inline below:
---
usr.sbin/vmd/Makefile | 2 +-
usr.sbin/vmd/vioraw.c | 73 ++
usr.sbin/vmd/vioscsi.c | 7 +++--
usr.sbin/vmd/virtio.c | 56 +-
usr.sbin/vmd/virtio.h | 16 ---
5 files changed, 124 insertions(+), 30 deletions(-)
create mode 100644 usr.sbin/vmd/vioraw.c
diff --git usr.sbin/vmd/Makefile usr.sbin/vmd/Makefile
index 7ea090f8886..24c1d1b1d4a 100644
--- usr.sbin/vmd/Makefile
+++ usr.sbin/vmd/Makefile
@@ -6,7 +6,7 @@ PROG= vmd
SRCS= vmd.c control.c log.c priv.c proc.c config.c vmm.c
SRCS+= vm.c loadfile_elf.c pci.c virtio.c i8259.c mc146818.c
SRCS+= ns8250.c i8253.c vmboot.c ufs.c disklabel.c dhcp.c packet.c
-SRCS+= parse.y atomicio.c vioscsi.c
+SRCS+= parse.y atomicio.c vioscsi.c vioraw.c
CFLAGS+= -Wall -I${.CURDIR}
CFLAGS+= -Wstrict-prototypes -Wmissing-prototypes
diff --git usr.sbin/vmd/vioraw.c usr.sbin/vmd/vioraw.c
new file mode 100644
index 000..0dcb8b3c9c2
--- /dev/null
+++ usr.sbin/vmd/vioraw.c
@@ -0,0 +1,73 @@
+/* $OpenBSD: $ */
+/*
+ * Copyright (c) 2018 Ori Bernstein
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include
+
+#include
+#include
+
+#include
+#include
+
+#include "vmd.h"
+#include "vmm.h"
+#include "virtio.h"
+
+static ssize_t
+raw_pread(void *file, char *buf, size_t len, off_t off)
+{
+ return pread(*(int *)file, buf, len, off);
+}
+
+static ssize_t
+raw_pwrite(void *file, char *buf, size_t len, off_t off)
+{
+ return pwrite(*(int *)file, buf, len, off);
+}
+
+static void
+raw_close(void *file)
+{
+ close(*(int *)file);
+ free(file);
+}
+
+/*
+ * Initializes a raw disk image backing file from an fd.
+ * Stores the number of 512 byte sectors in *szp,
+ * returning -1 for error, 0 for success.
+ */
+int
+virtio_init_raw(struct virtio_backing *file, off_t *szp, int fd)
+{
+ off_t sz;
+ int *fdp;
+
+ sz = lseek(fd, 0, SEEK_END);
+ if (sz == -1)
+ return -1;
+
+ fdp = malloc(sizeof(int));
+ *fdp = fd;
+ file->p = fdp;
+ file->pread = raw_pread;
+ file->pwrite = raw_pwrite;
+ file->close = raw_close;
+ *szp = sz;
+ return 0;
+}
+
diff --git usr.sbin/vmd/vioscsi.c usr.sbin/vmd/vioscsi.c
index 93867887598..af504f0550d 100644
--- usr.sbin/vmd/vioscsi.c
+++ usr.sbin/vmd/vioscsi.c
@@ -197,7 +197,7 @@ vioscsi_start_read(struct vioscsi_dev *dev, off_t block,
ssize_t n_blocks)
goto nomem;
info->len = n_blocks * VIOSCSI_BLOCK_SIZE_CDROM;
Re: Using shift on external keyboards in softraid passphrases from efiboot
On Fri, Aug 24, 2018 at 10:47:27AM +0200, Theo Buehler wrote:
> On Fri, Aug 24, 2018 at 11:50:51AM +0900, YASUOKA Masahiko wrote:
> > Hi,
> >
> > I think the diff should be brought to arm64 as well. ok?
>
> ok. But shouldn't armv7 also be kept in sync?
Exactly.
>
> >
> > On Thu, 23 Aug 2018 11:21:57 +0900 (JST)
> > YASUOKA Masahiko wrote:
> > > On Mon, 20 Aug 2018 13:50:13 +0200
> > > Theo Buehler wrote:
> > >> On Thu, Aug 16, 2018 at 09:51:32PM +0200, Frank Groeneveld wrote:
> > >>> I haven't been able to type the passphrase of my softraid device on
> > >>> boot when using an external keyboard on my Thinkpad X260. Finally I
> > >>> had some time to debug this problem and this is what I discovered.
> > >>>
> > >>> On a different laptop with EFI, the ReadKeyStroke call will not return
> > >>> a packet when shift is pressed on the external keyboard. On the
> > >>> Thinkpad however, a packet is returned with UnicodeChar == 0, which
> > >>> results in a wrong passphrase being used.
> > >>>
> > >>> This seems like a bug in the firmware to me, because according to some
> > >>> EFI specifications I found online, this should not return a packet.
> > >>> I've attached a simple patch that fixes this, but I'm not sure whether
> > >>> this might break things on different systems.
> > >>
> > >> I can't comment on the technical side of this patch but I can confirm
> > >> that it allows me to enter the password from an external keyboard with
> > >> my x280.
> > >
> > > In the spec,
> > >
> > > | The UnicodeChar is the actual printable character or is zero if the
> > > | key does not represent a printable character (control key, function
> > > | key, etc.).
> > >
> > > It seems that UnicodeChar can be zero. So I think the diff is OK even
> > > on the spec.
> > >
> > > If there is no futher comment I'll commit it. Thanks.
> >
> >
> > Index: sys/arch/arm64/stand/efiboot/efiboot.c
> > ===
> > RCS file: /cvs/src/sys/arch/arm64/stand/efiboot/efiboot.c,v
> > retrieving revision 1.20
> > diff -u -p -r1.20 efiboot.c
> > --- sys/arch/arm64/stand/efiboot/efiboot.c 23 Aug 2018 15:31:12 -
> > 1.20
> > +++ sys/arch/arm64/stand/efiboot/efiboot.c 24 Aug 2018 02:44:30 -
> > @@ -129,7 +129,7 @@ efi_cons_getc(dev_t dev)
> > }
> >
> > status = conin->ReadKeyStroke(conin, &key);
> > - while (status == EFI_NOT_READY) {
> > + while (status == EFI_NOT_READY || key.UnicodeChar == 0) {
> > if (dev & 0x80)
> > return (0);
> > /*
>
dl_iterate_phdr(3) manual tweak
Hi,
I've been working with dl_iterate_phdr(3) recently, and noticed a
missing detail from our manual page: when the callback returns non-zero,
iteration stops. This is documented in the Linux manual page, and I've
also verified this behaviour in our code:
dlfcn.c:
---8<---
for (object = _dl_objects; object != NULL; object = object->next) {
...
retval = callback(&info, sizeof (struct dl_phdr_info), data);
if (retval)
break;
}
--->8---
Any objections to the following change?
Index: dl_iterate_phdr.3
===
RCS file: /cvs/src/share/man/man3/dl_iterate_phdr.3,v
retrieving revision 1.4
diff -u -p -r1.4 dl_iterate_phdr.3
--- dl_iterate_phdr.3 5 Jun 2013 03:42:03 - 1.4
+++ dl_iterate_phdr.3 24 Aug 2018 09:19:02 -
@@ -34,6 +34,11 @@ for each shared object, passing it infor
program headers and the
.Fa data
argument.
+Iteration continues until either there are no more objects to
+iterate over, or until
+.Fa callback
+returns a non-zero value.
+.Pp
The information about the program headers is passed in a structure
that is defined as:
.Bd -literal
--
Best Regards
Edd Barrett
http://www.theunixzoo.co.uk
Re: dl_iterate_phdr(3) manual tweak
> Date: Fri, 24 Aug 2018 10:20:19 +0100
> From: Edd Barrett
>
> Hi,
>
> I've been working with dl_iterate_phdr(3) recently, and noticed a
> missing detail from our manual page: when the callback returns non-zero,
> iteration stops. This is documented in the Linux manual page, and I've
> also verified this behaviour in our code:
>
> dlfcn.c:
> ---8<---
> for (object = _dl_objects; object != NULL; object = object->next) {
> ...
> retval = callback(&info, sizeof (struct dl_phdr_info), data);
> if (retval)
> break;
> }
> --->8---
>
> Any objections to the following change?
correct, but I'd leave out the second "until".
> Index: dl_iterate_phdr.3
> ===
> RCS file: /cvs/src/share/man/man3/dl_iterate_phdr.3,v
> retrieving revision 1.4
> diff -u -p -r1.4 dl_iterate_phdr.3
> --- dl_iterate_phdr.3 5 Jun 2013 03:42:03 - 1.4
> +++ dl_iterate_phdr.3 24 Aug 2018 09:19:02 -
> @@ -34,6 +34,11 @@ for each shared object, passing it infor
> program headers and the
> .Fa data
> argument.
> +Iteration continues until either there are no more objects to
> +iterate over, or until
> +.Fa callback
> +returns a non-zero value.
> +.Pp
> The information about the program headers is passed in a structure
> that is defined as:
> .Bd -literal
>
> --
> Best Regards
> Edd Barrett
>
> http://www.theunixzoo.co.uk
>
>
Re: dl_iterate_phdr(3) manual tweak
On Fri, Aug 24, 2018 at 11:47:35AM +0200, Mark Kettenis wrote: > correct, but I'd leave out the second "until". I prefer with the second until, but I'm not super-fussed. Here's the diff without the second, OK? Index: dl_iterate_phdr.3 === RCS file: /cvs/src/share/man/man3/dl_iterate_phdr.3,v retrieving revision 1.4 diff -u -p -r1.4 dl_iterate_phdr.3 --- dl_iterate_phdr.3 5 Jun 2013 03:42:03 - 1.4 +++ dl_iterate_phdr.3 24 Aug 2018 09:58:53 - @@ -34,6 +34,11 @@ for each shared object, passing it infor program headers and the .Fa data argument. +Iteration continues until either there are no more objects to +iterate over, or +.Fa callback +returns a non-zero value. +.Pp The information about the program headers is passed in a structure that is defined as: .Bd -literal -- Best Regards Edd Barrett http://www.theunixzoo.co.uk
Re: ospfd: prevent additional ospfd from starting
On Fri, Aug 24, 2018 at 08:58:12AM +0200, Claudio Jeker wrote: > On Wed, Aug 22, 2018 at 12:12:10AM +0200, Remi Locherer wrote: > > On Tue, Aug 21, 2018 at 05:54:18PM +0100, Stuart Henderson wrote: > > > On 2018/08/21 17:16, Remi Locherer wrote: > > > > Hi tech, > > > > > > > > recently we had a short outage in our network. A script started an > > > > additional > > > > ospfd instance because the -n flag for config test was missing. > > > > > > This is a problem with bgpd as well, last time I did this it killed one > > > of the > > > *other* routers on the network (i.e. not just the one where I > > > accidentally ran > > > 2x bgpd...). > > > > > > > What then happend was not nice: > > > > - The new ospfd unlinked the control socket of the first ospfd > > > > - The new ospfd removed all routes from the first ospfd > > > > - The new ospfd was not able to build up an adjacency and therefore > > > > could > > > > not install the routes needed for a recovery. > > > > - Both ospfd instances were running but non-functional. > > > > > > > > Of course the faulty script is fixed by now. ;-) > > > > > > > > It would be nice if ospfd could prevent such a situation. > > > > > > > > Below diff does these things: > > > > - Detect a running ospfd by first doing a connect on the control socket. > > > > - Do not delete the control socket on exit. > > > > - This could delete the socket of another instance. > > > > - Unlinking the socket on shutdown will be in the way once we add > > > > pledge > > > > to the main process. It was removed recently from various daemons. > > > > > > This all sounds very sensible. > > > > > > > - Do not delete routes added by another process even if they have > > > > prio RTP_OSPF. Without this the new ospfd will remove all the routes > > > > of the first one. > > > > > > I'm unsure about this, the above changes stop the new ospfd from running > > > don't they, so that shouldn't be a problem? > > > > It stops to late. kr_init happens before and kill all existing routes with > > priority 32. And again in the shutdown function of ospfd. > > > > > > If an ospfd blows up for whatever reason, it would be quite inconvenient > > > if it needs manual route tweaks rather than just 'rcctl start ospfd' to > > > fix it .. > > > > Yes, this is not optimal. > > > > The new diff below defers kr_init until the ospf engine notifies the parent > > that the control socket is ready. In case the ospf engine exits because the > > control socket is already in use no routes are known that could be removed. > > > > With this ospfd keeps the behaviour of removing foreign routes with > > priority 32. > > > > Better? > > > > Why are we not checking the control socket in the parent? > Also it may be better to create the control socket in the parent and pass > it to the ospfe. This is what bgpd is doing and allows to change the path > during runtime with a config reload. This makes sense to me. I'll come up with a new diff once I found some time for it. But I'm not sure about changing the socket path with a reload. I plan to pledge (stdio rpath sendfd wroute) and eventually unveil (read ospfd.conf) the main process. > > Could there be a case where this causes ospfd to hang on start in the > connect? Not sure if we can sleep doing a connect() to a AF_UNIX socket.
fix assignment in if-clause in smsc(4)
Hi,
smsc(4) needs a fix to an assignment which I found with cppcheck. FreeBSD also
made this change 2 years ago on rev 295608.
OK?
Index: if_smsc.c
===
RCS file: /cvs/src/sys/dev/usb/if_smsc.c,v
retrieving revision 1.31
diff -u -p -u -r1.31 if_smsc.c
--- if_smsc.c 29 Jul 2017 17:24:04 - 1.31
+++ if_smsc.c 24 Aug 2018 10:31:21 -
@@ -774,7 +774,7 @@ smsc_chip_init(struct smsc_softc *sc)
smsc_write_reg(sc, SMSC_PM_CTRL, SMSC_PM_CTRL_PHY_RST);
if ((err = smsc_wait_for_bits(sc, SMSC_PM_CTRL,
- SMSC_PM_CTRL_PHY_RST) != 0)) {
+ SMSC_PM_CTRL_PHY_RST)) != 0) {
smsc_warn_printf(sc, "timed-out waiting for phy reset to "
"complete\n");
goto init_failed;
fix assignment in if-clause in upgt(4)
Hi,
Similar to the patch I just sent for smsc(4) this one also has a misplaced
parenthesis in an if-clause. NetBSD fixed this on rev 1.13.
OK?
Index: if_upgt.c
===
RCS file: /cvs/src/sys/dev/usb/if_upgt.c,v
retrieving revision 1.81
diff -u -p -u -r1.81 if_upgt.c
--- if_upgt.c 26 Oct 2017 15:00:28 - 1.81
+++ if_upgt.c 24 Aug 2018 10:53:58 -
@@ -1233,7 +1233,7 @@ upgt_media_change(struct ifnet *ifp)
DPRINTF(1, "%s: %s\n", sc->sc_dev.dv_xname, __func__);
- if ((error = ieee80211_media_change(ifp) != ENETRESET))
+ if ((error = ieee80211_media_change(ifp)) != ENETRESET)
return (error);
if (ifp->if_flags & (IFF_UP | IFF_RUNNING)) {
Re: fix assignment in if-clause in upgt(4)
On Fri, Aug 24, 2018 at 11:57:54AM +0100, Ricardo Mestre wrote:
> Hi,
>
> Similar to the patch I just sent for smsc(4) this one also has a misplaced
> parenthesis in an if-clause. NetBSD fixed this on rev 1.13.
>
> OK?
ok
> Index: if_upgt.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_upgt.c,v
> retrieving revision 1.81
> diff -u -p -u -r1.81 if_upgt.c
> --- if_upgt.c 26 Oct 2017 15:00:28 - 1.81
> +++ if_upgt.c 24 Aug 2018 10:53:58 -
> @@ -1233,7 +1233,7 @@ upgt_media_change(struct ifnet *ifp)
>
> DPRINTF(1, "%s: %s\n", sc->sc_dev.dv_xname, __func__);
>
> - if ((error = ieee80211_media_change(ifp) != ENETRESET))
> + if ((error = ieee80211_media_change(ifp)) != ENETRESET)
> return (error);
>
> if (ifp->if_flags & (IFF_UP | IFF_RUNNING)) {
>
Re: fix assignment in if-clause in smsc(4)
On Fri, Aug 24 2018, Ricardo Mestre wrote:
> Hi,
>
> smsc(4) needs a fix to an assignment which I found with cppcheck. FreeBSD also
> made this change 2 years ago on rev 295608.
>
> OK?
This diff shouldn't change the semantics but it looks saner this way
indeed. ok jca@
The return value of smsc_chip_init() isn't checked, should it be?
> Index: if_smsc.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_smsc.c,v
> retrieving revision 1.31
> diff -u -p -u -r1.31 if_smsc.c
> --- if_smsc.c 29 Jul 2017 17:24:04 - 1.31
> +++ if_smsc.c 24 Aug 2018 10:31:21 -
> @@ -774,7 +774,7 @@ smsc_chip_init(struct smsc_softc *sc)
> smsc_write_reg(sc, SMSC_PM_CTRL, SMSC_PM_CTRL_PHY_RST);
>
> if ((err = smsc_wait_for_bits(sc, SMSC_PM_CTRL,
> - SMSC_PM_CTRL_PHY_RST) != 0)) {
> + SMSC_PM_CTRL_PHY_RST)) != 0) {
> smsc_warn_printf(sc, "timed-out waiting for phy reset to "
> "complete\n");
> goto init_failed;
>
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: dl_iterate_phdr(3) manual tweak
On Fri, Aug 24 2018, Edd Barrett wrote: > On Fri, Aug 24, 2018 at 11:47:35AM +0200, Mark Kettenis wrote: >> correct, but I'd leave out the second "until". > > I prefer with the second until, but I'm not super-fussed. > > Here's the diff without the second, OK? ok jca@ > Index: dl_iterate_phdr.3 > === > RCS file: /cvs/src/share/man/man3/dl_iterate_phdr.3,v > retrieving revision 1.4 > diff -u -p -r1.4 dl_iterate_phdr.3 > --- dl_iterate_phdr.3 5 Jun 2013 03:42:03 - 1.4 > +++ dl_iterate_phdr.3 24 Aug 2018 09:58:53 - > @@ -34,6 +34,11 @@ for each shared object, passing it infor > program headers and the > .Fa data > argument. > +Iteration continues until either there are no more objects to > +iterate over, or > +.Fa callback > +returns a non-zero value. > +.Pp > The information about the program headers is passed in a structure > that is defined as: > .Bd -literal -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: fix assignment in if-clause in upgt(4)
On Fri, Aug 24 2018, Ricardo Mestre wrote:
> Hi,
>
> Similar to the patch I just sent for smsc(4) this one also has a misplaced
> parenthesis in an if-clause. NetBSD fixed this on rev 1.13.
>
> OK?
ok jca@
> Index: if_upgt.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_upgt.c,v
> retrieving revision 1.81
> diff -u -p -u -r1.81 if_upgt.c
> --- if_upgt.c 26 Oct 2017 15:00:28 - 1.81
> +++ if_upgt.c 24 Aug 2018 10:53:58 -
> @@ -1233,7 +1233,7 @@ upgt_media_change(struct ifnet *ifp)
>
> DPRINTF(1, "%s: %s\n", sc->sc_dev.dv_xname, __func__);
>
> - if ((error = ieee80211_media_change(ifp) != ENETRESET))
> + if ((error = ieee80211_media_change(ifp)) != ENETRESET)
> return (error);
>
> if (ifp->if_flags & (IFF_UP | IFF_RUNNING)) {
>
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: fix assignment in if-clause in smsc(4)
I think it should since it might return != 0, but what exactly should be
done in this case? I don't have such hardware to test those conditions
and if it keeps working anyway or not if it happens.
On 13:25 Fri 24 Aug , Jeremie Courreges-Anglas wrote:
> On Fri, Aug 24 2018, Ricardo Mestre wrote:
> > Hi,
> >
> > smsc(4) needs a fix to an assignment which I found with cppcheck. FreeBSD
> > also
> > made this change 2 years ago on rev 295608.
> >
> > OK?
>
> This diff shouldn't change the semantics but it looks saner this way
> indeed. ok jca@
>
> The return value of smsc_chip_init() isn't checked, should it be?
>
> > Index: if_smsc.c
> > ===
> > RCS file: /cvs/src/sys/dev/usb/if_smsc.c,v
> > retrieving revision 1.31
> > diff -u -p -u -r1.31 if_smsc.c
> > --- if_smsc.c 29 Jul 2017 17:24:04 - 1.31
> > +++ if_smsc.c 24 Aug 2018 10:31:21 -
> > @@ -774,7 +774,7 @@ smsc_chip_init(struct smsc_softc *sc)
> > smsc_write_reg(sc, SMSC_PM_CTRL, SMSC_PM_CTRL_PHY_RST);
> >
> > if ((err = smsc_wait_for_bits(sc, SMSC_PM_CTRL,
> > - SMSC_PM_CTRL_PHY_RST) != 0)) {
> > + SMSC_PM_CTRL_PHY_RST)) != 0) {
> > smsc_warn_printf(sc, "timed-out waiting for phy reset to "
> > "complete\n");
> > goto init_failed;
> >
>
> --
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
>
Re: vmd/vmctl: allow to boot cdrom-only VMs
Hi,
it’s not like that, maybe I wasn’t clear. Let me try to explain:
- you can boot from disk (-d)
- you can boot an OpenBSD kernel or use a custom BIOS (-b)
- you can boot from CDROM (-r)
vmd/vmctl checks if at least one option is available.
If you specify neither of these options, you can potentially boot from network
or via telekinesis. But vmd doesn’t support network boot (only with a PXE BIOS
payload that conflicts with sgabios) and I cannot judge your telekinesis skills
to make a VM boot without any kernel.
Reyk
> Am 23.08.2018 um 18:05 schrieb Todd T. Fries :
>
> This makes me wonder. Does it make sense to support booting a kernel without
> disks? Some people have heard of the phrase 'diskless' ;-)
>
> Penned by Reyk Floeter on 20180822 13:35.23, we have:
> | Hi,
> |
> | vmctl doesn't allow to boot VMs with only a CDROM. I see no reason
> | for it and vmd already allows CDROM-only.
> |
> | OK?
> |
> | Via https://twitter.com/wizardishungry/status/1032327323125727232
> | "Jon Williams @wizardishungry
> | @reykfloeter Could you consider allowing booting ISO-only vms in 6.4?
> | This is helpful for running container hosts (e.g. boot2docker)."
> |
> | Reyk
> |
> | Index: usr.sbin/vmctl/vmctl.c
> | ===
> | RCS file: /cvs/src/usr.sbin/vmctl/vmctl.c,v
> | retrieving revision 1.54
> | diff -u -p -u -p -r1.54 vmctl.c
> | --- usr.sbin/vmctl/vmctl.c12 Jul 2018 12:04:49 -1.54
> | +++ usr.sbin/vmctl/vmctl.c22 Aug 2018 18:29:44 -
> | @@ -98,8 +98,8 @@ vm_start(uint32_t start_id, const char *
> | errx(1, "too many disks");
> | else if (ndisks == 0)
> | warnx("starting without disks");
> | -if (kernel == NULL && ndisks == 0)
> | -errx(1, "no kernel or disk specified");
> | +if (kernel == NULL && ndisks == 0 && !iso)
> | +errx(1, "no kernel or disk/cdrom specified");
> | if (nnics == -1)
> | nnics = 0;
> | if (nnics > VMM_MAX_NICS_PER_VM)
>
> --
> Todd T. Fries . http://todd.fries.net/pgp.txt . @unix2mars . github:toddfries
Re: openssl s_time: check for SSL_write failure
On Wed, Aug 22 2018, Scott Cheloha wrote: > Check for failure. > > Also reuse retval for the buffer length in lieu of strlen. > > ok? Glancing at SSL_write(3), shouldn't this code loop if we get an error and SSL_get_error(3) returns SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE? Or perhaps it should just set SSL_MODE_AUTO_RETRY? > Index: s_time.c > === > RCS file: /cvs/src/usr.bin/openssl/s_time.c,v > retrieving revision 1.29 > diff -u -p -r1.29 s_time.c > --- s_time.c 22 Aug 2018 20:36:24 - 1.29 > +++ s_time.c 22 Aug 2018 22:08:54 - > @@ -380,7 +380,8 @@ run_test(SSL *scon) > fprintf(stderr, "URL too long\n"); > return 0; > } > - SSL_write(scon, buf, strlen(buf)); > + if (SSL_write(scon, buf, retval) <= 0) > + return 0; > while ((i = SSL_read(scon, buf, sizeof(buf))) > 0) > bytes_read += i; > } > -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: fix assignment in if-clause in upgt(4)
On Fri, Aug 24, 2018 at 11:57:54AM +0100, Ricardo Mestre wrote:
> Hi,
>
> Similar to the patch I just sent for smsc(4) this one also has a misplaced
> parenthesis in an if-clause. NetBSD fixed this on rev 1.13.
>
> OK?
OK claudio
> Index: if_upgt.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_upgt.c,v
> retrieving revision 1.81
> diff -u -p -u -r1.81 if_upgt.c
> --- if_upgt.c 26 Oct 2017 15:00:28 - 1.81
> +++ if_upgt.c 24 Aug 2018 10:53:58 -
> @@ -1233,7 +1233,7 @@ upgt_media_change(struct ifnet *ifp)
>
> DPRINTF(1, "%s: %s\n", sc->sc_dev.dv_xname, __func__);
>
> - if ((error = ieee80211_media_change(ifp) != ENETRESET))
> + if ((error = ieee80211_media_change(ifp)) != ENETRESET)
> return (error);
>
> if (ifp->if_flags & (IFF_UP | IFF_RUNNING)) {
>
--
:wq Claudio
init(8): remove special SIGSYS handling
The badsys() function has been here since import. Its comment
indicates that SIGSYS might arrive if the system doesn't support
sysctl, which is not applicable to us anymore, if ever.
The only legitimate case I can think of here, now, is the desire
to support dropping an old init(8) onto a new kernel and not having
the box panic immediately if it calls an obsolete syscall.
But that isn't something we support, no? And if the kernel ABI is
changing such that init(8) is calling an obsolete syscall, there are
steps one can take to migrate init(8) to the new ABI to prevent the
situation.
So SIGSYS should cause a disaster() exit upon first receipt like
all the other signals we haven't defined behavior for.
Thoughts? ok?
Index: init.c
===
RCS file: /cvs/src/sbin/init/init.c,v
retrieving revision 1.67
diff -u -p -r1.67 init.c
--- init.c 31 Jan 2018 15:57:44 - 1.67
+++ init.c 24 Aug 2018 15:12:11 -
@@ -91,7 +91,6 @@ void stall(char *, ...);
void warning(char *, ...);
void emergency(char *, ...);
void disaster(int);
-void badsys(int);
typedef enum {
invalid_state,
@@ -261,9 +260,8 @@ main(int argc, char *argv[])
* We catch or block signals rather than ignore them,
* so that they get reset on exec.
*/
- handle(badsys, SIGSYS, 0);
handle(disaster, SIGABRT, SIGFPE, SIGILL, SIGSEGV,
- SIGBUS, SIGXCPU, SIGXFSZ, 0);
+ SIGBUS, SIGSYS, SIGXCPU, SIGXFSZ, 0);
handle(transition_handler, SIGHUP, SIGINT, SIGTERM, SIGTSTP,
SIGUSR1, SIGUSR2, 0);
handle(alrm_handler, SIGALRM, 0);
@@ -377,22 +375,6 @@ emergency(char *message, ...)
va_start(ap, message);
vsyslog_r(LOG_EMERG, &sdata, message, ap);
va_end(ap);
-}
-
-/*
- * Catch a SIGSYS signal.
- *
- * These may arise if a system does not support sysctl.
- * We tolerate up to 25 of these, then throw in the towel.
- */
-void
-badsys(int sig)
-{
- static int badcount = 0;
-
- if (badcount++ < 25)
- return;
- disaster(sig);
}
/*
Re: init(8): remove special SIGSYS handling
Scott Cheloha wrote: > The badsys() function has been here since import. Its comment > indicates that SIGSYS might arrive if the system doesn't support > sysctl, which is not applicable to us anymore, if ever. > > The only legitimate case I can think of here, now, is the desire > to support dropping an old init(8) onto a new kernel and not having > the box panic immediately if it calls an obsolete syscall. > > But that isn't something we support, no? And if the kernel ABI is > changing such that init(8) is calling an obsolete syscall, there are > steps one can take to migrate init(8) to the new ABI to prevent the > situation. > > So SIGSYS should cause a disaster() exit upon first receipt like > all the other signals we haven't defined behavior for. Back in the old days (before horses had horseshoes), upgrades with bsd.rd or other hot-plug media were far from trivial. If the ABI changed, you wanted init to "limp" towards starting a single-user shell at least, otherwise you were prevented from next steps towards crossing the ABI bump by replacing init... I can see your argument that this isn't needed anymore.
Re: init(8): remove special SIGSYS handling
On Fri, Aug 24, 2018 at 10:02:21AM -0600, Theo de Raadt wrote: > Scott Cheloha wrote: > > > The badsys() function has been here since import. Its comment > > indicates that SIGSYS might arrive if the system doesn't support > > sysctl, which is not applicable to us anymore, if ever. > > > > The only legitimate case I can think of here, now, is the desire > > to support dropping an old init(8) onto a new kernel and not having > > the box panic immediately if it calls an obsolete syscall. > > > > But that isn't something we support, no? And if the kernel ABI is > > changing such that init(8) is calling an obsolete syscall, there are > > steps one can take to migrate init(8) to the new ABI to prevent the > > situation. > > > > So SIGSYS should cause a disaster() exit upon first receipt like > > all the other signals we haven't defined behavior for. > > Back in the old days (before horses had horseshoes), upgrades with > bsd.rd or other hot-plug media were far from trivial. If the ABI > changed, you wanted init to "limp" towards starting a single-user shell > at least, otherwise you were prevented from next steps towards crossing > the ABI bump by replacing init... > > I can see your argument that this isn't needed anymore. I mean, if there are nasty situations where you're trying to bring up a fubar'd box where getting init to exec the single-user shell makes or breaks your efforts, then I can just update the comment to indicate this. Do such situations occur anymore (anyone reading please chime in)? Even if rarely, that's still legit. I'm hunting this code because it mentions "no sysctl" as its raison d'etre, which is definitely not a reason for keeping it.
Re: init(8): remove special SIGSYS handling
Scott Cheloha wrote: > I mean, if there are nasty situations where you're trying to bring > up a fubar'd box where getting init to exec the single-user shell > makes or breaks your efforts, then I can just update the comment to > indicate this. > > Do such situations occur anymore (anyone reading please chime in)? > Even if rarely, that's still legit. > > I'm hunting this code because it mentions "no sysctl" as its raison > d'etre, which is definitely not a reason for keeping it. It is confusing that it says sysctl (rather than syscall). I don't see how SIGSYS gets delivered for a subset of sysctl, it is only delivered for a whole syscall being missing. Maybe this is an attempt at supporting ABI-crossing from the CSRG days, from before sysctl, to when sysctl was added, and then the code remained behind for other reasons? These days we have far more experience at ABI crossings, and are providing better solutions (seeing as we've adjusted/broken/changed the ABI only oh hundreds of times..)
Re: init(8): remove special SIGSYS handling
On Fri, Aug 24, 2018 at 10:21:08AM -0600, Theo de Raadt wrote: > Scott Cheloha wrote: > > > [...] > > > > I'm hunting this code because it mentions "no sysctl" as its raison > > d'etre, which is definitely not a reason for keeping it. > > It is confusing that it says sysctl (rather than syscall). I don't see > how SIGSYS gets delivered for a subset of sysctl, it is only delivered > for a whole syscall being missing. Maybe this is an attempt at > supporting ABI-crossing from the CSRG days, from before sysctl, to when > sysctl was added, and then the code remained behind for other reasons? Here is the diff where it first appeared: https://svnweb.freebsd.org/csrg/sbin/init/init.c?r1=58422&r2=58421&pathrev=58422 Apparently added to gracefully handle the case where sysctl(2) caused SIGSYS if they failed to retrieve or set the kernel security level. I think. KERN_SECURELVL itself was introduced only a few days prior: https://svnweb.freebsd.org/csrg/sys/sys/sysctl.h?r1=58329&r2=58328&pathrev=58329 And KERN_SECURELVL only became available via sysctl like a half hour prior to the /sbin/init commit adding support for it: https://svnweb.freebsd.org/csrg/sys/kern/kern_sysctl.c?r1=58415&r2=58414&pathrev=58415 sysctl itself was introduced only a month before all of this: https://svnweb.freebsd.org/csrg/sys/kern/kern_sysctl.c?r1=57843&r2=57842&pathrev=57843 ... so your theory makes sense. > These days we have far more experience at ABI crossings, and are > providing better solutions (seeing as we've adjusted/broken/changed the > ABI only oh hundreds of times..)
Re: init(8): remove special SIGSYS handling
Scott Cheloha wrote: > On Fri, Aug 24, 2018 at 10:21:08AM -0600, Theo de Raadt wrote: > > Scott Cheloha wrote: > > > > > [...] > > > > > > I'm hunting this code because it mentions "no sysctl" as its raison > > > d'etre, which is definitely not a reason for keeping it. > > > > It is confusing that it says sysctl (rather than syscall). I don't see > > how SIGSYS gets delivered for a subset of sysctl, it is only delivered > > for a whole syscall being missing. Maybe this is an attempt at > > supporting ABI-crossing from the CSRG days, from before sysctl, to when > > sysctl was added, and then the code remained behind for other reasons? > > Here is the diff where it first appeared: > > https://svnweb.freebsd.org/csrg/sbin/init/init.c?r1=58422&r2=58421&pathrev=58422 > > Apparently added to gracefully handle the case where sysctl(2) caused SIGSYS > if they failed to retrieve or set the kernel security level. I think. > > KERN_SECURELVL itself was introduced only a few days prior: > > https://svnweb.freebsd.org/csrg/sys/sys/sysctl.h?r1=58329&r2=58328&pathrev=58329 > > And KERN_SECURELVL only became available via sysctl like a half hour prior to > the /sbin/init commit adding support for it: > > https://svnweb.freebsd.org/csrg/sys/kern/kern_sysctl.c?r1=58415&r2=58414&pathrev=58415 > > sysctl itself was introduced only a month before all of this: > > https://svnweb.freebsd.org/csrg/sys/kern/kern_sysctl.c?r1=57843&r2=57842&pathrev=57843 > > ... so your theory makes sense. Thanks for doing the archaeological research! Quite a shock they encountered. The churn around early work on ktrace is also very amusing. I am really enjoying this moment. So ok deraadt for the diff.
Re: rasops(9) cursor drawing code
On Tue, Aug 21, 2018 at 08:07:03PM +0200, Mark Kettenis wrote: > > Date: Mon, 20 Aug 2018 20:55:16 +0200 (CEST) > > From: Mark Kettenis > > > > > Date: Mon, 20 Aug 2018 15:46:57 +0200 > > > From: Frederic Cambus > > > > > > There is an issue when moving the cursor backwards with the left arrow > > > key, characters disappear and only reappear when moving back forward > > > with the right arrow key. > > > > > > I also noticed some similar artefacts when using interactive programs > > > with wsmoused enabled. > > > > Ah, I didn't take the scrollback offset into account. Here is a > > better diff. > > As fcambus@ noticed, I attached the old diff again. Here is the right > one. This version fixes the issues I was seeing. OK fcambus@
Match ACPI devices on both _HID and _CID
ACPI devices typically have two methods that identify the device.
_HID returns a "hardware ID" and _CID returns a "compatible ID". The
idea is that _HID return an ID for the specific hardware. But if the
device is compatible with a more generic implementation, _CID can be
used to indicate that fact. We currently only match devices by using
the _HID. That means that we have to list the specific hardware IDs
of everything that's out there. It also means that we might not match
new hardware even though we have a working driver for it.
Diff below fixes this by also matching on _CID. The acpi_matchhids()
function will return 2 if the _HID matches a device in the list and
will return 1 if the _CID matches. That way a driver for a specific
device can override a more generic driver.
The diff then removes the _HID for the serial console on the Overdrive
1000 as that one has a _CID that matches the generic ARM PL011 UART.
ok?
Index: dev/acpi/acpi.c
===
RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
retrieving revision 1.357
diff -u -p -r1.357 acpi.c
--- dev/acpi/acpi.c 19 Aug 2018 08:23:47 - 1.357
+++ dev/acpi/acpi.c 24 Aug 2018 21:09:19 -
@@ -541,8 +541,13 @@ acpi_matchhids(struct acpi_attach_args *
{
if (aa->aaa_dev == NULL || aa->aaa_node == NULL)
return (0);
+
if (_acpi_matchhids(aa->aaa_dev, hids)) {
dnprintf(5, "driver %s matches at least one hid\n", driver);
+ return (2);
+ }
+ if (aa->aaa_cdev && _acpi_matchhids(aa->aaa_cdev, hids)) {
+ dnprintf(5, "driver %s matches at least one cid\n", driver);
return (1);
}
@@ -2977,8 +2982,8 @@ const char *acpi_skip_hids[] = {
"PNP0200", /* PC-class DMA Controller */
"PNP0201", /* EISA DMA Controller */
"PNP0800", /* Microsoft Sound System Compatible Device */
- "PNP0A03", /* PCI Bus */
#if defined(__amd64__) || defined(__i386__)
+ "PNP0A03", /* PCI Bus */
"PNP0A08", /* PCI Express Bus */
#endif
"PNP0C01", /* System Board */
@@ -3058,6 +3063,7 @@ acpi_foundhid(struct aml_node *node, voi
aaa.aaa_dmat = sc->sc_dmat;
aaa.aaa_node = node->parent;
aaa.aaa_dev = dev;
+ aaa.aaa_cdev = cdev;
if (acpi_matchhids(&aaa, acpi_skip_hids, "none") ||
acpi_matchhids(&aaa, acpi_isa_hids, "none"))
@@ -3152,6 +3158,7 @@ acpi_foundsbs(struct aml_node *node, voi
aaa.aaa_memt = sc->sc_memt;
aaa.aaa_node = node->parent;
aaa.aaa_dev = dev;
+ aaa.aaa_cdev = cdev;
config_found(self, &aaa, acpi_print);
node->parent->attached = 1;
Index: dev/acpi/acpivar.h
===
RCS file: /cvs/src/sys/dev/acpi/acpivar.h,v
retrieving revision 1.98
diff -u -p -r1.98 acpivar.h
--- dev/acpi/acpivar.h 19 Aug 2018 08:23:47 - 1.98
+++ dev/acpi/acpivar.h 24 Aug 2018 21:09:19 -
@@ -63,6 +63,7 @@ struct acpi_attach_args {
void*aaa_table;
struct aml_node *aaa_node;
const char *aaa_dev;
+ const char *aaa_cdev;
};
struct acpi_mem_map {
Index: dev/acpi/pluart_acpi.c
===
RCS file: /cvs/src/sys/dev/acpi/pluart_acpi.c,v
retrieving revision 1.2
diff -u -p -r1.2 pluart_acpi.c
--- dev/acpi/pluart_acpi.c 11 Aug 2018 16:04:49 - 1.2
+++ dev/acpi/pluart_acpi.c 24 Aug 2018 21:09:19 -
@@ -51,7 +51,6 @@ struct cfattach pluart_acpi_ca = {
};
const char *pluart_hids[] = {
- "AMDI0511",
"ARMH0011",
NULL
};
Re: Using shift on external keyboards in softraid passphrases from efiboot
On Fri, 24 Aug 2018 10:55:52 +0200
Patrick Wildt wrote:
> On Fri, Aug 24, 2018 at 10:47:27AM +0200, Theo Buehler wrote:
>> On Fri, Aug 24, 2018 at 11:50:51AM +0900, YASUOKA Masahiko wrote:
>> > Hi,
>> >
>> > I think the diff should be brought to arm64 as well. ok?
>>
>> ok. But shouldn't armv7 also be kept in sync?
>
> Exactly.
Thanks,
ok?
Index: sys/arch/arm64/stand/efiboot/efiboot.c
===
RCS file: /cvs/src/sys/arch/arm64/stand/efiboot/efiboot.c,v
retrieving revision 1.20
diff -u -p -r1.20 efiboot.c
--- sys/arch/arm64/stand/efiboot/efiboot.c 23 Aug 2018 15:31:12 -
1.20
+++ sys/arch/arm64/stand/efiboot/efiboot.c 24 Aug 2018 22:51:21 -
@@ -129,7 +129,7 @@ efi_cons_getc(dev_t dev)
}
status = conin->ReadKeyStroke(conin, &key);
- while (status == EFI_NOT_READY) {
+ while (status == EFI_NOT_READY || key.UnicodeChar == 0) {
if (dev & 0x80)
return (0);
/*
Index: sys/arch/armv7/stand/efiboot/efiboot.c
===
RCS file: /cvs/src/sys/arch/armv7/stand/efiboot/efiboot.c,v
retrieving revision 1.22
diff -u -p -r1.22 efiboot.c
--- sys/arch/armv7/stand/efiboot/efiboot.c 23 Aug 2018 15:31:12 -
1.22
+++ sys/arch/armv7/stand/efiboot/efiboot.c 24 Aug 2018 22:51:21 -
@@ -126,7 +126,7 @@ efi_cons_getc(dev_t dev)
}
status = conin->ReadKeyStroke(conin, &key);
- while (status == EFI_NOT_READY) {
+ while (status == EFI_NOT_READY || key.UnicodeChar == 0) {
if (dev & 0x80)
return (0);
/*
Re: Using shift on external keyboards in softraid passphrases from efiboot
On Sat, Aug 25, 2018 at 07:53:55AM +0900, YASUOKA Masahiko wrote:
> On Fri, 24 Aug 2018 10:55:52 +0200
> Patrick Wildt wrote:
> > On Fri, Aug 24, 2018 at 10:47:27AM +0200, Theo Buehler wrote:
> >> On Fri, Aug 24, 2018 at 11:50:51AM +0900, YASUOKA Masahiko wrote:
> >> > Hi,
> >> >
> >> > I think the diff should be brought to arm64 as well. ok?
> >>
> >> ok. But shouldn't armv7 also be kept in sync?
> >
> > Exactly.
>
> Thanks,
>
> ok?
ok
>
> Index: sys/arch/arm64/stand/efiboot/efiboot.c
> ===
> RCS file: /cvs/src/sys/arch/arm64/stand/efiboot/efiboot.c,v
> retrieving revision 1.20
> diff -u -p -r1.20 efiboot.c
> --- sys/arch/arm64/stand/efiboot/efiboot.c23 Aug 2018 15:31:12 -
> 1.20
> +++ sys/arch/arm64/stand/efiboot/efiboot.c24 Aug 2018 22:51:21 -
> @@ -129,7 +129,7 @@ efi_cons_getc(dev_t dev)
> }
>
> status = conin->ReadKeyStroke(conin, &key);
> - while (status == EFI_NOT_READY) {
> + while (status == EFI_NOT_READY || key.UnicodeChar == 0) {
> if (dev & 0x80)
> return (0);
> /*
> Index: sys/arch/armv7/stand/efiboot/efiboot.c
> ===
> RCS file: /cvs/src/sys/arch/armv7/stand/efiboot/efiboot.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 efiboot.c
> --- sys/arch/armv7/stand/efiboot/efiboot.c23 Aug 2018 15:31:12 -
> 1.22
> +++ sys/arch/armv7/stand/efiboot/efiboot.c24 Aug 2018 22:51:21 -
> @@ -126,7 +126,7 @@ efi_cons_getc(dev_t dev)
> }
>
> status = conin->ReadKeyStroke(conin, &key);
> - while (status == EFI_NOT_READY) {
> + while (status == EFI_NOT_READY || key.UnicodeChar == 0) {
> if (dev & 0x80)
> return (0);
> /*
>
fix hid constant conversion
The HID code uses hid_feature, hid_input, and hid_output constants
to refer to report types internally that then need to be converted
to their bus-level counterparts before actually getting sent out (so
hid_feature becomes UHID_FEATURE_REPORT for USB,
I2C_HID_REPORT_TYPE_FEATURE for i2c).
This conversion was hard-coded in ihidev when I wrote it, but
ihidev_[gs]et_report should assume the type passed is already an
i2c-level define, not a hid one. This is how uhidev does it.
So add a conversion routine callback that any hidmt callers need to
set so that hidmt can convert hid constants to the bus-level
versions before then calling the get/set report functions.
Also add a similar conversion function to uhidev for an upcoming
driver.
Index: dev/hid/hidmt.c
===
RCS file: /cvs/src/sys/dev/hid/hidmt.c,v
retrieving revision 1.7
diff -u -p -u -p -r1.7 hidmt.c
--- dev/hid/hidmt.c 30 Jul 2018 15:56:30 - 1.7
+++ dev/hid/hidmt.c 25 Aug 2018 02:21:18 -
@@ -126,7 +126,11 @@ hidmt_setup(struct device *self, struct
capsize = hid_report_size(desc, dlen, hid_feature, mt->sc_rep_cap);
rep = malloc(capsize, M_DEVBUF, M_NOWAIT | M_ZERO);
- if (mt->hidev_get_report(mt->sc_device, hid_feature, mt->sc_rep_cap,
+ if (mt->hidev_report_type_conv == NULL)
+ panic("no report type conversion function");
+
+ if (mt->hidev_get_report(mt->sc_device,
+ mt->hidev_report_type_conv(hid_feature), mt->sc_rep_cap,
rep, capsize)) {
printf("\n%s: failed getting capability report\n",
self->dv_xname);
@@ -278,8 +282,12 @@ hidmt_detach(struct hidmt *mt, int flags
int
hidmt_set_input_mode(struct hidmt *mt, uint16_t mode)
{
- return mt->hidev_set_report(mt->sc_device, hid_feature,
- mt->sc_rep_config, &mode, 2);
+ if (mt->hidev_report_type_conv == NULL)
+ panic("no report type conversion function");
+
+ return mt->hidev_set_report(mt->sc_device,
+ mt->hidev_report_type_conv(hid_feature),
+ mt->sc_rep_config, &mode, sizeof(mode));
}
void
Index: dev/hid/hidmtvar.h
===
RCS file: /cvs/src/sys/dev/hid/hidmtvar.h,v
retrieving revision 1.5
diff -u -p -u -p -r1.5 hidmtvar.h
--- dev/hid/hidmtvar.h 10 Oct 2017 20:31:50 - 1.5
+++ dev/hid/hidmtvar.h 25 Aug 2018 02:21:18 -
@@ -39,6 +39,7 @@ struct hidmt {
#define HIDMT_REVY 0x0001 /* Y-axis is reversed ("natural" scrolling) */
struct device *sc_device;
+ int (*hidev_report_type_conv)(int);
int (*hidev_get_report)(struct device *, int, int, void *,
int);
int (*hidev_set_report)(struct device *, int, int, void *,
Index: dev/i2c/ihidev.c
===
RCS file: /cvs/src/sys/dev/i2c/ihidev.c,v
retrieving revision 1.16
diff -u -p -u -p -r1.16 ihidev.c
--- dev/i2c/ihidev.c12 Jan 2018 08:11:47 - 1.16
+++ dev/i2c/ihidev.c25 Aug 2018 02:21:18 -
@@ -787,7 +787,6 @@ ihidev_get_report_desc(struct ihidev_sof
*size = sc->sc_reportlen;
}
-/* convert hid_* constants used throughout HID code to i2c HID equivalents */
int
ihidev_report_type_conv(int hid_type_id)
{
@@ -808,12 +807,8 @@ ihidev_get_report(struct device *dev, in
{
struct ihidev_softc *sc = (struct ihidev_softc *)dev;
struct i2c_hid_report_request rreq;
- int ctype;
- if ((ctype = ihidev_report_type_conv(type)) < 0)
- return (1);
-
- rreq.type = ctype;
+ rreq.type = type;
rreq.id = id;
rreq.data = data;
rreq.len = len;
@@ -831,12 +826,8 @@ ihidev_set_report(struct device *dev, in
{
struct ihidev_softc *sc = (struct ihidev_softc *)dev;
struct i2c_hid_report_request rreq;
- int ctype;
-
- if ((ctype = ihidev_report_type_conv(type)) < 0)
- return (1);
- rreq.type = ctype;
+ rreq.type = type;
rreq.id = id;
rreq.data = data;
rreq.len = len;
Index: dev/i2c/ihidev.h
===
RCS file: /cvs/src/sys/dev/i2c/ihidev.h,v
retrieving revision 1.5
diff -u -p -u -p -r1.5 ihidev.h
--- dev/i2c/ihidev.h29 Nov 2017 02:48:16 - 1.5
+++ dev/i2c/ihidev.h25 Aug 2018 02:21:18 -
@@ -128,5 +128,6 @@ int ihidev_open(struct ihidev *);
void ihidev_close(struct ihidev *);
int ihidev_ioctl(struct ihidev *, u_long, caddr_t, int, struct proc *);
+int ihidev_report_type_conv(int);
int ihidev_set_report(struct device *, int, int, void *, int);
int ihidev_get_report(struct device *, int, int, void *, int);
Index: dev/i2c/imt.c
===
RCS file: /cvs/src/sys/dev/i2c/imt
