Re: tcp sync cache refcount improvement

2023-09-03 Thread Vitaliy Makkoveev
> On 3 Sep 2023, at 21:08, Alexander Bluhm  wrote:
> 
> Hi,
> 
> Avoid a useless increment and decrement of of the tcp syn cache
> refcount by unexpanding the SYN_CACHE_TIMER_ARM() macro in the timer
> callback.
> 
> ok?
> 

ok mvs

> bluhm
> 
> Index: netinet/tcp_input.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v
> retrieving revision 1.390
> diff -u -p -r1.390 tcp_input.c
> --- netinet/tcp_input.c   28 Aug 2023 14:50:01 -  1.390
> +++ netinet/tcp_input.c   3 Sep 2023 18:04:13 -
> @@ -3159,19 +3159,6 @@ syn_cache_put(struct syn_cache *sc)
>   pool_put(_cache_pool, sc);
> }
> 
> -/*
> - * We don't estimate RTT with SYNs, so each packet starts with the default
> - * RTT and each timer step has a fixed timeout value.
> - */
> -#define  SYN_CACHE_TIMER_ARM(sc) 
> \
> -do { \
> - TCPT_RANGESET((sc)->sc_rxtcur,  \
> - TCPTV_SRTTDFLT * tcp_backoff[(sc)->sc_rxtshift], TCPTV_MIN, \
> - TCPTV_REXMTMAX);\
> - if (timeout_add_msec(&(sc)->sc_timer, (sc)->sc_rxtcur)) \
> - refcnt_take(&(sc)->sc_refcnt);  \
> -} while (/*CONSTCOND*/0)
> -
> void
> syn_cache_init(void)
> {
> @@ -3300,11 +3287,17 @@ syn_cache_insert(struct syn_cache *sc, s
>   }
> 
>   /*
> -  * Initialize the entry's timer.
> +  * Initialize the entry's timer.  We don't estimate RTT
> +  * with SYNs, so each packet starts with the default RTT
> +  * and each timer step has a fixed timeout value.
>*/
>   sc->sc_rxttot = 0;
>   sc->sc_rxtshift = 0;
> - SYN_CACHE_TIMER_ARM(sc);
> + TCPT_RANGESET(sc->sc_rxtcur,
> + TCPTV_SRTTDFLT * tcp_backoff[sc->sc_rxtshift], TCPTV_MIN,
> + TCPTV_REXMTMAX);
> + if (timeout_add_msec(>sc_timer, sc->sc_rxtcur))
> + refcnt_take(>sc_refcnt);
> 
>   /* Link it from tcpcb entry */
>   refcnt_take(>sc_refcnt);
> @@ -3365,15 +3358,12 @@ syn_cache_timer(void *arg)
> 
>   /* Advance the timer back-off. */
>   sc->sc_rxtshift++;
> - SYN_CACHE_TIMER_ARM(sc);
> + TCPT_RANGESET(sc->sc_rxtcur,
> + TCPTV_SRTTDFLT * tcp_backoff[sc->sc_rxtshift], TCPTV_MIN,
> + TCPTV_REXMTMAX);
> + if (!timeout_add_msec(>sc_timer, sc->sc_rxtcur))
> + syn_cache_put(sc);
> 
> - /*
> -  * Decrement reference of this timer.  We know there is another timer
> -  * as we just added it.  So just deref, free is not necessary.
> -  */
> - lastref = refcnt_rele(>sc_refcnt);
> - KASSERT(lastref == 0);
> - (void)lastref;
>   NET_UNLOCK();
>   return;
> 
> 



Re: vmd/vmm: remove an ioctl from the vcpu hotpath, go brrr

2023-09-03 Thread Dave Voutila


Mischa  writes:

> Nice!! Thanx Dave!
>
> Running go brrr as we speak.
> Testing with someone who is running Debian.

Great. I'll plan on committing this tomorrow afternoon (4 Sep) my time
unless I hear of any issues.



tcp sync cache refcount improvement

2023-09-03 Thread Alexander Bluhm
Hi,

Avoid a useless increment and decrement of of the tcp syn cache
refcount by unexpanding the SYN_CACHE_TIMER_ARM() macro in the timer
callback.

ok?

bluhm

Index: netinet/tcp_input.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v
retrieving revision 1.390
diff -u -p -r1.390 tcp_input.c
--- netinet/tcp_input.c 28 Aug 2023 14:50:01 -  1.390
+++ netinet/tcp_input.c 3 Sep 2023 18:04:13 -
@@ -3159,19 +3159,6 @@ syn_cache_put(struct syn_cache *sc)
pool_put(_cache_pool, sc);
 }
 
-/*
- * We don't estimate RTT with SYNs, so each packet starts with the default
- * RTT and each timer step has a fixed timeout value.
- */
-#defineSYN_CACHE_TIMER_ARM(sc) 
\
-do {   \
-   TCPT_RANGESET((sc)->sc_rxtcur,  \
-   TCPTV_SRTTDFLT * tcp_backoff[(sc)->sc_rxtshift], TCPTV_MIN, \
-   TCPTV_REXMTMAX);\
-   if (timeout_add_msec(&(sc)->sc_timer, (sc)->sc_rxtcur)) \
-   refcnt_take(&(sc)->sc_refcnt);  \
-} while (/*CONSTCOND*/0)
-
 void
 syn_cache_init(void)
 {
@@ -3300,11 +3287,17 @@ syn_cache_insert(struct syn_cache *sc, s
}
 
/*
-* Initialize the entry's timer.
+* Initialize the entry's timer.  We don't estimate RTT
+* with SYNs, so each packet starts with the default RTT
+* and each timer step has a fixed timeout value.
 */
sc->sc_rxttot = 0;
sc->sc_rxtshift = 0;
-   SYN_CACHE_TIMER_ARM(sc);
+   TCPT_RANGESET(sc->sc_rxtcur,
+   TCPTV_SRTTDFLT * tcp_backoff[sc->sc_rxtshift], TCPTV_MIN,
+   TCPTV_REXMTMAX);
+   if (timeout_add_msec(>sc_timer, sc->sc_rxtcur))
+   refcnt_take(>sc_refcnt);
 
/* Link it from tcpcb entry */
refcnt_take(>sc_refcnt);
@@ -3365,15 +3358,12 @@ syn_cache_timer(void *arg)
 
/* Advance the timer back-off. */
sc->sc_rxtshift++;
-   SYN_CACHE_TIMER_ARM(sc);
+   TCPT_RANGESET(sc->sc_rxtcur,
+   TCPTV_SRTTDFLT * tcp_backoff[sc->sc_rxtshift], TCPTV_MIN,
+   TCPTV_REXMTMAX);
+   if (!timeout_add_msec(>sc_timer, sc->sc_rxtcur))
+   syn_cache_put(sc);
 
-   /*
-* Decrement reference of this timer.  We know there is another timer
-* as we just added it.  So just deref, free is not necessary.
-*/
-   lastref = refcnt_rele(>sc_refcnt);
-   KASSERT(lastref == 0);
-   (void)lastref;
NET_UNLOCK();
return;
 



Re: PATCH: remove "dead code" in make

2023-09-03 Thread Marc Espie
On Thu, Aug 31, 2023 at 08:59:53AM +0200, Marc Espie wrote:
> A long time ago, I tried to host our fork of make, in the hope it would get
> picked up by other systems.
> 
> Accordingly, some features were added to mimic netbsd's extensions, hidden
> behind FEATURES macros.
> 
> Turns out that, for better or for worse, FreeBSD decided to go with NetBSD's
> fork of bmake, so this is just dead weight that makes stuff more complex.
> 
> Accordingly, config.h is no longer really needed, as it shrinks to two
> little defines.
> 
> (the only part worth looking at is varmodifiers... this survived a full build
> of base and xenocara without any issue, along with quite a few ports)

Slightly updated version. I hadn't bothered to actually remove config.h
and thus missed an include.

Also: all remaining modifiers are now strictly either word_apply or apply,
none does both, so I killed the extra code and added an assert just in case.

I could use some okays.  Fairly safe.

Index: Makefile
===
RCS file: /cvs/src/usr.bin/make/Makefile,v
retrieving revision 1.64
diff -u -p -r1.64 Makefile
--- Makefile13 Jan 2020 15:41:53 -  1.64
+++ Makefile3 Sep 2023 16:44:43 -
@@ -6,8 +6,7 @@ HOSTCFLAGS+= -I${.OBJDIR} -I${.CURDIR}
 CDIAGFLAGS=-Wall -W -Wno-char-subscripts -Wstrict-prototypes -pedantic \
-Wmissing-prototypes -Wdeclaration-after-statement -std=c99
 
-CDEFS+=-DHAS_PATHS_H
-CDEFS+=-DHAS_EXTENDED_GETCWD
+CDEFS+=-DMAKE_BSIZE=256 -DDEFMAXJOBS=4
 #CDEFS+=-DHAS_STATS
 
 DPADD += ${LIBUTIL}
Index: arch.c
===
RCS file: /cvs/src/usr.bin/make/arch.c,v
retrieving revision 1.93
diff -u -p -r1.93 arch.c
--- arch.c  17 Feb 2023 17:59:36 -  1.93
+++ arch.c  3 Sep 2023 16:44:43 -
@@ -82,7 +82,6 @@
 #include 
 #include 
 #include 
-#include "config.h"
 #include "defines.h"
 #include "buf.h"
 #include "dir.h"
Index: buf.c
===
RCS file: /cvs/src/usr.bin/make/buf.c,v
retrieving revision 1.29
diff -u -p -r1.29 buf.c
--- buf.c   13 Jan 2020 13:54:44 -  1.29
+++ buf.c   3 Sep 2023 16:44:43 -
@@ -75,7 +75,6 @@
 #include 
 #include 
 #include 
-#include "config.h"
 #include "defines.h"
 #include "buf.h"
 #include "stats.h"
Index: cmd_exec.c
===
RCS file: /cvs/src/usr.bin/make/cmd_exec.c,v
retrieving revision 1.12
diff -u -p -r1.12 cmd_exec.c
--- cmd_exec.c  31 Aug 2023 06:53:28 -  1.12
+++ cmd_exec.c  3 Sep 2023 16:44:43 -
@@ -30,7 +30,6 @@
 #include 
 #include 
 #include 
-#include "config.h"
 #include "defines.h"
 #include "cmd_exec.h"
 #include "buf.h"
Index: compat.c
===
RCS file: /cvs/src/usr.bin/make/compat.c,v
retrieving revision 1.93
diff -u -p -r1.93 compat.c
--- compat.c26 Jan 2020 12:41:21 -  1.93
+++ compat.c3 Sep 2023 16:44:43 -
@@ -40,7 +40,6 @@
 #include 
 #include 
 #include 
-#include "config.h"
 #include "defines.h"
 #include "dir.h"
 #include "engine.h"
Index: cond.c
===
RCS file: /cvs/src/usr.bin/make/cond.c,v
retrieving revision 1.54
diff -u -p -r1.54 cond.c
--- cond.c  21 Dec 2019 15:29:25 -  1.54
+++ cond.c  3 Sep 2023 16:44:44 -
@@ -42,7 +42,6 @@
 #include 
 #include 
 #include 
-#include "config.h"
 #include "defines.h"
 #include "dir.h"
 #include "buf.h"
Index: config.h
===
RCS file: config.h
diff -N config.h
--- config.h5 Jan 2022 02:00:55 -   1.21
+++ /dev/null   1 Jan 1970 00:00:00 -
@@ -1,120 +0,0 @@
-#ifndef CONFIG_H
-#define CONFIG_H
-
-/* $OpenBSD: config.h,v 1.21 2022/01/05 02:00:55 jsg Exp $ */
-/* $NetBSD: config.h,v 1.7 1996/11/06 17:59:03 christos Exp $  */
-
-/*
- * Copyright (c) 1988, 1989, 1990 The Regents of the University of California.
- * Copyright (c) 1988, 1989 by Adam de Boor
- * Copyright (c) 1989 by Berkeley Softworks
- * All rights reserved.
- *
- * This code is derived from software contributed to Berkeley by
- * Adam de Boor.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *notice, this list of conditions and the following disclaimer in the
- *documentation and/or other materials provided with the distribution.
- * 3. Neither the name of the University nor the names of its contributors
- *may be used to endorse or promote products derived from this software
- *without 

Re: vmd/vmm: remove an ioctl from the vcpu hotpath, go brrr

2023-09-03 Thread Mischa

Nice!! Thanx Dave!

Running go brrr as we speak.
Testing with someone who is running Debian.

Mischa

On 2023-09-01 21:50, Dave Voutila wrote:

Now that my i8259 fix is in, it's safe to expand the testing pool for
this diff. (Without that fix, users would definitely hit the hung block
device issue testing this one.) Hoping that folks that run non-OpenBSD
guests or strange configurations can give it a spin.

This change removes an ioctl(2) call from the vcpu thread hot path in
vmd. Instead of making that syscall to toggle on/off a pending 
interrupt
flag on the vcpu object in vmm(4), it adds a flag into the 
vm_run_params

struct sent with the VMM_IOC_RUN ioctl. The in-kernel vcpu runloop can
now toggle the pending interrupt state prior to vm entry.

mbuhl@ and phessler@ have run this diff on their machines. Current
observations are reduced average network latency for guests.

My terse measurements using the following btrace script show some
promising changes in terms of reducing ioctl syscalls:

  /* VMM_IOC_INTR: 0x800c5606 -> 2148292102 */
  syscall:ioctl:entry
  /arg1 == 2148292102/
  {
@total[tid] = count();
@running[tid] = count();
  }
  interval:hz:1
  {
print(@running);
clear(@running);
  }

Measuring from boot of an OpenBSD guest to after the guest finishes
relinking (based on my manual observation of the libevent thread
settling down in syscall rate), I see a huge reduction in VMM_IOC_INTR
ioctls for a single guest:

## -current
@total[433237]: 1325100  # vcpu thread (!!)
@total[187073]: 80239# libevent thread

## with diff
@total[550347]: 42   # vcpu thread (!!)
@total[256550]: 86946# libevent thread

Most of the VMM_IOC_INTR ioctls on the vcpu threads come from seabios
and the bootloader prodding some of the emulated hardware, but even
after the bootloader you'll see ~10-20k/s of ioctl's on -current
vs. ~4-5k/s with the diff.

At steady-state, the vcpu thread no longer makes the VMM_IOC_INTR calls
at all and you should see the libevent thread calling it at a rate 
~100/s

(probably hardclock?). *Without* the diff, I see a steady 650/s rate on
the vcpu thread at idle. *With* the diff, it's 0/s at idle. :)

To test:
- rebuild & install new kernel
- copy/symlink vmmvar.h into /usr/include/machine/
- rebuild & re-install vmd & vmctl
- reboot

-dv


diffstat refs/heads/master refs/heads/vmm-vrp_intr_pending
 M  sys/arch/amd64/amd64/vmm_machdep.c  |  10+   0-
 M  sys/arch/amd64/include/vmmvar.h |   1+   0-
 M  usr.sbin/vmd/vm.c   |   2+  16-

3 files changed, 13 insertions(+), 16 deletions(-)

diff refs/heads/master refs/heads/vmm-vrp_intr_pending
commit - 8afcf90fb39e4a84606e93137c2b6c20f44312cb
commit + 10eeb8a0414ec927b6282473c50043a7027d6b41
blob - 24a376a8f3bc94bc4a4203fe66c5994594adff46
blob + e3b6d10a0ae78b12ec2f3296f708b42540ce798e
--- sys/arch/amd64/amd64/vmm_machdep.c
+++ sys/arch/amd64/amd64/vmm_machdep.c
@@ -3973,6 +3973,11 @@ vcpu_run_vmx(struct vcpu *vcpu, struct 
vm_run_params *

 */
irq = vrp->vrp_irq;

+   if (vrp->vrp_intr_pending)
+   vcpu->vc_intr = 1;
+   else
+   vcpu->vc_intr = 0;
+
if (vrp->vrp_continue) {
switch (vcpu->vc_gueststate.vg_exit_reason) {
case VMX_EXIT_IO:
@@ -6381,6 +6386,11 @@ vcpu_run_svm(struct vcpu *vcpu, struct 
vm_run_params *


irq = vrp->vrp_irq;

+   if (vrp->vrp_intr_pending)
+   vcpu->vc_intr = 1;
+   else
+   vcpu->vc_intr = 0;
+
/*
 * If we are returning from userspace (vmd) because we exited
 * last time, fix up any needed vcpu state first. Which state
blob - e9f8384cccfde33034d7ac9782610f93eb5dc640
blob + 88545b54b35dd60280ba87403e343db9463d7419
--- sys/arch/amd64/include/vmmvar.h
+++ sys/arch/amd64/include/vmmvar.h
@@ -456,6 +456,7 @@ struct vm_run_params {
uint32_tvrp_vcpu_id;
uint8_t vrp_continue;   /* Continuing from an exit */
uint16_tvrp_irq;/* IRQ to inject */
+   uint8_t vrp_intr_pending;   /* Additional intrs pending? */

/* Input/output parameter to VMM_IOC_RUN */
struct vm_exit  *vrp_exit;  /* updated exit data */
blob - 5f598bcc14af5115372d34a4176254d377aad91c
blob + 447fc219adadf945de2bf25d5335993c2abdc26f
--- usr.sbin/vmd/vm.c
+++ usr.sbin/vmd/vm.c
@@ -1610,22 +1610,8 @@ vcpu_run_loop(void *arg)
} else
vrp->vrp_irq = 0x;

-   /* Still more pending? */
-   if (i8259_is_pending()) {
-   /*
-* XXX can probably avoid ioctls here by providing intr
-* in vrp
-*/
-   if (vcpu_pic_intr(vrp->vrp_vm_id,
-   vrp->vrp_vcpu_id, 1)) {
-   fatal("can't set INTR");
-   }
-   } 

Re: malloc write after free error checking

2023-09-03 Thread Otto Moerbeek
On Sun, Sep 03, 2023 at 09:30:49AM +0200, Otto Moerbeek wrote:

> > You can also be set upon a wrong foot: if an out of bounds write on a
> > adjacent chunk happens and lands in (another) free chunk, upon
> > allocation of that free chunk it will be reported as a "write after
> > free" case. It might even be that an allocation that was never
> > allocated and never freed is still "write after free" because of
> > "close" out of bounds writes. 
> > 
> > I'm thinking how to change the error message to reflect that.

I was thinking to change the message to "write to unallocated chunk",
trying to get the message trough that it also applies to chunks never
allocated but written to, not only to chunks that were freed and then
written to.

-Otto



Re: Add Intel Optane P1600X to pcidevs

2023-09-03 Thread Andreas Bartelt

Hi,

forgot to include the corresponding dmesg output:
nvme1 at pci20 dev 0 function 0 "Intel P1600X" rev 0x00: msix, NVMe 1.1
nvme1: INTEL SSDPEK1A118GA, firmware U5110550, serial BTOC14120X9P118B

Could this please be committed?

Best regards
Andreas

On 8/20/23 14:00, Andreas Bartelt wrote:

Hi,

Intel Optane P1600X Series SSDs are not yet identified in CURRENT (e.g., 
https://www.intel.com/content/www/us/en/products/sku/211867/intel-optane-ssd-p1600x-series-118gb-m-2-80mm-pcie-3-0-x4-3d-xpoint/specifications.html ).


The following patch adds a suitable description (tested with 118GB model).

Index: src/sys/dev/pci/pcidevs
===
RCS file: /cvs/src/sys/dev/pci/pcidevs,v
retrieving revision 1.2045
diff -u -p -r1.2045 pcidevs
--- src/sys/dev/pci/pcidevs    9 Aug 2023 21:27:47 -    1.2045
+++ src/sys/dev/pci/pcidevs    20 Aug 2023 11:53:01 -
@@ -4566,6 +4566,7 @@ product INTEL WL_8265_1    0x24fd    Dual Ban
  product INTEL 82820_HB    0x2501    82820 Host
  product INTEL 82820_AGP    0x250f    82820 AGP
  product INTEL OPTANE    0x2522    Optane
+product INTEL P1600X    0x2525    P1600X
  product INTEL WL_9260_1    0x2526    Dual Band Wireless-AC 9260
  product INTEL 82850_HB    0x2530    82850 Host
  product INTEL 82860_HB    0x2531    82860 Host




Re: vmd/vmm: remove an ioctl from the vcpu hotpath, go brrr

2023-09-03 Thread Mike Larkin
On Fri, Sep 01, 2023 at 03:50:31PM -0400, Dave Voutila wrote:
> Now that my i8259 fix is in, it's safe to expand the testing pool for
> this diff. (Without that fix, users would definitely hit the hung block
> device issue testing this one.) Hoping that folks that run non-OpenBSD
> guests or strange configurations can give it a spin.
>
> This change removes an ioctl(2) call from the vcpu thread hot path in
> vmd. Instead of making that syscall to toggle on/off a pending interrupt
> flag on the vcpu object in vmm(4), it adds a flag into the vm_run_params
> struct sent with the VMM_IOC_RUN ioctl. The in-kernel vcpu runloop can
> now toggle the pending interrupt state prior to vm entry.
>
> mbuhl@ and phessler@ have run this diff on their machines. Current
> observations are reduced average network latency for guests.
>
> My terse measurements using the following btrace script show some
> promising changes in terms of reducing ioctl syscalls:
>
>   /* VMM_IOC_INTR: 0x800c5606 -> 2148292102 */
>   syscall:ioctl:entry
>   /arg1 == 2148292102/
>   {
> @total[tid] = count();
> @running[tid] = count();
>   }
>   interval:hz:1
>   {
> print(@running);
> clear(@running);
>   }
>
> Measuring from boot of an OpenBSD guest to after the guest finishes
> relinking (based on my manual observation of the libevent thread
> settling down in syscall rate), I see a huge reduction in VMM_IOC_INTR
> ioctls for a single guest:
>
> ## -current
> @total[433237]: 1325100  # vcpu thread (!!)
> @total[187073]: 80239# libevent thread
>
> ## with diff
> @total[550347]: 42   # vcpu thread (!!)
> @total[256550]: 86946# libevent thread
>
> Most of the VMM_IOC_INTR ioctls on the vcpu threads come from seabios
> and the bootloader prodding some of the emulated hardware, but even
> after the bootloader you'll see ~10-20k/s of ioctl's on -current
> vs. ~4-5k/s with the diff.
>
> At steady-state, the vcpu thread no longer makes the VMM_IOC_INTR calls
> at all and you should see the libevent thread calling it at a rate ~100/s
> (probably hardclock?). *Without* the diff, I see a steady 650/s rate on
> the vcpu thread at idle. *With* the diff, it's 0/s at idle. :)
>
> To test:
> - rebuild & install new kernel
> - copy/symlink vmmvar.h into /usr/include/machine/
> - rebuild & re-install vmd & vmctl
> - reboot
>
> -dv
>
>

ok mlarkin, thanks!

> diffstat refs/heads/master refs/heads/vmm-vrp_intr_pending
>  M  sys/arch/amd64/amd64/vmm_machdep.c  |  10+   0-
>  M  sys/arch/amd64/include/vmmvar.h |   1+   0-
>  M  usr.sbin/vmd/vm.c   |   2+  16-
>
> 3 files changed, 13 insertions(+), 16 deletions(-)
>
> diff refs/heads/master refs/heads/vmm-vrp_intr_pending
> commit - 8afcf90fb39e4a84606e93137c2b6c20f44312cb
> commit + 10eeb8a0414ec927b6282473c50043a7027d6b41
> blob - 24a376a8f3bc94bc4a4203fe66c5994594adff46
> blob + e3b6d10a0ae78b12ec2f3296f708b42540ce798e
> --- sys/arch/amd64/amd64/vmm_machdep.c
> +++ sys/arch/amd64/amd64/vmm_machdep.c
> @@ -3973,6 +3973,11 @@ vcpu_run_vmx(struct vcpu *vcpu, struct vm_run_params *
>*/
>   irq = vrp->vrp_irq;
>
> + if (vrp->vrp_intr_pending)
> + vcpu->vc_intr = 1;
> + else
> + vcpu->vc_intr = 0;
> +
>   if (vrp->vrp_continue) {
>   switch (vcpu->vc_gueststate.vg_exit_reason) {
>   case VMX_EXIT_IO:
> @@ -6381,6 +6386,11 @@ vcpu_run_svm(struct vcpu *vcpu, struct vm_run_params *
>
>   irq = vrp->vrp_irq;
>
> + if (vrp->vrp_intr_pending)
> + vcpu->vc_intr = 1;
> + else
> + vcpu->vc_intr = 0;
> +
>   /*
>* If we are returning from userspace (vmd) because we exited
>* last time, fix up any needed vcpu state first. Which state
> blob - e9f8384cccfde33034d7ac9782610f93eb5dc640
> blob + 88545b54b35dd60280ba87403e343db9463d7419
> --- sys/arch/amd64/include/vmmvar.h
> +++ sys/arch/amd64/include/vmmvar.h
> @@ -456,6 +456,7 @@ struct vm_run_params {
>   uint32_tvrp_vcpu_id;
>   uint8_t vrp_continue;   /* Continuing from an exit */
>   uint16_tvrp_irq;/* IRQ to inject */
> + uint8_t vrp_intr_pending;   /* Additional intrs pending? */
>
>   /* Input/output parameter to VMM_IOC_RUN */
>   struct vm_exit  *vrp_exit;  /* updated exit data */
> blob - 5f598bcc14af5115372d34a4176254d377aad91c
> blob + 447fc219adadf945de2bf25d5335993c2abdc26f
> --- usr.sbin/vmd/vm.c
> +++ usr.sbin/vmd/vm.c
> @@ -1610,22 +1610,8 @@ vcpu_run_loop(void *arg)
>   } else
>   vrp->vrp_irq = 0x;
>
> - /* Still more pending? */
> - if (i8259_is_pending()) {
> - /*
> -  * XXX can probably avoid ioctls here by providing intr
> -  * in vrp
> -  */
> - if (vcpu_pic_intr(vrp->vrp_vm_id,
> - vrp->vrp_vcpu_id, 1)) {
> -  

Re: malloc write after free error checking

2023-09-03 Thread Otto Moerbeek
On Sun, Sep 03, 2023 at 09:21:18AM +0200, Otto Moerbeek wrote:

> Hello,
> 
> I'm seeing some reports of "write after free" reported by malloc by
> peolpe running current.  Malloc has become more strict since begining
> of June. Let me explain:
> 
> Small allocations share a page. e.g. a 4k page will hold 8 512 byte
> allocations.
> 
> When one such allocation is freed, it will be filled with a "free
> junk" pattern, stay for a short while in the delayed free list (and be
> subhject to write after free checks) and after that it will be marked
> free and it might be allocated again. On a new allocation the write
> after free check will also be done. That is the new part of my commit
> in June.  Another change is that on the allocation of the page
> containing chunks, all chunks wil be filled with a "free junk" (0xdf)
> patttern. 
> 
> The change has a consequence: the time span of the "write after free"
> checks is much longer, as it can take a long time for a chunk to get
> used again.
> 
> I do believe the changes itself are correct and can help findiung
> bugs in other code. 
> 
> You can also be set upon a wrong foot: if an out of bounds write on a
> adjacent chunk happens and lands in (another) free chunk, upon
> allocation of that free chunk it will be reported as a "write after
> free" case. It might even be that an allocation that was never
> allocated and never freed is still "write after free" because of
> "close" out of bounds writes. 
> 
> I'm thinking how to change the error message to reflect that.
> 
> If these extra checks hinder progress, it is possible to swicth them
> off using malloc option f (small F).

Oops, that should be j (small J).

> 
> For general debugging I recommend using malloc option S, is has all
> the extra strictness that can be enabled while still conforming to the
> malloc API.
> 
> Please be aware of these things while hunting for bugs.
> 
>   -Otto
> 



malloc write after free error checking

2023-09-03 Thread Otto Moerbeek
Hello,

I'm seeing some reports of "write after free" reported by malloc by
peolpe running current.  Malloc has become more strict since begining
of June. Let me explain:

Small allocations share a page. e.g. a 4k page will hold 8 512 byte
allocations.

When one such allocation is freed, it will be filled with a "free
junk" pattern, stay for a short while in the delayed free list (and be
subhject to write after free checks) and after that it will be marked
free and it might be allocated again. On a new allocation the write
after free check will also be done. That is the new part of my commit
in June.  Another change is that on the allocation of the page
containing chunks, all chunks wil be filled with a "free junk" (0xdf)
patttern. 

The change has a consequence: the time span of the "write after free"
checks is much longer, as it can take a long time for a chunk to get
used again.

I do believe the changes itself are correct and can help findiung
bugs in other code. 

You can also be set upon a wrong foot: if an out of bounds write on a
adjacent chunk happens and lands in (another) free chunk, upon
allocation of that free chunk it will be reported as a "write after
free" case. It might even be that an allocation that was never
allocated and never freed is still "write after free" because of
"close" out of bounds writes. 

I'm thinking how to change the error message to reflect that.

If these extra checks hinder progress, it is possible to swicth them
off using malloc option f (small F).

For general debugging I recommend using malloc option S, is has all
the extra strictness that can be enabled while still conforming to the
malloc API.

Please be aware of these things while hunting for bugs.

-Otto



Re: VisionFive 2

2023-09-03 Thread Robert Palm
Thanks for commenting.

Maybe this is useful

RISC-V Star64 JH7110: Power Up the Display Controller with U-Boot Bootloader:

https://lupyuen.codeberg.page/articles/display3.html

Am 2. Sept. 2023, 23:24, um 23:24, Mark Kettenis  
schrieb:
>> From: Robert Palm 
>> Date: Sat, 02 Sep 2023 18:34:14 +0200
>>
>> Hi Mark,
>>
>> as you presumably already know, it seems there exists a first
>> version for hdmi support:
>>
>https://patchwork.kernel.org/project/linux-riscv/cover/20230801101030.2040-1-keith.z...@starfivetech.com/
>>
>> Does this mean a new driver for the vf2 like rkdwhdmi would be
>> "needed" to support hdmi in OpenBSD?
>
>In principle support in u-boot would be enough to give OpenBSD a
>framebuffer console.
>
>> Am 3. Aug. 2023, 15:41, um 15:41, Mark Kettenis
> schrieb:
>> >> Date: Tue, 01 Aug 2023 23:11:43 +0200
>> >> From: Robert Palm 
>> >>
>> >> I own a VF 2 version 1.2a and can successfully install / boot the
>> >machine.
>> >>
>> >> The inner network port (dwqe1) works at 100 full duplex and
>receives
>> >
>> >> ipv4 via DHCP.
>> >>
>> >> The outer port currently doesn't seem to get an ip, but gets
>active
>> >> and in full-duplex 100.
>> >
>> >There is a reason why the VisonFive 2 isn't listed as supported on
>> >
>> >  https://www.openbsd.org/riscv64.html
>> >
>> >There are stll bugs and...
>> >
>> >> It seems a lot depends on proper .dtb files (which kind users
>shared
>> >> with me).
>> >
>> >Yes, and it is a total mess.  The device trees are being changed as
>> >support for the board is upstreamed in the Linux kernel.  But their
>> >firmware still provides their hacked up device trees that they use
>> >with their hacked up vendor kernel.
>> >
>> >> How did you create the .dtb files ?
>> >
>> >They're build from:
>> >
>>
>>https://github.com/starfive-tech/linux/tree/JH7110_VisionFive2_upstream
>> >
>> >But that branch keeps getting rebased, and they changed things
>again,
>> >so PCIe stopped working.  So I've decided to stick with what I have
>> >for development and wait until the device tree bindings have been
>> >accepted by the Linux maintainers.  Meanwhile, if you're running
>> >-current on one of these, expect your setup to break at some point
>in
>> >the future.
>> >
>> >> Do you plan to update them ?
>> >
>> >The plan is to provide usable device tree in ports as soon as there
>is
>> >an upstream Linux version that only needs minor patching.
>> >
>> >> They seem to be quite different to the "official" starfive
>releases
>> >> (which don't work for me with OpenBSD).
>> >
>> >As I said, that's just the typical unmaintainable vendor crap.
>> >
>> >> Do you plan more work on the VF2 ?
>> >
>> >I probably consider it done as soon as I finc the remaining dwqe(4)
>> >bugs have been found and fixed.
>> >
>> >Cheers,
>> >
>> >Mark
>> 
>>