Re: httpd.conf.5 TLS defaults
On Fri, Sep 04, 2020 at 08:48:48PM -0700, na...@airpost.net wrote: > This is TLS v1.2 & 1.3 now. Delete it here, since the referenced man page is > updated. Thanks, I'm ok with this diff. I had the diff below in my tree for a long time (I think it was prompted by a question of tj). I did mention the defaults since the other tls options (except client ca) do: Index: httpd.conf.5 === RCS file: /var/cvs/src/usr.sbin/httpd/httpd.conf.5,v retrieving revision 1.112 diff -u -p -r1.112 httpd.conf.5 --- httpd.conf.524 Aug 2020 15:49:10 - 1.112 +++ httpd.conf.526 Aug 2020 06:41:31 - @@ -649,12 +649,10 @@ is empty, OCSP stapling will not be used The default is to not use OCSP stapling. .It Ic protocols Ar string Specify the TLS protocols to enable for this server. -If not specified, the value -.Qq default -will be used (secure protocols; TLSv1.2-only). Refer to the .Xr tls_config_parse_protocols 3 -function for other valid protocol string values. +function for valid protocol string values. +By default, TLSv1.3 and TLSv1.2 will be used. .It Ic ticket lifetime Ar seconds Enable TLS session tickets with a .Ar seconds
httpd.conf.5 TLS defaults
This is TLS v1.2 & 1.3 now. Delete it here, since the referenced man page is updated. Index: httpd.conf.5 === RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v retrieving revision 1.112 diff -u -p -r1.112 httpd.conf.5 --- httpd.conf.524 Aug 2020 15:49:10 - 1.112 +++ httpd.conf.55 Sep 2020 03:44:14 - @@ -651,7 +651,7 @@ The default is to not use OCSP stapling. Specify the TLS protocols to enable for this server. If not specified, the value .Qq default -will be used (secure protocols; TLSv1.2-only). +will be used. Refer to the .Xr tls_config_parse_protocols 3 function for other valid protocol string values.
Probably useless patch to initial GDT
I doubt this will ever matter, but I think this is correct. I hope this isn't an annoying patch. Just trying to get my feet wet in the kernel. It compiled and ran on my machine for what it's worth. Chris Waddey Index: gidt.S === RCS file: /cvs/src/sys/arch/amd64/stand/libsa/gidt.S,v retrieving revision 1.12 diff -u -p -r1.12 gidt.S --- gidt.S 9 Nov 2019 17:58:46 - 1.12 +++ gidt.S 4 Sep 2020 23:11:23 - @@ -260,14 +260,14 @@ gdt: .byte (LINKADDR >> 16) & 0xff # midbase .byte SDT_MEMERAC | 0 | 0x80 # RXAC, dpl = 0, present .byte 0x0 | 0 | 0 | 0 # hilimit, xx, 16bit, byte granularity - .byte (LINKADDR >> 20) & 0xff # hibase + .byte (LINKADDR >> 24) & 0xff # hibase /* 0x20 : 16 bit data */ .word 0x # lolimit .word (LINKADDR & 0x) # lobase .byte (LINKADDR >> 16) & 0xff # midbase .byte SDT_MEMRWA | 0 | 0x80 # RWA, dpl = 0, present .byte 0x0 | 0 | 0 | 0 # hilimit, xx, 16bit, byte granularity - .byte (LINKADDR >> 20) & 0xff # hibase + .byte (LINKADDR >> 24) & 0xff # hibase .globl Gdtr Gdtr: .word . - gdt - 1
Re: timeout(9): add clock-based timeouts (attempt 2)
On Fri, Sep 04, 2020 at 05:55:40PM -0500, Scott Cheloha wrote:
> On Sat, Jul 25, 2020 at 08:46:08PM -0500, Scott Cheloha wrote:
> >
> > [...]
> >
> > I want to add clock-based timeouts to the kernel because tick-based
> > timeouts suffer from a few problems:
> >
> > [...]
> >
> > Basically, ticks are a poor approximation for the system clock. We
> > should use the real thing where possible.
> >
> > [...]
> >
> > Thoughts on this approach? Thoughts on the proposed API?
>
> 6 week bump.
>
> Attached is an rebased and streamlined diff.
>
> [...]
Updated diff fixes a pasto.
Index: kern/kern_timeout.c
===
RCS file: /cvs/src/sys/kern/kern_timeout.c,v
retrieving revision 1.79
diff -u -p -r1.79 kern_timeout.c
--- kern/kern_timeout.c 7 Aug 2020 00:45:25 - 1.79
+++ kern/kern_timeout.c 5 Sep 2020 01:54:42 -
@@ -1,4 +1,4 @@
-/* $OpenBSD: kern_timeout.c,v 1.79 2020/08/07 00:45:25 cheloha Exp $
*/
+/* $OpenBSD: kern_timeout.c,v 1.77 2020/08/01 08:40:20 anton Exp $ */
/*
* Copyright (c) 2001 Thomas Nordin
* Copyright (c) 2000-2001 Artur Grabowski
@@ -64,16 +64,27 @@ struct timeoutstat tostat; /* [T] stati
* of the global variable "ticks" when the timeout should be called. There are
* four levels with 256 buckets each.
*/
-#define BUCKETS 1024
+#define WHEELCOUNT 4
#define WHEELSIZE 256
#define WHEELMASK 255
#define WHEELBITS 8
+#define BUCKETS (WHEELCOUNT * WHEELSIZE)
-struct circq timeout_wheel[BUCKETS]; /* [T] Queues of timeouts */
+struct circq timeout_wheel[BUCKETS]; /* [T] Tick-based timeouts */
+struct circq timeout_wheel_kc[BUCKETS];/* [T] Clock-based timeouts */
struct circq timeout_new; /* [T] New, unscheduled timeouts */
struct circq timeout_todo; /* [T] Due or needs rescheduling */
struct circq timeout_proc; /* [T] Due + needs process context */
+time_t timeout_level_width[WHEELCOUNT];/* [I] Wheel level width
(seconds) */
+struct timespec tick_ts; /* [I] Length of a tick (1/hz secs) */
+
+struct kclock {
+ struct timespec kc_lastscan;/* [T] Clock time at last wheel scan */
+ struct timespec kc_late;/* [T] Late if due prior */
+ struct timespec kc_offset; /* [T] Offset from primary kclock */
+} timeout_kclock[KCLOCK_MAX];
+
#define MASKWHEEL(wheel, time) (((time) >> ((wheel)*WHEELBITS)) & WHEELMASK)
#define BUCKET(rel, abs) \
@@ -155,9 +166,15 @@ struct lock_type timeout_spinlock_type =
((needsproc) ? _sleeplock_obj : _spinlock_obj)
#endif
+void kclock_nanotime(int, struct timespec *);
void softclock(void *);
void softclock_create_thread(void *);
+void softclock_process_kclock_timeout(struct timeout *, int);
+void softclock_process_tick_timeout(struct timeout *, int);
void softclock_thread(void *);
+uint32_t timeout_bucket(struct timeout *);
+uint32_t timeout_maskwheel(uint32_t, const struct timespec *);
+void timeout_run(struct timeout *);
void timeout_proc_barrier(void *);
/*
@@ -207,13 +224,19 @@ timeout_sync_leave(int needsproc)
void
timeout_startup(void)
{
- int b;
+ int b, level;
CIRCQ_INIT(_new);
CIRCQ_INIT(_todo);
CIRCQ_INIT(_proc);
for (b = 0; b < nitems(timeout_wheel); b++)
CIRCQ_INIT(_wheel[b]);
+ for (b = 0; b < nitems(timeout_wheel_kc); b++)
+ CIRCQ_INIT(_wheel_kc[b]);
+
+ for (level = 0; level < nitems(timeout_level_width); level++)
+ timeout_level_width[level] = 2 << (level * WHEELBITS);
+ NSEC_TO_TIMESPEC(tick_nsec, _ts);
}
void
@@ -229,25 +252,39 @@ timeout_proc_init(void)
kthread_create_deferred(softclock_create_thread, NULL);
}
+static inline void
+_timeout_set(struct timeout *to, void (*fn)(void *), void *arg, int flags,
+int kclock)
+{
+ to->to_func = fn;
+ to->to_arg = arg;
+ to->to_flags = flags | TIMEOUT_INITIALIZED;
+ to->to_kclock = kclock;
+}
+
void
timeout_set(struct timeout *new, void (*fn)(void *), void *arg)
{
- timeout_set_flags(new, fn, arg, 0);
+ _timeout_set(new, fn, arg, 0, KCLOCK_NONE);
}
void
timeout_set_flags(struct timeout *to, void (*fn)(void *), void *arg, int flags)
{
- to->to_func = fn;
- to->to_arg = arg;
- to->to_process = NULL;
- to->to_flags = flags | TIMEOUT_INITIALIZED;
+ _timeout_set(to, fn, arg, flags, KCLOCK_NONE);
}
void
timeout_set_proc(struct timeout *new, void (*fn)(void *), void *arg)
{
- timeout_set_flags(new, fn, arg, TIMEOUT_PROC);
+ _timeout_set(new, fn, arg, TIMEOUT_PROC, KCLOCK_NONE);
+}
+
+void
+timeout_set_kclock(struct timeout *to, void (*fn)(void *), void *arg, int
flags,
+int kclock)
+{
+ _timeout_set(to, fn, arg, flags | TIMEOUT_KCLOCK, kclock);
}
int
@@ -257,6 +294,8 @@ timeout_add(struct timeout
Re: Allow builds on arm64 without DDB
On Fri, Sep 04, 2020 at 08:55:30AM -0600, Theo de Raadt wrote:
> Too much ifdef. Can you try a different approach -- leave the ipi code
> intact, but wrap the call to db_enter() with #ifdef DDB? The ipi won't
> be activated, so it should be fine for it to return 0.
Thanks Theo. I just tried to the follow (what seems to be) the standard
practice across the kernel of segregating all DDB-specific code. Pros
and cons to both approaches, obviously, but I can confirm the less
explicit approach works too:
Index: agintc.c
===
RCS file: /cvs/src/sys/arch/arm64/dev/agintc.c,v
retrieving revision 1.26
diff -u -p -u -p -r1.26 agintc.c
--- agintc.c17 Jul 2020 08:07:33 - 1.26
+++ agintc.c5 Sep 2020 01:32:13 -
@@ -1121,7 +1121,9 @@ int
agintc_ipi_ddb(void *v)
{
/* XXX */
+#ifdef DDB
db_enter();
+#endif
return 1;
}
Index: ampintc.c
===
RCS file: /cvs/src/sys/arch/arm64/dev/ampintc.c,v
retrieving revision 1.19
diff -u -p -u -p -r1.19 ampintc.c
--- ampintc.c 17 Jul 2020 08:07:33 - 1.19
+++ ampintc.c 5 Sep 2020 01:32:13 -
@@ -958,7 +958,9 @@ int
ampintc_ipi_ddb(void *v)
{
/* XXX */
+#ifdef DDB
db_enter();
+#endif
return 1;
}
> Matt Baulch wrote:
>
> > Currently, it's not possible to build on arm64 without the in-kernel
> > debugger.
> > This patch fixes the two offending files:
> >
> > Index: agintc.c
> > ===
> > RCS file: /cvs/src/sys/arch/arm64/dev/agintc.c,v
> > retrieving revision 1.26
> > diff -u -p -u -p -r1.26 agintc.c
> > --- agintc.c17 Jul 2020 08:07:33 - 1.26
> > +++ agintc.c4 Sep 2020 13:18:33 -
> > @@ -156,7 +156,9 @@ struct agintc_softc {
> > struct agintc_dmamem*sc_pend;
> > struct interrupt_controller sc_ic;
> > int sc_ipi_num[2]; /* id for NOP and DDB ipi */
> > +#ifdef DDB
> > int sc_ipi_reason[MAXCPUS]; /* NOP or DDB caused */
> > +#endif
> > void*sc_ipi_irq[2]; /* irqhandle for each ipi */
> > };
> > struct agintc_softc *agintc_sc;
> > @@ -226,7 +228,9 @@ voidagintc_wait_rwp(struct agintc_soft
> > void agintc_r_wait_rwp(struct agintc_softc *sc);
> > uint32_t agintc_r_ictlr(void);
> >
> > +#ifdef DDB
> > intagintc_ipi_ddb(void *v);
> > +#endif
> > intagintc_ipi_nop(void *v);
> > intagintc_ipi_combined(void *);
> > void agintc_send_ipi(struct cpu_info *, int);
> > @@ -576,15 +580,19 @@ agintc_attach(struct device *parent, str
> > sc->sc_ipi_irq[0] = agintc_intr_establish(ipiirq[0],
> > IPL_IPI|IPL_MPSAFE, NULL, agintc_ipi_combined, sc, "ipi");
> > sc->sc_ipi_num[ARM_IPI_NOP] = ipiirq[0];
> > +#ifdef DDB
> > sc->sc_ipi_num[ARM_IPI_DDB] = ipiirq[0];
> > +#endif
> > break;
> > case 2:
> > sc->sc_ipi_irq[0] = agintc_intr_establish(ipiirq[0],
> > IPL_IPI|IPL_MPSAFE, NULL, agintc_ipi_nop, sc, "ipinop");
> > sc->sc_ipi_num[ARM_IPI_NOP] = ipiirq[0];
> > +#ifdef DDB
> > sc->sc_ipi_irq[1] = agintc_intr_establish(ipiirq[1],
> > IPL_IPI|IPL_MPSAFE, NULL, agintc_ipi_ddb, sc, "ipiddb");
> > sc->sc_ipi_num[ARM_IPI_DDB] = ipiirq[1];
> > +#endif
> > break;
> > default:
> > panic("nipi unexpected number %d", nipi);
> > @@ -1117,6 +1125,7 @@ agintc_r_wait_rwp(struct agintc_softc *s
> > }
> >
> > #ifdef MULTIPROCESSOR
> > +#ifdef DDB
> > int
> > agintc_ipi_ddb(void *v)
> > {
> > @@ -1124,6 +1133,7 @@ agintc_ipi_ddb(void *v)
> > db_enter();
> > return 1;
> > }
> > +#endif
> >
> > int
> > agintc_ipi_nop(void *v)
> > @@ -1135,14 +1145,16 @@ agintc_ipi_nop(void *v)
> > int
> > agintc_ipi_combined(void *v)
> > {
> > +#ifdef DDB
> > struct agintc_softc *sc = v;
> >
> > if (sc->sc_ipi_reason[cpu_number()] == ARM_IPI_DDB) {
> > sc->sc_ipi_reason[cpu_number()] = ARM_IPI_NOP;
> > return agintc_ipi_ddb(v);
> > - } else {
> > - return agintc_ipi_nop(v);
> > }
> > +#endif
> > +
> > + return agintc_ipi_nop(v);
> > }
> >
> > void
> > @@ -1154,9 +1166,11 @@ agintc_send_ipi(struct cpu_info *ci, int
> > if (ci == curcpu() && id == ARM_IPI_NOP)
> > return;
> >
> > +#ifdef DDB
> > /* never overwrite IPI_DDB with IPI_NOP */
> > if (id == ARM_IPI_DDB)
> > sc->sc_ipi_reason[ci->ci_cpuid] = id;
> > +#endif
> >
> > /* will only send 1 cpu */
> > sendmask = (ci->ci_mpidr & MPIDR_AFF3) << 16;
> > Index: ampintc.c
> > ===
> > RCS file: /cvs/src/sys/arch/arm64/dev/ampintc.c,v
> > retrieving
Re: incorrect result from getppid for ptraced processes
On Fri, Sep 4, 2020 at 2:59 PM Mateusz Guzik wrote:
> On 9/5/20, Philip Guenther wrote:
> > On Fri, Sep 4, 2020 at 1:06 PM Mateusz Guzik wrote:
> >
> >> On 9/4/20, Vitaliy Makkoveev wrote:
> >> > On Fri, Sep 04, 2020 at 05:24:42PM +0200, Mateusz Guzik wrote:
> >> >> getppid blindly follows the parent pointer and reads the pid.
> >> >>
> >> >> The problem is that ptrace reparents the traced process, so in
> >> >> particular if you gdb -p $something, the target proc will start
> seeing
> >> >> gdb instead of its actual parent.
> >> >>
> >> >> There is a lot to say about the entire reparenting business or
> storing
> >> >> the original pid in ps_oppid (instead of some form of a reference to
> >> >> the process).
> >> >>
> >> >> However, I think the most feasible fix for now is the same thing
> >> >> FreeBSD did: *always* store the actual parent pid in ps_oppid. This
> >> >> means all repareting will keep updating it (most notably when
> >> >> abandoning children on exit), while ptrace will skip that part.
> >> >>
> >> >> Side effect of such a change be that getppid will stop requiring the
> >> >> kernel lock.
> >> >>
> >> >
> >> > Thanks for report. But we are in beta stage now so such modification
> is
> >> > impossible until next iteration.
> >> >
> >> > Since original parent identifier is stored as `ps_oppid' while process
> >> > is traced we just return it to userland for this case. This is the way
> >> > I
> >> > propose to fix this bug for now.
> >> >
> >> > Comments? OKs?
> >> >
> >> > Index: sys/kern/kern_prot.c
> >> > ===
> >> > RCS file: /cvs/src/sys/kern/kern_prot.c,v
> >> > retrieving revision 1.76
> >> > diff -u -p -r1.76 kern_prot.c
> >> > --- sys/kern/kern_prot.c 9 Jul 2019 12:23:25 - 1.76
> >> > +++ sys/kern/kern_prot.c 4 Sep 2020 21:12:15 -
> >> > @@ -84,7 +84,11 @@ int
> >> > sys_getppid(struct proc *p, void *v, register_t *retval)
> >> > {
> >> >
> >> > - *retval = p->p_p->ps_pptr->ps_pid;
> >> > + if (p->p_p->ps_flags & PS_TRACED)
> >> > + *retval = p->p_p->ps_oppid;
> >> > + else
> >> > + *retval = p->p_p->ps_pptr->ps_pid;
> >> > +
> >> > return (0);
> >> > }
> >>
> >> This is definitely a bare minimum fix, but it does the job.
> >>
> >
> > ptrace() has behaved like this for the life of OpenBSD and an indefinite
> > number of years previous in the BSD releases. What has happened that a
> > definitely incomplete fix is needed Right Now?
>
> I don't see how this reads as a demand this is fixed Right Now.
>
I didn't call it a demand, but the point stands: what has changed?
I don't see how the fix is incomplete either. It can be done better
> with more effort, but AFAICS the above results in correct behavior.
>
There are at least 2 other uses of ps_pptr->ps_pid that should also change,
unless you like coredumps and ps disagreeing with getppid(), and someone
needs to think how it affects doas.
Philip Guenther
Re: incorrect result from getppid for ptraced processes
On 9/5/20, Philip Guenther wrote:
> On Fri, Sep 4, 2020 at 1:06 PM Mateusz Guzik wrote:
>
>> On 9/4/20, Vitaliy Makkoveev wrote:
>> > On Fri, Sep 04, 2020 at 05:24:42PM +0200, Mateusz Guzik wrote:
>> >> getppid blindly follows the parent pointer and reads the pid.
>> >>
>> >> The problem is that ptrace reparents the traced process, so in
>> >> particular if you gdb -p $something, the target proc will start seeing
>> >> gdb instead of its actual parent.
>> >>
>> >> There is a lot to say about the entire reparenting business or storing
>> >> the original pid in ps_oppid (instead of some form of a reference to
>> >> the process).
>> >>
>> >> However, I think the most feasible fix for now is the same thing
>> >> FreeBSD did: *always* store the actual parent pid in ps_oppid. This
>> >> means all repareting will keep updating it (most notably when
>> >> abandoning children on exit), while ptrace will skip that part.
>> >>
>> >> Side effect of such a change be that getppid will stop requiring the
>> >> kernel lock.
>> >>
>> >
>> > Thanks for report. But we are in beta stage now so such modification is
>> > impossible until next iteration.
>> >
>> > Since original parent identifier is stored as `ps_oppid' while process
>> > is traced we just return it to userland for this case. This is the way
>> > I
>> > propose to fix this bug for now.
>> >
>> > Comments? OKs?
>> >
>> > Index: sys/kern/kern_prot.c
>> > ===
>> > RCS file: /cvs/src/sys/kern/kern_prot.c,v
>> > retrieving revision 1.76
>> > diff -u -p -r1.76 kern_prot.c
>> > --- sys/kern/kern_prot.c 9 Jul 2019 12:23:25 - 1.76
>> > +++ sys/kern/kern_prot.c 4 Sep 2020 21:12:15 -
>> > @@ -84,7 +84,11 @@ int
>> > sys_getppid(struct proc *p, void *v, register_t *retval)
>> > {
>> >
>> > - *retval = p->p_p->ps_pptr->ps_pid;
>> > + if (p->p_p->ps_flags & PS_TRACED)
>> > + *retval = p->p_p->ps_oppid;
>> > + else
>> > + *retval = p->p_p->ps_pptr->ps_pid;
>> > +
>> > return (0);
>> > }
>>
>> This is definitely a bare minimum fix, but it does the job.
>>
>
> ptrace() has behaved like this for the life of OpenBSD and an indefinite
> number of years previous in the BSD releases. What has happened that a
> definitely incomplete fix is needed Right Now?
>
I don't see how this reads as a demand this is fixed Right Now.
I don't see how the fix is incomplete either. It can be done better
with more effort, but AFAICS the above results in correct behavior.
--
Mateusz Guzik
Re: incorrect result from getppid for ptraced processes
On Fri, Sep 4, 2020 at 1:06 PM Mateusz Guzik wrote:
> On 9/4/20, Vitaliy Makkoveev wrote:
> > On Fri, Sep 04, 2020 at 05:24:42PM +0200, Mateusz Guzik wrote:
> >> getppid blindly follows the parent pointer and reads the pid.
> >>
> >> The problem is that ptrace reparents the traced process, so in
> >> particular if you gdb -p $something, the target proc will start seeing
> >> gdb instead of its actual parent.
> >>
> >> There is a lot to say about the entire reparenting business or storing
> >> the original pid in ps_oppid (instead of some form of a reference to
> >> the process).
> >>
> >> However, I think the most feasible fix for now is the same thing
> >> FreeBSD did: *always* store the actual parent pid in ps_oppid. This
> >> means all repareting will keep updating it (most notably when
> >> abandoning children on exit), while ptrace will skip that part.
> >>
> >> Side effect of such a change be that getppid will stop requiring the
> >> kernel lock.
> >>
> >
> > Thanks for report. But we are in beta stage now so such modification is
> > impossible until next iteration.
> >
> > Since original parent identifier is stored as `ps_oppid' while process
> > is traced we just return it to userland for this case. This is the way I
> > propose to fix this bug for now.
> >
> > Comments? OKs?
> >
> > Index: sys/kern/kern_prot.c
> > ===
> > RCS file: /cvs/src/sys/kern/kern_prot.c,v
> > retrieving revision 1.76
> > diff -u -p -r1.76 kern_prot.c
> > --- sys/kern/kern_prot.c 9 Jul 2019 12:23:25 - 1.76
> > +++ sys/kern/kern_prot.c 4 Sep 2020 21:12:15 -
> > @@ -84,7 +84,11 @@ int
> > sys_getppid(struct proc *p, void *v, register_t *retval)
> > {
> >
> > - *retval = p->p_p->ps_pptr->ps_pid;
> > + if (p->p_p->ps_flags & PS_TRACED)
> > + *retval = p->p_p->ps_oppid;
> > + else
> > + *retval = p->p_p->ps_pptr->ps_pid;
> > +
> > return (0);
> > }
>
> This is definitely a bare minimum fix, but it does the job.
>
ptrace() has behaved like this for the life of OpenBSD and an indefinite
number of years previous in the BSD releases. What has happened that a
definitely incomplete fix is needed Right Now?
Philip Guenther
Re: timeout(9): add clock-based timeouts (attempt 2)
On Sat, Jul 25, 2020 at 08:46:08PM -0500, Scott Cheloha wrote: > > [...] > > I want to add clock-based timeouts to the kernel because tick-based > timeouts suffer from a few problems: > > [...] > > Basically, ticks are a poor approximation for the system clock. We > should use the real thing where possible. > > [...] > > Thoughts on this approach? Thoughts on the proposed API? 6 week bump. Attached is an rebased and streamlined diff. Let's try again: This patch adds support for timeouts scheduled against the hardware timecounter. I call these "kclock timeouts". They are distinct from the current tick-based timeouts because ticks are "software time", not "real time". For now we have one kclock, KCLOCK_UPTIME, which corresponds to nanouptime(9). In the future I intend to add support for runtime and UTC kclocks. Why do we want kclock timeouts at all? 1. Kclock timeouts expire at an actual time, not a tick. They have nanosecond resolution and are NTP-sensitive. Thus, they will *never* fire early. 2. One upshot of nanosecond resolution is that we don't need to "round up" to the next tick when scheduling a timeout to prevent early execution. The extra resolution allows us to reduce latency in some contexts. 3. Kclock timeouts cover the entire range of the kernel timeline. We can remove the "tick loops" like the one sys_nanosleep(). 4. Kclock timeouts are scheduled as absolute deadlines. This makes supporting absolute timeouts trivial, which means we can add support for clock_nanosleep(2) and the absolute pthread timeouts to the kernel. Kclock timeouts aren't actually used anywhere yet, so merging this patch will not break anything like last time (CC bluhm@). In a subsequent diff I will put them to use in tsleep_nsec(9) etc. This will enable those interfaces to block for less than a tick, which in turn will allow userspace to block for less than a tick in e.g. futex(2), and poll(2). pd@ has verified that this fixes the "time problem" in OpenBSD vmm(4) VMs (CC pd@). You initialize kclock timeouts with timeout_set_kclock(). You schedule them with timeout_in_nsec(), a relative timeout interface that accepts a count of nanoseconds. If your timeout is in some other unit (seconds, milliseconds, whatever) you must convert it to nanoseconds before scheduling. Something like this will work: timeout_in_nsec(_timeout, SEC_TO_NSEC(1)); There won't be a flavored API supporting every conceivable time unit. In the future I will expose an absolute timeout interface and a periodic timeout rescheduling interface. We don't need either of these interfaces to start, though. Tick-based timeouts and kclock-based timeouts are *not* compatible. You cannot schedule a kclock timeout with timeout_add(9). You cannot schedule a tick-based timeout with timeout_in_nsec(9). I have added KASSERTs to prevent this. Scheduling a kclock timeout with timeout_in_nsec() is more costly than scheduling a tick-based timeout with timeout_add(9) because you have to read the hardware timecounter. The cost will vary with your clock: bad clocks have lots of overhead, good clocks have low-to-no overhead. The programmer will need to decide if the potential overhead is too high when employing these timeouts. In most cases the overhead will not be a problem. The network stack is one spot where it might be. Processing the kclock timeout wheel during hardclock(9) adds negligible overhead to that routine. Processing a kclock timeout during softclock() is roughly 4 times as expensive as processing a tick-based timeout. At idle on my 2Ghz amd64 machine tick-based timeouts take ~125 cycles to process while kclock timeouts take ~500 cycles. The average cost seems to drop as more kclock timeouts are processed, though I can't really explain why. Thoughts? ok? Index: kern/kern_timeout.c === RCS file: /cvs/src/sys/kern/kern_timeout.c,v retrieving revision 1.79 diff -u -p -r1.79 kern_timeout.c --- kern/kern_timeout.c 7 Aug 2020 00:45:25 - 1.79 +++ kern/kern_timeout.c 4 Sep 2020 22:41:12 - @@ -1,4 +1,4 @@ -/* $OpenBSD: kern_timeout.c,v 1.79 2020/08/07 00:45:25 cheloha Exp $ */ +/* $OpenBSD: kern_timeout.c,v 1.77 2020/08/01 08:40:20 anton Exp $ */ /* * Copyright (c) 2001 Thomas Nordin * Copyright (c) 2000-2001 Artur Grabowski @@ -64,16 +64,27 @@ struct timeoutstat tostat; /* [T] stati * of the global variable "ticks" when the timeout should be called. There are * four levels with 256 buckets each. */ -#define BUCKETS 1024 +#define WHEELCOUNT 4 #define WHEELSIZE 256 #define WHEELMASK 255 #define WHEELBITS 8 +#define BUCKETS (WHEELCOUNT * WHEELSIZE) -struct circq timeout_wheel[BUCKETS]; /* [T] Queues of timeouts */ +struct circq timeout_wheel[BUCKETS]; /* [T] Tick-based timeouts */ +struct circq timeout_wheel_kc[BUCKETS];/* [T] Clock-based timeouts */ struct circq
Re: incorrect result from getppid for ptraced processes
On 9/4/20, Vitaliy Makkoveev wrote:
> On Fri, Sep 04, 2020 at 05:24:42PM +0200, Mateusz Guzik wrote:
>> getppid blindly follows the parent pointer and reads the pid.
>>
>> The problem is that ptrace reparents the traced process, so in
>> particular if you gdb -p $something, the target proc will start seeing
>> gdb instead of its actual parent.
>>
>> There is a lot to say about the entire reparenting business or storing
>> the original pid in ps_oppid (instead of some form of a reference to
>> the process).
>>
>> However, I think the most feasible fix for now is the same thing
>> FreeBSD did: *always* store the actual parent pid in ps_oppid. This
>> means all repareting will keep updating it (most notably when
>> abandoning children on exit), while ptrace will skip that part.
>>
>> Side effect of such a change be that getppid will stop requiring the
>> kernel lock.
>>
>
> Thanks for report. But we are in beta stage now so such modification is
> impossible until next iteration.
>
> Since original parent identifier is stored as `ps_oppid' while process
> is traced we just return it to userland for this case. This is the way I
> propose to fix this bug for now.
>
> Comments? OKs?
>
> Index: sys/kern/kern_prot.c
> ===
> RCS file: /cvs/src/sys/kern/kern_prot.c,v
> retrieving revision 1.76
> diff -u -p -r1.76 kern_prot.c
> --- sys/kern/kern_prot.c 9 Jul 2019 12:23:25 - 1.76
> +++ sys/kern/kern_prot.c 4 Sep 2020 21:12:15 -
> @@ -84,7 +84,11 @@ int
> sys_getppid(struct proc *p, void *v, register_t *retval)
> {
>
> - *retval = p->p_p->ps_pptr->ps_pid;
> + if (p->p_p->ps_flags & PS_TRACED)
> + *retval = p->p_p->ps_oppid;
> + else
> + *retval = p->p_p->ps_pptr->ps_pid;
> +
> return (0);
> }
>
>
This is definitely a bare minimum fix, but it does the job.
--
Mateusz Guzik
dmesg submitting tool
A problem I see: It's often hard to submit a dmesg.
These days, sending e-mail from an arbitrary machine is "difficult",
as it usually requires use of an e-mail relay. Gathering and sending
a dmesg via many common e-mail clients often ends up mangling the
dmesg in various ways.
Goals for a solution:
* Should not create any additional work for developers.
* Should not require any significant additions to the most basic
OpenBSD install.
* Should be able to hunt for a port with clear, outbound access.
* Should use a sane protocol (not FTP).
* Should ultimately deliver a familiar, useful, plain text e-mail to
OpenBSD developers.
* Should do some basic checking to minimize spam and mangled dmesgs
from ever hitting the OpenBSD Developers.
* Should allow users to review what is about to be sent to OpenBSD.
* Should be stupidly fast and simple for users to use.
* Should be possible to become part of the base OpenBSD install if
decided useful and desirable.
What I came up with:
The script, "senddmesg" and a system to process the submissions,
forwarding on via e-mail to dm...@openbsd.org
Protocol: The best option would probably be an HTTP(s) PUT, but the
tools to do that are not part of OpenBSD's base install (well...probably
could be done by perl, but that's above my skill set). So...looks to me
like sftp is the answer. Of course, many networks block port 22, so I
set up a redirect on the receiver to allow SFTP over ports 443, 80, and
53 in addition to port 22.
The sending script starts by checking if it is running as root. If not,
it advises the user that's ok, but less data can be sent, and gives the
user an opportunity to cancel and re-run as root. It then tries to to do
an SFTP connection to the target system over each of the ports in order
of likelihood for success. A very short timeout is used (currently one
second) so that if the SFTP client can't get through, it fails and moves
on quickly.
Once a viable port is found, dmesg, usbdevs, pcidump and sysctl hw is
run, collected into a mktemp(1) file, and the user is put into $EDITOR
or vi to look at what is being sent. When they exit vi, they are
prompted if they wish to send the file or not.
If they wish to proceed, the file is gzip'd, and sftp'd to the dmesg
collection system. A two second delay takes place, and a another SFTP
connection is made to try to retrieve a status file, which will report
if the file was accepted and forwarded on to dm...@openbsd.org or
rejected
On the receiving end, a script looks for new, uploaded files. If the
file has been received matches the filename format of a likely dmesg
upload, it's unpacked, checked for a few basic things, and if it checks
out, the dmesg file is forwarded to the OpenBSD developers. Files are
left where they could be accessed by another user less than one second,
files are uploaded with randomized names (from mktemp(1)), and the
directory the chroot user can write to is mode 370, so the uploading
user can't see other uploads that haven't been processed yet.
A Demo system is set up at dmesg.egoslike.us
You can download the senddmesg script:
$ sftp dm...@dmesg.egoslike.us:/outgoing/senddmesg
It's a fairly simple ksh script, look it over to make sure I'm not
doing something malicious or stupid, then run it to send a dmesg to *me*.
If you use it to send a dmesg, IT IS NOT GOING TO THE OPENBSD DEVELOPERS
AT THIS TIME. I'm not auto-forwarding e-mail until a developer says
"do it".
The dmesg user has a 30kB filesize ulimit and no password, and a few
tricks to make it difficult to use it to pass messages from user to
user via this system.
This is a proof of concept only. I really don't like the current
senddmesg script, there are quite a few things I want it to do better,
including having options for non-interactive ("just send the dang
thing!") and command line driven. However, before I spend a lot
of time making it suck less, I would like some indication that
developers think this is worth going further with.
IF developers wish me to go forward with this, we can talk about
hosting options (or keep using this system, a VPS hosted by Vultr).
I am willing to keep running it for the project.
Nick.
Re: incorrect result from getppid for ptraced processes
On Fri, Sep 04, 2020 at 05:24:42PM +0200, Mateusz Guzik wrote:
> getppid blindly follows the parent pointer and reads the pid.
>
> The problem is that ptrace reparents the traced process, so in
> particular if you gdb -p $something, the target proc will start seeing
> gdb instead of its actual parent.
>
> There is a lot to say about the entire reparenting business or storing
> the original pid in ps_oppid (instead of some form of a reference to
> the process).
>
> However, I think the most feasible fix for now is the same thing
> FreeBSD did: *always* store the actual parent pid in ps_oppid. This
> means all repareting will keep updating it (most notably when
> abandoning children on exit), while ptrace will skip that part.
>
> Side effect of such a change be that getppid will stop requiring the
> kernel lock.
>
Thanks for report. But we are in beta stage now so such modification is
impossible until next iteration.
Since original parent identifier is stored as `ps_oppid' while process
is traced we just return it to userland for this case. This is the way I
propose to fix this bug for now.
Comments? OKs?
Index: sys/kern/kern_prot.c
===
RCS file: /cvs/src/sys/kern/kern_prot.c,v
retrieving revision 1.76
diff -u -p -r1.76 kern_prot.c
--- sys/kern/kern_prot.c9 Jul 2019 12:23:25 - 1.76
+++ sys/kern/kern_prot.c4 Sep 2020 21:12:15 -
@@ -84,7 +84,11 @@ int
sys_getppid(struct proc *p, void *v, register_t *retval)
{
- *retval = p->p_p->ps_pptr->ps_pid;
+ if (p->p_p->ps_flags & PS_TRACED)
+ *retval = p->p_p->ps_oppid;
+ else
+ *retval = p->p_p->ps_pptr->ps_pid;
+
return (0);
}
Re: [PATCH] Add IOMMU support for Intel VT-d and AMD-Vi
> Date: Fri, 4 Sep 2020 00:50:44 -0500
> From: Jordan Hargrave
A few hints below...
> > > +
> > > +/* Page Table Entry per domain */
> > > +static struct ivhd_dte hwdte[65536] __aligned(PAGE_SIZE);
> > > +
> > > +/* Alias mapping */
> > > +#define SID_INVALID 0x8000L
> > > +static uint32_t sid_flag[65536];
> >
> > Can we avoid having these large arrays, or at least allocate them
> > dynamically? That would also avoid the explicit alignment which is
> > somewhat nasty since it affects the entire kernel.
>
> OK. But the hwdte does need the 2M area to be all contiguous but it is not
> needed for DMAR/Intel. You *can* have up to 8 different device table entries
> though to split up the area.
The appropriate interface to use in this context is
bus_dmamem_alloc(9). You can specify alignment, and if you set nsegs
to 1, you will get memory that is physicaly contiguous.
To map the memory into kernel address space you'll need create a map
using bus_dmamap_create(9) and map it using bus_dmamem_map(9). Then
instead of using pmap_extract(9) you use bus_dmamap_load_raw(9) which
then populates the physical addresses.
Many of the drivers written by dlg@ define convenience functions to do
all these steps, although interestingly enough he tends to use
bus_dmamap_load(9) instead of bus_dmamap_load_raw(9) which is
sub-optimal.
> > > +
> > > +struct domain_dev {
> > > + int sid;
> > > + int sec;
> > > + int sub;
> > > + TAILQ_ENTRY(domain_dev) link;
> > > +};
> > > +
> > > +struct domain {
> > > + struct iommu_softc *iommu;
> > > + int did;
> > > + int gaw;
> > > + struct pte_entry*pte;
> > > + paddr_t ptep;
> > > + struct bus_dma_tag dmat;
> > > + int flag;
> > > +
> > > + struct mutexexlck;
> > > + charexname[32];
> > > + struct extent *iovamap;
> > > + TAILQ_HEAD(,domain_dev) devices;
> > > + TAILQ_ENTRY(domain) link;
> > > +};
> > > +
> > > +#define DOM_DEBUG 0x1
> > > +#define DOM_NOMAP 0x2
> > > +
> > > +struct dmar_devlist {
> > > + int type;
> > > + int bus;
> > > + int ndp;
> > > + struct acpidmar_devpath *dp;
> > > + TAILQ_ENTRY(dmar_devlist) link;
> > > +};
> > > +
> > > +TAILQ_HEAD(devlist_head, dmar_devlist);
> > > +
> > > +struct ivhd_devlist {
> > > + int start_id;
> > > + int end_id;
> > > + int cfg;
> > > + TAILQ_ENTRY(ivhd_devlist) link;
> > > +};
> > > +
> > > +struct rmrr_softc {
> > > + TAILQ_ENTRY(rmrr_softc) link;
> > > + struct devlist_head devices;
> > > + int segment;
> > > + uint64_tstart;
> > > + uint64_tend;
> > > +};
> > > +
> > > +struct atsr_softc {
> > > + TAILQ_ENTRY(atsr_softc) link;
> > > + struct devlist_head devices;
> > > + int segment;
> > > + int flags;
> > > +};
> > > +
> > > +struct iommu_pic {
> > > + struct pic pic;
> > > + struct iommu_softc *iommu;
> > > +};
> > > +
> > > +#define IOMMU_FLAGS_CATCHALL 0x1
> > > +#define IOMMU_FLAGS_BAD 0x2
> > > +#define IOMMU_FLAGS_SUSPEND 0x4
> > > +
> > > +struct iommu_softc {
> > > + TAILQ_ENTRY(iommu_softc)link;
> > > + struct devlist_head devices;
> > > + int id;
> > > + int flags;
> > > + int segment;
> > > +
> > > + struct mutexreg_lock;
> > > +
> > > + bus_space_tag_t iot;
> > > + bus_space_handle_t ioh;
> > > +
> > > + uint64_tcap;
> > > + uint64_tecap;
> > > + uint32_tgcmd;
> > > +
> > > + int mgaw;
> > > + int agaw;
> > > + int ndoms;
> > > +
> > > + struct root_entry *root;
> > > + struct context_entry*ctx[256];
> > > +
> > > + void*intr;
> > > + struct iommu_picpic;
> > > + int fedata;
> > > + uint64_tfeaddr;
> > > + uint64_trtaddr;
> > > +
> > > + // Queued Invalidation
> > > + int qi_head;
> > > + int qi_tail;
> > > + paddr_t qip;
> > > + struct qi_entry *qi;
> > > +
> > > + struct domain *unity;
> > > + TAILQ_HEAD(,domain) domains;
> > > +
> > > + // AMD iommu
> > > + struct ivhd_dte *dte;
> > > + void*cmd_tbl;
> > > + void*evt_tbl;
> > > + paddr_t cmd_tblp;
> > > + paddr_t evt_tblp;
> > > + uint64_twv[128] __aligned(4096);
> >
> > This wv array isn't used as far as I can tell.
>
> Ah I was doing some testing on the
incorrect result from getppid for ptraced processes
getppid blindly follows the parent pointer and reads the pid. The problem is that ptrace reparents the traced process, so in particular if you gdb -p $something, the target proc will start seeing gdb instead of its actual parent. There is a lot to say about the entire reparenting business or storing the original pid in ps_oppid (instead of some form of a reference to the process). However, I think the most feasible fix for now is the same thing FreeBSD did: *always* store the actual parent pid in ps_oppid. This means all repareting will keep updating it (most notably when abandoning children on exit), while ptrace will skip that part. Side effect of such a change be that getppid will stop requiring the kernel lock. -- Mateusz Guzik
Re: Allow builds on arm64 without DDB
Too much ifdef. Can you try a different approach -- leave the ipi code
intact, but wrap the call to db_enter() with #ifdef DDB? The ipi won't
be activated, so it should be fine for it to return 0.
Matt Baulch wrote:
> Currently, it's not possible to build on arm64 without the in-kernel debugger.
> This patch fixes the two offending files:
>
> Index: agintc.c
> ===
> RCS file: /cvs/src/sys/arch/arm64/dev/agintc.c,v
> retrieving revision 1.26
> diff -u -p -u -p -r1.26 agintc.c
> --- agintc.c 17 Jul 2020 08:07:33 - 1.26
> +++ agintc.c 4 Sep 2020 13:18:33 -
> @@ -156,7 +156,9 @@ struct agintc_softc {
> struct agintc_dmamem*sc_pend;
> struct interrupt_controller sc_ic;
> int sc_ipi_num[2]; /* id for NOP and DDB ipi */
> +#ifdef DDB
> int sc_ipi_reason[MAXCPUS]; /* NOP or DDB caused */
> +#endif
> void*sc_ipi_irq[2]; /* irqhandle for each ipi */
> };
> struct agintc_softc *agintc_sc;
> @@ -226,7 +228,9 @@ void agintc_wait_rwp(struct agintc_soft
> void agintc_r_wait_rwp(struct agintc_softc *sc);
> uint32_t agintc_r_ictlr(void);
>
> +#ifdef DDB
> int agintc_ipi_ddb(void *v);
> +#endif
> int agintc_ipi_nop(void *v);
> int agintc_ipi_combined(void *);
> void agintc_send_ipi(struct cpu_info *, int);
> @@ -576,15 +580,19 @@ agintc_attach(struct device *parent, str
> sc->sc_ipi_irq[0] = agintc_intr_establish(ipiirq[0],
> IPL_IPI|IPL_MPSAFE, NULL, agintc_ipi_combined, sc, "ipi");
> sc->sc_ipi_num[ARM_IPI_NOP] = ipiirq[0];
> +#ifdef DDB
> sc->sc_ipi_num[ARM_IPI_DDB] = ipiirq[0];
> +#endif
> break;
> case 2:
> sc->sc_ipi_irq[0] = agintc_intr_establish(ipiirq[0],
> IPL_IPI|IPL_MPSAFE, NULL, agintc_ipi_nop, sc, "ipinop");
> sc->sc_ipi_num[ARM_IPI_NOP] = ipiirq[0];
> +#ifdef DDB
> sc->sc_ipi_irq[1] = agintc_intr_establish(ipiirq[1],
> IPL_IPI|IPL_MPSAFE, NULL, agintc_ipi_ddb, sc, "ipiddb");
> sc->sc_ipi_num[ARM_IPI_DDB] = ipiirq[1];
> +#endif
> break;
> default:
> panic("nipi unexpected number %d", nipi);
> @@ -1117,6 +1125,7 @@ agintc_r_wait_rwp(struct agintc_softc *s
> }
>
> #ifdef MULTIPROCESSOR
> +#ifdef DDB
> int
> agintc_ipi_ddb(void *v)
> {
> @@ -1124,6 +1133,7 @@ agintc_ipi_ddb(void *v)
> db_enter();
> return 1;
> }
> +#endif
>
> int
> agintc_ipi_nop(void *v)
> @@ -1135,14 +1145,16 @@ agintc_ipi_nop(void *v)
> int
> agintc_ipi_combined(void *v)
> {
> +#ifdef DDB
> struct agintc_softc *sc = v;
>
> if (sc->sc_ipi_reason[cpu_number()] == ARM_IPI_DDB) {
> sc->sc_ipi_reason[cpu_number()] = ARM_IPI_NOP;
> return agintc_ipi_ddb(v);
> - } else {
> - return agintc_ipi_nop(v);
> }
> +#endif
> +
> + return agintc_ipi_nop(v);
> }
>
> void
> @@ -1154,9 +1166,11 @@ agintc_send_ipi(struct cpu_info *ci, int
> if (ci == curcpu() && id == ARM_IPI_NOP)
> return;
>
> +#ifdef DDB
> /* never overwrite IPI_DDB with IPI_NOP */
> if (id == ARM_IPI_DDB)
> sc->sc_ipi_reason[ci->ci_cpuid] = id;
> +#endif
>
> /* will only send 1 cpu */
> sendmask = (ci->ci_mpidr & MPIDR_AFF3) << 16;
> Index: ampintc.c
> ===
> RCS file: /cvs/src/sys/arch/arm64/dev/ampintc.c,v
> retrieving revision 1.19
> diff -u -p -u -p -r1.19 ampintc.c
> --- ampintc.c 17 Jul 2020 08:07:33 - 1.19
> +++ ampintc.c 4 Sep 2020 13:18:33 -
> @@ -141,7 +141,9 @@ struct ampintc_softc {
> uint8_t sc_cpu_mask[ICD_ICTR_CPU_M + 1];
> struct evcount sc_spur;
> struct interrupt_controller sc_ic;
> +#ifdef DDB
> int sc_ipi_reason[ICD_ICTR_CPU_M + 1];
> +#endif
> int sc_ipi_num[2];
> };
> struct ampintc_softc *ampintc;
> @@ -196,7 +198,9 @@ void ampintc_intr_barrier(void *);
>
> int ampintc_ipi_combined(void *);
> int ampintc_ipi_nop(void *);
> +#ifdef DDB
> int ampintc_ipi_ddb(void *);
> +#endif
> void ampintc_send_ipi(struct cpu_info *, int);
>
> struct cfattach ampintc_ca = {
> @@ -347,15 +351,19 @@ ampintc_attach(struct device *parent, st
> ampintc_intr_establish(ipiirq[0], IST_EDGE_RISING,
> IPL_IPI|IPL_MPSAFE, NULL, ampintc_ipi_combined, sc, "ipi");
> sc->sc_ipi_num[ARM_IPI_NOP] = ipiirq[0];
> +#ifdef DDB
> sc->sc_ipi_num[ARM_IPI_DDB] = ipiirq[0];
> +#endif
> break;
> case 2:
> ampintc_intr_establish(ipiirq[0], IST_EDGE_RISING,
>
Allow builds on arm64 without DDB
Currently, it's not possible to build on arm64 without the in-kernel debugger.
This patch fixes the two offending files:
Index: agintc.c
===
RCS file: /cvs/src/sys/arch/arm64/dev/agintc.c,v
retrieving revision 1.26
diff -u -p -u -p -r1.26 agintc.c
--- agintc.c17 Jul 2020 08:07:33 - 1.26
+++ agintc.c4 Sep 2020 13:18:33 -
@@ -156,7 +156,9 @@ struct agintc_softc {
struct agintc_dmamem*sc_pend;
struct interrupt_controller sc_ic;
int sc_ipi_num[2]; /* id for NOP and DDB ipi */
+#ifdef DDB
int sc_ipi_reason[MAXCPUS]; /* NOP or DDB caused */
+#endif
void*sc_ipi_irq[2]; /* irqhandle for each ipi */
};
struct agintc_softc *agintc_sc;
@@ -226,7 +228,9 @@ voidagintc_wait_rwp(struct agintc_soft
void agintc_r_wait_rwp(struct agintc_softc *sc);
uint32_t agintc_r_ictlr(void);
+#ifdef DDB
intagintc_ipi_ddb(void *v);
+#endif
intagintc_ipi_nop(void *v);
intagintc_ipi_combined(void *);
void agintc_send_ipi(struct cpu_info *, int);
@@ -576,15 +580,19 @@ agintc_attach(struct device *parent, str
sc->sc_ipi_irq[0] = agintc_intr_establish(ipiirq[0],
IPL_IPI|IPL_MPSAFE, NULL, agintc_ipi_combined, sc, "ipi");
sc->sc_ipi_num[ARM_IPI_NOP] = ipiirq[0];
+#ifdef DDB
sc->sc_ipi_num[ARM_IPI_DDB] = ipiirq[0];
+#endif
break;
case 2:
sc->sc_ipi_irq[0] = agintc_intr_establish(ipiirq[0],
IPL_IPI|IPL_MPSAFE, NULL, agintc_ipi_nop, sc, "ipinop");
sc->sc_ipi_num[ARM_IPI_NOP] = ipiirq[0];
+#ifdef DDB
sc->sc_ipi_irq[1] = agintc_intr_establish(ipiirq[1],
IPL_IPI|IPL_MPSAFE, NULL, agintc_ipi_ddb, sc, "ipiddb");
sc->sc_ipi_num[ARM_IPI_DDB] = ipiirq[1];
+#endif
break;
default:
panic("nipi unexpected number %d", nipi);
@@ -1117,6 +1125,7 @@ agintc_r_wait_rwp(struct agintc_softc *s
}
#ifdef MULTIPROCESSOR
+#ifdef DDB
int
agintc_ipi_ddb(void *v)
{
@@ -1124,6 +1133,7 @@ agintc_ipi_ddb(void *v)
db_enter();
return 1;
}
+#endif
int
agintc_ipi_nop(void *v)
@@ -1135,14 +1145,16 @@ agintc_ipi_nop(void *v)
int
agintc_ipi_combined(void *v)
{
+#ifdef DDB
struct agintc_softc *sc = v;
if (sc->sc_ipi_reason[cpu_number()] == ARM_IPI_DDB) {
sc->sc_ipi_reason[cpu_number()] = ARM_IPI_NOP;
return agintc_ipi_ddb(v);
- } else {
- return agintc_ipi_nop(v);
}
+#endif
+
+ return agintc_ipi_nop(v);
}
void
@@ -1154,9 +1166,11 @@ agintc_send_ipi(struct cpu_info *ci, int
if (ci == curcpu() && id == ARM_IPI_NOP)
return;
+#ifdef DDB
/* never overwrite IPI_DDB with IPI_NOP */
if (id == ARM_IPI_DDB)
sc->sc_ipi_reason[ci->ci_cpuid] = id;
+#endif
/* will only send 1 cpu */
sendmask = (ci->ci_mpidr & MPIDR_AFF3) << 16;
Index: ampintc.c
===
RCS file: /cvs/src/sys/arch/arm64/dev/ampintc.c,v
retrieving revision 1.19
diff -u -p -u -p -r1.19 ampintc.c
--- ampintc.c 17 Jul 2020 08:07:33 - 1.19
+++ ampintc.c 4 Sep 2020 13:18:33 -
@@ -141,7 +141,9 @@ struct ampintc_softc {
uint8_t sc_cpu_mask[ICD_ICTR_CPU_M + 1];
struct evcount sc_spur;
struct interrupt_controller sc_ic;
+#ifdef DDB
int sc_ipi_reason[ICD_ICTR_CPU_M + 1];
+#endif
int sc_ipi_num[2];
};
struct ampintc_softc *ampintc;
@@ -196,7 +198,9 @@ void ampintc_intr_barrier(void *);
int ampintc_ipi_combined(void *);
int ampintc_ipi_nop(void *);
+#ifdef DDB
int ampintc_ipi_ddb(void *);
+#endif
voidampintc_send_ipi(struct cpu_info *, int);
struct cfattachampintc_ca = {
@@ -347,15 +351,19 @@ ampintc_attach(struct device *parent, st
ampintc_intr_establish(ipiirq[0], IST_EDGE_RISING,
IPL_IPI|IPL_MPSAFE, NULL, ampintc_ipi_combined, sc, "ipi");
sc->sc_ipi_num[ARM_IPI_NOP] = ipiirq[0];
+#ifdef DDB
sc->sc_ipi_num[ARM_IPI_DDB] = ipiirq[0];
+#endif
break;
case 2:
ampintc_intr_establish(ipiirq[0], IST_EDGE_RISING,
IPL_IPI|IPL_MPSAFE, NULL, ampintc_ipi_nop, sc, "ipinop");
sc->sc_ipi_num[ARM_IPI_NOP] = ipiirq[0];
+#ifdef DDB
ampintc_intr_establish(ipiirq[1], IST_EDGE_RISING,
IPL_IPI|IPL_MPSAFE, NULL, ampintc_ipi_ddb, sc, "ipiddb");
sc->sc_ipi_num[ARM_IPI_DDB] = ipiirq[1];
+#endif
