Re: [macppc 6.7-beta] clang backend error: "adde Constant" issue

2020-04-27 Thread George Koehler
Moving from bugs@ to tech@,
because some people might miss a clang diff on bugs@.

This diff modifies LLVM's DAGCombiner to skip an optimization if it
would make an illegal ISD::ADDE node.  This fixes fatal errors from
powerpc clang when building ports net/libtorrent-rasterbar and
devel/avr/binutils on PowerPC.  The fatal errors look like,

On Sun, 19 Apr 2020 17:26:49 +0200
Charlene Wendling  wrote:

> fatal error: error in backend: Cannot select: 0x90a1c1b0: i1,glue = adde 
> Constant:i1<0>, Constant:i1<-1>, 0xa6a2b1b0:1
>   0xa6a2b828: i1 = Constant<0>
>   0x90a1c630: i1 = Constant<-1>
>   0xa6a2b1b0: i32,glue = addc 0x90a1c3a8, Constant:i32<-1>

An example to cause this error, given unsigned 32-bit *p, is
if (*p > ((*p + 15ULL) & ~15ULL)) ...

The SelectionDAG goes through phases.  The DAG starts with an i64 add
*p + 15ULL, but 32-bit PowerPC has no i64 ops, so the legalize-types
phase converts the i64 add to an i32 addc/adde pair.  The DAG keeps
some i1 ops because clang -mcrbits (the default) enables i1 ops on
condition register bits.  DAGCombiner is optimizing the i1 ops when it
truncates the i32 adde to an i1 adde, but i1 adde is an illegal op.

Legalize-ops runs next, and would convert illegal ops to legal ops,
but it can't fix i1 adde, because it has no case for ISD::ADDE.
LegalizeIntegerTypes.cpp:1966 knows this, and refuses to insert an
illegal adde, but DAGCombiner::visitTRUNCATE didn't know.

This diff might affect other arches.  I guess that Aarch64, ARM, MIPS,
Sparc don't have integer ops smaller than i32, and X86 doesn't use
ISD::ADDE, so this diff might not affect them.

OK to commit?

Index: lib/CodeGen//SelectionDAG/DAGCombiner.cpp
===
RCS file: /cvs/src/gnu/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp,v
retrieving revision 1.1.1.8
diff -u -p -r1.1.1.8 DAGCombiner.cpp
--- lib/CodeGen//SelectionDAG/DAGCombiner.cpp   23 Jun 2019 21:36:48 -  
1.1.1.8
+++ lib/CodeGen//SelectionDAG/DAGCombiner.cpp   27 Apr 2020 21:48:18 -
@@ -9904,9 +9904,10 @@ SDValue DAGCombiner::visitTRUNCATE(SDNod
   // (trunc adde(X, Y, Carry)) -> (adde trunc(X), trunc(Y), Carry)
   // (trunc addcarry(X, Y, Carry)) -> (addcarry trunc(X), trunc(Y), Carry)
   // When the adde's carry is not used.
+  // Don't make an illegal adde: LegalizeDAG can't expand nor promote it.
   if ((N0.getOpcode() == ISD::ADDE || N0.getOpcode() == ISD::ADDCARRY) &&
   N0.hasOneUse() && !N0.getNode()->hasAnyUseOfValue(1) &&
-  (!LegalOperations || TLI.isOperationLegal(N0.getOpcode(), VT))) {
+  TLI.isOperationLegal(N0.getOpcode(), VT)) {
 SDLoc SL(N);
 auto X = DAG.getNode(ISD::TRUNCATE, SL, VT, N0.getOperand(0));
 auto Y = DAG.getNode(ISD::TRUNCATE, SL, VT, N0.getOperand(1));



Re: vi: add expandtab option

2020-04-27 Thread David Gwynne



> On 28 Apr 2020, at 13:31, Todd C. Miller  wrote:
> 
> On Thu, 02 Apr 2020 16:21:47 -0600, "Todd C. Miller" wrote:
> 
>> In vim, the expandtab option expands tabs to spaces in insert mode
>> as well as when shifting and indenting/outdenting.  This is very
>> useful when working on a code-base where the style dictates using
>> spaces instead of tabs for indentation.
>> 
>> NetBSD added an implementation of expandtab to their vi some time
>> ago, but theirs doesn't convert tabs to spaces in insert mode.  I've
>> adapted the NetBSD patch and added support for expanding tabs in
>> insert mode, unless escaped via ^V.
>> 
>> The option is off by default (of course).
>> 
>> Comments?  Please, no tabs vs spaces flame wars.
> 
> Ping?  It would be nice for this to make 6.7.

im ok with it.

i've only read it, but it makes sense. i mostly like the idea that i wouldn't 
have to to use (or install) another editor if i need this.

dlg

> 
> - todd
> 
> Index: usr.bin/vi/common/options.c
> ===
> RCS file: /cvs/src/usr.bin/vi/common/options.c,v
> retrieving revision 1.27
> diff -u -p -u -r1.27 options.c
> --- usr.bin/vi/common/options.c   21 May 2019 09:24:58 -  1.27
> +++ usr.bin/vi/common/options.c   2 Apr 2020 20:43:14 -
> @@ -69,6 +69,8 @@ OPTLIST const optlist[] = {
>   {"escapetime",  NULL,   OPT_NUM,0},
> /* O_ERRORBELLS   4BSD */
>   {"errorbells",  NULL,   OPT_0BOOL,  0},
> +/* O_EXPANDTAB   NetBSD 5.0 */
> + {"expandtab",   NULL,   OPT_0BOOL,  0},
> /* O_EXRC System V (undocumented) */
>   {"exrc",NULL,   OPT_0BOOL,  0},
> /* O_EXTENDED   4.4BSD */
> @@ -207,6 +209,7 @@ static OABBREV const abbrev[] = {
>   {"co",  O_COLUMNS}, /*   4.4BSD */
>   {"eb",  O_ERRORBELLS},  /* 4BSD */
>   {"ed",  O_EDCOMPATIBLE},/* 4BSD */
> + {"et",  O_EXPANDTAB},   /* NetBSD 5.0 */
>   {"ex",  O_EXRC},/* System V (undocumented) */
>   {"ht",  O_HARDTABS},/* 4BSD */
>   {"ic",  O_IGNORECASE},  /* 4BSD */
> Index: usr.bin/vi/docs/USD.doc/vi.man/vi.1
> ===
> RCS file: /cvs/src/usr.bin/vi/docs/USD.doc/vi.man/vi.1,v
> retrieving revision 1.77
> diff -u -p -u -r1.77 vi.1
> --- usr.bin/vi/docs/USD.doc/vi.man/vi.1   4 Oct 2019 20:12:01 -   
> 1.77
> +++ usr.bin/vi/docs/USD.doc/vi.man/vi.1   2 Apr 2020 22:05:31 -
> @@ -1606,6 +1606,11 @@ and
> characters to move forward to the next
> .Ar shiftwidth
> column boundary.
> +If the
> +.Cm expandtab
> +option is set, only insert
> +.Aq space
> +characters.
> .Pp
> .It Aq Cm erase
> .It Aq Cm control-H
> @@ -2343,6 +2348,16 @@ key mapping.
> .Nm ex
> only.
> Announce error messages with a bell.
> +.It Cm expandtab , et Bq off
> +Expand
> +.Aq tab
> +characters to
> +.Aq space
> +when inserting, replacing or shifting text, autoindenting,
> +indenting with
> +.Aq Ic control-T ,
> +or outdenting with
> +.Aq Ic control-D .
> .It Cm exrc , ex Bq off
> Read the startup files in the local directory.
> .It Cm extended Bq off
> Index: usr.bin/vi/docs/USD.doc/vi.ref/set.opt.roff
> ===
> RCS file: /cvs/src/usr.bin/vi/docs/USD.doc/vi.ref/set.opt.roff,v
> retrieving revision 1.12
> diff -u -p -u -r1.12 set.opt.roff
> --- usr.bin/vi/docs/USD.doc/vi.ref/set.opt.roff   8 Aug 2016 15:09:33 
> -   1.12
> +++ usr.bin/vi/docs/USD.doc/vi.ref/set.opt.roff   2 Apr 2020 22:05:27 
> -
> @@ -96,7 +96,9 @@ the first nonblank character of the line
> Lines are indented using tab characters to the extent possible (based on
> the value of the
> .OP tabstop
> -option) and then using space characters as necessary.
> +option, and if
> +.OP expandtab
> +is not set) and then using space characters as necessary.
> For commands inserting text into the middle of a line, any blank characters
> to the right of the cursor are discarded, and the first nonblank character
> to the right of the cursor is aligned as described above.
> @@ -400,6 +402,17 @@ only.
> error messages are normally presented in inverse video.
> If that is not possible for the terminal, setting this option causes
> error messages to be announced by ringing the terminal bell.
> +.KY expandtab
> +.IP "expandtab, et [off]"
> +Expand
> +.LI 
> +characters to
> +.LI 
> +when inserting, replacing or shifting text, autoindenting,
> +indenting with
> +.CO ,
> +or outdenting with
> +.CO .
> .KY exrc
> .IP "exrc, ex [off]"
> If this option is turned on in the EXINIT environment variables,
> Index: usr.bin/vi/ex/ex_shift.c
> ===
> RCS file: /cvs/src/usr.bin/vi/ex/ex_shift.c,v
> retrieving revision 1.8

Re: vi: add expandtab option

2020-04-27 Thread Todd C . Miller
On Thu, 02 Apr 2020 16:21:47 -0600, "Todd C. Miller" wrote:

> In vim, the expandtab option expands tabs to spaces in insert mode
> as well as when shifting and indenting/outdenting.  This is very
> useful when working on a code-base where the style dictates using
> spaces instead of tabs for indentation.
>
> NetBSD added an implementation of expandtab to their vi some time
> ago, but theirs doesn't convert tabs to spaces in insert mode.  I've
> adapted the NetBSD patch and added support for expanding tabs in
> insert mode, unless escaped via ^V.
>
> The option is off by default (of course).
>
> Comments?  Please, no tabs vs spaces flame wars.

Ping?  It would be nice for this to make 6.7.

 - todd

Index: usr.bin/vi/common/options.c
===
RCS file: /cvs/src/usr.bin/vi/common/options.c,v
retrieving revision 1.27
diff -u -p -u -r1.27 options.c
--- usr.bin/vi/common/options.c 21 May 2019 09:24:58 -  1.27
+++ usr.bin/vi/common/options.c 2 Apr 2020 20:43:14 -
@@ -69,6 +69,8 @@ OPTLIST const optlist[] = {
{"escapetime",  NULL,   OPT_NUM,0},
 /* O_ERRORBELLS4BSD */
{"errorbells",  NULL,   OPT_0BOOL,  0},
+/* O_EXPANDTAB NetBSD 5.0 */
+   {"expandtab",   NULL,   OPT_0BOOL,  0},
 /* O_EXRC  System V (undocumented) */
{"exrc",NULL,   OPT_0BOOL,  0},
 /* O_EXTENDED4.4BSD */
@@ -207,6 +209,7 @@ static OABBREV const abbrev[] = {
{"co",  O_COLUMNS}, /*   4.4BSD */
{"eb",  O_ERRORBELLS},  /* 4BSD */
{"ed",  O_EDCOMPATIBLE},/* 4BSD */
+   {"et",  O_EXPANDTAB},   /* NetBSD 5.0 */
{"ex",  O_EXRC},/* System V (undocumented) */
{"ht",  O_HARDTABS},/* 4BSD */
{"ic",  O_IGNORECASE},  /* 4BSD */
Index: usr.bin/vi/docs/USD.doc/vi.man/vi.1
===
RCS file: /cvs/src/usr.bin/vi/docs/USD.doc/vi.man/vi.1,v
retrieving revision 1.77
diff -u -p -u -r1.77 vi.1
--- usr.bin/vi/docs/USD.doc/vi.man/vi.1 4 Oct 2019 20:12:01 -   1.77
+++ usr.bin/vi/docs/USD.doc/vi.man/vi.1 2 Apr 2020 22:05:31 -
@@ -1606,6 +1606,11 @@ and
 characters to move forward to the next
 .Ar shiftwidth
 column boundary.
+If the
+.Cm expandtab
+option is set, only insert
+.Aq space
+characters.
 .Pp
 .It Aq Cm erase
 .It Aq Cm control-H
@@ -2343,6 +2348,16 @@ key mapping.
 .Nm ex
 only.
 Announce error messages with a bell.
+.It Cm expandtab , et Bq off
+Expand
+.Aq tab
+characters to
+.Aq space
+when inserting, replacing or shifting text, autoindenting,
+indenting with
+.Aq Ic control-T ,
+or outdenting with
+.Aq Ic control-D .
 .It Cm exrc , ex Bq off
 Read the startup files in the local directory.
 .It Cm extended Bq off
Index: usr.bin/vi/docs/USD.doc/vi.ref/set.opt.roff
===
RCS file: /cvs/src/usr.bin/vi/docs/USD.doc/vi.ref/set.opt.roff,v
retrieving revision 1.12
diff -u -p -u -r1.12 set.opt.roff
--- usr.bin/vi/docs/USD.doc/vi.ref/set.opt.roff 8 Aug 2016 15:09:33 -   
1.12
+++ usr.bin/vi/docs/USD.doc/vi.ref/set.opt.roff 2 Apr 2020 22:05:27 -
@@ -96,7 +96,9 @@ the first nonblank character of the line
 Lines are indented using tab characters to the extent possible (based on
 the value of the
 .OP tabstop
-option) and then using space characters as necessary.
+option, and if
+.OP expandtab
+is not set) and then using space characters as necessary.
 For commands inserting text into the middle of a line, any blank characters
 to the right of the cursor are discarded, and the first nonblank character
 to the right of the cursor is aligned as described above.
@@ -400,6 +402,17 @@ only.
 error messages are normally presented in inverse video.
 If that is not possible for the terminal, setting this option causes
 error messages to be announced by ringing the terminal bell.
+.KY expandtab
+.IP "expandtab, et [off]"
+Expand
+.LI 
+characters to
+.LI 
+when inserting, replacing or shifting text, autoindenting,
+indenting with
+.CO ,
+or outdenting with
+.CO .
 .KY exrc
 .IP "exrc, ex [off]"
 If this option is turned on in the EXINIT environment variables,
Index: usr.bin/vi/ex/ex_shift.c
===
RCS file: /cvs/src/usr.bin/vi/ex/ex_shift.c,v
retrieving revision 1.8
diff -u -p -u -r1.8 ex_shift.c
--- usr.bin/vi/ex/ex_shift.c6 Jan 2016 22:28:52 -   1.8
+++ usr.bin/vi/ex/ex_shift.c2 Apr 2020 20:53:16 -
@@ -127,10 +127,13 @@ shift(SCR *sp, EXCMD *cmdp, enum which r
 * Build a new indent string and count the number of
 * characters it uses.
 */
-   for (tbp = bp, newidx = 0;
-   newcol >= O_VAL(sp, O_TABSTOP); 

Re: iked(8): remove insecure EC2N curves

2020-04-27 Thread Klemens Nanni
On Tue, Apr 28, 2020 at 01:09:09AM +0200, Tobias Heider wrote:
> the EC2N family of curves have been marked as insecure for at least 10 years.
> In fact, IANA has stopped listing them altogether [1].
> Their former IDs are now 'reserved'.
> 
> I think it's time for us to drop them as well.
OK kn



msi-x for ixl(4)

2020-04-27 Thread Jonathan Matthew
This makes ixl(4) use MSI-X where available.  The hardware is set up
for the same kind of approach as we're heading towards in em(4) and
ix(4) - interrupts for admin commands and events (link state etc.)
can only be delivered to vector 0, and the natural approach is to
map rx and tx queues to other vectors, so that's what I've done here.

The driver was already set up for multiple rx/tx queues (though it only
uses one still), so the diff sets up one vector per queue.  The vector
setup here involves creating linked lists of interrupt causes, which
are identified by a queue type (tx or rx) and a queue index.  The
queues also need to be told which msix vector they interrupt on.
This is done through per-vector and per-queue registers.

I've tested this with and without msix on amd64 with this nic:
ixl0 at pci14 dev 0 function 0 "Intel X710 SFP+" rev 0x02: port 3, FW 6.0.48754 
API 1.7, msix, 1 queue

ok?

Index: if_ixl.c
===
RCS file: /cvs/src/sys/dev/pci/if_ixl.c,v
retrieving revision 1.47
diff -u -p -r1.47 if_ixl.c
--- if_ixl.c22 Apr 2020 07:09:40 -  1.47
+++ if_ixl.c28 Apr 2020 00:24:02 -
@@ -1092,6 +1092,13 @@ struct ixl_atq {
 };
 SIMPLEQ_HEAD(ixl_atq_list, ixl_atq);
 
+struct ixl_queue_intr {
+   struct ixl_softc*sc;
+   int  queue;
+   void*ihc;
+   char name[8];
+};
+
 struct ixl_softc {
struct devicesc_dev;
struct arpcomsc_ac;
@@ -1103,6 +1110,7 @@ struct ixl_softc {
pci_intr_handle_tsc_ih;
void*sc_ihc;
pcitag_t sc_tag;
+   struct ixl_queue_intr   *sc_qintr;
 
bus_dma_tag_tsc_dmat;
bus_space_tag_t  sc_memt;
@@ -1160,6 +1168,8 @@ struct ixl_softc {
 static voidixl_clear_hw(struct ixl_softc *);
 static int ixl_pf_reset(struct ixl_softc *);
 
+static int ixl_setup_msix(struct ixl_softc *, struct pci_attach_args *);
+
 static int ixl_dmamem_alloc(struct ixl_softc *, struct ixl_dmamem *,
bus_size_t, u_int);
 static voidixl_dmamem_free(struct ixl_softc *, struct ixl_dmamem *);
@@ -1214,7 +1224,8 @@ static void   ixl_media_status(struct ifne
 static voidixl_watchdog(struct ifnet *);
 static int ixl_ioctl(struct ifnet *, u_long, caddr_t);
 static voidixl_start(struct ifqueue *);
-static int ixl_intr(void *);
+static int ixl_intr0(void *);
+static int ixl_intr_queue(void *);
 static int ixl_up(struct ixl_softc *);
 static int ixl_down(struct ixl_softc *);
 static int ixl_iff(struct ixl_softc *);
@@ -1524,13 +1535,24 @@ ixl_attach(struct device *parent, struct
goto shutdown;
}
 
-   if (pci_intr_map_msi(pa, >sc_ih) != 0 &&
-   pci_intr_map(pa, >sc_ih) != 0) {
-   printf(", unable to map interrupt\n");
-   goto shutdown;
+   if (pci_intr_map_msix(pa, 0, >sc_ih) == 0) {
+   sc->sc_qintr = mallocarray(sizeof(struct ixl_queue_intr),
+   ixl_nqueues(sc), M_DEVBUF, M_WAITOK|M_CANFAIL|M_ZERO);
+   if (sc->sc_qintr == NULL) {
+   printf(", unable to allocate queue interrupts\n");
+   goto shutdown;
+   }
+   } else {
+   if (pci_intr_map_msi(pa, >sc_ih) != 0 &&
+   pci_intr_map(pa, >sc_ih) != 0) {
+   printf(", unable to map interrupt\n");
+   goto shutdown;
+   }
}
 
-   printf(", %s, address %s\n", pci_intr_string(sc->sc_pc, sc->sc_ih),
+   printf(", %s, %d queue%s, address %s\n",
+   pci_intr_string(sc->sc_pc, sc->sc_ih), ixl_nqueues(sc),
+   (ixl_nqueues(sc) > 1 ? "s" : ""),
ether_sprintf(sc->sc_ac.ac_enaddr));
 
if (ixl_hmc(sc) != 0) {
@@ -1585,13 +1607,18 @@ ixl_attach(struct device *parent, struct
}
 
sc->sc_ihc = pci_intr_establish(sc->sc_pc, sc->sc_ih,
-   IPL_NET | IPL_MPSAFE, ixl_intr, sc, DEVNAME(sc));
+   IPL_NET | IPL_MPSAFE, ixl_intr0, sc, DEVNAME(sc));
if (sc->sc_ihc == NULL) {
printf("%s: unable to establish interrupt handler\n",
DEVNAME(sc));
goto free_scratch;
}
 
+   if (ixl_setup_msix(sc, pa) != 0) {
+   /* error printed by ixl_setup_msix */
+   goto free_scratch;
+   }
+
ifp->if_softc = sc;
ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
ifp->if_xflags = IFXF_MPSAFE;
@@ -1667,6 +1694,9 @@ shutdown:
BUS_DMASYNC_POSTREAD|BUS_DMASYNC_POSTWRITE);
 
ixl_arq_unfill(sc);
+
+   free(sc->sc_qintr, M_DEVBUF, ixl_nqueues(sc) *
+   sizeof(struct ixl_queue_intr));
 free_arq:
ixl_dmamem_free(sc, >sc_arq);
 free_atq:
@@ -1903,25 

iked(8): remove insecure EC2N curves

2020-04-27 Thread Tobias Heider
Hi,

the EC2N family of curves have been marked as insecure for at least 10 years.
In fact, IANA has stopped listing them altogether [1].
Their former IDs are now 'reserved'.

I think it's time for us to drop them as well.

ok?

[1] 
https://www.iana.org/assignments/ikev2-parameters/ikev2-parameters.xhtml#ikev2-parameters-8

Index: dh.c
===
RCS file: /cvs/src/sbin/iked/dh.c,v
retrieving revision 1.22
diff -u -p -r1.22 dh.c
--- dh.c2 Apr 2019 09:42:55 -   1.22
+++ dh.c27 Apr 2020 22:58:24 -
@@ -35,7 +35,7 @@ int   modp_getlen(struct group *);
 intmodp_create_exchange(struct group *, uint8_t *);
 intmodp_create_shared(struct group *, uint8_t *, uint8_t *);
 
-/* EC2N/ECP */
+/* ECP */
 intec_init(struct group *);
 intec_getlen(struct group *);
 intec_secretlen(struct group *);
@@ -83,8 +83,6 @@ const struct group_id ike_groups[] = {
"",
"02"
},
-   { GROUP_EC2N, 3, 155, NULL, NULL, NID_ipsec3 },
-   { GROUP_EC2N, 4, 185, NULL, NULL, NID_ipsec4 },
{ GROUP_MODP, 5, 1536,
"C90FDAA22168C234C4C6628B80DC1CD1"
"29024E088A67CC74020BBEA63B139B22514A08798E3404DD"
@@ -290,7 +288,6 @@ group_get(uint32_t id)
group->exchange = modp_create_exchange;
group->shared = modp_create_shared;
break;
-   case GROUP_EC2N:
case GROUP_ECP:
group->init = ec_init;
group->getlen = ec_getlen;
Index: dh.h
===
RCS file: /cvs/src/sbin/iked/dh.h,v
retrieving revision 1.11
diff -u -p -r1.11 dh.h
--- dh.h27 Oct 2017 14:26:35 -  1.11
+++ dh.h27 Apr 2020 22:58:24 -
@@ -21,7 +21,6 @@
 
 enum group_type {
GROUP_MODP  = 0,
-   GROUP_EC2N  = 1,
GROUP_ECP   = 2,
GROUP_CURVE25519= 3
 };
Index: iked.conf.5
===
RCS file: /cvs/src/sbin/iked/iked.conf.5,v
retrieving revision 1.66
diff -u -p -r1.66 iked.conf.5
--- iked.conf.5 27 Apr 2020 22:40:09 -  1.66
+++ iked.conf.5 27 Apr 2020 22:58:24 -
@@ -909,8 +909,6 @@ keyword:
 .It Em Name Ta Em Group Ta Em Size Ta Em Type
 .It Li modp768 Ta grp1 Ta 768 Ta "MODP"
 .It Li modp1024 Ta grp2 Ta 1024 Ta "MODP"
-.It Li ec2n155 Ta grp3 Ta 155 Ta "EC2N [insecure]"
-.It Li ec2n185 Ta grp4 Ta 185 Ta "EC2N [insecure]"
 .It Li modp1536 Ta grp5 Ta 1536 Ta "MODP"
 .It Li modp2048 Ta grp14 Ta 2048 Ta "MODP"
 .It Li modp3072 Ta grp15 Ta 3072 Ta "MODP"
@@ -931,11 +929,8 @@ keyword:
 .Pp
 The currently supported group types are either
 MODP (exponentiation groups modulo a prime),
-EC2N (elliptic curve groups over GF[2^N]),
 ECP (elliptic curve groups modulo a prime),
 or Curve25519.
-Please note that the EC2N groups are considered as insecure and only
-provided for backwards compatibility.
 .Sh FILES
 .Bl -tag -width /etc/examples/iked.conf -compact
 .It Pa /etc/iked.conf
Index: ikev2.h
===
RCS file: /cvs/src/sbin/iked/ikev2.h,v
retrieving revision 1.31
diff -u -p -r1.31 ikev2.h
--- ikev2.h 3 Dec 2019 12:38:34 -   1.31
+++ ikev2.h 27 Apr 2020 22:58:24 -
@@ -230,8 +230,6 @@ extern struct iked_constmap ikev2_xforma
 #define IKEV2_XFORMDH_NONE 0   /* No DH */
 #define IKEV2_XFORMDH_MODP_768 1   /* DH Group 1 */
 #define IKEV2_XFORMDH_MODP_10242   /* DH Group 2 */
-#define IKEV2_XFORMDH_EC2N_155 3   /* DH Group 3 */
-#define IKEV2_XFORMDH_EC2N_185 4   /* DH Group 3 */
 #define IKEV2_XFORMDH_MODP_15365   /* DH Group 5 */
 #define IKEV2_XFORMDH_MODP_204814  /* DH Group 14 */
 #define IKEV2_XFORMDH_MODP_307215  /* DH Group 15 */
Index: parse.y
===
RCS file: /cvs/src/sbin/iked/parse.y,v
retrieving revision 1.95
diff -u -p -r1.95 parse.y
--- parse.y 26 Apr 2020 16:55:47 -  1.95
+++ parse.y 27 Apr 2020 22:58:24 -
@@ -223,10 +223,6 @@ const struct ipsec_xf groupxfs[] = {
{ "grp1",   IKEV2_XFORMDH_MODP_768 },
{ "modp1024",   IKEV2_XFORMDH_MODP_1024 },
{ "grp2",   IKEV2_XFORMDH_MODP_1024 },
-   { "ec2n155",IKEV2_XFORMDH_EC2N_155 },
-   { "grp3",   IKEV2_XFORMDH_EC2N_155 },
-   { "ec2n185",IKEV2_XFORMDH_EC2N_185 },
-   { "grp4",   IKEV2_XFORMDH_EC2N_185 },
{ "modp1536",   IKEV2_XFORMDH_MODP_1536 },
{ "grp5",   IKEV2_XFORMDH_MODP_1536 },
{ "modp2048",   IKEV2_XFORMDH_MODP_2048 },



alpha installation notes INSTALL.alpha

2020-04-27 Thread Sebastian Benoit
Hi,

there have been no floppy images since the 6.2 release. This removes mention
of boot floppies from the INSTALL.alpha notes. Maybe someone who knows
something about alpha machines can do a check?

comments or oks?

diff --git distrib/notes/alpha/contents distrib/notes/alpha/contents
index eccbc79af6c..bb3867a540b 100644
--- distrib/notes/alpha/contents
+++ distrib/notes/alpha/contents
@@ -5,35 +5,6 @@ OpenBSDminiroot
It can be copied to the swap partition of an existing
disk to allow installing or upgrading to OpenBSD OSREV.
 
-OpenBSDfloppy
-   This floppy image will boot on the following MACHINE
-   models:
-   - AlphaStation 200, 250, 255, 400
-   - AlphaServer 300 and 400
-   - AlphaStation 500, 600
-   - AlphaStation 600A, 1200
-   - AlphaServer 800, 1000, 1000A, 1200, 4000 and 4100
-   - AXPpci33 based machines, including ``Noname'',
- UDB, Multia
-   - EB164 based machines, including PC164, 164SX,
- and 164LX
-   - Personal Workstation (Miata)
-
-   floppyB{:--:}OSrev.fs   Another MACHINE boot and installation floppy; 
see
-   below.
-   This floppy image will boot on the following MACHINE
-   models:
-   - Alpha Processor, Inc. UP1000, UP1100, UP2000 and
- UP2000+
-   - XP900, XP1000, CS20, DS10, DS20, DS20L, ES40 and
- 264DP
-
-   floppyC{:--:}OSrev.fs   Another MACHINE boot and installation floppy; 
see
-   below.
-   This floppy image will boot on the following MACHINE
-   models:
-   - Tadpole AlphaBook
-
 OpenBSDdistsets
 
 OpenBSDbsd
@@ -57,8 +28,6 @@ OpenBSDcd
netboot.mop The OpenBSD/MACHINE network boot loader, for MOP
protocol.
 
-OpenBSDfloppydesc(three,Each,s)
-
 DistributionDescription(eight)
 
 OpenBSDbase(101660534,244170752)
diff --git distrib/notes/alpha/install distrib/notes/alpha/install
index 1c0033aafa9..3ab01311789 100644
--- distrib/notes/alpha/install
+++ distrib/notes/alpha/install
@@ -3,23 +3,9 @@ OpenBSDInstallPrelude
 
 There are several ways to install OpenBSD onto a disk.  The easiest way is
 to boot from the bootable CD-ROM mini image, then install from your favorite
-source. You can also use one of the OpenBSD installation floppies, if your
-machine has a floppy drive. Network booting is supported through means of
+source. Network booting is supported through means of
 dhcpd(8) and tftpd(8).
 
-Booting from Floppy Disk installation media:
-
-   At the SRM console prompt, enter
-   boot dva0
-   You should see info about the primary and secondary boot
-   and then the kernel should start to load.  It will take a
-   while to load the kernel from the floppy, most likely more
-   than a minute.  If some action doesn't eventually happen,
-   or the spinning cursor has stopped and nothing further has
-   happened, or the machine spontaneously reboots, then either
-   you have a bad boot floppy (in which case you should try
-   another) or your alpha is not currently supported by OpenBSD.
-
 Booting from CD-ROM installation media:
 
At the SRM console prompt, enter
@@ -36,12 +22,12 @@ Booting from CD-ROM installation media:
boot DEVICE
where DEVICE is the dka device name.
 
-   You should see info about the primary and secondary boot
-   and then the kernel should start to load.  If the kernel
-   fails to load or the spinning cursor has stopped and nothing
-   further has happened, you either have a hardware problem or
-   your MACHINE is not currently supported by OpenBSD; try booting
-   from a floppy instead if possible.
+   You should see info about the primary and secondary boot and then the
+   kernel should start to load.  If the kernel fails to load or the
+   spinning cursor has stopped and nothing further has happened, you
+   either have a hardware problem or your MACHINE is not currently
+   supported by OpenBSD; try booting from the network instead if
+   possible.
 
 Booting from Network:
 
@@ -124,7 +110,7 @@ Booting from Network:
Once loaded, the boot loader will mount /alpha over NFS and load
the kernel from there.
 
-Installing using the Floppy, CD-ROM or Network procedure:
+Installing using the CD-ROM or Network procedure:
 
 OpenBSDInstallPart2
 
diff --git distrib/notes/alpha/prep distrib/notes/alpha/prep
index 29fda76e463..4aadfc21f83 100644
--- distrib/notes/alpha/prep
+++ distrib/notes/alpha/prep
@@ -63,8 +63,7 @@ Using the SRM console:
 

Re: [PATCH] printf: Add %B conversion specifier for binary

2020-04-27 Thread Theo de Raadt
Ingo Schwarze  wrote:

> Alejandro Colomar wrote on Mon, Apr 27, 2020 at 08:26:38PM +0200:
> 
> > This patch adds a new feature to the ``printf`` family of functions:
> > ``%B`` conversion specifier for printing unsigned numbers in binary.
> 
> No.  We do not want non-standard extensions to standard functions
> unless they provide unusually important benefit and/or are very
> widely supported elsewhere.
> 
> > I also sent today a patch to add this specifier to glibc.  They are
> > concerned about adding a new non-standard specifier, but if more C libs
> > are going to add it at the same time, it may become a thing.
> 
> Usually, glibc is quite proactive about adding non-standard stuff
> even if it is not really useful.  If even they reject the idea,
> OpenBSD certainly isn't the place to start campaigning.

"campaigning" -- good word.

When non-standard APIs or additions are added, the usual reason is
that a very common operation, done by-hand each time, tend to be
highly error prone.

Printing in binary?  Not common.  And I doubt it is error prone, either.



Re: [PATCH] printf: Add %B conversion specifier for binary

2020-04-27 Thread Ingo Schwarze
Hi,

Alejandro Colomar wrote on Mon, Apr 27, 2020 at 08:26:38PM +0200:

> This patch adds a new feature to the ``printf`` family of functions:
> ``%B`` conversion specifier for printing unsigned numbers in binary.

No.  We do not want non-standard extensions to standard functions
unless they provide unusually important benefit and/or are very
widely supported elsewhere.

> I also sent today a patch to add this specifier to glibc.  They are
> concerned about adding a new non-standard specifier, but if more C libs
> are going to add it at the same time, it may become a thing.

Usually, glibc is quite proactive about adding non-standard stuff
even if it is not really useful.  If even they reject the idea,
OpenBSD certainly isn't the place to start campaigning.

Yours,
  Ingo



Re: [PATCH] printf: Add %B conversion specifier for binary

2020-04-27 Thread Theo de Raadt
Alejandro Colomar  wrote:

> I also sent today a patch to add this specifier to glibc.  They are
> concerned about adding a new non-standard specifier, but if more C libs
> are going to add it at the same time, it may become a thing.

Sorry, but I doubt any of this is going to happen.

The action this performs is trivialy done by hand the extremely rare
pieces of code which need it.



[PATCH] printf: Add %B conversion specifier for binary

2020-04-27 Thread Alejandro Colomar

Hi all,

This patch adds a new feature to the ``printf`` family of functions:

``%B`` conversion specifier for printing unsigned numbers in binary.

Behaviour is exactly as with ``%X``, only changing the base (16 -> 2).

``%b`` is already in use by some ``printf(1)`` implementations, so I
didn't use it for binary.  Anyway, binary doesn't have letters, so only
the ``0b``/``0B`` specifier would change.

I also documented the new specifier in the man pages.

Disclaimer: I couldn't test it myself, so test it before applying it.

I also sent today a patch to add this specifier to glibc.  They are
concerned about adding a new non-standard specifier, but if more C libs
are going to add it at the same time, it may become a thing.

Alex.


From 1a41d44571ccaf9ffaf36b2c2b96dd34e48eb5b7 Mon Sep 17 00:00:00 2001
From: Alejandro Colomar 
Date: Mon, 27 Apr 2020 19:15:55 +0200
Subject: [PATCH 1/2] printf: Add %B conversion specifier for printing binary

---
 lib/libc/stdio/vfprintf.c  | 28 
 lib/libc/stdio/vfwprintf.c | 28 
 2 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/lib/libc/stdio/vfprintf.c b/lib/libc/stdio/vfprintf.c
index 1d451a84f66..1e5cd3ad89b 100644
--- a/lib/libc/stdio/vfprintf.c
+++ b/lib/libc/stdio/vfprintf.c
@@ -310,9 +310,9 @@ __vfprintf(FILE *fp, const char *fmt0, __va_list ap)
char *dtoaresult = NULL;
 #endif

-   uintmax_t _umax;/* integer arguments %[diouxX] */
-   enum { OCT, DEC, HEX } base;/* base for %[diouxX] conversion */
-   int dprec;  /* a copy of prec if %[diouxX], 0 otherwise */
+   uintmax_t _umax;/* integer arguments %[BdiouxX] */
+   enum { BIN, OCT, DEC, HEX } base; /* base for %[BdiouxX] conversion */
+   int dprec;  /* a copy of prec if %[BdiouxX], 0 otherwise */
int realsz; /* field size expanded by dprec */
int size;   /* size of converted field or string */
const char *xdigs;  /* digits for %[xX] conversion */
@@ -320,7 +320,7 @@ __vfprintf(FILE *fp, const char *fmt0, __va_list ap)
struct __suio uio;  /* output information: summary */
struct __siov iov[NIOV];/* ... and individual io vectors */
char buf[BUF];  /* buffer with space for digits of uintmax_t */
-   char ox[2]; /* space for 0x; ox[1] is either x, X, or \0 */
+   char ox[2]; /* space for 0x; ox[1] is either x, X,B or \0 */
union arg *argtable;/* args, built due to positional arg */
union arg statargtable[STATIC_ARG_TBL_SIZE];
size_t argtablesiz;
@@ -891,6 +891,10 @@ fp_common:
_umax = UARG();
base = DEC;
goto nosign;
+   case 'B':
+   _umax = UARG();
+   base = BIN;
+   goto bin;
case 'X':
xdigs = xdigs_upper;
goto hex;
@@ -898,8 +902,8 @@ fp_common:
xdigs = xdigs_lower;
 hex:   _umax = UARG();
base = HEX;
-   /* leading 0x/X only if non-zero */
-   if (flags & ALT && _umax != 0)
+   /* leading 0x/X/B only if non-zero */
+bin:   if (flags & ALT && _umax != 0)
ox[1] = ch;

/* unsigned conversions */
@@ -925,6 +929,13 @@ number:if ((dprec = prec) >= 0)
 * a variable; hence this switch.
 */
switch (base) {
+   case BIN:
+   do {
+   *--cp = to_char(_umax & 1);
+   _umax >>= 1;
+   } while (_umax);
+   break;
+
case OCT:
do {
*--cp = to_char(_umax & 7);
@@ -980,7 +991,7 @@ number: if ((dprec = prec) >= 0)
 * first be prefixed by any sign or other prefix; otherwise,
 * it should be blank padded before the prefix is emitted.
 * After any left-hand padding and prefixing, emit zeroes
-* required by a decimal %[diouxX] precision, then print the
+* required by a decimal %[BdiouxX] precision, then print the
 * string proper, then emit zeroes required by any leftover
 * floating precision; finally, if LADJUST, pad with blanks.
 *
@@ -1000,7 +1011,7 @@ number:   if ((dprec = prec) >= 0)
/* prefix */
 

Re: drm(4) kqfilter & EVFILT_READ

2020-04-27 Thread Martin Pieuchot
On 28/04/20(Tue) 01:54, Jonathan Gray wrote:
> On Mon, Apr 27, 2020 at 04:52:33PM +0200, Martin Pieuchot wrote:
> > Diff below extends the existing drmkqfilter() to support EVFILT_READ.
> > This makes drm(4)'s kqueue support in pair with poll().
> > 
> > The event list queried in the filt_drmread() should be protected by the
> > `event_lock' mutex.  This could be done by using the `kdev' backpointer
> > as shown in comment.  However this would require some plumbing to make
> > use of `minor'.  The side effect of this approach would be to reduce the
> > diff with Linux.
> 
> You seem to be confusing kdev and dev in the drm_minor struct.
> at least in linux kdev is 'struct device *' and dev is
> 'struct drm_device *' (which has the event lock).
> I have a tree which uses the drm_minor struct but it is part of a much
> larger diff.

Thanks for pointing that out!  Do you see another way to comply to the
locking then?

> Could you explain what prompted this diff?

My on-going audit of the poll & kqueue interfaces to ensure they are
coherent.



net80211/athn/iwm/iwx: add nwflag nomimo

2020-04-27 Thread Stefan Sperling
We currently do not support 11n mode on devices which do not have all
antenna ports connected. So if e.g. an athn(4) card is installed into
an APU or Alix, we require that users plug pigtails into all antenna
connectors on the card, and mount a corresponding number of antennas
on the case. Given hardware restrictions this may not always be possible.

I happen to have an Alix case with only one hole for an antenna, and I am now
running a MIMO-capable AR9280 card on this board. Without this patch I can
either run the card in 11a/g mode, or I could screw a hole for another
antenna into the case. But 11n mode isn't a viable option; if either side
decides to send a MIMO data frame this frame will be lost.

There is no technical requirement for this restriction since 11n mode
can operate with just MCS 0-7. There are 11n devices which do not even
support MIMO and 11n mode already works fine with such devices.

However, on devices which support MIMO we would need a way to tell how
many antennas are physically connected in order to make a decision.

Automatically detecting dead antennas requires tedious driver-specific
heuristics, and in some cases isn't even a possibility since firmware may
restrict what the driver can see. E.g. I've been told by an iwlwifi developer
that current Intel firmware doesn't bother with detecting missing antennas.

What we can easily offer is another nwflag to disable MIMO at run-time.
With this set, drivers will operate their device as if it didn't support MIMO,
net80211 can stop announcing support for MIMO rates, and 11n mode works.

Tested on iwm(4) and athn(4), and works as expected for me.

ok?

(Apply this patch, run 'make includes', and recompile ifconfig and the kernel.)
 
diff 6608380519f922f07dba3821961f4d7330327041 
ad4a4e1aff0295006baf16de294666e4b1293b93
blob - f6efc3bc13e84ddb4e7821b23637e3404668caef
blob + c954658d613a805b0e003654429ff8ae68c3c6a7
--- sbin/ifconfig/ifconfig.8
+++ sbin/ifconfig/ifconfig.8
@@ -1053,6 +1053,13 @@ flag will disable the direct bridging of frames betwee
 nodes when operating in Host AP mode.
 Setting this flag will block and filter direct inter-station
 communications.
+.It nomimo
+The
+.Ql nomimo
+flag will disable MIMO reception and transmission even if the driver
+and wireless network device support MIMO.
+This flag can be used to work around packet loss in 11n mode if the
+wireless network device has unpopulated extra antenna connectors.
 .It stayauth
 The
 .Ql stayauth
blob - 8153923216917c1f8fdb77c3ac4de21feba10e1d
blob + 7d2d13f168c1d6eb7e50756c8956e0b05a3c8270
--- sys/dev/ic/athn.c
+++ sys/dev/ic/athn.c
@@ -180,6 +180,53 @@ struct cfdriver athn_cd = {
NULL, "athn", DV_IFNET
 };
 
+void
+athn_config_ht(struct athn_softc *sc)
+{
+   struct ieee80211com *ic = >sc_ic;
+   int i, ntxstreams, nrxstreams;
+
+   if ((sc->flags & ATHN_FLAG_11N) == 0)
+   return;
+
+   /* Set HT capabilities. */
+   ic->ic_htcaps = (IEEE80211_HTCAP_SMPS_DIS <<
+   IEEE80211_HTCAP_SMPS_SHIFT);
+#ifdef notyet
+   ic->ic_htcaps |= IEEE80211_HTCAP_CBW20_40 |
+   IEEE80211_HTCAP_SGI40 |
+   IEEE80211_HTCAP_DSSSCCK40;
+#endif
+   ic->ic_htxcaps = 0;
+#ifdef notyet
+   if (AR_SREV_9271(sc) || AR_SREV_9287_10_OR_LATER(sc))
+   ic->ic_htcaps |= IEEE80211_HTCAP_SGI20;
+   if (AR_SREV_9380_10_OR_LATER(sc))
+   ic->ic_htcaps |= IEEE80211_HTCAP_LDPC;
+   if (AR_SREV_9280_10_OR_LATER(sc)) {
+   ic->ic_htcaps |= IEEE80211_HTCAP_TXSTBC;
+   ic->ic_htcaps |= 1 << IEEE80211_HTCAP_RXSTBC_SHIFT;
+   }
+#endif
+   ntxstreams = sc->ntxchains;
+   nrxstreams = sc->nrxchains;
+   if (!AR_SREV_9380_10_OR_LATER(sc)) {
+   ntxstreams = MIN(ntxstreams, 2);
+   nrxstreams = MIN(nrxstreams, 2);
+   }
+   /* Set supported HT rates. */
+   if (ic->ic_userflags & IEEE80211_F_NOMIMO)
+   ntxstreams = nrxstreams = 1;
+   memset(ic->ic_sup_mcs, 0, sizeof(ic->ic_sup_mcs));
+   for (i = 0; i < nrxstreams; i++)
+   ic->ic_sup_mcs[i] = 0xff;
+   ic->ic_tx_mcs_set = IEEE80211_TX_MCS_SET_DEFINED;
+   if (ntxstreams != nrxstreams) {
+   ic->ic_tx_mcs_set |= IEEE80211_TX_RX_MCS_NOT_EQUAL;
+   ic->ic_tx_mcs_set |= (ntxstreams - 1) << 2;
+   }
+}
+
 int
 athn_attach(struct athn_softc *sc)
 {
@@ -302,44 +349,8 @@ athn_attach(struct athn_softc *sc)
IEEE80211_C_SHPREAMBLE |/* Short preamble supported. */
IEEE80211_C_PMGT;   /* Power saving supported. */
 
-   if (sc->flags & ATHN_FLAG_11N) {
-   int i, ntxstreams, nrxstreams;
+   athn_config_ht(sc);
 
-   /* Set HT capabilities. */
-   ic->ic_htcaps = (IEEE80211_HTCAP_SMPS_DIS <<
-   IEEE80211_HTCAP_SMPS_SHIFT);
-#ifdef notyet
-   ic->ic_htcaps |= IEEE80211_HTCAP_CBW20_40 |
-   

Re: drm(4) kqfilter & EVFILT_READ

2020-04-27 Thread Jonathan Gray
On Mon, Apr 27, 2020 at 04:52:33PM +0200, Martin Pieuchot wrote:
> Diff below extends the existing drmkqfilter() to support EVFILT_READ.
> This makes drm(4)'s kqueue support in pair with poll().
> 
> The event list queried in the filt_drmread() should be protected by the
> `event_lock' mutex.  This could be done by using the `kdev' backpointer
> as shown in comment.  However this would require some plumbing to make
> use of `minor'.  The side effect of this approach would be to reduce the
> diff with Linux.

You seem to be confusing kdev and dev in the drm_minor struct.
at least in linux kdev is 'struct device *' and dev is
'struct drm_device *' (which has the event lock).
I have a tree which uses the drm_minor struct but it is part of a much
larger diff.

Could you explain what prompted this diff?

> 
> I'd be interested to hear if somebody sees a different approach.
> 
> That said, as long as the KERNEL_LOCK() is taken in all these code paths
> it should be safe to commit the filter as it is.
> 
> Comments, Oks?
> 
> Index: sys/conf.h
> ===
> RCS file: /cvs/src/sys/sys/conf.h,v
> retrieving revision 1.150
> diff -u -p -r1.150 conf.h
> --- sys/conf.h21 Apr 2020 08:29:27 -  1.150
> +++ sys/conf.h27 Apr 2020 14:43:48 -
> @@ -439,7 +439,7 @@ extern struct cdevsw cdevsw[];
>   (dev_type_stop((*))) enodev, 0, selfalse, \
>   (dev_type_mmap((*))) enodev }
>  
> -/* open, close, read, ioctl, poll, mmap, nokqfilter */
> +/* open, close, read, ioctl, poll, mmap, kqfilter */
>  #define  cdev_drm_init(c,n){ \
>   dev_init(c,n,open), dev_init(c,n,close), dev_init(c, n, read), \
>   (dev_type_write((*))) enodev, dev_init(c,n,ioctl), \
> Index: dev/pci/drm/drm_drv.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/drm_drv.c,v
> retrieving revision 1.174
> diff -u -p -r1.174 drm_drv.c
> --- dev/pci/drm/drm_drv.c 7 Apr 2020 13:27:51 -   1.174
> +++ dev/pci/drm/drm_drv.c 27 Apr 2020 14:43:48 -
> @@ -484,6 +484,30 @@ filt_drmkms(struct knote *kn, long hint)
>   return (kn->kn_fflags != 0);
>  }
>  
> +void
> +filt_drmreaddetach(struct knote *kn)
> +{
> + struct drm_file *file_priv = kn->kn_hook;
> + int s;
> +
> + s = spltty();
> + klist_remove(_priv->rsel.si_note, kn);
> + splx(s);
> +}
> +
> +int
> +filt_drmread(struct knote *kn, long hint)
> +{
> + struct drm_file *file_priv = kn->kn_hook;
> + int  val = 0;
> +
> + //mtx_enter(_priv->minor->kdev->event_lock);
> + val = !list_empty(_priv->event_list);
> + //mtx_leave(_priv->minor->kdev->event_lock);
> +
> + return (val);
> +}
> +
>  const struct filterops drm_filtops = {
>   .f_flags= FILTEROP_ISFD,
>   .f_attach   = NULL,
> @@ -491,30 +515,51 @@ const struct filterops drm_filtops = {
>   .f_event= filt_drmkms,
>  };
>  
> +const struct filterops drmread_filtops = {
> + .f_flags= FILTEROP_ISFD,
> + .f_attach   = NULL,
> + .f_detach   = filt_drmreaddetach,
> + .f_event= filt_drmread,
> +};
> +
>  int
>  drmkqfilter(dev_t kdev, struct knote *kn)
>  {
>   struct drm_device   *dev = NULL;
> - int s;
> + struct drm_file *file_priv = NULL;
> + int  s;
>  
>   dev = drm_get_device_from_kdev(kdev);
>   if (dev == NULL || dev->dev_private == NULL)
>   return (ENXIO);
>  
>   switch (kn->kn_filter) {
> + case EVFILT_READ:
> + mutex_lock(>struct_mutex);
> + file_priv = drm_find_file_by_minor(dev, minor(kdev));
> + mutex_unlock(>struct_mutex);
> + if (file_priv == NULL)
> + return (ENXIO);
> +
> + kn->kn_fop = _filtops;
> + kn->kn_hook = file_priv;
> +
> + s = spltty();
> + klist_insert(_priv->rsel.si_note, kn);
> + splx(s);
> + break;
>   case EVFILT_DEVICE:
>   kn->kn_fop = _filtops;
> + kn->kn_hook = dev;
> +
> + s = spltty();
> + klist_insert(>note, kn);
> + splx(s);
>   break;
>   default:
>   return (EINVAL);
>   }
>  
> - kn->kn_hook = dev;
> -
> - s = spltty();
> - klist_insert(>note, kn);
> - splx(s);
> -
>   return (0);
>  }
>  
> @@ -772,7 +817,6 @@ out:
>   return (gotone);
>  }
>  
> -/* XXX kqfilter ... */
>  int
>  drmpoll(dev_t kdev, int events, struct proc *p)
>  {
> 
> 



Re: IPv6 Support for umb(4)

2020-04-27 Thread Gerhard Roth

On 4/27/20 4:53 PM, Theo de Raadt wrote:

Gerhard Roth  wrote:


Hi Theo,

On 4/27/20 4:39 PM, Theo de Raadt wrote:

Is this code in umb_decode_ip_configuration() reached again, if
you do a late ifconfig (don't set inet6 at up time, but set it
later)


no, seting inet6 later doesn't work. On MBIM level I have to tell the
device *before* the CONNECT whether I want IPv6 support or not. And
there is no way to change that selection later on while we are
connected.

The only way to do this transparently would be an implicit disconnect
followed by a reconnect in if_umb.c. But then I would need some trigger
that is called every time someone does 'ifconfig umb0 inet6 eui64' or
'ifconfig umb0 -inet6'. And I'm not sure whether the implicit
temporary link loss is appreciated by the user.


Then this diff seems incorrect.

The alternative of disconnecting, is not nice.

Perhaps umb can always request inet6 at startup, but not actually expose
it to the network stack.  Then enabling the software inet6 flag can expose
inet6 to the network layer, disabling the inet6 flag can remove exposure
to the network layer, but the actual address and support is always attempted
by the driver?




That would work. Will try this.



Re: IPv6 Support for umb(4)

2020-04-27 Thread Theo de Raadt
Gerhard Roth  wrote:

> Hi Theo,
> 
> On 4/27/20 4:39 PM, Theo de Raadt wrote:
> > Is this code in umb_decode_ip_configuration() reached again, if
> > you do a late ifconfig (don't set inet6 at up time, but set it
> > later)
> 
> no, seting inet6 later doesn't work. On MBIM level I have to tell the
> device *before* the CONNECT whether I want IPv6 support or not. And
> there is no way to change that selection later on while we are
> connected.
> 
> The only way to do this transparently would be an implicit disconnect
> followed by a reconnect in if_umb.c. But then I would need some trigger
> that is called every time someone does 'ifconfig umb0 inet6 eui64' or
> 'ifconfig umb0 -inet6'. And I'm not sure whether the implicit
> temporary link loss is appreciated by the user.

Then this diff seems incorrect.

The alternative of disconnecting, is not nice.

Perhaps umb can always request inet6 at startup, but not actually expose
it to the network stack.  Then enabling the software inet6 flag can expose
inet6 to the network layer, disabling the inet6 flag can remove exposure
to the network layer, but the actual address and support is always attempted
by the driver?



drm(4) kqfilter & EVFILT_READ

2020-04-27 Thread Martin Pieuchot
Diff below extends the existing drmkqfilter() to support EVFILT_READ.
This makes drm(4)'s kqueue support in pair with poll().

The event list queried in the filt_drmread() should be protected by the
`event_lock' mutex.  This could be done by using the `kdev' backpointer
as shown in comment.  However this would require some plumbing to make
use of `minor'.  The side effect of this approach would be to reduce the
diff with Linux.

I'd be interested to hear if somebody sees a different approach.

That said, as long as the KERNEL_LOCK() is taken in all these code paths
it should be safe to commit the filter as it is.

Comments, Oks?

Index: sys/conf.h
===
RCS file: /cvs/src/sys/sys/conf.h,v
retrieving revision 1.150
diff -u -p -r1.150 conf.h
--- sys/conf.h  21 Apr 2020 08:29:27 -  1.150
+++ sys/conf.h  27 Apr 2020 14:43:48 -
@@ -439,7 +439,7 @@ extern struct cdevsw cdevsw[];
(dev_type_stop((*))) enodev, 0, selfalse, \
(dev_type_mmap((*))) enodev }
 
-/* open, close, read, ioctl, poll, mmap, nokqfilter */
+/* open, close, read, ioctl, poll, mmap, kqfilter */
 #define  cdev_drm_init(c,n){ \
dev_init(c,n,open), dev_init(c,n,close), dev_init(c, n, read), \
(dev_type_write((*))) enodev, dev_init(c,n,ioctl), \
Index: dev/pci/drm/drm_drv.c
===
RCS file: /cvs/src/sys/dev/pci/drm/drm_drv.c,v
retrieving revision 1.174
diff -u -p -r1.174 drm_drv.c
--- dev/pci/drm/drm_drv.c   7 Apr 2020 13:27:51 -   1.174
+++ dev/pci/drm/drm_drv.c   27 Apr 2020 14:43:48 -
@@ -484,6 +484,30 @@ filt_drmkms(struct knote *kn, long hint)
return (kn->kn_fflags != 0);
 }
 
+void
+filt_drmreaddetach(struct knote *kn)
+{
+   struct drm_file *file_priv = kn->kn_hook;
+   int s;
+
+   s = spltty();
+   klist_remove(_priv->rsel.si_note, kn);
+   splx(s);
+}
+
+int
+filt_drmread(struct knote *kn, long hint)
+{
+   struct drm_file *file_priv = kn->kn_hook;
+   int  val = 0;
+
+   //mtx_enter(_priv->minor->kdev->event_lock);
+   val = !list_empty(_priv->event_list);
+   //mtx_leave(_priv->minor->kdev->event_lock);
+
+   return (val);
+}
+
 const struct filterops drm_filtops = {
.f_flags= FILTEROP_ISFD,
.f_attach   = NULL,
@@ -491,30 +515,51 @@ const struct filterops drm_filtops = {
.f_event= filt_drmkms,
 };
 
+const struct filterops drmread_filtops = {
+   .f_flags= FILTEROP_ISFD,
+   .f_attach   = NULL,
+   .f_detach   = filt_drmreaddetach,
+   .f_event= filt_drmread,
+};
+
 int
 drmkqfilter(dev_t kdev, struct knote *kn)
 {
struct drm_device   *dev = NULL;
-   int s;
+   struct drm_file *file_priv = NULL;
+   int  s;
 
dev = drm_get_device_from_kdev(kdev);
if (dev == NULL || dev->dev_private == NULL)
return (ENXIO);
 
switch (kn->kn_filter) {
+   case EVFILT_READ:
+   mutex_lock(>struct_mutex);
+   file_priv = drm_find_file_by_minor(dev, minor(kdev));
+   mutex_unlock(>struct_mutex);
+   if (file_priv == NULL)
+   return (ENXIO);
+
+   kn->kn_fop = _filtops;
+   kn->kn_hook = file_priv;
+
+   s = spltty();
+   klist_insert(_priv->rsel.si_note, kn);
+   splx(s);
+   break;
case EVFILT_DEVICE:
kn->kn_fop = _filtops;
+   kn->kn_hook = dev;
+
+   s = spltty();
+   klist_insert(>note, kn);
+   splx(s);
break;
default:
return (EINVAL);
}
 
-   kn->kn_hook = dev;
-
-   s = spltty();
-   klist_insert(>note, kn);
-   splx(s);
-
return (0);
 }
 
@@ -772,7 +817,6 @@ out:
return (gotone);
 }
 
-/* XXX kqfilter ... */
 int
 drmpoll(dev_t kdev, int events, struct proc *p)
 {



Re: IPv6 Support for umb(4)

2020-04-27 Thread Gerhard Roth

Hi Theo,

On 4/27/20 4:39 PM, Theo de Raadt wrote:

Is this code in umb_decode_ip_configuration() reached again, if
you do a late ifconfig (don't set inet6 at up time, but set it
later)


no, seting inet6 later doesn't work. On MBIM level I have to tell the 
device *before* the CONNECT whether I want IPv6 support or not. And 
there is no way to change that selection later on while we are connected.


The only way to do this transparently would be an implicit disconnect
followed by a reconnect in if_umb.c. But then I would need some trigger
that is called every time someone does 'ifconfig umb0 inet6 eui64' or 
'ifconfig umb0 -inet6'. And I'm not sure whether the implicit temporary 
link loss is appreciated by the user.


Gerhard




That is how other network interfaces work.  I'm trying to make
sure this behaviour isn't too weird (ie. requiring a down, then up).

Gerhard Roth  wrote:


And since IPv6 is now optional for umb(4), we can just skip
evaluation of the IPv6 part of the IP configuration, if it
wasn't enabled.

Gerhard


Index: sys/dev/usb/if_umb.c
===
RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
retrieving revision 1.33
diff -u -p -u -p -r1.33 if_umb.c
--- sys/dev/usb/if_umb.c27 Apr 2020 11:16:51 -  1.33
+++ sys/dev/usb/if_umb.c27 Apr 2020 13:56:09 -
@@ -1937,6 +1937,10 @@ tryv6:;
/*
 * IPv6 configuation
 */
+   if ((sc->sc_flags & UMBFLG_NO_INET6) ||
+   in6ifa_ifpforlinklocal(GET_IFP(sc), 0) == NULL)
+   goto done;
+
avail_v6 = letoh32(ic->ipv6_available);
if (avail_v6 == 0) {
if (ifp->if_flags & IFF_DEBUG)





Re: IPv6 Support for umb(4)

2020-04-27 Thread Theo de Raadt
Is this code in umb_decode_ip_configuration() reached again, if
you do a late ifconfig (don't set inet6 at up time, but set it
later)

That is how other network interfaces work.  I'm trying to make
sure this behaviour isn't too weird (ie. requiring a down, then up).

Gerhard Roth  wrote:

> And since IPv6 is now optional for umb(4), we can just skip
> evaluation of the IPv6 part of the IP configuration, if it
> wasn't enabled.
> 
> Gerhard
> 
> 
> Index: sys/dev/usb/if_umb.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
> retrieving revision 1.33
> diff -u -p -u -p -r1.33 if_umb.c
> --- sys/dev/usb/if_umb.c  27 Apr 2020 11:16:51 -  1.33
> +++ sys/dev/usb/if_umb.c  27 Apr 2020 13:56:09 -
> @@ -1937,6 +1937,10 @@ tryv6:;
>   /*
>* IPv6 configuation
>*/
> + if ((sc->sc_flags & UMBFLG_NO_INET6) ||
> + in6ifa_ifpforlinklocal(GET_IFP(sc), 0) == NULL)
> + goto done;
> +
>   avail_v6 = letoh32(ic->ipv6_available);
>   if (avail_v6 == 0) {
>   if (ifp->if_flags & IFF_DEBUG)
> 



Re: IPv6 Support for umb(4)

2020-04-27 Thread Gerhard Roth
And since IPv6 is now optional for umb(4), we can just skip
evaluation of the IPv6 part of the IP configuration, if it
wasn't enabled.

Gerhard


Index: sys/dev/usb/if_umb.c
===
RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
retrieving revision 1.33
diff -u -p -u -p -r1.33 if_umb.c
--- sys/dev/usb/if_umb.c27 Apr 2020 11:16:51 -  1.33
+++ sys/dev/usb/if_umb.c27 Apr 2020 13:56:09 -
@@ -1937,6 +1937,10 @@ tryv6:;
/*
 * IPv6 configuation
 */
+   if ((sc->sc_flags & UMBFLG_NO_INET6) ||
+   in6ifa_ifpforlinklocal(GET_IFP(sc), 0) == NULL)
+   goto done;
+
avail_v6 = letoh32(ic->ipv6_available);
if (avail_v6 == 0) {
if (ifp->if_flags & IFF_DEBUG)



Re: patch: Enable dock audio on Thinkpad dock (Thinkpad L460)

2020-04-27 Thread Abel Abraham Camarillo Ojeda
On Monday, April 27, 2020, Jan Klemkow  wrote:

> On Thu, Apr 16, 2020 at 05:59:44PM -0500, Abel Abraham Camarillo Ojeda
> wrote:
> > On Tuesday, February 11, 2020, Abel Abraham Camarillo Ojeda <
> acam...@verlet.org> wrote:
> > > On Wednesday, January 8, 2020, Abel Abraham Camarillo Ojeda <
> acam...@verlet.org> wrote:
> > >> On Mon, Dec 30, 2019 at 1:24 PM Abel Abraham Camarillo Ojeda <
> acam...@verlet.org> wrote:
> > >>
> > >>> The following enables audio via the dock station port in my
> > >>> thinkpad L460.  But, anyone knows if its possible to automatically
> > >>> disable the laptop speaker when I plug in the audio port in the
> > >>> dock? it doesn't appear to have a *_sense, ideas?
> > >>>
> > >>> this also enables the annoying beep (echo -e "\a"; in console)
> > >>>
> > >>> patch inline and attached:
> > >> Hi, comments, oks?
> > > Anyone?
> > Hi, any ok, comments?
>
> Hi Abel,
>
> Thanks for your diff.  Please add an applicable diff inline in your mail
> next time and don't attach it.


Thanks Jan, I attached and inlined the diff because
gmail and diff mangling ...

Thanks for review


>
> I can't test the diff with the right hardware.  But, the patch applies,
> builds and doesn't break audio on my ThinkPad X1C6.  The diff also looks
> fine for me.
>
> bye,
> Jan
>
> Index: azalia_codec.c
> ===
> RCS file: /cvs/src/sys/dev/pci/azalia_codec.c,v
> retrieving revision 1.178
> diff -u -p -r1.178 azalia_codec.c
> --- azalia_codec.c  14 Oct 2019 02:04:35 -  1.178
> +++ azalia_codec.c  27 Apr 2020 07:42:45 -
> @@ -159,6 +159,17 @@ azalia_codec_init_vtbl(codec_t *this)
> this->subid == 0x503c17aa)
> this->qrks |= AZ_QRK_WID_TPDOCK2;
> break;
> +   case 0x10ec0293:
> +   this->name = "Realtek ALC293";
> +   this->qrks |= AZ_QRK_WID_CDIN_1C | AZ_QRK_WID_BEEP_1D;
> +
> +   /*
> +* Enable dock audio on Thinkpad docks
> +* 0x17aa : 0x5051 = Thinkpad L460
> +*/
> +   if (this->subid == 0x505117aa)
> +   this->qrks |= AZ_QRK_WID_TPDOCK2;
> +   break;
> case 0x10ec0298:
> this->name = "Realtek ALC298";
> if (this->subid == 0x320019e5 ||
>


Re: Raspberry Pi GPIO

2020-04-27 Thread Mark Kettenis
> Date: Mon, 27 Apr 2020 12:33:24 +0100
> From: Stuart Henderson 
> 
> On 2020/04/26 12:56, Mark Kettenis wrote:
> > Diff below adds GPIO support to bcmgpio(4).  It also adds the bits to
> > attach gpio(4) such that GPIO pins can be controlled from userland.
> > This makes sense on boards like the Raspberry Pi and the
> > implementation makes sure that pins used by kernel drivers can't be
> > touched.
> > 
> > ok?
> 
> Only tested with inputs so far, but works for me. OK sthen@
> 

If you put:

  /usr/sbin/gpioctl gpio0 42 set out led0

in /etc/rc.securelevel you can turn on the green LED with:

  gpioctl gpio0 led0 on

Cheers,

Mark



Re: Raspberry Pi GPIO

2020-04-27 Thread Stuart Henderson
On 2020/04/26 12:56, Mark Kettenis wrote:
> Diff below adds GPIO support to bcmgpio(4).  It also adds the bits to
> attach gpio(4) such that GPIO pins can be controlled from userland.
> This makes sense on boards like the Raspberry Pi and the
> implementation makes sure that pins used by kernel drivers can't be
> touched.
> 
> ok?

Only tested with inputs so far, but works for me. OK sthen@



Re: IPv6 Support for umb(4)

2020-04-27 Thread Gerhard Roth
Hello Claudio,

On Mon, 27 Apr 2020 11:51:50 +0200 Claudio Jeker  
wrote:
> On Mon, Apr 27, 2020 at 10:26:01AM +0200, Gerhard Roth wrote:
> > Should we change umb(4) so that it only grabs an IPv6 address
> > in case somebody does a "ifconfig umb0 inet6 eui64" first?
> > 
> > Anyone willing to ok the patch below?  
> 
> see below
>  
> > On 2/19/20 9:19 AM, Gerhard Roth wrote:  
> > > On Wed, 19 Feb 2020 08:45:39 +0100 Claudio Jeker 
> > >  wrote:  
> > > > On Tue, Feb 18, 2020 at 11:16:54PM +, Stuart Henderson wrote:  
> > > > > On 2020/02/18 13:40, Gerhard Roth wrote:  
> > > > > > > > Yes, I tried MBIM_CONTEXT_IPTYPE_IPV4ANDV6 myself first but to 
> > > > > > > > no
> > > > > > > > avail. The switched to MBIM_CONTEXT_IPTYPE_IPV4V6 and everything
> > > > > > > > was fine.  
> > > > > > > 
> > > > > > > Obviously it needs to switch based on INET6, but with the current
> > > > > > > mechanism used for handling v6 in OpenBSD, shouldn't it disable v6
> > > > > > > unless the interface has a link-local configured? (So the usual 
> > > > > > > method
> > > > > > > to enable v6 and bring an interface up would be "inet6 eui64" and 
> > > > > > > "up").
> > > > > > > That is how pppoe works.  
> > > > > > 
> > > > > > 
> > > > > > Hi Stuart,
> > > > > > 
> > > > > > you mean like that?  
> > > > > 
> > > > > Yes, that looks right - sorry I don't have a working umb to test 
> > > > > though!  
> > > > 
> > > > I guess we should then also adjust the manpage to make sure people know
> > > > how to enable IPv6 in hostname.umb0  
> > > 
> > > 
> > > Took a look at pppoe.4 and tried to extract what is needed for umb.4.
> > > Updated diff below.
> > > 
> > > Gerhard
> > > 
> > > 
> > > Index: share/man/man4/umb.4
> > > ===
> > > RCS file: /cvs/src/share/man/man4/umb.4,v
> > > retrieving revision 1.10
> > > diff -u -p -u -p -r1.10 umb.4
> > > --- share/man/man4/umb.4  18 Feb 2020 08:09:37 -  1.10
> > > +++ share/man/man4/umb.4  19 Feb 2020 08:14:01 -
> > > @@ -40,6 +40,19 @@ will remain in this state until the MBIM
> > >   In case the device is connected to an "always-on" USB port,
> > >   it may be possible to connect to a provider without entering the
> > >   PIN again even if the system was rebooted.
> > > +.Pp
> > > +To use IPv6, configure a link-local address before bringing
> > > +the interface up.
> > > +Some devices require the
> > > +.Sy AUTOCONF6
> > > +flag on the interface.  
> 
> I think I asked this already, how does one know if autoconf is needed or
> not. Is that only device specific or also provider dependent?
> Adding autoconf will enable slaacd(8) on the device. What kind of troubles
> could result from this on a device that do not need it?
> In general this paragraph does not really help me to understand the
> situation better.

Basically, the MBIM protocol allows us to obtain the IPv6
address the ISP has given to us with the MBIM_CID_IP_CONFIGURATION
query. And all the provider I've met have done so, so far.

Yet job@ had a Japanese ISP that didn't give any IP address
via the MBIM protocol but wanted us to use regular IPv6
protocols to obtain the IP address.

Now this is a little bit too much detail for a man page.
And there is no technical way to find out what is required apart
from trial and error.

So I rephrased the sentence to:

If the device is able to connect to the ISP's network
but doesn't show an IPv6 address, setting the
AUTOCONF6 on the interface before bringing it up may help.

(see updated diff below)
Does that sound better?

> 
> > > +.Pp
> > > +A typical
> > > +.Pa /etc/hostname.umb0
> > > +looks like this:
> > > +.Bd -literal -offset indent
> > > +pin 1234 apn ISP-APN-Name inet6 eui64 autoconf -roaming up
> > > +.Ed  
> 
> In my opinion this is a bad example, it mixes a lot of different options
> which are not explained to the user in that man page. Also I think it is
> bad practice to put everything on one line. ifconfig(8) is rather
> sensitive when it comes to multiple commands in a single invocation.
> If I look at this and compare it with my hostname.umb0 file
> pin 
> up
> I have more questions, what is apn and why would I need, where do I find
> what I need to put there. ifconfig umb0 does not show anything named apn.

The Access Point Name (APN) shows up in ifconfig as soon as you
set one. Explaining the meaning of the APN is IMHO beyond the
scope of a section 4 man page. For example, we don't explain
the meaning of SSID for WiFi, either.


> Also why is -roaming after inet6 eui64 autoconf? Isn't -roaming default?
> 
> Ideally the first paragraph explains the IPv6 setup well enough that no
> example config is needed here.

Ok, removed the example.


> 
> > >   .Sh HARDWARE
> > >   The following devices should work:
> > >   .Pp
> > > Index: sys/dev/usb/if_umb.c
> > > ===
> > > RCS file: 

Re: maxrtc(4)

2020-04-27 Thread Mark Kettenis
> Date: Sat, 25 Apr 2020 20:20:37 +0200 (CEST)
> From: Mark Kettenis 
> 
> The cvs log tells me this driver was written to privide an alternative
> clock for the Sun Fire V210.  That is probably why it prints a message
> about overwriting the rtc handler.  But the driver was never enabled
> on sparc64 (or anywhere else for that matter) and overwriting the rtc
> is normal behaviour on arm64.  So this diff removes that message.
> 
> The official device tree binding name for this device is
> "dallas,ds1307" so check for that in the match function.
> 
> ok?

ping?

> Index: dev/i2c/ds1307.c
> ===
> RCS file: /cvs/src/sys/dev/i2c/ds1307.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 ds1307.c
> --- dev/i2c/ds1307.c  20 Jun 2016 13:42:42 -  1.1
> +++ dev/i2c/ds1307.c  25 Apr 2020 18:05:37 -
> @@ -96,7 +96,8 @@ maxrtc_match(struct device *parent, void
>  {
>   struct i2c_attach_args *ia = arg;
>  
> - if (strcmp(ia->ia_name, "ds1307") == 0)
> + if (strcmp(ia->ia_name, "dallas,ds1307") == 0 ||
> + strcmp(ia->ia_name, "ds1307") == 0)
>   return (1);
>  
>   return (0);
> @@ -123,12 +124,6 @@ maxrtc_attach(struct device *parent, str
>   if (maxrtc_set_24h_mode(sc) == -1)
>   return;
>  
> - /* register our time handlers */
> - if (todr_handle != NULL) {
> - printf("%s: overwriting existing rtc handler\n",
> - sc->sc_dev.dv_xname);
> - }
> - /* XXX just overwrite existing rtc handler? */
>   todr_handle = >sc_todr;
>  }
>  
> 
> 



Re: pcfrtc(4) fix

2020-04-27 Thread Mark Kettenis
> Date: Sat, 25 Apr 2020 00:41:38 +0200 (CEST)
> From: Mark Kettenis 
> 
> The chip will set the OSF flag whenever the internal oscillator stops
> running.  That happens for example wen the battry runs out of juice.
> The idea as that we can check this flag to decide whether we should
> trust the time in the chip.  The driver tries to implement that but
> fails because it always clears the flag in the attach function.
> 
> Diff below fixes that by leaving the flag alone in the attach
> function, but clearing it when we set the time.
> 
> ok?

ping?

> Index: dev/i2c/pcf8523.c
> ===
> RCS file: /cvs/src/sys/dev/i2c/pcf8523.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 pcf8523.c
> --- dev/i2c/pcf8523.c 20 May 2016 20:33:53 -  1.3
> +++ dev/i2c/pcf8523.c 24 Apr 2020 22:37:20 -
> @@ -165,13 +165,6 @@ pcfrtc_attach(struct device *parent, str
>   /* Report battery status. */
>   reg = pcfrtc_reg_read(sc, PCF8523_CONTROL3);
>   printf(": battery %s\n", (reg & PCF8523_CONTROL3_BLF) ? "low" : "ok");
> -
> - /* Clear OS flag.  */
> - reg = pcfrtc_reg_read(sc, PCF8523_SECONDS);
> - if (reg & PCF8523_SECONDS_OS) {
> - reg &= ~PCF8523_SECONDS_OS;
> - pcfrtc_reg_write(sc, PCF8523_SECONDS, reg);
> - }
>  }
>  
>  int
> @@ -194,11 +187,19 @@ pcfrtc_settime(struct todr_chip_handle *
>  {
>   struct pcfrtc_softc *sc = ch->cookie;
>   struct clock_ymdhms dt;
> + uint8_t reg;
>  
>   clock_secs_to_ymdhms(tv->tv_sec, );
> -
>   if (pcfrtc_clock_write(sc, ) == 0)
>   return (-1);
> +
> + /* Clear OS flag.  */
> + reg = pcfrtc_reg_read(sc, PCF8523_SECONDS);
> + if (reg & PCF8523_SECONDS_OS) {
> + reg &= ~PCF8523_SECONDS_OS;
> + pcfrtc_reg_write(sc, PCF8523_SECONDS, reg);
> + }
> +
>   return (0);
>  }
>  
> 
> 



Re: Simplify NET_LOCK() variations

2020-04-27 Thread Martin Pieuchot
On 16/04/20(Thu) 11:08, Alexandr Nedvedicky wrote:
> [...] 
> > @@ -356,7 +367,17 @@ taskq_thread(void *xtq)
> >  {
> > struct taskq *tq = xtq;
> > struct task work;
> > -   int last;
> > +   int last, i;
> > +
> > +   mtx_enter(>tq_mtx);
> > +   for (i = 0; i < tq->tq_nthreads; i++) {
> > +   if (tq->tq_threads[i] == NULL) {
> > +   tq->tq_threads[i] = curproc;
> > +   break;
> > +   }
> > +   }
> > +   KASSERT(i < tq->tq_nthreads);
> > +   mtx_leave(>tq_mtx);
> >  
> > if (ISSET(tq->tq_flags, TASKQ_MPSAFE))
> > KERNEL_UNLOCK();
> > @@ -381,4 +402,17 @@ taskq_thread(void *xtq)
> > wakeup_one(>tq_running);
> >  
> > kthread_exit(0);
> > +}
> 
> sorry if my question sounds stupid. The snippet above indicates
> the task queue threads stay around forever (once created), right?
> I'm asking, because I see no place where we do 'tq_threads[i] = NULL'.

That's a bug, updated diff below.

If there's a consensus that this is a way to move forward, it would make
sense to commit it after unlock. 

Index: kern/kern_task.c
===
RCS file: /cvs/src/sys/kern/kern_task.c,v
retrieving revision 1.27
diff -u -p -r1.27 kern_task.c
--- kern/kern_task.c19 Dec 2019 17:40:11 -  1.27
+++ kern/kern_task.c27 Apr 2020 10:11:17 -
@@ -47,6 +47,7 @@ struct taskq {
unsigned int tq_nthreads;
unsigned int tq_flags;
const char  *tq_name;
+   struct proc **tq_threads;
 
struct mutex tq_mtx;
struct task_list tq_worklist;
@@ -55,6 +56,7 @@ struct taskq {
 #endif
 };
 
+struct proc *systqproc;
 static const char taskq_sys_name[] = "systq";
 
 struct taskq taskq_sys = {
@@ -64,6 +66,7 @@ struct taskq taskq_sys = {
1,
0,
taskq_sys_name,
+   ,
MUTEX_INITIALIZER(IPL_HIGH),
TAILQ_HEAD_INITIALIZER(taskq_sys.tq_worklist),
 #ifdef WITNESS
@@ -74,6 +77,7 @@ struct taskq taskq_sys = {
 #endif
 };
 
+struct proc *systqmpproc;
 static const char taskq_sys_mp_name[] = "systqmp";
 
 struct taskq taskq_sys_mp = {
@@ -83,6 +87,7 @@ struct taskq taskq_sys_mp = {
1,
TASKQ_MPSAFE,
taskq_sys_mp_name,
+   ,
MUTEX_INITIALIZER(IPL_HIGH),
TAILQ_HEAD_INITIALIZER(taskq_sys_mp.tq_worklist),
 #ifdef WITNESS
@@ -129,6 +134,8 @@ taskq_create(const char *name, unsigned 
tq->tq_nthreads = nthreads;
tq->tq_name = name;
tq->tq_flags = flags;
+   tq->tq_threads = mallocarray(nthreads, sizeof(*tq->tq_threads),
+   M_DEVBUF, M_WAITOK|M_ZERO);
 
mtx_init_flags(>tq_mtx, ipl, name, 0);
TAILQ_INIT(>tq_worklist);
@@ -172,6 +179,8 @@ taskq_destroy(struct taskq *tq)
}
mtx_leave(>tq_mtx);
 
+   free(tq->tq_threads, M_DEVBUF,
+   tq->tq_nthreads * sizeof(*tq->tq_threads));
free(tq, M_DEVBUF, sizeof(*tq));
 }
 
@@ -186,6 +195,8 @@ taskq_create_thread(void *arg)
switch (tq->tq_state) {
case TQ_S_DESTROYED:
mtx_leave(>tq_mtx);
+   free(tq->tq_threads, M_DEVBUF,
+   tq->tq_nthreads * sizeof(*tq->tq_threads));
free(tq, M_DEVBUF, sizeof(*tq));
return;
 
@@ -356,7 +367,17 @@ taskq_thread(void *xtq)
 {
struct taskq *tq = xtq;
struct task work;
-   int last;
+   int last, i;
+
+   mtx_enter(>tq_mtx);
+   for (i = 0; i < tq->tq_nthreads; i++) {
+   if (tq->tq_threads[i] == NULL) {
+   tq->tq_threads[i] = curproc;
+   break;
+   }
+   }
+   KASSERT(i < tq->tq_nthreads);
+   mtx_leave(>tq_mtx);
 
if (ISSET(tq->tq_flags, TASKQ_MPSAFE))
KERNEL_UNLOCK();
@@ -372,6 +393,12 @@ taskq_thread(void *xtq)
 
mtx_enter(>tq_mtx);
last = (--tq->tq_running == 0);
+   for (i = 0; i < tq->tq_nthreads; i++) {
+   if (tq->tq_threads[i] == curproc) {
+   tq->tq_threads[i] = NULL;
+   break;
+   }
+   }
mtx_leave(>tq_mtx);
 
if (ISSET(tq->tq_flags, TASKQ_MPSAFE))
@@ -381,4 +408,17 @@ taskq_thread(void *xtq)
wakeup_one(>tq_running);
 
kthread_exit(0);
+}
+
+int
+in_taskq(struct taskq *tq)
+{
+   int i;
+
+   for (i = 0; i < tq->tq_nthreads; i++) {
+   if (curproc == tq->tq_threads[i])
+   return (1);
+   }
+
+   return (0);
 }
Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.603
diff -u -p -r1.603 if.c
--- net/if.c12 Apr 2020 07:04:03 -  1.603
+++ net/if.c27 Apr 2020 10:05:28 -
@@ -250,6 +250,47 @@ struct task if_input_task_locked = TASK_
 struct rwlock 

Re: IPv6 Support for umb(4)

2020-04-27 Thread Claudio Jeker
On Mon, Apr 27, 2020 at 10:26:01AM +0200, Gerhard Roth wrote:
> Should we change umb(4) so that it only grabs an IPv6 address
> in case somebody does a "ifconfig umb0 inet6 eui64" first?
> 
> Anyone willing to ok the patch below?

see below
 
> On 2/19/20 9:19 AM, Gerhard Roth wrote:
> > On Wed, 19 Feb 2020 08:45:39 +0100 Claudio Jeker  
> > wrote:
> > > On Tue, Feb 18, 2020 at 11:16:54PM +, Stuart Henderson wrote:
> > > > On 2020/02/18 13:40, Gerhard Roth wrote:
> > > > > > > Yes, I tried MBIM_CONTEXT_IPTYPE_IPV4ANDV6 myself first but to no
> > > > > > > avail. The switched to MBIM_CONTEXT_IPTYPE_IPV4V6 and everything
> > > > > > > was fine.
> > > > > > 
> > > > > > Obviously it needs to switch based on INET6, but with the current
> > > > > > mechanism used for handling v6 in OpenBSD, shouldn't it disable v6
> > > > > > unless the interface has a link-local configured? (So the usual 
> > > > > > method
> > > > > > to enable v6 and bring an interface up would be "inet6 eui64" and 
> > > > > > "up").
> > > > > > That is how pppoe works.
> > > > > 
> > > > > 
> > > > > Hi Stuart,
> > > > > 
> > > > > you mean like that?
> > > > 
> > > > Yes, that looks right - sorry I don't have a working umb to test though!
> > > 
> > > I guess we should then also adjust the manpage to make sure people know
> > > how to enable IPv6 in hostname.umb0
> > 
> > 
> > Took a look at pppoe.4 and tried to extract what is needed for umb.4.
> > Updated diff below.
> > 
> > Gerhard
> > 
> > 
> > Index: share/man/man4/umb.4
> > ===
> > RCS file: /cvs/src/share/man/man4/umb.4,v
> > retrieving revision 1.10
> > diff -u -p -u -p -r1.10 umb.4
> > --- share/man/man4/umb.418 Feb 2020 08:09:37 -  1.10
> > +++ share/man/man4/umb.419 Feb 2020 08:14:01 -
> > @@ -40,6 +40,19 @@ will remain in this state until the MBIM
> >   In case the device is connected to an "always-on" USB port,
> >   it may be possible to connect to a provider without entering the
> >   PIN again even if the system was rebooted.
> > +.Pp
> > +To use IPv6, configure a link-local address before bringing
> > +the interface up.
> > +Some devices require the
> > +.Sy AUTOCONF6
> > +flag on the interface.

I think I asked this already, how does one know if autoconf is needed or
not. Is that only device specific or also provider dependent?
Adding autoconf will enable slaacd(8) on the device. What kind of troubles
could result from this on a device that do not need it?
In general this paragraph does not really help me to understand the
situation better.

> > +.Pp
> > +A typical
> > +.Pa /etc/hostname.umb0
> > +looks like this:
> > +.Bd -literal -offset indent
> > +pin 1234 apn ISP-APN-Name inet6 eui64 autoconf -roaming up
> > +.Ed

In my opinion this is a bad example, it mixes a lot of different options
which are not explained to the user in that man page. Also I think it is
bad practice to put everything on one line. ifconfig(8) is rather
sensitive when it comes to multiple commands in a single invocation.
If I look at this and compare it with my hostname.umb0 file
pin 
up
I have more questions, what is apn and why would I need, where do I find
what I need to put there. ifconfig umb0 does not show anything named apn.
Also why is -roaming after inet6 eui64 autoconf? Isn't -roaming default?

Ideally the first paragraph explains the IPv6 setup well enough that no
example config is needed here.

> >   .Sh HARDWARE
> >   The following devices should work:
> >   .Pp
> > Index: sys/dev/usb/if_umb.c
> > ===
> > RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
> > retrieving revision 1.32
> > diff -u -p -u -p -r1.32 if_umb.c
> > --- sys/dev/usb/if_umb.c18 Feb 2020 08:09:37 -  1.32
> > +++ sys/dev/usb/if_umb.c18 Feb 2020 12:35:45 -
> > @@ -2583,7 +2583,8 @@ umb_send_connect(struct umb_softc *sc, i
> > c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4);
> >   #ifdef INET6
> > /* XXX FIXME: support IPv6-only mode, too */
> > -   if ((sc->sc_flags & UMBFLG_NO_INET6) == 0)
> > +   if ((sc->sc_flags & UMBFLG_NO_INET6) == 0 &&
> > +   in6ifa_ifpforlinklocal(GET_IFP(sc), 0) != NULL)
> > c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4V6);
> >   #endif
> > memcpy(c->context, umb_uuid_context_internet, sizeof (c->context));
> > 

The code diff is fine by me.

-- 
:wq Claudio



Re: patch: Enable dock audio on Thinkpad dock (Thinkpad L460)

2020-04-27 Thread Jan Klemkow
On Thu, Apr 16, 2020 at 05:59:44PM -0500, Abel Abraham Camarillo Ojeda wrote:
> On Tuesday, February 11, 2020, Abel Abraham Camarillo Ojeda 
>  wrote:
> > On Wednesday, January 8, 2020, Abel Abraham Camarillo Ojeda 
> >  wrote:
> >> On Mon, Dec 30, 2019 at 1:24 PM Abel Abraham Camarillo Ojeda 
> >>  wrote:
> >>
> >>> The following enables audio via the dock station port in my
> >>> thinkpad L460.  But, anyone knows if its possible to automatically
> >>> disable the laptop speaker when I plug in the audio port in the
> >>> dock? it doesn't appear to have a *_sense, ideas?
> >>>
> >>> this also enables the annoying beep (echo -e "\a"; in console)
> >>>
> >>> patch inline and attached:
> >> Hi, comments, oks?
> > Anyone?
> Hi, any ok, comments?

Hi Abel,

Thanks for your diff.  Please add an applicable diff inline in your mail
next time and don't attach it.

I can't test the diff with the right hardware.  But, the patch applies,
builds and doesn't break audio on my ThinkPad X1C6.  The diff also looks
fine for me.

bye,
Jan

Index: azalia_codec.c
===
RCS file: /cvs/src/sys/dev/pci/azalia_codec.c,v
retrieving revision 1.178
diff -u -p -r1.178 azalia_codec.c
--- azalia_codec.c  14 Oct 2019 02:04:35 -  1.178
+++ azalia_codec.c  27 Apr 2020 07:42:45 -
@@ -159,6 +159,17 @@ azalia_codec_init_vtbl(codec_t *this)
this->subid == 0x503c17aa)
this->qrks |= AZ_QRK_WID_TPDOCK2;
break;
+   case 0x10ec0293:
+   this->name = "Realtek ALC293";
+   this->qrks |= AZ_QRK_WID_CDIN_1C | AZ_QRK_WID_BEEP_1D;
+
+   /*
+* Enable dock audio on Thinkpad docks
+* 0x17aa : 0x5051 = Thinkpad L460
+*/
+   if (this->subid == 0x505117aa)
+   this->qrks |= AZ_QRK_WID_TPDOCK2;
+   break;
case 0x10ec0298:
this->name = "Realtek ALC298";
if (this->subid == 0x320019e5 ||



Re: diff: em(4) deadlock fix

2020-04-27 Thread Martin Pieuchot
On 21/04/20(Tue) 15:54, Jan Klemkow wrote:
> Hi,
> 
> The following diff fixes a deadlock in em(4) when system runs out of
> mbuf(9)s.
> 
> Tested with current on amd64 with:
> em0 at pci0 dev 25 function 0 "Intel I217-LM" rev 0x05: msi, mac 0x1e phy 
> 0xb, address d0:50:99:c1:67:94
> 
> When the system runs out of mbufs, the receive ring of the em-NIC also
> run out of mbufs.  If the driver puts an mbuf from the NICs receive ring
> with em_rxeof, it calls em_rxrefill to refill the ring.  If its not
> possible to refill the ring and the receive ring is empty, it sets a
> timer to retry refilling later till new mbufs are available.

So the current logic assumes that as long as the ring contains descriptors
em_rxrefill() failures aren't fatal and it is not necessary to schedule a
refill.  Why?

Unless we understand this it is difficult to reason about a fix.  Have
you looked at other drivers, is it a common pattern?

> In my case, the function em_rxeof is unable to pull the last one or two
> mbufs from the receive ring, because the descriptor status hasn't set
> the "descriptor done" flag (E1000_RXD_STAT_DD).  Thus, the ring has one
> or two remaining mbufs, and the timeout to put new ones in is never set.
> When new mbufs are available later, the em driver stays in that
> situation.

Are you saying there's always some descriptor on the ring?  Did you
investigate why?

> The diff always sets the timeout to put new mbufs in the receive ring,
> when it has lesser mbufs then the "low water mark".

This changes the logic explained above.  With it the fatal condition
changes from 0 to `lwm'.  Another possible fix would be to always
schedule a timeout as soon as em_rxrefill() fails.

I'm not advocating for a particular change, I'm suggesting we understand
the current logic and its implications.

This bugs reminds me of sthen@'s change to be able to stability use NFS
on a port machine.  I wonder if they have something in common.

> A side effect is: when the mbufs in use (alive) are below then the "low
> water mark", the "current water mark" raises up to value of the "high
> water mark".  When new mbufs are available the number of mbufs in use
> also raises to that value.  Because, the "current water mark" just sinks
> during net livelocks.
> 
> I'm sure this path is not the best solution.  Maybe someone else has a
> better idea to avoid that situation?!
> 
> Bye,
> Jan
> 
> Index: dev/pci/if_em.c
> ===
> RCS file: /cvs/src/sys/dev/pci/if_em.c,v
> retrieving revision 1.349
> diff -u -p -r1.349 if_em.c
> --- dev/pci/if_em.c   24 Mar 2020 09:30:06 -  1.349
> +++ dev/pci/if_em.c   21 Apr 2020 12:02:28 -
> @@ -2809,7 +2809,8 @@ em_rxrefill(void *arg)
>  
>   if (em_rxfill(que))
>   E1000_WRITE_REG(>hw, RDT(que->me), que->rx.sc_rx_desc_head);
> - else if (if_rxr_inuse(>rx.sc_rx_ring) == 0)
> + else if (if_rxr_inuse(>rx.sc_rx_ring) <
> + if_rxr_lwm(>rx.sc_rx_ring))
>   timeout_add(>rx_refill, 1);
>  }
>  
> Index: net/if_var.h
> ===
> RCS file: /cvs/src/sys/net/if_var.h,v
> retrieving revision 1.104
> diff -u -p -r1.104 if_var.h
> --- net/if_var.h  12 Apr 2020 07:02:43 -  1.104
> +++ net/if_var.h  21 Apr 2020 11:58:17 -
> @@ -389,6 +389,7 @@ u_int if_rxr_get(struct if_rxring *, u_i
>  #define if_rxr_put(_r, _c)   do { (_r)->rxr_alive -= (_c); } while (0)
>  #define if_rxr_inuse(_r) ((_r)->rxr_alive)
>  #define if_rxr_cwm(_r)   ((_r)->rxr_cwm)
> +#define if_rxr_lwm(_r)   ((_r)->rxr_lwm)
>  
>  int  if_rxr_info_ioctl(struct if_rxrinfo *, u_int, struct if_rxring_info *);
>  int  if_rxr_ioctl(struct if_rxrinfo *, const char *, u_int,
> 



Re: [patch] Check for -1 explicitly in getpeereid.c

2020-04-27 Thread Martin Vahlensieck
On Sun, Apr 26, 2020 at 03:30:51PM -0600, Theo de Raadt wrote:
> Patrick Wildt  wrote:
> 
> > I don't know userland very well, so I have a question.  In the middle of
> > 2019 there have been plenty of changes in regards to changing checks of
> > syscalls from < 0 to a more strict == -1, like this one in isakmpd:
> > 
> > 
> > revision 1.26
> > date: 2019/06/28 13:32:44;  author: deraadt;  state: Exp;  lines: +2 -2;  
> > commitid: JJ6Ck4WTrgUiEjJp;
> > When system calls indicate an error they return -1, not some arbitrary
> > value < 0.  errno is only updated in this case.  Change all (most?)
> > callers of syscalls to follow this better, and let's see if this strictness
> > helps us in the future.
> > 
> 
> I have about 4000 more changes like that, but I'm stuck with trying to
> push it further forward.  For various reasons, some of which can be
> guessed from this thread.
> 
> > getsockopt(), I think, is also a system call.  And the manpage indicates
> > that a failure is always -1, and not some arbitrary number:
> > 
> > RETURN VALUES
> >  Upon successful completion, the value 0 is returned; otherwise the
> >  value -1 is returned and the global variable errno is set to indicate 
> > the
> >  error.
> > 
> > What is the difference between the diff in this mail, and the changes
> > done in the middle of last year?
> 
> The difference is this is direct checking of the syscalls.
> 
> Versus checking at a higher layer of abstraction, or conversion of
> that result to something else.
> 
> Say you have an interface which returns precisely 0 and -1 for two conditions.
> Well then it has a large set of out-of-range values which (a) won't occur
> but (b) if they occur, how do you handle them?  At which layer?  
> 
> The range of numbers returned really express 3 conditions.  One which is
> impossible, yet if it happens, do you want to convert the impossible to
> success, or to failure?
> 
> In the recently supplied diff, a return value of 50 at the system call
> layer, is returned into a library returning 0 (success).  Furthermore, the
> diff itself proposes treating the out-of-range impossible as a success,
> and accesses memory which is very probably unintialized.
> 
> > getsockopt() isn't allowed to return
> > anything else but 0 and 1, right?  Though I guess the current check
> > (error != 0) is the one that also catches instances where getsockopt()
> > isn't behaving well, even though it shouldn't.  But then, with the -1
> > check, wouldn't we be catching more instances of syscalls misbehaving
> > if we checked for < -1?
> 
> Correct.  I hope you have reached the same indecision point as me.
> 
> I feel uncomfortable changing all checking-points to 3-way decision.
> And imagine what a modern compiler would do there...
> 

The way you put it is obvious how stupid my diff was.  I did not
understand that it does not call the libc wrapper.  Evidently there are
still some things going on I do not understand.

Sorry for all the inconveniences caused.

Best,

Martin



Re: IPv6 Support for umb(4)

2020-04-27 Thread Gerhard Roth

Should we change umb(4) so that it only grabs an IPv6 address
in case somebody does a "ifconfig umb0 inet6 eui64" first?

Anyone willing to ok the patch below?



On 2/19/20 9:19 AM, Gerhard Roth wrote:

On Wed, 19 Feb 2020 08:45:39 +0100 Claudio Jeker  
wrote:

On Tue, Feb 18, 2020 at 11:16:54PM +, Stuart Henderson wrote:

On 2020/02/18 13:40, Gerhard Roth wrote:

Yes, I tried MBIM_CONTEXT_IPTYPE_IPV4ANDV6 myself first but to no
avail. The switched to MBIM_CONTEXT_IPTYPE_IPV4V6 and everything
was fine.


Obviously it needs to switch based on INET6, but with the current
mechanism used for handling v6 in OpenBSD, shouldn't it disable v6
unless the interface has a link-local configured? (So the usual method
to enable v6 and bring an interface up would be "inet6 eui64" and "up").
That is how pppoe works.



Hi Stuart,

you mean like that?


Yes, that looks right - sorry I don't have a working umb to test though!


I guess we should then also adjust the manpage to make sure people know
how to enable IPv6 in hostname.umb0



Took a look at pppoe.4 and tried to extract what is needed for umb.4.
Updated diff below.

Gerhard


Index: share/man/man4/umb.4
===
RCS file: /cvs/src/share/man/man4/umb.4,v
retrieving revision 1.10
diff -u -p -u -p -r1.10 umb.4
--- share/man/man4/umb.418 Feb 2020 08:09:37 -  1.10
+++ share/man/man4/umb.419 Feb 2020 08:14:01 -
@@ -40,6 +40,19 @@ will remain in this state until the MBIM
  In case the device is connected to an "always-on" USB port,
  it may be possible to connect to a provider without entering the
  PIN again even if the system was rebooted.
+.Pp
+To use IPv6, configure a link-local address before bringing
+the interface up.
+Some devices require the
+.Sy AUTOCONF6
+flag on the interface.
+.Pp
+A typical
+.Pa /etc/hostname.umb0
+looks like this:
+.Bd -literal -offset indent
+pin 1234 apn ISP-APN-Name inet6 eui64 autoconf -roaming up
+.Ed
  .Sh HARDWARE
  The following devices should work:
  .Pp
Index: sys/dev/usb/if_umb.c
===
RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
retrieving revision 1.32
diff -u -p -u -p -r1.32 if_umb.c
--- sys/dev/usb/if_umb.c18 Feb 2020 08:09:37 -  1.32
+++ sys/dev/usb/if_umb.c18 Feb 2020 12:35:45 -
@@ -2583,7 +2583,8 @@ umb_send_connect(struct umb_softc *sc, i
c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4);
  #ifdef INET6
/* XXX FIXME: support IPv6-only mode, too */
-   if ((sc->sc_flags & UMBFLG_NO_INET6) == 0)
+   if ((sc->sc_flags & UMBFLG_NO_INET6) == 0 &&
+   in6ifa_ifpforlinklocal(GET_IFP(sc), 0) != NULL)
c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4V6);
  #endif
memcpy(c->context, umb_uuid_context_internet, sizeof (c->context));





Re: athn(4): configure Tx interrupt mitigation

2020-04-27 Thread Stefan Sperling
Anyone?

On Wed, Apr 22, 2020 at 07:28:02PM +0200, Stefan Sperling wrote:
> We currently configure interrupt mitigation for Rx, but not for Tx.
> 
> And there is also a global Tx/Rx interrupt limit which can be configured
> via the MIRT register. Setting this could prevent Tx/Rx interrupt storms.
> 
> This change doesn't really buy us anything during regular use of the
> device, but during my testing it didn't hurt either.
> 
> OK?
> 
> diff 9b00dd5006bdfc0cf9d71dac4defe47f2ccfda0d 
> c50b0f0215d7dd5773e0ee8623d8aa2c7d2107c6
> blob - 88784561c133b42b2b23dff2cea0075cf2ffdd27
> blob + b3532eb8263f678c80a44257fe4f55820458a18c
> --- sys/dev/ic/athn.c
> +++ sys/dev/ic/athn.c
> @@ -2428,6 +2428,12 @@ athn_hw_reset(struct athn_softc *sc, struct ieee80211_
>   /* Setup Rx interrupt mitigation. */
>   AR_WRITE(sc, AR_RIMT, SM(AR_RIMT_FIRST, 2000) | SM(AR_RIMT_LAST, 500));
>  
> + /* Setup Tx interrupt mitigation. */
> + AR_WRITE(sc, AR_TIMT, SM(AR_TIMT_FIRST, 2000) | SM(AR_TIMT_LAST, 500));
> +
> + /* Set maximum interrupt rate threshold (in micro seconds). */
> + AR_WRITE(sc, AR_MIRT, SM(AR_MIRT_RATE_THRES, 2000));
> +
>   ops->init_baseband(sc);
>  
>   if ((error = athn_init_calib(sc, c, extc)) != 0) {
> blob - 92b937ce3b3a0977a0f8548051863c39656a4862
> blob + e14c096b90b5350712548d245a9bfd919b66f23b
> --- sys/dev/ic/athnreg.h
> +++ sys/dev/ic/athnreg.h
> @@ -280,6 +280,10 @@
>  /* Bits for AR_IER. */
>  #define AR_IER_ENABLE0x0001
>  
> +/* Bits for AR_MIRT. */
> +#define AR_MIRT_RATE_THRES_M 0x
> +#define AR_MIRT_RATE_THRES_S 0 
> +
>  /* Bits for AR_TIMT. */
>  #define AR_TIMT_LAST_M   0x
>  #define AR_TIMT_LAST_S   0
> 
>