Re: is anyone maintaining hifn(4)?

[emily ingalls <emily@ingalls.rocks>]

> 
> 3. mstohz turns out to be kind of a lousy API -- it doesn't (always?)
>   round up, so you should use MIN(1, mstohz(10)) in case anyone tries
>   to run this code with HZ=10 in which case mstohz(10) = 0 leads to
>   no pause at all.
> 

i'm think you meant meant MAX(1, mstohz(10)) here, i don't think MIN makes sense unless i'm misunderstanding.

aside from that point: i believe i've addressed the other points you mentioned, and an updated patch follows. let me know if you have any further comments on this or if there's something i missed.

i've played around with this a bit on a Cyrix 6x86 system, a dual Pentium II system, and my 486 system. everything still works as expected on the former two, and the 486 is still exhibiting the same behavior as before (i.e. not working, but working better than before the patch).



diff -ru a/sys/dev/pci/hifn7751.c b/sys/dev/pci/hifn7751.c
--- a/sys/dev/pci/hifn7751.c    2018-12-27 06:03:54.000000000 -0800
+++ b/sys/dev/pci/hifn7751.c    2021-07-24 02:12:14.293549726 -0700
@@ -60,6 +60,7 @@
 #include <sys/mbuf.h>
 #include <sys/device.h>
 #include <sys/module.h>
+#include <sys/workqueue.h>

 #ifdef __OpenBSD__
 #include <crypto/crypto.h>
@@ -152,6 +153,7 @@
 static void    hifn_rng_locked(void *);
 static void    hifn_tick(void *);
 static void    hifn_abort(struct hifn_softc *);
+static void    hifn_abort_worker(struct work *, void *);
 static void    hifn_alloc_slot(struct hifn_softc *, int *, int *, int *,
                                int *);
 static void    hifn_write_4(struct hifn_softc *, int, bus_size_t, u_int32_t);
@@ -435,6 +437,14 @@
            BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE);

        mutex_init(&sc->sc_mtx, MUTEX_DEFAULT, IPL_VM);
+       cv_init(&sc->sc_cv, "hifnabrt");
+
+       if (workqueue_create(&sc->sc_wq, "hifnabrt",
+               hifn_abort_worker, sc, PRI_NONE, IPL_VM, 0))
+                       panic("%s: could not create workqueue: hifnabrt",
+                           device_xname(self));
+
+       sc->sc_aborting = 0;

        if (sc->sc_flags & (HIFN_HAS_PUBLIC | HIFN_HAS_RNG)) {
                hifn_init_pubrng(sc);
@@ -474,6 +484,13 @@
 {
        struct hifn_softc *sc = device_private(self);

+       /* wait for pending aborts to complete */
+       mutex_enter(&sc->sc_mtx);
+       while(sc->sc_aborting == 1)
+               cv_wait(&sc->sc_cv, &sc->sc_mtx);
+       mutex_exit(&sc->sc_mtx);
+
+       sc->sc_aborting = 1;
        hifn_abort(sc);

        hifn_reset_board(sc, 1);
@@ -1073,6 +1090,21 @@
                WRITE_REG_0(sc, HIFN_0_PUCNFG, HIFN_PUCNFG_COMPSING |
                    HIFN_PUCNFG_TCALLPHASES |
                    HIFN_PUCNFG_TCDRVTOTEM | HIFN_PUCNFG_BUS32);
+               /*
+                * per datasheet: set PLL speed and charge current,
+                * and turn on the PLL bypass
+                */
+               WRITE_REG_1(sc, HIFN_1_PLL, HIFN_PLL_7956_INIT);
+               /* then, wait at least 10ms for the PLL to lock */
+               kpause("hifnpll", false, MAX(1, mstohz(10)), NULL);
+               /* then, disable PLL bypass */
+               WRITE_REG_1(sc, HIFN_1_PLL, HIFN_PLL_7956_INIT2);
+               /* pause again for good measure */
+               kpause("hifnpll", false, MAX(1, mstohz(10)), NULL);
+               /*
+                * write the final PLL config value, with packet engine and
+                * key engine set to use the PLL output
+                */
                WRITE_REG_1(sc, HIFN_1_PLL, HIFN_PLL_7956);
        } else {
                WRITE_REG_0(sc, HIFN_0_PUCNFG, HIFN_PUCNFG_COMPSING |
@@ -1691,6 +1723,11 @@
        u_int32_t cmdlen;
        int     cmdi, resi, err = 0;

+       KASSERT(mutex_owned(&sc->sc_mtx));
+
+       if (sc->sc_aborting)
+               return (ERESTART);
+
        if (bus_dmamap_create(sc->sc_dmat, HIFN_MAX_DMALEN, MAX_SCATTER,
            HIFN_MAX_SEGLEN, 0, BUS_DMA_NOWAIT, &cmd->src_map))
                return (ENOMEM);
@@ -2044,7 +2081,10 @@
        if (restart) {
                printf("%s: abort, resetting.\n", device_xname(sc->sc_dv));
                hifnstats.hst_abort++;
-               hifn_abort(sc);
+               if (!sc->sc_aborting) {
+                       sc->sc_aborting = true;
+                       workqueue_enqueue(sc->sc_wq, &sc->sc_work, NULL);
+               }
                goto out;
        }

@@ -2533,6 +2573,9 @@
        struct cryptop *crp;
        int i, u;

+       KASSERT(sc->sc_aborting);
+       ASSERT_SLEEPABLE();
+
        i = dma->resk; u = dma->resu;
        while (u != 0) {
                cmd = dma->hifn_commands[i];
@@ -2600,6 +2643,21 @@
 }

 static void
+hifn_abort_worker(struct work *wk, void *hsc)
+{
+       struct hifn_softc *sc = hsc;
+
+       hifn_abort(sc);
+       mutex_enter(&sc->sc_mtx);
+
+       sc->sc_aborting = 0;
+       cv_signal(&sc->sc_cv);
+       crypto_unblock(sc->sc_cid, CRYPTO_SYMQ|CRYPTO_ASYMQ);
+
+       mutex_exit(&sc->sc_mtx);
+}
+
+static void
 hifn_callback(struct hifn_softc *sc, struct hifn_command *cmd, u_int8_t *resbuf)
 {
        struct hifn_dma *dma = sc->sc_dma;
@@ -2870,6 +2928,11 @@
        int cmdi, resi;
        u_int32_t cmdlen;

+       KASSERT(mutex_owned(&sc->sc_mtx));
+
+       if (sc->sc_aborting)
+               return (ERESTART);
+
        if ((dma->cmdu + 1) > HIFN_D_CMD_RSIZE ||
            (dma->resu + 1) > HIFN_D_CMD_RSIZE)
                return (ENOMEM);
diff -ru a/sys/dev/pci/hifn7751reg.h b/sys/dev/pci/hifn7751reg.h
--- a/sys/dev/pci/hifn7751reg.h 2005-12-11 04:22:49.000000000 -0800
+++ b/sys/dev/pci/hifn7751reg.h 2021-07-17 23:42:13.372045735 -0700
@@ -361,7 +361,16 @@
 /*
  * PLL config register
  */
-#define        HIFN_PLL_7956           0x00001d18      /* 7956 PLL config value */
+
+/* 7956 PLL config values
+ *
+ * set PLL to use onboard reference, rather than a potentially
+ * unknown and unstable bus clock. setting PLL correctly requires
+ * three steps.
+ */
+#define HIFN_PLL_7956_INIT     0x00001c03 /* initial state, setting pll clock source */
+#define HIFN_PLL_7956_INIT2    0x00001c01 /* after PLL stabilizes, clear PLL bypass bit */
+#define        HIFN_PLL_7956           0x00001c19 /* set packet and key engines to use PLL */

 /*********************************************************************
  * Structs for board commands
diff -ru a/sys/dev/pci/hifn7751var.h b/sys/dev/pci/hifn7751var.h
--- a/sys/dev/pci/hifn7751var.h 2015-04-14 13:32:36.000000000 -0700
+++ b/sys/dev/pci/hifn7751var.h 2021-07-23 20:22:40.214682622 -0700
@@ -184,12 +184,16 @@
        int                     sc_needwakeup;  /* ops q'd wating on resources */
        int                     sc_curbatch;    /* # ops submitted w/o int */
        int                     sc_suspended;
+       int                     sc_aborting;
        struct hifn_session sc_sessions[2048];
        pci_chipset_tag_t sc_pci_pc;
        pcitag_t sc_pci_tag;
        bus_size_t sc_waw_lastreg;
        int sc_waw_lastgroup;
        kmutex_t                sc_mtx;
+       kcondvar_t              sc_cv;
+       struct workqueue        *sc_wq;
+       struct work             sc_work;
 };

 #define WRITE_REG_0(sc,reg,val)                hifn_write_4((sc), 0, (reg), (val))