Re: [RFC] acpi: add acpitimer_delay(), acpihpet_delay()
On Thu, Sep 08, 2022 at 08:24:11AM -0500, Scott Cheloha wrote: > On Thu, Sep 08, 2022 at 05:52:43AM +0300, Pavel Korovin wrote: > > On 09/07, Scott Cheloha wrote: > > > Just to make sure that my changes to acpihpet(4) actually caused > > > the problem, I have a few more questions: > > > > > > 1. When did you change the OS type? > > > > 03 August, after that, there was a local snpashot built from sources > > fetched on 17 Aug 2022 22:12:57 +0300 which wasn't affected. > > > > > 2. What was the compilation date of the kernel where you first saw the > > >problem? > > > > Locally built snapshot from sources fetched on Wed, 31 Aug 2022 02:05:34 > > +0300. > > > > > 3. If you boot an unpatched kernel does the problem manifest in > > >exactly the same way? > > > > As I said, I mistakenly changed the OS type not to "FreeBSD Pre-11 > > versions (32-bit)", but to "FreeBSD 11 (32-bit)". > > The problem affects only VMs which have Guest OS Version set to "FreeBSD > > Pre-11 versions (32-bit)" on snapshots built after 31 Aug 2022. > > > > Sample outputs from the machine running older snapshot which is not > > affected: > > > > $ sysctl kern.version | head -n1 > > kern.version=OpenBSD 7.2-beta (GENERIC.MP) #1: Thu Aug 18 15:15:13 MSK > > 2022 > > > > $ sysctl | grep tsc > > kern.timecounter.hardware=tsc > > kern.timecounter.choice=i8254(0) acpihpet0(1000) tsc(2000) > > acpitimer0(1000) > > machdep.tscfreq=1500017850 > > machdep.invarianttsc=1 > > Please send a full bug report to b...@openbsd.org. > > Include full a dmesg for the affected machine. > > Something else with VMWare is involved here and I'm not enough of an > expert to say what. More eyes will be helpful. > > Your problem is *probably* caused by my changes, but there are too > many moving parts with all the different VMWare configurations for me > to narrow down what the issue is definitively. I have committed the acpihpet(4) fix. Please send a full bug report to b...@openbsd.org so we can continue poking at this. I have some test code I'd like you to try out so we can get a better look at the TSC calibration process on your machine.
Re: soreceive() with shared netlock for raw sockets
On Sat, Sep 10, 2022 at 10:49:40PM +0300, Vitaliy Makkoveev wrote:
> As it was done for udp and divert sockets.
I have no stess test for parallel receive yet.
Diff looks reasonable.
OK bluhm@
> Index: sys/netinet/ip_var.h
> ===
> RCS file: /cvs/src/sys/netinet/ip_var.h,v
> retrieving revision 1.104
> diff -u -p -r1.104 ip_var.h
> --- sys/netinet/ip_var.h 3 Sep 2022 22:43:38 - 1.104
> +++ sys/netinet/ip_var.h 10 Sep 2022 19:41:56 -
> @@ -258,6 +258,8 @@ intrip_output(struct mbuf *, struct so
> struct mbuf *);
> int rip_attach(struct socket *, int);
> int rip_detach(struct socket *);
> +void rip_lock(struct socket *);
> +void rip_unlock(struct socket *);
> int rip_bind(struct socket *so, struct mbuf *, struct proc *);
> int rip_connect(struct socket *, struct mbuf *);
> int rip_disconnect(struct socket *);
> Index: sys/netinet/raw_ip.c
> ===
> RCS file: /cvs/src/sys/netinet/raw_ip.c,v
> retrieving revision 1.147
> diff -u -p -r1.147 raw_ip.c
> --- sys/netinet/raw_ip.c 3 Sep 2022 22:43:38 - 1.147
> +++ sys/netinet/raw_ip.c 10 Sep 2022 19:41:56 -
> @@ -106,6 +106,8 @@ struct inpcbtable rawcbtable;
> const struct pr_usrreqs rip_usrreqs = {
> .pru_attach = rip_attach,
> .pru_detach = rip_detach,
> + .pru_lock = rip_lock,
> + .pru_unlock = rip_unlock,
> .pru_bind = rip_bind,
> .pru_connect= rip_connect,
> .pru_disconnect = rip_disconnect,
> @@ -220,12 +222,19 @@ rip_input(struct mbuf **mp, int *offp, i
> else
> n = m_copym(m, 0, M_COPYALL, M_NOWAIT);
> if (n != NULL) {
> + int ret;
> +
> if (inp->inp_flags & INP_CONTROLOPTS ||
> inp->inp_socket->so_options & SO_TIMESTAMP)
> ip_savecontrol(inp, , ip, n);
> - if (sbappendaddr(inp->inp_socket,
> +
> + mtx_enter(>inp_mtx);
> + ret = sbappendaddr(inp->inp_socket,
> >inp_socket->so_rcv,
> - sintosa(), n, opts) == 0) {
> + sintosa(), n, opts);
> + mtx_leave(>inp_mtx);
> +
> + if (ret == 0) {
> /* should notify about lost packet */
> m_freem(n);
> m_freem(opts);
> @@ -498,6 +507,24 @@ rip_detach(struct socket *so)
> in_pcbdetach(inp);
>
> return (0);
> +}
> +
> +void
> +rip_lock(struct socket *so)
> +{
> + struct inpcb *inp = sotoinpcb(so);
> +
> + NET_ASSERT_LOCKED();
> + mtx_enter(>inp_mtx);
> +}
> +
> +void
> +rip_unlock(struct socket *so)
> +{
> + struct inpcb *inp = sotoinpcb(so);
> +
> + NET_ASSERT_LOCKED();
> + mtx_leave(>inp_mtx);
> }
>
> int
> Index: sys/netinet6/ip6_var.h
> ===
> RCS file: /cvs/src/sys/netinet6/ip6_var.h,v
> retrieving revision 1.102
> diff -u -p -r1.102 ip6_var.h
> --- sys/netinet6/ip6_var.h3 Sep 2022 22:43:38 - 1.102
> +++ sys/netinet6/ip6_var.h10 Sep 2022 19:41:56 -
> @@ -353,6 +353,8 @@ int rip6_output(struct mbuf *, struct so
> struct mbuf *);
> int rip6_attach(struct socket *, int);
> int rip6_detach(struct socket *);
> +void rip6_lock(struct socket *);
> +void rip6_unlock(struct socket *);
> int rip6_bind(struct socket *, struct mbuf *, struct proc *);
> int rip6_connect(struct socket *, struct mbuf *);
> int rip6_disconnect(struct socket *);
> Index: sys/netinet6/raw_ip6.c
> ===
> RCS file: /cvs/src/sys/netinet6/raw_ip6.c,v
> retrieving revision 1.168
> diff -u -p -r1.168 raw_ip6.c
> --- sys/netinet6/raw_ip6.c3 Sep 2022 22:43:38 - 1.168
> +++ sys/netinet6/raw_ip6.c10 Sep 2022 19:41:56 -
> @@ -108,6 +108,8 @@ struct cpumem *rip6counters;
> const struct pr_usrreqs rip6_usrreqs = {
> .pru_attach = rip6_attach,
> .pru_detach = rip6_detach,
> + .pru_lock = rip6_lock,
> + .pru_unlock = rip6_unlock,
> .pru_bind = rip6_bind,
> .pru_connect= rip6_connect,
> .pru_disconnect = rip6_disconnect,
> @@ -261,13 +263,20 @@ rip6_input(struct mbuf **mp, int *offp,
> else
> n = m_copym(m, 0, M_COPYALL, M_NOWAIT);
> if (n != NULL) {
> + int ret;
> +
> if (in6p->inp_flags & IN6P_CONTROLOPTS)
> ip6_savecontrol(in6p, n, );
> /* strip intermediate headers */
> m_adj(n, *offp);
> - if
Re: Change pru_rcvd() return type to the type of void
On Sat, Sep 10, 2022 at 10:18:15PM +0300, Vitaliy Makkoveev wrote:
> We have no interest on pru_rcvd() return value. Also, we call pru_rcvd()
> only if the socket's protocol have PR_WANTRCVD flag set. Such sockets
> are route domain, tcp(4) and unix(4) sockets.
>
> This diff keeps the PR_WANTRCVD check. In other hand we could always
> call pru_rcvd() and do "pru_rcvd != NULL" check within, but in the
> future with per buffer locking, we could have some re-locking around
> pru_rcvd() call and I want to do it outside wrapper.
OK bluhm@
> Index: sys/kern/uipc_usrreq.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
> retrieving revision 1.185
> diff -u -p -r1.185 uipc_usrreq.c
> --- sys/kern/uipc_usrreq.c3 Sep 2022 22:43:38 - 1.185
> +++ sys/kern/uipc_usrreq.c10 Sep 2022 18:51:42 -
> @@ -363,7 +363,7 @@ uipc_shutdown(struct socket *so)
> return (0);
> }
>
> -int
> +void
> uipc_rcvd(struct socket *so)
> {
> struct socket *so2;
> @@ -390,8 +390,6 @@ uipc_rcvd(struct socket *so)
> default:
> panic("uipc 2");
> }
> -
> - return (0);
> }
>
> int
> Index: sys/net/rtsock.c
> ===
> RCS file: /cvs/src/sys/net/rtsock.c,v
> retrieving revision 1.355
> diff -u -p -r1.355 rtsock.c
> --- sys/net/rtsock.c 8 Sep 2022 10:22:06 - 1.355
> +++ sys/net/rtsock.c 10 Sep 2022 18:51:42 -
> @@ -115,7 +115,7 @@ int route_attach(struct socket *, int);
> int route_detach(struct socket *);
> int route_disconnect(struct socket *);
> int route_shutdown(struct socket *);
> -int route_rcvd(struct socket *);
> +void route_rcvd(struct socket *);
> int route_send(struct socket *, struct mbuf *, struct mbuf *,
> struct mbuf *);
> int route_abort(struct socket *);
> @@ -299,7 +299,7 @@ route_shutdown(struct socket *so)
> return (0);
> }
>
> -int
> +void
> route_rcvd(struct socket *so)
> {
> struct rtpcb *rop = sotortpcb(so);
> @@ -314,8 +314,6 @@ route_rcvd(struct socket *so)
> ((sbspace(rop->rop_socket, >rop_socket->so_rcv) ==
> rop->rop_socket->so_rcv.sb_hiwat)))
> rop->rop_flags &= ~ROUTECB_FLAG_FLUSH;
> -
> - return (0);
> }
>
> int
> Index: sys/netinet/tcp_usrreq.c
> ===
> RCS file: /cvs/src/sys/netinet/tcp_usrreq.c,v
> retrieving revision 1.207
> diff -u -p -r1.207 tcp_usrreq.c
> --- sys/netinet/tcp_usrreq.c 3 Sep 2022 22:43:38 - 1.207
> +++ sys/netinet/tcp_usrreq.c 10 Sep 2022 18:51:42 -
> @@ -792,18 +792,17 @@ out:
> /*
> * After a receive, possibly send window update to peer.
> */
> -int
> +void
> tcp_rcvd(struct socket *so)
> {
> struct inpcb *inp;
> struct tcpcb *tp;
> - int error;
> short ostate;
>
> soassertlocked(so);
>
> - if ((error = tcp_sogetpcb(so, , )))
> - return (error);
> + if (tcp_sogetpcb(so, , ))
> + return;
>
> if (so->so_options & SO_DEBUG)
> ostate = tp->t_state;
> @@ -820,7 +819,6 @@ tcp_rcvd(struct socket *so)
>
> if (so->so_options & SO_DEBUG)
> tcp_trace(TA_USER, ostate, tp, tp, NULL, PRU_RCVD, 0);
> - return (0);
> }
>
> /*
> Index: sys/netinet/tcp_var.h
> ===
> RCS file: /cvs/src/sys/netinet/tcp_var.h,v
> retrieving revision 1.157
> diff -u -p -r1.157 tcp_var.h
> --- sys/netinet/tcp_var.h 3 Sep 2022 22:43:38 - 1.157
> +++ sys/netinet/tcp_var.h 10 Sep 2022 18:51:42 -
> @@ -725,7 +725,7 @@ inttcp_connect(struct socket *, struct
> int tcp_accept(struct socket *, struct mbuf *);
> int tcp_disconnect(struct socket *);
> int tcp_shutdown(struct socket *);
> -int tcp_rcvd(struct socket *);
> +void tcp_rcvd(struct socket *);
> int tcp_send(struct socket *, struct mbuf *, struct mbuf *,
>struct mbuf *);
> int tcp_abort(struct socket *);
> Index: sys/sys/protosw.h
> ===
> RCS file: /cvs/src/sys/sys/protosw.h,v
> retrieving revision 1.55
> diff -u -p -r1.55 protosw.h
> --- sys/sys/protosw.h 5 Sep 2022 14:56:09 - 1.55
> +++ sys/sys/protosw.h 10 Sep 2022 18:51:42 -
> @@ -72,7 +72,7 @@ struct pr_usrreqs {
> int (*pru_accept)(struct socket *, struct mbuf *);
> int (*pru_disconnect)(struct socket *);
> int (*pru_shutdown)(struct socket *);
> - int (*pru_rcvd)(struct socket *);
> + void(*pru_rcvd)(struct socket *);
> int (*pru_send)(struct socket *, struct mbuf *, struct mbuf *,
> struct mbuf *);
> int (*pru_abort)(struct socket *);
> @@ -336,12 +336,10 @@ pru_shutdown(struct socket *so)
> return
Re: installboot: powerpc64: fix passing explicit stage files
On Mon, Sep 12, 2022 at 12:37:53PM +, Klemens Nanni wrote:
> This fixes installboot regress on powerpc64, forgot to send it earlier.
>
> The same diff already landed for macppc; efi already the same fix
> md_init() but without the cleanup that entails for macppc and powerpc64.
>
> From macppc_installboot.c r1.6 "Fix passing explicit stage files":
>
> Using `stage1' leads to a bit more cleanup since early MI installboot.c
> handles `-r', i.e. write_filesystem() no longer has needs to do the
> fileprefix() dance itself.
>
> OK?
Same story/diff for octeon.
Index: octeon_installboot.c
===
RCS file: /cvs/src/usr.sbin/installboot/octeon_installboot.c,v
retrieving revision 1.6
diff -u -p -r1.6 octeon_installboot.c
--- octeon_installboot.c9 Sep 2022 15:53:16 - 1.6
+++ octeon_installboot.c12 Sep 2022 12:39:57 -
@@ -61,6 +61,8 @@ static intfindmbrfat(int, struct diskla
void
md_init(void)
{
+ stages = 1;
+ stage1 = "/usr/mdec/boot";
}
void
@@ -164,12 +166,9 @@ write_filesystem(struct disklabel *dl, c
struct msdosfs_args args;
char cmd[60];
char dst[PATH_MAX];
- char *src;
- size_t mntlen, pathlen, srclen;
+ size_t mntlen, pathlen;
int rslt;
- src = NULL;
-
/* Create directory for temporary mount point. */
strlcpy(dst, "/tmp/installboot.XX", sizeof(dst));
if (mkdtemp(dst) == NULL)
@@ -231,17 +230,11 @@ write_filesystem(struct disklabel *dl, c
warn("unable to build /boot path");
goto umount;
}
- src = fileprefix(root, "/usr/mdec/boot");
- if (src == NULL) {
- rslt = -1;
- goto umount;
- }
- srclen = strlen(src);
if (verbose)
fprintf(stderr, "%s %s to %s\n",
- (nowrite ? "would copy" : "copying"), src, dst);
+ (nowrite ? "would copy" : "copying"), stage1, dst);
if (!nowrite) {
- rslt = filecopy(src, dst);
+ rslt = filecopy(stage1, dst);
if (rslt == -1)
goto umount;
}
@@ -258,8 +251,6 @@ rmdir:
dst[mntlen] = '\0';
if (rmdir(dst) == -1)
err(1, "rmdir('%s') failed", dst);
-
- free(src);
if (rslt == -1)
exit(1);
installboot: powerpc64: fix passing explicit stage files
This fixes installboot regress on powerpc64, forgot to send it earlier.
The same diff already landed for macppc; efi already the same fix
md_init() but without the cleanup that entails for macppc and powerpc64.
>From macppc_installboot.c r1.6 "Fix passing explicit stage files":
Using `stage1' leads to a bit more cleanup since early MI installboot.c
handles `-r', i.e. write_filesystem() no longer has needs to do the
fileprefix() dance itself.
OK?
Index: powerpc64_installboot.c
===
RCS file: /cvs/src/usr.sbin/installboot/powerpc64_installboot.c,v
retrieving revision 1.5
diff -u -p -r1.5 powerpc64_installboot.c
--- powerpc64_installboot.c 9 Sep 2022 15:53:16 - 1.5
+++ powerpc64_installboot.c 12 Sep 2022 12:31:44 -
@@ -63,6 +63,8 @@ char duid[20];
void
md_init(void)
{
+ stages = 1;
+ stage1 = "/usr/mdec/boot";
}
void
@@ -172,12 +174,9 @@ write_filesystem(struct disklabel *dl, c
char cmd[60];
char dir[PATH_MAX];
char dst[PATH_MAX];
- char *src;
- size_t mntlen, srclen;
+ size_t mntlen;
int rslt;
- src = NULL;
-
/* Create directory for temporary mount point. */
strlcpy(dir, "/tmp/installboot.XX", sizeof(dst));
if (mkdtemp(dir) == NULL)
@@ -239,17 +238,11 @@ write_filesystem(struct disklabel *dl, c
warn("unable to build /boot path");
goto umount;
}
- src = fileprefix(root, "/usr/mdec/boot");
- if (src == NULL) {
- rslt = -1;
- goto umount;
- }
- srclen = strlen(src);
if (verbose)
fprintf(stderr, "%s %s to %s\n",
- (nowrite ? "would copy" : "copying"), src, dst);
+ (nowrite ? "would copy" : "copying"), stage1, dst);
if (!nowrite) {
- rslt = filecopy(src, dst);
+ rslt = filecopy(stage1, dst);
if (rslt == -1)
goto umount;
}
@@ -292,8 +285,6 @@ rmdir:
dst[mntlen] = '\0';
if (rmdir(dir) == -1)
err(1, "rmdir('%s') failed", dir);
-
- free(src);
if (rslt == -1)
exit(1);
Re: acpihpet(4): acpihpet_delay: only use lower 32 bits of counter
On Fri, Sep 09, 2022 at 07:32:58AM -0500, Scott Cheloha wrote:
> On Fri, Sep 09, 2022 at 03:59:01PM +1000, Jonathan Gray wrote:
> > On Thu, Sep 08, 2022 at 08:31:21PM -0500, Scott Cheloha wrote:
> > > On Sat, Aug 27, 2022 at 09:28:06PM -0500, Scott Cheloha wrote:
> > > > Whoops, forgot about the split read problem. My mistake.
> > > >
> > > > Because 32-bit platforms cannot do bus_space_read_8 atomically, and
> > > > i386 can use acpihpet(4), we can only safely use the lower 32 bits of
> > > > the counter in acpihpet_delay() (unless we want two versions of
> > > > acpihpet_delay()... which I don't).
> > > >
> > > > Switch from acpihpet_r() to bus_space_read_4(9) and accumulate cycles
> > > > as we do in acpihpet_delay(). Unlike acpitimer(4), the HPET is a
> > > > 64-bit counter so we don't need to mask the difference between val1
> > > > and val2.
> > > >
> > > > [...]
> > >
> > > 12 day ping.
> > >
> > > This needs fixing before it causes problems.
> >
> > the hpet spec says to set a bit to force a 32-bit counter on
> > 32-bit platforms
> >
> > see 2.4.7 Issues related to 64-bit Timers with 32-bit CPUs, in
> > https://www.intel.com/content/dam/www/public/us/en/documents/technical-specifications/software-developers-hpet-spec-1-0a.pdf
>
> I don't follow your meaning. Putting the HPET in 32-bit mode doesn't
> help us here, it would just break acpihpet_delay() in a different way.
>
> The problem is that acpihpet_delay() is written as a 64-bit delay(9)
> and there's no way to do that safely on i386 without introducing extra
> overhead.
>
> The easiest and cheapest fix is to rewrite acpihpet_delay() as a
> 32-bit delay(9), i.e. we count cycles until we pass a threshold.
> acpitimer_delay() in acpi/acpitimer.c is a 32-bit delay(9) and it
> works great, let's just do the same thing again here.
>
> ok?
Seems to handle wrapping.
ok jsg@
>
> Index: acpihpet.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpihpet.c,v
> retrieving revision 1.28
> diff -u -p -r1.28 acpihpet.c
> --- acpihpet.c25 Aug 2022 18:01:54 - 1.28
> +++ acpihpet.c9 Sep 2022 12:29:41 -
> @@ -281,13 +281,19 @@ acpihpet_attach(struct device *parent, s
> void
> acpihpet_delay(int usecs)
> {
> - uint64_t c, s;
> + uint64_t count = 0, cycles;
> struct acpihpet_softc *sc = hpet_timecounter.tc_priv;
> + uint32_t val1, val2;
>
> - s = acpihpet_r(sc->sc_iot, sc->sc_ioh, HPET_MAIN_COUNTER);
> - c = usecs * hpet_timecounter.tc_frequency / 100;
> - while (acpihpet_r(sc->sc_iot, sc->sc_ioh, HPET_MAIN_COUNTER) - s < c)
> + val2 = bus_space_read_4(sc->sc_iot, sc->sc_ioh, HPET_MAIN_COUNTER);
> + cycles = usecs * hpet_timecounter.tc_frequency / 100;
> + while (count < cycles) {
> CPU_BUSY_CYCLE();
> + val1 = val2;
> + val2 = bus_space_read_4(sc->sc_iot, sc->sc_ioh,
> + HPET_MAIN_COUNTER);
> + count += val2 - val1;
> + }
> }
>
> u_int
>
>
