Re: is anyone maintaining hifn(4)?

emily ingalls <[email protected]> Sat, 24 Jul 2021 02:15:22 -0700
Newsgroups gmane.os.netbsd.devel.crypto
Message-ID <[email protected]>
>=20
> 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=3D10 in which case mstohz(10) =3D 0 leads =
to
>   no pause at all.
>=20

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 =3D 0;

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

+       /* wait for pending aborts to complete */
+       mutex_enter(&sc->sc_mtx);
+       while(sc->sc_aborting =3D=3D 1)
+               cv_wait(&sc->sc_cv, &sc->sc_mtx);
+       mutex_exit(&sc->sc_mtx);
+
+       sc->sc_aborting =3D 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 =3D 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 =3D 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 =3D dma->resk; u =3D dma->resu;
        while (u !=3D 0) {
                cmd =3D dma->hifn_commands[i];
@@ -2600,6 +2643,21 @@
 }

 static void
+hifn_abort_worker(struct work *wk, void *hsc)
+{
+       struct hifn_softc *sc =3D hsc;
+
+       hifn_abort(sc);
+       mutex_enter(&sc->sc_mtx);
+
+       sc->sc_aborting =3D 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 =3D 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))=