Re: Move SS_CANTRCVMORE and SS_RCVATMARK to corresponding SBS_*
On Sun, Dec 11, 2022 at 07:55:44PM -0700, Theo de Raadt wrote:
> The problem you have introduced is that so_state is copied from sockets
> to a sysctl record, where it also shows up as so_state.
>
> At least fstat(8) looks at this field, and uses the old SS_CANTSENDMORE
> bit you have deleted. The userland-visible name change is an API break.
> Furthermore you moved it, but your new so_snd.sb_state value is not
> exported in sysctl, and if we add this to sysctl it will be quite a big
> ABI break. I suspect that is too expensive.
>
> So what you need to here is leave the SS_* bits with their existing names.
> You cannot change their names. It is exported API. You cannot change their
> bit values either, it is exported ABI in sysctl.
>
> What you can do is optionally use some bits in the socket, some bits in
> so_snd, and some bits in so_rcv (I assume that is the future). And then
> in kern_sysctl.c around line 1187 you can merge the bits, since they are
> still unique:
>
>kf->so_state = so->so_state | so->so_snd.sb_state | so->so_rcv.sb_state
>
> This will mean fstat can see all the bits in the one sysctl sub-variable.
>
> It means you must go very quickly rename, or you should consider backing
> your commit out and trying again fresh.
>
`sb_state' will contain some new bits so I guess it's better to restote
old SS_CANTSENDMORE and set it if SBS_CANTSENDMORE is set:
Index: sys/kern/kern_sysctl.c
===
RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.408
diff -u -p -r1.408 kern_sysctl.c
--- sys/kern/kern_sysctl.c 7 Nov 2022 14:25:44 - 1.408
+++ sys/kern/kern_sysctl.c 12 Dec 2022 07:31:26 -
@@ -1185,6 +1185,8 @@ fill_file(struct kinfo_file *kf, struct
kf->so_type = so->so_type;
kf->so_state = so->so_state;
+ if (so->so_rcv.sb_state & SBS_CANTSENDMORE)
+ kf->so_state |= SS_CANTSENDMORE;
if (show_pointers)
kf->so_pcb = PTRTOINT64(so->so_pcb);
else
Index: sys/sys/socketvar.h
===
RCS file: /cvs/src/sys/sys/socketvar.h,v
retrieving revision 1.113
diff -u -p -r1.113 socketvar.h
--- sys/sys/socketvar.h 11 Dec 2022 21:19:08 - 1.113
+++ sys/sys/socketvar.h 12 Dec 2022 07:31:26 -
@@ -149,6 +149,7 @@ struct socket {
#defineSS_ISCONNECTED 0x002 /* socket connected to a peer */
#defineSS_ISCONNECTING 0x004 /* in process of connecting to
peer */
#defineSS_ISDISCONNECTING 0x008 /* in process of disconnecting
*/
+#define SS_CANTSENDMORE0x010 /* can't send more data to peer
*/
#defineSS_CANTRCVMORE 0x020 /* can't receive more data from
peer */
#defineSS_RCVATMARK0x040 /* at mark on input */
#defineSS_ISDISCONNECTED 0x800 /* socket disconnected from
peer */
Re: hostname.if(5): lladdr tweaks
On Fri, Dec 09, 2022 at 09:32:13PM +, Jason McIntyre wrote: > hi. > > two points about the recent ability to use lladdr: > > - the example of "bridge0" made sense when bridge was regarded as a > separate entity and not integrated with ifconfig. plus a list of one > example looks rubbish. now that we have a second example (lladdr) and > bridge is not flagged as a special case, i think we can simply the > text and reduce it to two examples > > - i'm not sure about using "lladdr". although we use this term in > ifconfig(8), we explain it. and people may miss it if they are thinking > of mac address. i've attempted to both write the term fully as "link > layer local address" and add a "(MAC)". i suppose you could argue that > people who think of the term as "lladdr" might miss that (!) but i > don;t think that is a real worry. > > so here's my cut at tweaking... > jmc > on the back of afresh's prioroty flip diff, here's a revision of my diff after some input from deraadt. it will need adjusting again if we flip priority... jmc Index: hostname.if.5 === RCS file: /cvs/src/share/man/man5/hostname.if.5,v retrieving revision 1.80 diff -u -p -r1.80 hostname.if.5 --- hostname.if.5 5 Dec 2022 20:12:00 - 1.80 +++ hostname.if.5 12 Dec 2022 06:48:15 - @@ -40,13 +40,16 @@ The .Nm hostname.*\& files contain information regarding the configuration of each network interface. -The interface can be referenced by name or lladdr, such as -.Pa hostname.fxp0 , -.Pa hostname.00:00:5e:00:53:af , -or -.Pa hostname.bridge0 . -One file should exist for each interface that is to be configured, -with priority given to configuration by interface name over lladdr. +Interfaces are referenced by name and number, +such as +.Dq hostname.fxp0 . +For some machines, +autoconfiguration makes this system inconsistent, +so interfaces can alternatively be referenced by +their link layer address (lladdr), +such as +.Dq hostname.00:00:5e:00:53:af . +Priority is given to configuration by interface name/number over lladdr. A configuration file is not needed for lo0. .Pp The configuration information is expressed in a line-by-line packed format
Re: Move SS_CANTRCVMORE and SS_RCVATMARK to corresponding SBS_*
The problem you have introduced is that so_state is copied from sockets to a sysctl record, where it also shows up as so_state. At least fstat(8) looks at this field, and uses the old SS_CANTSENDMORE bit you have deleted. The userland-visible name change is an API break. Furthermore you moved it, but your new so_snd.sb_state value is not exported in sysctl, and if we add this to sysctl it will be quite a big ABI break. I suspect that is too expensive. So what you need to here is leave the SS_* bits with their existing names. You cannot change their names. It is exported API. You cannot change their bit values either, it is exported ABI in sysctl. What you can do is optionally use some bits in the socket, some bits in so_snd, and some bits in so_rcv (I assume that is the future). And then in kern_sysctl.c around line 1187 you can merge the bits, since they are still unique: kf->so_state = so->so_state | so->so_snd.sb_state | so->so_rcv.sb_state This will mean fstat can see all the bits in the one sysctl sub-variable. It means you must go very quickly rename, or you should consider backing your commit out and trying again fresh.
Re: Move SS_CANTRCVMORE and SS_RCVATMARK to corresponding SBS_*
This diff changes the exposed API/ABI, and breaks userland --
which has obviously not experienced a test build.
This is really obvious. If you change a kernel .h file, you have to
check all uses of the file. kernel .h files are installed into /usr/include/sys
and all userland must.
So the minimum work is to build base.
Why did you skip that step?
Since you commited this, the tree is now broken.
Vitaliy Makkoveev wrote:
> This continues previous "Introduce `sb_state' and move
> SS_CANTSENDMORE..." diff.
>
> Move socket's SS_CANTRCVMORE and SS_RCVATMARK bits to socket's `so_rcv'
> buffer state bits. Moved separately of SS_CANTSENDMORE to make review
> and check easier.
>
> As with previous diff, left remaining SS_ bits as is.
>
> Index: sys/kern/sys_socket.c
> ===
> RCS file: /cvs/src/sys/kern/sys_socket.c,v
> retrieving revision 1.57
> diff -u -p -r1.57 sys_socket.c
> --- sys/kern/sys_socket.c 11 Dec 2022 21:19:08 - 1.57
> +++ sys/kern/sys_socket.c 11 Dec 2022 23:01:34 -
> @@ -119,7 +119,7 @@ soo_ioctl(struct file *fp, u_long cmd, c
> break;
>
> case SIOCATMARK:
> - *(int *)data = (so->so_state_RCVATMARK) != 0;
> + *(int *)data = (so->so_rcv.sb_state & SBS_RCVATMARK) != 0;
> break;
>
> default:
> @@ -149,7 +149,8 @@ soo_stat(struct file *fp, struct stat *u
> memset(ub, 0, sizeof (*ub));
> ub->st_mode = S_IFSOCK;
> solock(so);
> - if ((so->so_state & SS_CANTRCVMORE) == 0 || so->so_rcv.sb_cc != 0)
> + if ((so->so_rcv.sb_state & SBS_CANTRCVMORE) == 0 ||
> + so->so_rcv.sb_cc != 0)
> ub->st_mode |= S_IRUSR | S_IRGRP | S_IROTH;
> if ((so->so_snd.sb_state & SBS_CANTSENDMORE) == 0)
> ub->st_mode |= S_IWUSR | S_IWGRP | S_IWOTH;
> Index: sys/kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.292
> diff -u -p -r1.292 uipc_socket.c
> --- sys/kern/uipc_socket.c11 Dec 2022 21:19:08 - 1.292
> +++ sys/kern/uipc_socket.c11 Dec 2022 23:01:34 -
> @@ -867,7 +867,7 @@ restart:
> so->so_error = 0;
> goto release;
> }
> - if (so->so_state & SS_CANTRCVMORE) {
> + if (so->so_rcv.sb_state & SBS_CANTRCVMORE) {
> if (m)
> goto dontblock;
> else if (so->so_rcv.sb_cc == 0)
> @@ -1023,7 +1023,7 @@ dontblock:
> so, so->so_type, m, m->m_type);
> #endif
> }
> - so->so_state &= ~SS_RCVATMARK;
> + so->so_rcv.sb_state &= ~SBS_RCVATMARK;
> len = uio->uio_resid;
> if (so->so_oobmark && len > so->so_oobmark - offset)
> len = so->so_oobmark - offset;
> @@ -1100,7 +1100,7 @@ dontblock:
> if ((flags & MSG_PEEK) == 0) {
> so->so_oobmark -= len;
> if (so->so_oobmark == 0) {
> - so->so_state |= SS_RCVATMARK;
> + so->so_rcv.sb_state |= SBS_RCVATMARK;
> break;
> }
> } else {
> @@ -1120,7 +1120,8 @@ dontblock:
>*/
> while (flags & MSG_WAITALL && m == NULL && uio->uio_resid > 0 &&
> !sosendallatonce(so) && !nextrecord) {
> - if (so->so_error || so->so_state & SS_CANTRCVMORE)
> + if (so->so_rcv.sb_state & SBS_CANTRCVMORE ||
> + so->so_error)
> break;
> SBLASTRECORDCHK(>so_rcv, "soreceive sbwait 2");
> SBLASTMBUFCHK(>so_rcv, "soreceive sbwait 2");
> @@ -1160,7 +1161,8 @@ dontblock:
> pru_rcvd(so);
> }
> if (orig_resid == uio->uio_resid && orig_resid &&
> - (flags & MSG_EOR) == 0 && (so->so_state & SS_CANTRCVMORE) == 0) {
> + (flags & MSG_EOR) == 0 &&
> + (so->so_rcv.sb_state & SBS_CANTRCVMORE) == 0) {
> sbunlock(so, >so_rcv);
> goto restart;
> }
> @@ -1632,13 +1634,13 @@ somove(struct socket *so, int wait)
> pru_rcvd(so);
>
> /* Receive buffer did shrink by len bytes, adjust oob. */
> - state = so->so_state;
> - so->so_state &= ~SS_RCVATMARK;
> + state = so->so_rcv.sb_state;
> + so->so_rcv.sb_state &= ~SBS_RCVATMARK;
> oobmark = so->so_oobmark;
> so->so_oobmark = oobmark > len ? oobmark - len : 0;
> if (oobmark) {
> if (oobmark == len)
> - so->so_state |= SS_RCVATMARK;
> + so->so_rcv.sb_state |=
Move SS_CANTRCVMORE and SS_RCVATMARK to corresponding SBS_*
This continues previous "Introduce `sb_state' and move
SS_CANTSENDMORE..." diff.
Move socket's SS_CANTRCVMORE and SS_RCVATMARK bits to socket's `so_rcv'
buffer state bits. Moved separately of SS_CANTSENDMORE to make review
and check easier.
As with previous diff, left remaining SS_ bits as is.
Index: sys/kern/sys_socket.c
===
RCS file: /cvs/src/sys/kern/sys_socket.c,v
retrieving revision 1.57
diff -u -p -r1.57 sys_socket.c
--- sys/kern/sys_socket.c 11 Dec 2022 21:19:08 - 1.57
+++ sys/kern/sys_socket.c 11 Dec 2022 23:01:34 -
@@ -119,7 +119,7 @@ soo_ioctl(struct file *fp, u_long cmd, c
break;
case SIOCATMARK:
- *(int *)data = (so->so_state_RCVATMARK) != 0;
+ *(int *)data = (so->so_rcv.sb_state & SBS_RCVATMARK) != 0;
break;
default:
@@ -149,7 +149,8 @@ soo_stat(struct file *fp, struct stat *u
memset(ub, 0, sizeof (*ub));
ub->st_mode = S_IFSOCK;
solock(so);
- if ((so->so_state & SS_CANTRCVMORE) == 0 || so->so_rcv.sb_cc != 0)
+ if ((so->so_rcv.sb_state & SBS_CANTRCVMORE) == 0 ||
+ so->so_rcv.sb_cc != 0)
ub->st_mode |= S_IRUSR | S_IRGRP | S_IROTH;
if ((so->so_snd.sb_state & SBS_CANTSENDMORE) == 0)
ub->st_mode |= S_IWUSR | S_IWGRP | S_IWOTH;
Index: sys/kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.292
diff -u -p -r1.292 uipc_socket.c
--- sys/kern/uipc_socket.c 11 Dec 2022 21:19:08 - 1.292
+++ sys/kern/uipc_socket.c 11 Dec 2022 23:01:34 -
@@ -867,7 +867,7 @@ restart:
so->so_error = 0;
goto release;
}
- if (so->so_state & SS_CANTRCVMORE) {
+ if (so->so_rcv.sb_state & SBS_CANTRCVMORE) {
if (m)
goto dontblock;
else if (so->so_rcv.sb_cc == 0)
@@ -1023,7 +1023,7 @@ dontblock:
so, so->so_type, m, m->m_type);
#endif
}
- so->so_state &= ~SS_RCVATMARK;
+ so->so_rcv.sb_state &= ~SBS_RCVATMARK;
len = uio->uio_resid;
if (so->so_oobmark && len > so->so_oobmark - offset)
len = so->so_oobmark - offset;
@@ -1100,7 +1100,7 @@ dontblock:
if ((flags & MSG_PEEK) == 0) {
so->so_oobmark -= len;
if (so->so_oobmark == 0) {
- so->so_state |= SS_RCVATMARK;
+ so->so_rcv.sb_state |= SBS_RCVATMARK;
break;
}
} else {
@@ -1120,7 +1120,8 @@ dontblock:
*/
while (flags & MSG_WAITALL && m == NULL && uio->uio_resid > 0 &&
!sosendallatonce(so) && !nextrecord) {
- if (so->so_error || so->so_state & SS_CANTRCVMORE)
+ if (so->so_rcv.sb_state & SBS_CANTRCVMORE ||
+ so->so_error)
break;
SBLASTRECORDCHK(>so_rcv, "soreceive sbwait 2");
SBLASTMBUFCHK(>so_rcv, "soreceive sbwait 2");
@@ -1160,7 +1161,8 @@ dontblock:
pru_rcvd(so);
}
if (orig_resid == uio->uio_resid && orig_resid &&
- (flags & MSG_EOR) == 0 && (so->so_state & SS_CANTRCVMORE) == 0) {
+ (flags & MSG_EOR) == 0 &&
+ (so->so_rcv.sb_state & SBS_CANTRCVMORE) == 0) {
sbunlock(so, >so_rcv);
goto restart;
}
@@ -1632,13 +1634,13 @@ somove(struct socket *so, int wait)
pru_rcvd(so);
/* Receive buffer did shrink by len bytes, adjust oob. */
- state = so->so_state;
- so->so_state &= ~SS_RCVATMARK;
+ state = so->so_rcv.sb_state;
+ so->so_rcv.sb_state &= ~SBS_RCVATMARK;
oobmark = so->so_oobmark;
so->so_oobmark = oobmark > len ? oobmark - len : 0;
if (oobmark) {
if (oobmark == len)
- so->so_state |= SS_RCVATMARK;
+ so->so_rcv.sb_state |= SBS_RCVATMARK;
if (oobmark >= len)
oobmark = 0;
}
@@ -1647,13 +1649,13 @@ somove(struct socket *so, int wait)
* Handle oob data. If any malloc fails, ignore error.
* TCP urgent data is not very reliable anyway.
*/
- while (((state & SS_RCVATMARK) || oobmark) &&
+ while (((state & SBS_RCVATMARK) || oobmark) &&
(so->so_options & SO_OOBINLINE)) {
struct mbuf *o = NULL;
- if
Switch priority of hostname.if lladdr over name
This would be the diff to swap priority of name/lladdr as some folks
wanted to see. I still don't recommend having both as you can still
have surprising outcomes with more complex configurations.
The install.sub includes an "if_name_to_lladdr" function from the
diff to support creating a hostname.lladdr during install. If we don't
decide to support that, I'd probably in-line that as is done in
netstart.
I did find that my iwm doesn't like being configured by lladdr at this
point of the upgrade because it doesn't have firmware loaded when it's
looking to see if it's a valid hostname.lladdr, but the iwm's lladdr is
all zeros still.
Index: etc/netstart
===
RCS file: /cvs/src/etc/netstart,v
retrieving revision 1.230
diff -u -p -r1.230 netstart
--- etc/netstart5 Dec 2022 20:12:00 - 1.230
+++ etc/netstart11 Dec 2022 20:44:34 -
@@ -144,11 +144,14 @@ ifstart() {
[[ -z $_line ]] && return
_if=$_line
_line=
-
- if [[ -e /etc/hostname.$_if ]]; then
- print -u2 "${0##*/}: $_hn: /etc/hostname.$_if overrides"
+ else
+ _line=$(ifconfig $_if | sed -n
's/^[[:space:]]*lladdr[[:space:]]//p')
+ if [[ -n $_line && -n $(ifconfig -M $_line) \
+ && -e /etc/hostname.$_line ]]; then
+ print -u2 "${0##*/}: $_hn: /etc/hostname.$_line:
overrides"
return
fi
+ _line=
fi
# Interface names must be alphanumeric only. We check to avoid
Index: distrib/miniroot/install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1215
diff -u -p -r1.1215 install.sub
--- distrib/miniroot/install.sub5 Dec 2022 20:12:00 - 1.1215
+++ distrib/miniroot/install.sub11 Dec 2022 20:44:34 -
@@ -356,6 +356,15 @@ get_ifs() {
done
}
+# Maps an interface name to lladdr,
+# filtered by whether it's valid for use by ifconfig -M
+if_name_to_lladdr() {
+ local _lladdr
+ _lladdr=$(ifconfig $1 2>/dev/null |
+ sed -n 's/^[[:space:]]*lladdr[[:space:]]//p')
+ [[ -n $_lladdr && -n $(ifconfig -M $_lladdr) ]] && echo $_lladdr
+}
+
# Return the device name of the disk device $1, which may be a disklabel UID.
get_dkdev_name() {
local _dev=${1#/dev/} _d
@@ -2434,7 +2443,10 @@ ifstart() {
if [[ $_if == ??:??:??:??:??:?? ]]; then
_if=$(ifconfig -M $_if)
[[ -z $_if ]] && return # invalid interface
- [[ -e /mnt/etc/hostname.$_if ]] && return # duplicate config
+ else
+ _line=$(if_name_to_lladdr $_if)
+ [[ -n $_line && -e /mnt/etc/hostname.$_line ]] && continue
+ _line=
fi
# Create interface if it does not yet exist.
Index: share/man/man5/hostname.if.5
===
RCS file: /cvs/src/share/man/man5/hostname.if.5,v
retrieving revision 1.80
diff -u -p -r1.80 hostname.if.5
--- share/man/man5/hostname.if.55 Dec 2022 20:12:00 - 1.80
+++ share/man/man5/hostname.if.511 Dec 2022 20:44:34 -
@@ -46,7 +46,7 @@ The interface can be referenced by name
or
.Pa hostname.bridge0 .
One file should exist for each interface that is to be configured,
-with priority given to configuration by interface name over lladdr.
+with priority given to configuration by interface lladdr over name.
A configuration file is not needed for lo0.
.Pp
The configuration information is expressed in a line-by-line packed format
add 2 transmeta devices to pcidevs
I have a laptop with these Transmeta devices: pchb0 at pci0 dev 0 function 0 vendor "Transmeta", unknown product 0x0060 rev 0x00 ppb0 at pci0 dev 1 function 0 vendor "Transmeta", unknown product 0x0061 rev 0x00 NetBSD describes device 0061 as the integrated North Bridge, but I think this is incorrect as this is actually an AGP bridge: product TRANSMETA TM8000NB 0x0061 TM8000 Integrated North Bridge The PCI repository has these entries, which I feel is more correct than NetBSD: 0060TM8000 Northbridge 0061TM8000 AGP bridge However, reading the "Efficeon BIOS Programmers Guide", I chose to describe device as 0060 as HyperTransport instead. The Efficeon processor has a virtual north bridge that can communicate with the south bridge over HyperTransport and can communicate with the graphics controller over an AGP bridge. For reference, see Chapter 2, Figure 1 showing the system block diagram. Some contemporaneous news coverage on HyperTransport is available here for reference: https://www.eetimes.com/transmeta-links-next-generation-processor-to-hypertransport/ ok? Index: pcidevs === RCS file: /cvs/src/sys/dev/pci/pcidevs,v retrieving revision 1.2014 diff -u -p -u -r1.2014 pcidevs --- pcidevs 4 Dec 2022 03:13:52 - 1.2014 +++ pcidevs 11 Dec 2022 18:51:04 - @@ -9030,6 +9030,8 @@ product TOSHIBA2 TFIRO0x0701 Infrared product TOSHIBA2 SDCARD0x0805 SD /* Transmeta products */ +product TRANSMETA TM8000_HT0x0060 TM8000 HyperTransport +product TRANSMETA TM8000_AGP 0x0061 TM8000 AGP product TRANSMETA NB 0x0295 Northbridge product TRANSMETA LONGRUN_NB 0x0395 LongRun Northbridge product TRANSMETA SDRAM0x0396 SDRAM
Re: agtimer(4/armv7): switch to clockintr
> Date: Sun, 11 Dec 2022 09:13:05 -0600 > From: Scott Cheloha > > On Sun, Dec 11, 2022 at 01:34:39PM +0100, Mark Kettenis wrote: > > > Date: Fri, 9 Dec 2022 09:37:11 -0600 > > > From: Scott Cheloha > > > > > > On Thu, Dec 08, 2022 at 11:35:34AM +0100, Jeremie Courreges-Anglas wrote: > > > > On Wed, Dec 07 2022, Scott Cheloha wrote: > > > > > ARMv7 has four interrupt clocks available. I think it'll be easier to > > > > > review/test if we do the clockintr switch driver by driver instead of > > > > > all at once in a massive single email. When all four driver patches > > > > > are confirmed to work, I'll commit them. > > > > > > > > > > Here's a patch to switch agtimer(4/armv7) to clockintr. > > > > > > > > > > - Remove agtimer-specific clock interrupt scheduling bits > > > > > and randomized statclock bits. > > > > > > > > > > - Wire up agtimer_intrclock. > > > > > > > > > > I am looking for a tester to help me get it compiling, > > > > > > > > Fails to build because of a signature mismatch for agtimer_trigger(), > > > > updated diff below. > > > > > > > > > and then run it > > > > > through a kernel-release-upgrade cycle. > > > > > > > > That's not what you're asking for, but no regression spotted on a cubox > > > > machine - which seems to use amptimer(4) according to dmesg. > > > > > > Thanks miod@/jca@! > > > > > > So right now are looking for a tester for the attached patch, which > > > *does* compile. > > > > > > An armv7 machine with agtimer(4/armv7). > > > > A fixed version (I applied your original diff and then fixed it up), > > works on a Banana Pi with an Allwinner A20 that uses agtimer(4). > > > > ok kettenis@ > > Thanks for testing! > > To ensure that the patch committed is the one that actually works, > please send me your fixed-up patch. Already packed away my board. I just adjusted the agtimer_trigger() prototype. > Also, did you build a release and upgrade from it? If that's not > possible (or it would take weeks) I can proceed as-is... but I would > like to avoid breaking snaps and getting yelled at. More effort than I'm willing to spend on it (a;ready moved on to the A10 board). But Theo uses a cubox-i to build snaps/release, so you should be safe here ;). > You have some time, though. I can't commit any of the four arm/armv7 > "clockintr switch" patches until they're all confirmed to work.
Re: agtimer(4/armv7): switch to clockintr
On Sun, Dec 11, 2022 at 01:34:39PM +0100, Mark Kettenis wrote: > > Date: Fri, 9 Dec 2022 09:37:11 -0600 > > From: Scott Cheloha > > > > On Thu, Dec 08, 2022 at 11:35:34AM +0100, Jeremie Courreges-Anglas wrote: > > > On Wed, Dec 07 2022, Scott Cheloha wrote: > > > > ARMv7 has four interrupt clocks available. I think it'll be easier to > > > > review/test if we do the clockintr switch driver by driver instead of > > > > all at once in a massive single email. When all four driver patches > > > > are confirmed to work, I'll commit them. > > > > > > > > Here's a patch to switch agtimer(4/armv7) to clockintr. > > > > > > > > - Remove agtimer-specific clock interrupt scheduling bits > > > > and randomized statclock bits. > > > > > > > > - Wire up agtimer_intrclock. > > > > > > > > I am looking for a tester to help me get it compiling, > > > > > > Fails to build because of a signature mismatch for agtimer_trigger(), > > > updated diff below. > > > > > > > and then run it > > > > through a kernel-release-upgrade cycle. > > > > > > That's not what you're asking for, but no regression spotted on a cubox > > > machine - which seems to use amptimer(4) according to dmesg. > > > > Thanks miod@/jca@! > > > > So right now are looking for a tester for the attached patch, which > > *does* compile. > > > > An armv7 machine with agtimer(4/armv7). > > A fixed version (I applied your original diff and then fixed it up), > works on a Banana Pi with an Allwinner A20 that uses agtimer(4). > > ok kettenis@ Thanks for testing! To ensure that the patch committed is the one that actually works, please send me your fixed-up patch. Also, did you build a release and upgrade from it? If that's not possible (or it would take weeks) I can proceed as-is... but I would like to avoid breaking snaps and getting yelled at. You have some time, though. I can't commit any of the four arm/armv7 "clockintr switch" patches until they're all confirmed to work.
Re: agtimer(4/armv7): switch to clockintr
> Date: Fri, 9 Dec 2022 09:37:11 -0600
> From: Scott Cheloha
>
> On Thu, Dec 08, 2022 at 11:35:34AM +0100, Jeremie Courreges-Anglas wrote:
> > On Wed, Dec 07 2022, Scott Cheloha wrote:
> > > ARMv7 has four interrupt clocks available. I think it'll be easier to
> > > review/test if we do the clockintr switch driver by driver instead of
> > > all at once in a massive single email. When all four driver patches
> > > are confirmed to work, I'll commit them.
> > >
> > > Here's a patch to switch agtimer(4/armv7) to clockintr.
> > >
> > > - Remove agtimer-specific clock interrupt scheduling bits
> > > and randomized statclock bits.
> > >
> > > - Wire up agtimer_intrclock.
> > >
> > > I am looking for a tester to help me get it compiling,
> >
> > Fails to build because of a signature mismatch for agtimer_trigger(),
> > updated diff below.
> >
> > > and then run it
> > > through a kernel-release-upgrade cycle.
> >
> > That's not what you're asking for, but no regression spotted on a cubox
> > machine - which seems to use amptimer(4) according to dmesg.
>
> Thanks miod@/jca@!
>
> So right now are looking for a tester for the attached patch, which
> *does* compile.
>
> An armv7 machine with agtimer(4/armv7).
A fixed version (I applied your original diff and then fixed it up),
works on a Banana Pi with an Allwinner A20 that uses agtimer(4).
ok kettenis@
> Index: sys/arch/arm/include/cpu.h
> ===
> RCS file: /cvs/src/sys/arch/arm/include/cpu.h,v
> retrieving revision 1.61
> diff -u -p -r1.61 cpu.h
> --- sys/arch/arm/include/cpu.h6 Jul 2021 09:34:06 - 1.61
> +++ sys/arch/arm/include/cpu.h9 Dec 2022 15:35:11 -
> @@ -149,6 +149,7 @@ void arm32_vector_init(vaddr_t, int);
> * Per-CPU information. For now we assume one CPU.
> */
>
> +#include
> #include
> #include
> #include
> @@ -198,7 +199,7 @@ struct cpu_info {
> #ifdef GPROF
> struct gmonparam *ci_gmon;
> #endif
> -
> + struct clockintr_queue ci_queue;
> charci_panicbuf[512];
> };
>
> Index: sys/arch/arm/include/_types.h
> ===
> RCS file: /cvs/src/sys/arch/arm/include/_types.h,v
> retrieving revision 1.19
> diff -u -p -r1.19 _types.h
> --- sys/arch/arm/include/_types.h 5 Mar 2018 01:15:25 - 1.19
> +++ sys/arch/arm/include/_types.h 9 Dec 2022 15:35:11 -
> @@ -35,6 +35,8 @@
> #ifndef _ARM__TYPES_H_
> #define _ARM__TYPES_H_
>
> +#define __HAVE_CLOCKINTR
> +
> #if defined(_KERNEL)
> typedef struct label_t {
> long val[11];
> Index: sys/arch/arm/cortex/agtimer.c
> ===
> RCS file: /cvs/src/sys/arch/arm/cortex/agtimer.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 agtimer.c
> --- sys/arch/arm/cortex/agtimer.c 12 Mar 2022 14:40:41 - 1.15
> +++ sys/arch/arm/cortex/agtimer.c 9 Dec 2022 15:35:11 -
> @@ -18,8 +18,10 @@
>
> #include
> #include
> +#include
> #include
> #include
> +#include
> #include
>
> #include
> @@ -51,28 +53,12 @@ static struct timecounter agtimer_timeco
> .tc_priv = NULL,
> };
>
> -struct agtimer_pcpu_softc {
> - uint64_tpc_nexttickevent;
> - uint64_tpc_nextstatevent;
> - u_int32_t pc_ticks_err_sum;
> -};
> -
> struct agtimer_softc {
> struct device sc_dev;
> int sc_node;
> -
> - struct agtimer_pcpu_softc sc_pstat[MAXCPUS];
> -
> - u_int32_t sc_ticks_err_cnt;
> u_int32_t sc_ticks_per_second;
> - u_int32_t sc_ticks_per_intr;
> - u_int32_t sc_statvar;
> - u_int32_t sc_statmin;
> -
> -#ifdef AMPTIMER_DEBUG
> - struct evcount sc_clk_count;
> - struct evcount sc_stat_count;
> -#endif
> + uint64_tsc_nsec_cycle_ratio;
> + uint64_tsc_nsec_max;
> };
>
> int agtimer_match(struct device *, void *, void *);
> @@ -93,6 +79,14 @@ struct cfdriver agtimer_cd = {
> NULL, "agtimer", DV_DULL
> };
>
> +void agtimer_rearm(void *, uint64_t);
> +void agtimer_trigger(void *);
> +
> +struct intrclock agtimer_intrclock = {
> + .ic_rearm = agtimer_rearm,
> + .ic_trigger = agtimer_trigger
> +};
> +
> uint64_t
> agtimer_readcnt64(void)
> {
> @@ -155,16 +149,13 @@ agtimer_attach(struct device *parent, st
> agtimer_frequency =
> OF_getpropint(sc->sc_node, "clock-frequency", agtimer_frequency);
> sc->sc_ticks_per_second = agtimer_frequency;
> -
> + sc->sc_nsec_cycle_ratio =
> + sc->sc_ticks_per_second * (1ULL << 32) / 10;
> + sc->sc_nsec_max = UINT64_MAX / sc->sc_nsec_cycle_ratio;
> printf(": %d kHz\n", sc->sc_ticks_per_second / 1000);
>
> /* XXX: disable user
