arm64 setjmp/longjmp once again.

2017-03-09 Thread Dale Rahn 
Yesterday's comments about setjmp caused me to dig more deeply into the
function today. Here are several fixes. There is still one possibly
outstanding issue with fpsr. When reading thru the calling convention
it seemed to indicate that the rounding mode was not to be assumed.
(Document is 
http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf)

 These fields [in FPSR] are not preserved across a public interface and may
 have any value on entry to a subroutine.

This would seem to indicate that none of those fields need to be preserved,
however should fpcr (rounding modes) be saved and restored?  The text
doesn't seem to give any indication about saving that state across routines.

setjmp.h had never been updated for arm64, the defines were all from arm32.
It was necessary to move the signal mask because after saving the full
floating point context, the old save location was used by the fp context.
Also the context is quite a bit larger than necessary 64 * 8 bytes,
where the structure is only around 31 * 8 bytes in context.

setjmp.S and _setjmp.S were modified to save the full 128 bit floating
point registers instead of only the 64 bit portion of the registers.

_setjmp.S was modified to have _longjmp always return non-zero, effectively
the same diff as was applied to setjmp.S recently.

Finally, the regress/lib/libc/setjmp-signal test was wrong, it was
attempting to write to a null pointer, however the compiler was optimizing
away the NULL deref and replacing it with a breakpoint instruction.
Adding a volatile defeats the compiler optimization, this may not be
the best fix for this issue. Is there better suggestions?

Unfortunately, these fixes do not explain the sh coredumps that were being
seen, in fact this test is not readily producing them on the RPI3 board.
However other sporatic errors were still being seen while running other
code and working on these changes.


Index: sys/arch/arm64/include/setjmp.h
===
RCS file: /cvs/src/sys/arch/arm64/include/setjmp.h,v
retrieving revision 1.1
diff -u -p -r1.1 setjmp.h
--- sys/arch/arm64/include/setjmp.h 17 Dec 2016 23:38:33 -  1.1
+++ sys/arch/arm64/include/setjmp.h 10 Mar 2017 06:41:15 -
@@ -16,27 +16,32 @@
  * Description of the setjmp buffer
  *
  * word  0 magic number(dependant on creator)
- *   1 -  3f4  fp register 4
- *  4 -  6 f5  fp register 5
- *  7 -  9 f6  fp register 6
- * 10 - 12 f7  fp register 7
- * 13  fpsrfp status register
- * 14  r4  register 4
- * 15  r5  register 5
- * 16  r6  register 6
- * 17  r7  register 7
- * 18  r8  register 8
- * 19  r9  register 9
- * 20  r10 register 10 (sl)
- * 21  r11 register 11 (fp)
- * 22  r12 register 12 (ip)
- * 23  r13 register 13 (sp)
- * 24  r14 register 14 (lr)
- * 25  signal mask (dependant on magic)
- * 26  (con't)
- * 27  (con't)
- * 28  (con't)
- *
+ *   1 sp  stack
+ *   2 x19 register 19
+ *   3 x20 register 20
+ *   4 x21 register 21
+ *   5 x22 register 22
+ *   6 x23 register 23
+ *   7 x24 register 24
+ *   8 x25 register 25
+ *   9 x26 register 26
+ *  10 x27 register 27
+ *  11 x28 register 28
+ *  12 x29 register 29
+ *  13 x30 register 30
+ * 14  q8  fp register 8
+ * 16  q9  fp register 9
+ * 18  q10 fp register 10
+ * 20  q11 fp register 11
+ * 22  q12 fp register 12
+ * 24  q13 fp register 13
+ * 26  q14 fp register 14
+ * 28  v15 fp register 15
+ * 30  signal mask (dependant on magic)
+ * 31  (con't)
+ * 32  (con't)
+ * 33  (con't)
+ *  
  * The magic number number identifies the jmp_buf and
  * how the buffer was created as well as providing
  * a sanity check.
@@ -61,23 +66,28 @@
 /* Valid for all jmp_buf's */
 
 #define _JB_MAGIC   0
-#define _JB_REG_F4  1
-#define _JB_REG_F5  4
-#define _JB_REG_F6  7
-#define _JB_REG_F7 10
-#define _JB_REG_FPSR   13
-#define _JB_REG_R4 14
-#define _JB_REG_R5 15
-#define _JB_REG_R6 16
-#define _JB_REG_R7 17
-#define _JB_REG_R8 18
-#define _JB_REG_R9 19
-#define _JB_REG_R1020
-#define

Re: armv7 memory segments

2017-03-09 Thread Jonathan Gray 
On Thu, Mar 09, 2017 at 10:38:20PM +0100, Mark Kettenis wrote:
> For some reason u-boot on the exynos platform decides to split up the
> memory in banks with a maximum size of 256MB.  Since my Odroid XU4 has
> 2GB of memory, I end up with more memory segments than the two
> supported by our kernel.  The diff below increases the number of
> segments to 32, which is what we use on sparc64 and macppc.
> 
> However, increasing the number of segments didn't actually make more
> memory available.  Turns out the code that is supposed to add
> additonal memory segments isn't working.  The reason is that we
> reinitialize the fdt to use the virtual mapping.  This makes the old
> pointer to the /memory node invalid.  Doing another lookup fixes
> things.  And the lookup really shouldn't fail at this point since we
> already established that the /memory node exists.
> 
> ok?

ok jsg@

The only other armv7 machine I can remember that needed more
than one segment was the utilite.  With atags it reported

atag mem start 0x1000 size 0x4000
atag mem start 0x8000 size 0x4000

http://marc.info/?l=openbsd-misc=142125974324412=2

> 
> P.S. arm64 currently lacks code to load additional memory segments.
>  That needs to be fixed.
> 
> Index: arch/armv7/include/vmparam.h
> ===
> RCS file: /cvs/src/sys/arch/armv7/include/vmparam.h,v
> retrieving revision 1.5
> diff -u -p -r1.5 vmparam.h
> --- arch/armv7/include/vmparam.h  24 Jan 2017 13:33:06 -  1.5
> +++ arch/armv7/include/vmparam.h  9 Mar 2017 21:27:45 -
> @@ -83,8 +83,8 @@
>   * max number of non-contig chunks of physical RAM you can have
>   */
>  
> -#define  VM_PHYSSEG_MAX  2
> -#define  VM_PHYSSEG_STRATVM_PSTRAT_RANDOM
> +#define  VM_PHYSSEG_MAX  32
> +#define  VM_PHYSSEG_STRATVM_PSTRAT_BSEARCH
>  
>  /*
>   * this indicates that we can't add RAM to the VM system after the
> Index: arch/armv7/armv7/armv7_machdep.c
> ===
> RCS file: /cvs/src/sys/arch/armv7/armv7/armv7_machdep.c,v
> retrieving revision 1.45
> diff -u -p -r1.45 armv7_machdep.c
> --- arch/armv7/armv7/armv7_machdep.c  25 Oct 2016 00:04:59 -  1.45
> +++ arch/armv7/armv7/armv7_machdep.c  9 Mar 2017 21:27:45 -
> @@ -754,6 +754,7 @@ initarm(void *arg0, void *arg1, void *ar
>   physsegs--;
>   }
>  
> + node = fdt_find_node("/memory");
>   for (i = 1; i < physsegs; i++) {
>   if (fdt_get_reg(node, i, ))
>   break;
>

Improved support for Apple trackpads: tests needed

2017-03-09 Thread Ulf Brosziewski 
This patch for ubcmtp makes it use the multitouch-input functions of
wsmouse. It's the first driver that would apply the "tracking" variant
(wsmouse_mtframe).

No wonders will result from the change, but the two-finger gestures that
involve movement - scrolling and click-and-drag with two fingers on a
clickpad - should work without flaws.

A quick way to check whether all touch positions are available and the
selection of the active touch works properly is two-finger-scrolling,
performed with only one finger moving, then switching to the other one.

Please note that click-and-drag will still require that the "ClickPad"
attribute is set in the synaptics(4) configuration.

The patch has been tested on a MacBookPro 8,2 (from 2011). It would be
nice to learn that it doesn't introduce regressions on older or newer
models.

If you don't use a brand-new version of the synaptics driver, you may
encounter the following bug: If the X cursor is in a window with
scrollable content and you put two finger on the touchpad without moving
them, the window content may quickly scroll up and down by a small
distance. This bug is fixed in the latest version.


Index: dev/usb/ubcmtp.c
===
RCS file: /cvs/src/sys/dev/usb/ubcmtp.c,v
retrieving revision 1.12
diff -u -p -r1.12 ubcmtp.c
--- dev/usb/ubcmtp.c30 Mar 2016 23:34:12 -  1.12
+++ dev/usb/ubcmtp.c9 Mar 2017 22:17:50 -
@@ -125,6 +125,12 @@ struct ubcmtp_finger {
 #define UBCMTP_SN_COORD250
 #define UBCMTP_SN_ORIENT   10
 
+/* Identify clickpads in ubcmtp_configure. */
+#define IS_CLICKPAD(ubcmtp_type) (ubcmtp_type != UBCMTP_TYPE1)
+
+/* Use a constant, synaptics-compatible pressure value for now. */
+#define DEFAULT_PRESSURE   40
+
 struct ubcmtp_limit {
int limit;
int min;
@@ -316,17 +322,6 @@ static struct ubcmtp_dev ubcmtp_devices[
},
 };
 
-/* coordinates for each tracked finger */
-struct ubcmtp_pos {
-   int down;
-   int x;
-   int y;
-   int z;
-   int w;
-   int dx;
-   int dy;
-};
-
 struct ubcmtp_softc {
struct device   sc_dev; /* base device */
 
@@ -355,7 +350,8 @@ struct ubcmtp_softc {
uint32_tsc_status;
 #define UBCMTP_ENABLED 1
 
-   struct ubcmtp_pos   pos[UBCMTP_MAX_FINGERS];
+   struct mtpoint  frame[UBCMTP_MAX_FINGERS];
+   int contacts;
int btn;
 };
 
@@ -519,6 +515,24 @@ ubcmtp_activate(struct device *self, int
 }
 
 int
+ubcmtp_configure(struct ubcmtp_softc *sc)
+{
+   struct wsmousehw *hw = wsmouse_get_hw(sc->sc_wsmousedev);
+
+   hw->type = WSMOUSE_TYPE_ELANTECH;   /* see ubcmtp_ioctl */
+   hw->hw_type = (IS_CLICKPAD(sc->dev_type->type)
+   ? WSMOUSEHW_CLICKPAD : WSMOUSEHW_TOUCHPAD);
+   hw->x_min = sc->dev_type->l_x.min;
+   hw->x_max = sc->dev_type->l_x.max;
+   hw->y_min = sc->dev_type->l_y.min;
+   hw->y_max = sc->dev_type->l_y.max;
+   hw->mt_slots = UBCMTP_MAX_FINGERS;
+   hw->flags = WSMOUSEHW_MT_TRACKING;
+
+   return wsmouse_configure(sc->sc_wsmousedev, NULL, 0);
+}
+
+int
 ubcmtp_enable(void *v)
 {
struct ubcmtp_softc *sc = v;
@@ -534,6 +548,9 @@ ubcmtp_enable(void *v)
return (1);
}
 
+   if (ubcmtp_configure(sc))
+   return (1);
+
if (ubcmtp_setup_pipes(sc) == 0) {
sc->sc_status |= UBCMTP_ENABLED;
return (0);
@@ -608,6 +625,7 @@ ubcmtp_ioctl(void *v, unsigned long cmd,
wsmode);
return (EINVAL);
}
+   wsmouse_set_mode(sc->sc_wsmousedev, wsmode);
 
DPRINTF("%s: changing mode to %s\n",
sc->sc_dev.dv_xname, (wsmode == WSMOUSE_COMPAT ? "compat" :
@@ -779,7 +797,7 @@ ubcmtp_tp_intr(struct usbd_xfer *xfer, v
struct ubcmtp_softc *sc = priv;
struct ubcmtp_finger *pkt;
u_int32_t pktlen;
-   int i, diff = 0, finger = 0, fingers = 0, s, t;
+   int i, s, btn, contacts, fingers;
 
if (usbd_is_dying(sc->sc_udev) || !(sc->sc_status & UBCMTP_ENABLED))
return;
@@ -803,76 +821,30 @@ ubcmtp_tp_intr(struct usbd_xfer *xfer, v
pkt = (struct ubcmtp_finger *)(sc->tp_pkt + sc->tp_offset);
fingers = (pktlen - sc->tp_offset) / sizeof(struct ubcmtp_finger);
 
+   contacts = 0;
for (i = 0; i < fingers; i++) {
-   if ((int16_t)letoh16(pkt[i].touch_major) == 0) {
-   /* finger lifted */
-   sc->pos[i].down = 0;
-   diff = 1;
-   continue;
-   }
-
-   if (!sc->pos[finger].down) {
-   sc->pos[finger].down = 1;
-   diff = 1;
-   }
-
-   if ((t =

Re: sysctl machdep.fakepckbc=1

2017-03-09 Thread Mark Kettenis 
> Date: Thu, 9 Mar 2017 16:13:01 +0100
> From: Martin Pieuchot 
> 
> On 09/03/17(Thu) 14:48, Mark Kettenis wrote:
> > > Date: Thu, 9 Mar 2017 14:04:04 +0100
> > > From: Martin Pieuchot 
> > > 
> > > Many of us have x86 machines with a USB keyboard that's unusable in
> > > ddb(4).  That's generally because the BIOS presents a fake pckbd(4)
> > > to the OS:
> > > 
> > >   pckbc0 at isa0 port 0x60/5 irq 1 irq 12
> > >   pckbd0 at pckbc0 (kbd slot)
> > >   wskbd0 at pckbd0: console keyboard, using wsdisplay0
> > > 
> > > Some BIOSes allow you to disable this emulation.  While this might
> > > allow you to have a working USB keyboard in ddb(4) it generally means
> > > you lose input in the bootloader.
> > > 
> > > So the diff below introduces a new sysctl(3) to work around this problem.
> > > Adding the following line in your /etc/sysctl.conf will force your first
> > > USB keyboard to re-attach itself after cold boot and become the default
> > > console keyboard.  That means you now have a working keyboard in ddb(4)!
> > > 
> > >   sysctl machdep.fakepckbc=1
> > > 
> > > I hope this will improve the content of bug reports.
> > 
> > Can't you already achieve this by disabling pckbc(4) in the kernel
> > config?  So something like this:
> > 
> > # config -ef /bsd
> > ...
> > ukc> disable pckbc
> > 250 pckbc0 disabled
> > ukc> quit
> 
> This doesn't work.
> 
> The reason is that pckbc_cnattach() doesn't care about the value of
> ``cf_fstate''.  We could fix that of course but I see two downside:
> 
> First we want "boot bsd -c" and early ddb(4) to work.  At least until
> the *hci(4) driver did its handshake with the BIOS we want a pckbc
> input console.

Right.  We want the SMM-based pckbc(4) emulation that the BIOS
provides to continue to work until we whack the usb controller.

> Second downside is this one:
> 
> > Only downside is that it doesn't stick.  If you install a new kernel
> > you need to repeat the excercise.
> 
> This is only good for development, not for users having an unexpected
> panic.

It's a pity that we need a knob for this, but it seems to be very
hard, if not impossible, to check whether the emulation is there.  So
your diff makes sense.  Apart from the bikeshed about the name
(fakepckbdc seems a bit weird, usbconsole makes more sense to me),
this is ok kettenis@

armv7 memory segments

2017-03-09 Thread Mark Kettenis 
For some reason u-boot on the exynos platform decides to split up the
memory in banks with a maximum size of 256MB.  Since my Odroid XU4 has
2GB of memory, I end up with more memory segments than the two
supported by our kernel.  The diff below increases the number of
segments to 32, which is what we use on sparc64 and macppc.

However, increasing the number of segments didn't actually make more
memory available.  Turns out the code that is supposed to add
additonal memory segments isn't working.  The reason is that we
reinitialize the fdt to use the virtual mapping.  This makes the old
pointer to the /memory node invalid.  Doing another lookup fixes
things.  And the lookup really shouldn't fail at this point since we
already established that the /memory node exists.

ok?

P.S. arm64 currently lacks code to load additional memory segments.
 That needs to be fixed.

Index: arch/armv7/include/vmparam.h
===
RCS file: /cvs/src/sys/arch/armv7/include/vmparam.h,v
retrieving revision 1.5
diff -u -p -r1.5 vmparam.h
--- arch/armv7/include/vmparam.h24 Jan 2017 13:33:06 -  1.5
+++ arch/armv7/include/vmparam.h9 Mar 2017 21:27:45 -
@@ -83,8 +83,8 @@
  * max number of non-contig chunks of physical RAM you can have
  */
 
-#defineVM_PHYSSEG_MAX  2
-#defineVM_PHYSSEG_STRATVM_PSTRAT_RANDOM
+#defineVM_PHYSSEG_MAX  32
+#defineVM_PHYSSEG_STRATVM_PSTRAT_BSEARCH
 
 /*
  * this indicates that we can't add RAM to the VM system after the
Index: arch/armv7/armv7/armv7_machdep.c
===
RCS file: /cvs/src/sys/arch/armv7/armv7/armv7_machdep.c,v
retrieving revision 1.45
diff -u -p -r1.45 armv7_machdep.c
--- arch/armv7/armv7/armv7_machdep.c25 Oct 2016 00:04:59 -  1.45
+++ arch/armv7/armv7/armv7_machdep.c9 Mar 2017 21:27:45 -
@@ -754,6 +754,7 @@ initarm(void *arg0, void *arg1, void *ar
physsegs--;
}
 
+   node = fdt_find_node("/memory");
for (i = 1; i < physsegs; i++) {
if (fdt_get_reg(node, i, ))
break;

Re: doas: add fd variable, use fd instead of i, to main()

2017-03-09 Thread Ted Unangst 
Michael Forney wrote:
> > +   exit(fd != -1);
> 
> While you're at it, I think this exit status is backwards. Currently
> `doas -L` exits 0 if the open failed.

yeah, oops. fixed. thanks.

OpenBSD errata, Mar 9, 2017

2017-03-09 Thread Sebastian Benoit 
Prevent integer overflow in PF when calculating the adaptive timeout.

Mainly states of established TCP connections would be affected
resulting in immediate state removal once the numer of states is
bigger than adaptive.start.

Disabling adative timeouts with
  set timeout { adaptive.start 0, adaptive.end 0 }
is a workaround to avoid this bug.

Issue found and initial diff by Mathieu Blanc (mathieu.blanc at cea dot fr)

The problem has been fixed in -current. For 5.9 and 6.0 the following
errata patches are available.

https://ftp.openbsd.org/pub/OpenBSD/patches/6.0/common/019_pf.patch.sig

https://ftp.openbsd.org/pub/OpenBSD/patches/5.9/common/036_pf.patch.sig

Re: acme-client -t switch?

2017-03-09 Thread Sebastian Benoit 
Stuart Henderson(s...@spacehopper.org) on 2017.03.07 21:56:56 +:
> On 2017/03/07 13:24, Devin Reade wrote:
> > Expanding on my previous email, it looks like the git version of
> > acme-client has a different implementation than what was implemented
> > in the version first committed (and later removed) from the OpenBSD
> > CVS sources.  The latter (CVS) version was calling "doas sh ..."
> > whereas the former (git) version writes challenges to stdout which
> > can then be processed by the invoking program.  Looking at the logs,
> > it appears the git version is a reworking of the functionality.
> > 
> > For the record, I was asking about introducing the mechanism (stdout)
> > currently used by the git version.
> > 
> > Devin
> > 
> 
> Both OpenBSD's version and the original have seen independent changes
> after import, they have now diverged quite a lot.
> 
> Since this came up.. what does anyone think about adding the original
> version back to ports? (personally, I could do with moving things away
> from the python version, but I need dns-01..)

I'm planing on re-introducing the functinality.

Re: doas: add fd variable, use fd instead of i, to main()

2017-03-09 Thread Michael Forney 
On Sun, Nov 27, 2016 at 12:31 AM, Hajime Edakawa
 wrote:
> Dear tech,
>
> I guessed it more better to use fd instead of i.
>
> Would this be OK?
>
> Sincerely, tech
> Edakawa
>
> Index: doas.c
> ===
> RCS file: /cvs/src/usr.bin/doas/doas.c,v
> retrieving revision 1.68
> diff -u -p -u -r1.68 doas.c
> --- doas.c  5 Oct 2016 23:28:28 -   1.68
> +++ doas.c  27 Nov 2016 08:20:45 -
> @@ -256,7 +256,7 @@ main(int argc, char **argv)
> uid_t target = 0;
> gid_t groups[NGROUPS_MAX + 1];
> int ngroups;
> -   int i, ch;
> +   int i, ch, fd;
> int sflag = 0;
> int nflag = 0;
> char cwdpath[PATH_MAX];
> @@ -279,10 +279,10 @@ main(int argc, char **argv)
> confpath = optarg;
> break;
> case 'L':
> -   i = open("/dev/tty", O_RDWR);
> -   if (i != -1)
> -   ioctl(i, TIOCCLRVERAUTH);
> -   exit(i != -1);
> +   fd = open("/dev/tty", O_RDWR);
> +   if (fd != -1)
> +   ioctl(fd, TIOCCLRVERAUTH);
> +   exit(fd != -1);

While you're at it, I think this exit status is backwards. Currently
`doas -L` exits 0 if the open failed.

> case 'u':
> if (parseuid(optarg, ) != 0)
> errx(1, "unknown user");
>

Re: USB polling + xhci(4) fix

2017-03-09 Thread Mark Kettenis 
> Date: Thu, 9 Mar 2017 16:04:44 +0100
> From: Martin Pieuchot 
> 
> On 09/03/17(Thu) 15:04, Mark Kettenis wrote:
> > > Date: Thu, 9 Mar 2017 14:32:29 +0100
> > > From: Martin Pieuchot 
> > > 
> > > Diff below move per HC driver polling code to the stack.  I know this
> > > code contains a use-after-free and this will be addressed in a later
> > > diff.  For the moment merge 4 copies into 1 such that I don't have to
> > > fix all of them.
> > > 
> > > As a side effect, this fix xhci(4) polling mode.  That means you can
> > > now use ddb(4) with an xhci(4)-attached keyboard.
> > > 
> > > ok?
> > 
> > Looks like a good idea to me.  Hwoever...
> 
> > [...]
> > > + if (polling) {
> > > + int timo;
> > > +
> > > + for (timo = xfer->timeout; timo >= 0; timo--) {
> > > + usb_delay_ms(bus, 1);
> > 
> > The xxx_waitintr() implementations have a sc->sc_bus.dying check here.
> > I'd expect that that check would still be necessary...
> 
> That check is needed when coming back from a sleep.  In polling mode
> usb_delay_ms() calls delay(9) so we know that all the resources are
> still there.
> 
> But it might be better to leave that for another diff.  Revised diff
> below.

Thanks for the explanation.  Looks good to me.  ok kettenis@OB

> Index: usbdi.c
> ===
> RCS file: /cvs/src/sys/dev/usb/usbdi.c,v
> retrieving revision 1.87
> diff -u -p -r1.87 usbdi.c
> --- usbdi.c   6 Mar 2017 12:13:58 -   1.87
> +++ usbdi.c   9 Mar 2017 14:58:44 -
> @@ -279,6 +279,8 @@ usbd_status
>  usbd_transfer(struct usbd_xfer *xfer)
>  {
>   struct usbd_pipe *pipe = xfer->pipe;
> + struct usbd_bus *bus = pipe->device->bus;
> + int polling = bus->use_polling;
>   usbd_status err;
>   int flags, s;
>  
> @@ -298,8 +300,6 @@ usbd_transfer(struct usbd_xfer *xfer)
>  
>   /* If there is no buffer, allocate one. */
>   if ((xfer->rqflags & URQ_DEV_DMABUF) == 0) {
> - struct usbd_bus *bus = pipe->device->bus;
> -
>  #ifdef DIAGNOSTIC
>   if (xfer->rqflags & URQ_AUTO_DMABUF)
>   printf("usbd_transfer: has old buffer!\n");
> @@ -325,8 +325,6 @@ usbd_transfer(struct usbd_xfer *xfer)
>   if (err != USBD_IN_PROGRESS && err) {
>   /* The transfer has not been queued, so free buffer. */
>   if (xfer->rqflags & URQ_AUTO_DMABUF) {
> - struct usbd_bus *bus = pipe->device->bus;
> -
>   usb_freemem(bus, >dmabuf);
>   xfer->rqflags &= ~URQ_AUTO_DMABUF;
>   }
> @@ -338,19 +336,40 @@ usbd_transfer(struct usbd_xfer *xfer)
>   /* Sync transfer, wait for completion. */
>   if (err != USBD_IN_PROGRESS)
>   return (err);
> +
>   s = splusb();
> - while (!xfer->done) {
> - if (pipe->device->bus->use_polling)
> - panic("usbd_transfer: not done");
> - flags = PRIBIO | (xfer->flags & USBD_CATCH ? PCATCH : 0);
> -
> - err = tsleep(xfer, flags, "usbsyn", 0);
> - if (err && !xfer->done) {
> - usbd_abort_pipe(pipe);
> - if (err == EINTR)
> - xfer->status = USBD_INTERRUPTED;
> - else
> - xfer->status = USBD_TIMEOUT;
> + if (polling) {
> + int timo;
> +
> + for (timo = xfer->timeout; timo >= 0; timo--) {
> + usb_delay_ms(bus, 1);
> + if (bus->dying) {
> + xfer->status = USBD_IOERROR;
> + usb_transfer_complete(xfer);
> + break;
> + }
> +
> + usbd_dopoll(pipe->device);
> + if (xfer->done)
> + break;
> + }
> +
> + if (timo < 0) {
> + xfer->status = USBD_TIMEOUT;
> + usb_transfer_complete(xfer);
> + }
> + } else {
> + while (!xfer->done) {
> + flags = PRIBIO|(xfer->flags & USBD_CATCH ? PCATCH : 0);
> +
> + err = tsleep(xfer, flags, "usbsyn", 0);
> + if (err && !xfer->done) {
> + usbd_abort_pipe(pipe);
> + if (err == EINTR)
> + xfer->status = USBD_INTERRUPTED;
> + else
> + xfer->status = USBD_TIMEOUT;
> + }
>   }
>   }
>   splx(s);
> Index: ehci.c
> ===
> RCS file: /cvs/src/sys/dev/usb/ehci.c,v
> retrieving revision 1.195
> diff -u -p -r1.195 ehci.c
> --- ehci.c8 Nov 2016 10:31:30 -   1.195
> +++ ehci.c9 Mar

Re: sysctl machdep.fakepckbc=1

2017-03-09 Thread Martin Pieuchot 
On 09/03/17(Thu) 14:48, Mark Kettenis wrote:
> > Date: Thu, 9 Mar 2017 14:04:04 +0100
> > From: Martin Pieuchot 
> > 
> > Many of us have x86 machines with a USB keyboard that's unusable in
> > ddb(4).  That's generally because the BIOS presents a fake pckbd(4)
> > to the OS:
> > 
> > pckbc0 at isa0 port 0x60/5 irq 1 irq 12
> > pckbd0 at pckbc0 (kbd slot)
> > wskbd0 at pckbd0: console keyboard, using wsdisplay0
> > 
> > Some BIOSes allow you to disable this emulation.  While this might
> > allow you to have a working USB keyboard in ddb(4) it generally means
> > you lose input in the bootloader.
> > 
> > So the diff below introduces a new sysctl(3) to work around this problem.
> > Adding the following line in your /etc/sysctl.conf will force your first
> > USB keyboard to re-attach itself after cold boot and become the default
> > console keyboard.  That means you now have a working keyboard in ddb(4)!
> > 
> > sysctl machdep.fakepckbc=1
> > 
> > I hope this will improve the content of bug reports.
> 
> Can't you already achieve this by disabling pckbc(4) in the kernel
> config?  So something like this:
> 
> # config -ef /bsd
> ...
> ukc> disable pckbc
> 250 pckbc0 disabled
> ukc> quit

This doesn't work.

The reason is that pckbc_cnattach() doesn't care about the value of
``cf_fstate''.  We could fix that of course but I see two downside:

First we want "boot bsd -c" and early ddb(4) to work.  At least until
the *hci(4) driver did its handshake with the BIOS we want a pckbc
input console.

Second downside is this one:

> Only downside is that it doesn't stick.  If you install a new kernel
> you need to repeat the excercise.

This is only good for development, not for users having an unexpected
panic.

Re: ip_ipip.c / gif(4) percpu counters

2017-03-09 Thread David Hill 
On Thu, Mar 09, 2017 at 03:49:21PM +0100, Jeremie Courreges-Anglas wrote:
> Martin Pieuchot  writes:
> 
> > On 08/03/17(Wed) 12:03, Jeremie Courreges-Anglas wrote:
> >> [...] 
> >> So here's a refreshed diff that initializes the counters directly from
> >> ip_init().  I remove the ipip_init() wrapper to make it clear that
> >> ip_init() is responsible for the job.
> >> 
> >> (Still) ok?
> >
> > I'm against adding more "#if PEUDOIF" in the stack everywhere and it
> > makes no sense to move more globals outside of ip_ipip.c.
> 
> The previous diff was just plain wrong.
> 
> > If you really don't want to use .pr_init, then call ipip_input() from
> > ip_input().
> 
> Actually I'm fine with .pr_init.  There are other protosw entries that
> reach the counters but one shouldn't remove entries carelessly anyway.
> 
> Is this better?
> 
Definitely. OK dhill@
> 
> Index: netinet/in_proto.c
> ===
> RCS file: /d/cvs/src/sys/netinet/in_proto.c,v
> retrieving revision 1.74
> diff -u -p -r1.74 in_proto.c
> --- netinet/in_proto.c2 Mar 2017 08:58:24 -   1.74
> +++ netinet/in_proto.c8 Mar 2017 11:56:45 -
> @@ -239,7 +239,8 @@ struct protosw inetsw[] = {
>.pr_output = rip_output,
>.pr_ctloutput  = rip_ctloutput,
>.pr_usrreq = rip_usrreq,
> -  .pr_sysctl = ipip_sysctl
> +  .pr_sysctl = ipip_sysctl,
> +  .pr_init   = ipip_init
>  },
>  {
>.pr_type   = SOCK_RAW,
> @@ -284,7 +285,8 @@ struct protosw inetsw[] = {
>.pr_output = rip_output,
>.pr_ctloutput  = rip_ctloutput,
>.pr_usrreq = rip_usrreq,
> -  .pr_sysctl = ipip_sysctl
> +  .pr_sysctl = ipip_sysctl,
> +  .pr_init   = ipip_init
>  },
>  #ifdef INET6
>  {
> Index: netinet/ip_ipip.c
> ===
> RCS file: /d/cvs/src/sys/netinet/ip_ipip.c,v
> retrieving revision 1.71
> diff -u -p -r1.71 ip_ipip.c
> --- netinet/ip_ipip.c 29 Jan 2017 19:58:47 -  1.71
> +++ netinet/ip_ipip.c 8 Mar 2017 11:59:04 -
> @@ -84,7 +84,13 @@
>   */
>  int ipip_allow = 0;
>  
> -struct ipipstat ipipstat;
> +struct cpumem *ipipcounters;
> +
> +void
> +ipip_init(void)
> +{
> + ipipcounters = counters_alloc(ipips_ncounters);
> +}
>  
>  /*
>   * Really only a wrapper for ipip_input(), for use with pr_input.
> @@ -95,7 +101,7 @@ ip4_input(struct mbuf **mp, int *offp, i
>   /* If we do not accept IP-in-IP explicitly, drop.  */
>   if (!ipip_allow && ((*mp)->m_flags & (M_AUTH|M_CONF)) == 0) {
>   DPRINTF(("ip4_input(): dropped due to policy\n"));
> - ipipstat.ipips_pdrops++;
> + ipipstat_inc(ipips_pdrops);
>   m_freem(*mp);
>   return IPPROTO_DONE;
>   }
> @@ -129,7 +135,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   u_int8_t v;
>   sa_family_t af;
>  
> - ipipstat.ipips_ipackets++;
> + ipipstat_inc(ipips_ipackets);
>  
>   m_copydata(m, 0, 1, );
>  
> @@ -143,7 +149,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   break;
>  #endif
>   default:
> - ipipstat.ipips_family++;
> + ipipstat_inc(ipips_family);
>   m_freem(m);
>   return IPPROTO_DONE;
>   }
> @@ -152,7 +158,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   if (m->m_len < hlen) {
>   if ((m = m_pullup(m, hlen)) == NULL) {
>   DPRINTF(("ipip_input(): m_pullup() failed\n"));
> - ipipstat.ipips_hdrops++;
> + ipipstat_inc(ipips_hdrops);
>   return IPPROTO_DONE;
>   }
>   }
> @@ -179,7 +185,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>  
>   /* Sanity check */
>   if (m->m_pkthdr.len < sizeof(struct ip)) {
> - ipipstat.ipips_hdrops++;
> + ipipstat_inc(ipips_hdrops);
>   m_freem(m);
>   return IPPROTO_DONE;
>   }
> @@ -195,7 +201,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   break;
>  #endif
>   default:
> - ipipstat.ipips_family++;
> + ipipstat_inc(ipips_family);
>   m_freem(m);
>   return IPPROTO_DONE;
>   }
> @@ -206,7 +212,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   if (m->m_len < hlen) {
>   if ((m = m_pullup(m, hlen)) == NULL) {
>   DPRINTF(("ipip_input(): m_pullup() failed\n"));
> - ipipstat.ipips_hdrops++;
> + ipipstat_inc(ipips_hdrops);
>   return IPPROTO_DONE;
>   }
>   }
> @@ -229,7 +235,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   ECN_ALLOWED_IPSEC : ECN_ALLOWED;
>   if (!ip_ecn_egress(mode, , >ip_tos)) {
>   DPRINTF(("ipip_input(): ip_ecn_egress() failed"));
> - ipipstat.ipips_pdrops++;
> + ipipstat_inc(ipips_pdrops);

Re: USB polling + xhci(4) fix

2017-03-09 Thread Martin Pieuchot 
On 09/03/17(Thu) 15:04, Mark Kettenis wrote:
> > Date: Thu, 9 Mar 2017 14:32:29 +0100
> > From: Martin Pieuchot 
> > 
> > Diff below move per HC driver polling code to the stack.  I know this
> > code contains a use-after-free and this will be addressed in a later
> > diff.  For the moment merge 4 copies into 1 such that I don't have to
> > fix all of them.
> > 
> > As a side effect, this fix xhci(4) polling mode.  That means you can
> > now use ddb(4) with an xhci(4)-attached keyboard.
> > 
> > ok?
> 
> Looks like a good idea to me.  Hwoever...

> [...]
> > +   if (polling) {
> > +   int timo;
> > +
> > +   for (timo = xfer->timeout; timo >= 0; timo--) {
> > +   usb_delay_ms(bus, 1);
> 
> The xxx_waitintr() implementations have a sc->sc_bus.dying check here.
> I'd expect that that check would still be necessary...

That check is needed when coming back from a sleep.  In polling mode
usb_delay_ms() calls delay(9) so we know that all the resources are
still there.

But it might be better to leave that for another diff.  Revised diff
below.

Index: usbdi.c
===
RCS file: /cvs/src/sys/dev/usb/usbdi.c,v
retrieving revision 1.87
diff -u -p -r1.87 usbdi.c
--- usbdi.c 6 Mar 2017 12:13:58 -   1.87
+++ usbdi.c 9 Mar 2017 14:58:44 -
@@ -279,6 +279,8 @@ usbd_status
 usbd_transfer(struct usbd_xfer *xfer)
 {
struct usbd_pipe *pipe = xfer->pipe;
+   struct usbd_bus *bus = pipe->device->bus;
+   int polling = bus->use_polling;
usbd_status err;
int flags, s;
 
@@ -298,8 +300,6 @@ usbd_transfer(struct usbd_xfer *xfer)
 
/* If there is no buffer, allocate one. */
if ((xfer->rqflags & URQ_DEV_DMABUF) == 0) {
-   struct usbd_bus *bus = pipe->device->bus;
-
 #ifdef DIAGNOSTIC
if (xfer->rqflags & URQ_AUTO_DMABUF)
printf("usbd_transfer: has old buffer!\n");
@@ -325,8 +325,6 @@ usbd_transfer(struct usbd_xfer *xfer)
if (err != USBD_IN_PROGRESS && err) {
/* The transfer has not been queued, so free buffer. */
if (xfer->rqflags & URQ_AUTO_DMABUF) {
-   struct usbd_bus *bus = pipe->device->bus;
-
usb_freemem(bus, >dmabuf);
xfer->rqflags &= ~URQ_AUTO_DMABUF;
}
@@ -338,19 +336,40 @@ usbd_transfer(struct usbd_xfer *xfer)
/* Sync transfer, wait for completion. */
if (err != USBD_IN_PROGRESS)
return (err);
+
s = splusb();
-   while (!xfer->done) {
-   if (pipe->device->bus->use_polling)
-   panic("usbd_transfer: not done");
-   flags = PRIBIO | (xfer->flags & USBD_CATCH ? PCATCH : 0);
-
-   err = tsleep(xfer, flags, "usbsyn", 0);
-   if (err && !xfer->done) {
-   usbd_abort_pipe(pipe);
-   if (err == EINTR)
-   xfer->status = USBD_INTERRUPTED;
-   else
-   xfer->status = USBD_TIMEOUT;
+   if (polling) {
+   int timo;
+
+   for (timo = xfer->timeout; timo >= 0; timo--) {
+   usb_delay_ms(bus, 1);
+   if (bus->dying) {
+   xfer->status = USBD_IOERROR;
+   usb_transfer_complete(xfer);
+   break;
+   }
+
+   usbd_dopoll(pipe->device);
+   if (xfer->done)
+   break;
+   }
+
+   if (timo < 0) {
+   xfer->status = USBD_TIMEOUT;
+   usb_transfer_complete(xfer);
+   }
+   } else {
+   while (!xfer->done) {
+   flags = PRIBIO|(xfer->flags & USBD_CATCH ? PCATCH : 0);
+
+   err = tsleep(xfer, flags, "usbsyn", 0);
+   if (err && !xfer->done) {
+   usbd_abort_pipe(pipe);
+   if (err == EINTR)
+   xfer->status = USBD_INTERRUPTED;
+   else
+   xfer->status = USBD_TIMEOUT;
+   }
}
}
splx(s);
Index: ehci.c
===
RCS file: /cvs/src/sys/dev/usb/ehci.c,v
retrieving revision 1.195
diff -u -p -r1.195 ehci.c
--- ehci.c  8 Nov 2016 10:31:30 -   1.195
+++ ehci.c  9 Mar 2017 13:19:42 -
@@ -117,7 +117,6 @@ int ehci_setaddr(struct usbd_device *, 
 void   ehci_poll(struct usbd_bus *);
 void   ehci_softintr(void *);
 intehci_intr1(struct ehci_softc *);
-void   ehci_waitintr(struct

Re: ip_ipip.c / gif(4) percpu counters

2017-03-09 Thread Jeremie Courreges-Anglas 
Martin Pieuchot  writes:

> On 08/03/17(Wed) 12:03, Jeremie Courreges-Anglas wrote:
>> [...] 
>> So here's a refreshed diff that initializes the counters directly from
>> ip_init().  I remove the ipip_init() wrapper to make it clear that
>> ip_init() is responsible for the job.
>> 
>> (Still) ok?
>
> I'm against adding more "#if PEUDOIF" in the stack everywhere and it
> makes no sense to move more globals outside of ip_ipip.c.

The previous diff was just plain wrong.

> If you really don't want to use .pr_init, then call ipip_input() from
> ip_input().

Actually I'm fine with .pr_init.  There are other protosw entries that
reach the counters but one shouldn't remove entries carelessly anyway.

Is this better?


Index: netinet/in_proto.c
===
RCS file: /d/cvs/src/sys/netinet/in_proto.c,v
retrieving revision 1.74
diff -u -p -r1.74 in_proto.c
--- netinet/in_proto.c  2 Mar 2017 08:58:24 -   1.74
+++ netinet/in_proto.c  8 Mar 2017 11:56:45 -
@@ -239,7 +239,8 @@ struct protosw inetsw[] = {
   .pr_output   = rip_output,
   .pr_ctloutput= rip_ctloutput,
   .pr_usrreq   = rip_usrreq,
-  .pr_sysctl   = ipip_sysctl
+  .pr_sysctl   = ipip_sysctl,
+  .pr_init = ipip_init
 },
 {
   .pr_type = SOCK_RAW,
@@ -284,7 +285,8 @@ struct protosw inetsw[] = {
   .pr_output   = rip_output,
   .pr_ctloutput= rip_ctloutput,
   .pr_usrreq   = rip_usrreq,
-  .pr_sysctl   = ipip_sysctl
+  .pr_sysctl   = ipip_sysctl,
+  .pr_init = ipip_init
 },
 #ifdef INET6
 {
Index: netinet/ip_ipip.c
===
RCS file: /d/cvs/src/sys/netinet/ip_ipip.c,v
retrieving revision 1.71
diff -u -p -r1.71 ip_ipip.c
--- netinet/ip_ipip.c   29 Jan 2017 19:58:47 -  1.71
+++ netinet/ip_ipip.c   8 Mar 2017 11:59:04 -
@@ -84,7 +84,13 @@
  */
 int ipip_allow = 0;
 
-struct ipipstat ipipstat;
+struct cpumem *ipipcounters;
+
+void
+ipip_init(void)
+{
+   ipipcounters = counters_alloc(ipips_ncounters);
+}
 
 /*
  * Really only a wrapper for ipip_input(), for use with pr_input.
@@ -95,7 +101,7 @@ ip4_input(struct mbuf **mp, int *offp, i
/* If we do not accept IP-in-IP explicitly, drop.  */
if (!ipip_allow && ((*mp)->m_flags & (M_AUTH|M_CONF)) == 0) {
DPRINTF(("ip4_input(): dropped due to policy\n"));
-   ipipstat.ipips_pdrops++;
+   ipipstat_inc(ipips_pdrops);
m_freem(*mp);
return IPPROTO_DONE;
}
@@ -129,7 +135,7 @@ ipip_input(struct mbuf **mp, int *offp, 
u_int8_t v;
sa_family_t af;
 
-   ipipstat.ipips_ipackets++;
+   ipipstat_inc(ipips_ipackets);
 
m_copydata(m, 0, 1, );
 
@@ -143,7 +149,7 @@ ipip_input(struct mbuf **mp, int *offp, 
break;
 #endif
default:
-   ipipstat.ipips_family++;
+   ipipstat_inc(ipips_family);
m_freem(m);
return IPPROTO_DONE;
}
@@ -152,7 +158,7 @@ ipip_input(struct mbuf **mp, int *offp, 
if (m->m_len < hlen) {
if ((m = m_pullup(m, hlen)) == NULL) {
DPRINTF(("ipip_input(): m_pullup() failed\n"));
-   ipipstat.ipips_hdrops++;
+   ipipstat_inc(ipips_hdrops);
return IPPROTO_DONE;
}
}
@@ -179,7 +185,7 @@ ipip_input(struct mbuf **mp, int *offp, 
 
/* Sanity check */
if (m->m_pkthdr.len < sizeof(struct ip)) {
-   ipipstat.ipips_hdrops++;
+   ipipstat_inc(ipips_hdrops);
m_freem(m);
return IPPROTO_DONE;
}
@@ -195,7 +201,7 @@ ipip_input(struct mbuf **mp, int *offp, 
break;
 #endif
default:
-   ipipstat.ipips_family++;
+   ipipstat_inc(ipips_family);
m_freem(m);
return IPPROTO_DONE;
}
@@ -206,7 +212,7 @@ ipip_input(struct mbuf **mp, int *offp, 
if (m->m_len < hlen) {
if ((m = m_pullup(m, hlen)) == NULL) {
DPRINTF(("ipip_input(): m_pullup() failed\n"));
-   ipipstat.ipips_hdrops++;
+   ipipstat_inc(ipips_hdrops);
return IPPROTO_DONE;
}
}
@@ -229,7 +235,7 @@ ipip_input(struct mbuf **mp, int *offp, 
ECN_ALLOWED_IPSEC : ECN_ALLOWED;
if (!ip_ecn_egress(mode, , >ip_tos)) {
DPRINTF(("ipip_input(): ip_ecn_egress() failed"));
-   ipipstat.ipips_pdrops++;
+   ipipstat_inc(ipips_pdrops);
m_freem(m);
return IPPROTO_DONE;
}
@@ -249,7 +255,7 @@ ipip_input(struct mbuf **mp, int *offp, 
itos = (ntohl(ip6->ip6_flow) >> 20) & 0xff;
if

Re: USB polling + xhci(4) fix

2017-03-09 Thread Mark Kettenis 
> Date: Thu, 9 Mar 2017 14:32:29 +0100
> From: Martin Pieuchot 
> 
> Diff below move per HC driver polling code to the stack.  I know this
> code contains a use-after-free and this will be addressed in a later
> diff.  For the moment merge 4 copies into 1 such that I don't have to
> fix all of them.
> 
> As a side effect, this fix xhci(4) polling mode.  That means you can
> now use ddb(4) with an xhci(4)-attached keyboard.
> 
> ok?

Looks like a good idea to me.  Hwoever...

> 
> Index: usbdi.c
> ===
> RCS file: /cvs/src/sys/dev/usb/usbdi.c,v
> retrieving revision 1.87
> diff -u -p -r1.87 usbdi.c
> --- usbdi.c   6 Mar 2017 12:13:58 -   1.87
> +++ usbdi.c   9 Mar 2017 13:27:06 -
> @@ -279,6 +279,8 @@ usbd_status
>  usbd_transfer(struct usbd_xfer *xfer)
>  {
>   struct usbd_pipe *pipe = xfer->pipe;
> + struct usbd_bus *bus = pipe->device->bus;
> + int polling = bus->use_polling;
>   usbd_status err;
>   int flags, s;
>  
> @@ -298,8 +300,6 @@ usbd_transfer(struct usbd_xfer *xfer)
>  
>   /* If there is no buffer, allocate one. */
>   if ((xfer->rqflags & URQ_DEV_DMABUF) == 0) {
> - struct usbd_bus *bus = pipe->device->bus;
> -
>  #ifdef DIAGNOSTIC
>   if (xfer->rqflags & URQ_AUTO_DMABUF)
>   printf("usbd_transfer: has old buffer!\n");
> @@ -325,8 +325,6 @@ usbd_transfer(struct usbd_xfer *xfer)
>   if (err != USBD_IN_PROGRESS && err) {
>   /* The transfer has not been queued, so free buffer. */
>   if (xfer->rqflags & URQ_AUTO_DMABUF) {
> - struct usbd_bus *bus = pipe->device->bus;
> -
>   usb_freemem(bus, >dmabuf);
>   xfer->rqflags &= ~URQ_AUTO_DMABUF;
>   }
> @@ -338,19 +336,34 @@ usbd_transfer(struct usbd_xfer *xfer)
>   /* Sync transfer, wait for completion. */
>   if (err != USBD_IN_PROGRESS)
>   return (err);
> +
>   s = splusb();
> - while (!xfer->done) {
> - if (pipe->device->bus->use_polling)
> - panic("usbd_transfer: not done");
> - flags = PRIBIO | (xfer->flags & USBD_CATCH ? PCATCH : 0);
> -
> - err = tsleep(xfer, flags, "usbsyn", 0);
> - if (err && !xfer->done) {
> - usbd_abort_pipe(pipe);
> - if (err == EINTR)
> - xfer->status = USBD_INTERRUPTED;
> - else
> - xfer->status = USBD_TIMEOUT;
> + if (polling) {
> + int timo;
> +
> + for (timo = xfer->timeout; timo >= 0; timo--) {
> + usb_delay_ms(bus, 1);

The xxx_waitintr() implementations have a sc->sc_bus.dying check here.
I'd expect that that check would still be necessary...

> + usbd_dopoll(pipe->device);
> + if (xfer->done)
> + break;
> + }
> +
> + if (timo < 0) {
> + xfer->status = USBD_TIMEOUT;
> + usb_transfer_complete(xfer);
> + }
> + } else {
> + while (!xfer->done) {
> + flags = PRIBIO|(xfer->flags & USBD_CATCH ? PCATCH : 0);
> +
> + err = tsleep(xfer, flags, "usbsyn", 0);
> + if (err && !xfer->done) {
> + usbd_abort_pipe(pipe);
> + if (err == EINTR)
> + xfer->status = USBD_INTERRUPTED;
> + else
> + xfer->status = USBD_TIMEOUT;
> + }
>   }
>   }
>   splx(s);
> Index: ehci.c
> ===
> RCS file: /cvs/src/sys/dev/usb/ehci.c,v
> retrieving revision 1.195
> diff -u -p -r1.195 ehci.c
> --- ehci.c8 Nov 2016 10:31:30 -   1.195
> +++ ehci.c9 Mar 2017 13:19:42 -
> @@ -117,7 +117,6 @@ int   ehci_setaddr(struct usbd_device *, 
>  void ehci_poll(struct usbd_bus *);
>  void ehci_softintr(void *);
>  int  ehci_intr1(struct ehci_softc *);
> -void ehci_waitintr(struct ehci_softc *, struct usbd_xfer *);
>  void ehci_check_intr(struct ehci_softc *, struct usbd_xfer *);
>  void ehci_check_qh_intr(struct ehci_softc *, struct usbd_xfer *);
>  void ehci_check_itd_intr(struct ehci_softc *, struct usbd_xfer *);
> @@ -918,37 +917,6 @@ ehci_idone(struct usbd_xfer *xfer)
>   DPRINTFN(/*12*/2, ("ehci_idone: ex=%p done\n", ex));
>  }
>  
> -/*
> - * Wait here until controller claims to have an interrupt.
> - * Then call ehci_intr and return.  Use timeout to avoid waiting
> - * too long.
> - */
> -void
> -ehci_waitintr(struct ehci_softc *sc, struct usbd_xfer *xfer)
> -{
> - int timo;
> -

Re: sysctl machdep.fakepckbc=1

2017-03-09 Thread Mark Kettenis 
> Date: Thu, 9 Mar 2017 14:04:04 +0100
> From: Martin Pieuchot 
> 
> Many of us have x86 machines with a USB keyboard that's unusable in
> ddb(4).  That's generally because the BIOS presents a fake pckbd(4)
> to the OS:
> 
>   pckbc0 at isa0 port 0x60/5 irq 1 irq 12
>   pckbd0 at pckbc0 (kbd slot)
>   wskbd0 at pckbd0: console keyboard, using wsdisplay0
> 
> Some BIOSes allow you to disable this emulation.  While this might
> allow you to have a working USB keyboard in ddb(4) it generally means
> you lose input in the bootloader.
> 
> So the diff below introduces a new sysctl(3) to work around this problem.
> Adding the following line in your /etc/sysctl.conf will force your first
> USB keyboard to re-attach itself after cold boot and become the default
> console keyboard.  That means you now have a working keyboard in ddb(4)!
> 
>   sysctl machdep.fakepckbc=1
> 
> I hope this will improve the content of bug reports.

Can't you already achieve this by disabling pckbc(4) in the kernel
config?  So something like this:

# config -ef /bsd
...
ukc> disable pckbc
250 pckbc0 disabled
ukc> quit

Only downside is that it doesn't stick.  If you install a new kernel
you need to repeat the excercise.

USB polling + xhci(4) fix

2017-03-09 Thread Martin Pieuchot 
Diff below move per HC driver polling code to the stack.  I know this
code contains a use-after-free and this will be addressed in a later
diff.  For the moment merge 4 copies into 1 such that I don't have to
fix all of them.

As a side effect, this fix xhci(4) polling mode.  That means you can
now use ddb(4) with an xhci(4)-attached keyboard.

ok?

Index: usbdi.c
===
RCS file: /cvs/src/sys/dev/usb/usbdi.c,v
retrieving revision 1.87
diff -u -p -r1.87 usbdi.c
--- usbdi.c 6 Mar 2017 12:13:58 -   1.87
+++ usbdi.c 9 Mar 2017 13:27:06 -
@@ -279,6 +279,8 @@ usbd_status
 usbd_transfer(struct usbd_xfer *xfer)
 {
struct usbd_pipe *pipe = xfer->pipe;
+   struct usbd_bus *bus = pipe->device->bus;
+   int polling = bus->use_polling;
usbd_status err;
int flags, s;
 
@@ -298,8 +300,6 @@ usbd_transfer(struct usbd_xfer *xfer)
 
/* If there is no buffer, allocate one. */
if ((xfer->rqflags & URQ_DEV_DMABUF) == 0) {
-   struct usbd_bus *bus = pipe->device->bus;
-
 #ifdef DIAGNOSTIC
if (xfer->rqflags & URQ_AUTO_DMABUF)
printf("usbd_transfer: has old buffer!\n");
@@ -325,8 +325,6 @@ usbd_transfer(struct usbd_xfer *xfer)
if (err != USBD_IN_PROGRESS && err) {
/* The transfer has not been queued, so free buffer. */
if (xfer->rqflags & URQ_AUTO_DMABUF) {
-   struct usbd_bus *bus = pipe->device->bus;
-
usb_freemem(bus, >dmabuf);
xfer->rqflags &= ~URQ_AUTO_DMABUF;
}
@@ -338,19 +336,34 @@ usbd_transfer(struct usbd_xfer *xfer)
/* Sync transfer, wait for completion. */
if (err != USBD_IN_PROGRESS)
return (err);
+
s = splusb();
-   while (!xfer->done) {
-   if (pipe->device->bus->use_polling)
-   panic("usbd_transfer: not done");
-   flags = PRIBIO | (xfer->flags & USBD_CATCH ? PCATCH : 0);
-
-   err = tsleep(xfer, flags, "usbsyn", 0);
-   if (err && !xfer->done) {
-   usbd_abort_pipe(pipe);
-   if (err == EINTR)
-   xfer->status = USBD_INTERRUPTED;
-   else
-   xfer->status = USBD_TIMEOUT;
+   if (polling) {
+   int timo;
+
+   for (timo = xfer->timeout; timo >= 0; timo--) {
+   usb_delay_ms(bus, 1);
+   usbd_dopoll(pipe->device);
+   if (xfer->done)
+   break;
+   }
+
+   if (timo < 0) {
+   xfer->status = USBD_TIMEOUT;
+   usb_transfer_complete(xfer);
+   }
+   } else {
+   while (!xfer->done) {
+   flags = PRIBIO|(xfer->flags & USBD_CATCH ? PCATCH : 0);
+
+   err = tsleep(xfer, flags, "usbsyn", 0);
+   if (err && !xfer->done) {
+   usbd_abort_pipe(pipe);
+   if (err == EINTR)
+   xfer->status = USBD_INTERRUPTED;
+   else
+   xfer->status = USBD_TIMEOUT;
+   }
}
}
splx(s);
Index: ehci.c
===
RCS file: /cvs/src/sys/dev/usb/ehci.c,v
retrieving revision 1.195
diff -u -p -r1.195 ehci.c
--- ehci.c  8 Nov 2016 10:31:30 -   1.195
+++ ehci.c  9 Mar 2017 13:19:42 -
@@ -117,7 +117,6 @@ int ehci_setaddr(struct usbd_device *, 
 void   ehci_poll(struct usbd_bus *);
 void   ehci_softintr(void *);
 intehci_intr1(struct ehci_softc *);
-void   ehci_waitintr(struct ehci_softc *, struct usbd_xfer *);
 void   ehci_check_intr(struct ehci_softc *, struct usbd_xfer *);
 void   ehci_check_qh_intr(struct ehci_softc *, struct usbd_xfer *);
 void   ehci_check_itd_intr(struct ehci_softc *, struct usbd_xfer *);
@@ -918,37 +917,6 @@ ehci_idone(struct usbd_xfer *xfer)
DPRINTFN(/*12*/2, ("ehci_idone: ex=%p done\n", ex));
 }
 
-/*
- * Wait here until controller claims to have an interrupt.
- * Then call ehci_intr and return.  Use timeout to avoid waiting
- * too long.
- */
-void
-ehci_waitintr(struct ehci_softc *sc, struct usbd_xfer *xfer)
-{
-   int timo;
-   u_int32_t intrs;
-
-   xfer->status = USBD_IN_PROGRESS;
-   for (timo = xfer->timeout; timo >= 0; timo--) {
-   usb_delay_ms(>sc_bus, 1);
-   if (sc->sc_bus.dying)
-   break;
-   intrs = EHCI_STS_INTRS(EOREAD4(sc, EHCI_USBSTS)) &
-   sc->sc_eintrs;
-   if

sysctl machdep.fakepckbc=1

2017-03-09 Thread Martin Pieuchot 
Many of us have x86 machines with a USB keyboard that's unusable in
ddb(4).  That's generally because the BIOS presents a fake pckbd(4)
to the OS:

pckbc0 at isa0 port 0x60/5 irq 1 irq 12
pckbd0 at pckbc0 (kbd slot)
wskbd0 at pckbd0: console keyboard, using wsdisplay0

Some BIOSes allow you to disable this emulation.  While this might
allow you to have a working USB keyboard in ddb(4) it generally means
you lose input in the bootloader.

So the diff below introduces a new sysctl(3) to work around this problem.
Adding the following line in your /etc/sysctl.conf will force your first
USB keyboard to re-attach itself after cold boot and become the default
console keyboard.  That means you now have a working keyboard in ddb(4)!

sysctl machdep.fakepckbc=1

I hope this will improve the content of bug reports.

I'm happy to change the name if you don't like ``fakepckbc'' maybe
``usbconsole''? 

Note that USB keyboards attached to xhci(4) still don't work in ddb(4).

Index: sbin/sysctl/sysctl.8
===
RCS file: /cvs/src/sbin/sysctl/sysctl.8,v
retrieving revision 1.209
diff -u -p -r1.209 sysctl.8
--- sbin/sysctl/sysctl.84 Mar 2017 17:40:23 -   1.209
+++ sbin/sysctl/sysctl.89 Mar 2017 12:52:32 -
@@ -370,6 +370,9 @@ and a few require a kernel compiled with
 .It machdep.sse Ta integer Ta no
 .It machdep.sse2 Ta integer Ta no
 .It machdep.xcrypt Ta integer Ta no
+.It machdep.lidsuspend Ta integer Ta yes
+.It machdep.lidaction Ta integer Ta yes
+.It machdep.fakepckbc Ta integer Ta yes
 .It machdep.allowaperture Ta integer Ta yes
 .It machdep.led_blink Ta integer Ta yes
 .It machdep.ceccerrs Ta integer Ta no
Index: sys/arch/amd64/amd64/machdep.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/machdep.c,v
retrieving revision 1.225
diff -u -p -r1.225 machdep.c
--- sys/arch/amd64/amd64/machdep.c  7 Mar 2017 11:49:42 -   1.225
+++ sys/arch/amd64/amd64/machdep.c  9 Mar 2017 12:22:52 -
@@ -146,6 +146,11 @@ extern int db_console;
 #include 
 #endif /* HIBERNATE */
 
+#include "pckbc.h"
+#if NPCKBC > 0
+#include 
+#endif
+
 /* the following is used externally (sysctl_hw) */
 char machine[] = MACHINE;
 
@@ -189,6 +194,7 @@ paddr_t tramp_pdirpa;
 
 int kbd_reset;
 int lid_action = 1;
+int fake_pckbc;
 
 /*
  * safepri is a safe priority for sleep to set for a spin-wait
@@ -506,6 +512,16 @@ cpu_sysctl(int *name, u_int namelen, voi
lid_action = val;
}
return (error);
+#if NPCKBC > 0
+   case CPU_FAKEPCKBC:
+   if (fake_pckbc)
+   return (sysctl_rdint(oldp, oldlenp, newp, fake_pckbc));
+
+   error = sysctl_int(oldp, oldlenp, newp, newlen, _pckbc);
+   if (fake_pckbc)
+   pckbc_release_console();
+   return (error);
+#endif
default:
return (EOPNOTSUPP);
}
Index: sys/arch/amd64/include/cpu.h
===
RCS file: /cvs/src/sys/arch/amd64/include/cpu.h,v
retrieving revision 1.108
diff -u -p -r1.108 cpu.h
--- sys/arch/amd64/include/cpu.h2 Mar 2017 10:38:10 -   1.108
+++ sys/arch/amd64/include/cpu.h9 Mar 2017 12:50:00 -
@@ -426,7 +426,8 @@ void mp_setperf_init(void);
 #define CPU_XCRYPT 12  /* supports VIA xcrypt in userland */
 #define CPU_LIDSUSPEND 13  /* lid close causes a suspend */
 #define CPU_LIDACTION  14  /* action caused by lid close */
-#define CPU_MAXID  15  /* number of valid machdep ids */
+#define CPU_FAKEPCKBC  15  /* fake pckbc(4) conroller */
+#define CPU_MAXID  16  /* number of valid machdep ids */
 
 #defineCTL_MACHDEP_NAMES { \
{ 0, 0 }, \
@@ -444,6 +445,7 @@ void mp_setperf_init(void);
{ "xcrypt", CTLTYPE_INT }, \
{ "lidsuspend", CTLTYPE_INT }, \
{ "lidaction", CTLTYPE_INT }, \
+   { "fakepckbc", CTLTYPE_INT }, \
 }
 
 /*
Index: sys/arch/i386/i386/machdep.c
===
RCS file: /cvs/src/sys/arch/i386/i386/machdep.c,v
retrieving revision 1.597
diff -u -p -r1.597 machdep.c
--- sys/arch/i386/i386/machdep.c7 Mar 2017 11:49:42 -   1.597
+++ sys/arch/i386/i386/machdep.c9 Mar 2017 12:23:32 -
@@ -168,6 +168,11 @@ extern struct proc *npxproc;
 #include 
 #endif /* HIBERNATE */
 
+#include "pckbc.h"
+#if NPCKBC > 0
+#include 
+#endif
+
 #include "vmm.h"
 
 void   replacesmap(void);
@@ -235,6 +240,7 @@ voidvia_update_sensor(void *args);
 #endif
 int kbd_reset;
 int lid_action = 1;
+int fake_pckbc;
 
 /*
  * safepri is a safe priority for sleep to set for a spin-wait
@@ -3529,6 +3535,7 @@ int
 cpu_sysctl(int *name, u_int namelen, void *oldp,

Re: pfsync if_get conversion

2017-03-09 Thread Martin Pieuchot 
On 09/03/17(Thu) 08:48, Stefan Sperling wrote:
> This diff converts a struct ifnet pointer in pfsync's softc into an
> ifindex with corresponding if_get()/if_put() calls.

This avoid the panic but obfuscate the problem.  What you want is a
detachhook such that if the parent interface is destroyed you don't
leave a sale pointer.

You can look at how vlan(4) or other pseudo drivers do it.