Re: malloc canaries for > page sized objects

[Theo Buehler]

On Sun, Oct 16, 2016 at 01:14:35PM +0200, Otto Moerbeek wrote:
> Hi,
> 
> this diff is somewhat big since I decided to rewrite wrterror() to be
> able to get better error messages.
> 
> Please review and test this with malloc option C (and other flags). An
> example run:
> 
> a.out(85360) in free(): chunk canary corrupted 0x13c68f696000 0x18a92@0x18a88
> 
> This means I overwrote a byte at offset 0x18a92 in a chunk of size
> 0x18a88 that is located at 0x13c68f696000.
> 
> Only max 32 bytes after the requested size are filled with 0xdb and
> checked on free. 

This is very nice and reads very well.  I tested it with C and CJ and S
on amd64 and it seems to work fine.

A few small comments about the new wrterror():

In

+   snprintf(pidbuf, sizeof(pidbuf), "%s(%d) in %s(): ", __progname,
+   getpid(), d->func ? d->func : "unknown");

I think you should truncate an insanely long __progname to 50 characters
to avoid overwriting the more important info that follows:

snprintf(pidbuf, sizeof(pidbuf), "%.50s(%d) in %s(): ", __progname,
getpid(), d->func ? d->func : "unknown");

because the longest possible string after __progname is

"(12345) in posix_memalign(): "

which has 29 characters, so there is room for 50 non-NULs in pidbuf[80],
and these should be plenty enough to identify the offending program.


I'm slightly concerned about running strlen on pidbuf[] and buf[] in
case {,v}snprintf fail.  Wouldn't it be cleaner to do something like
this:

if ((ret = snprintf(pidbuf, ...)) >= 0) {
iov[0].iov_base = pidbuf;
iov[0].iov_len = ret;
} else {
/* hardcoded message */
}

and a small style nit:

> @@ -1181,6 +1177,13 @@ omalloc(struct dir_info *pool, size_t sz
>   memset(p, SOME_JUNK,
>   psz - mopts.malloc_guard);
>   }
> + else if (mopts.chunk_canaries) {

} else if (mopts.chunk_canaries) { 

> + size_t csz = psz - mopts.malloc_guard - sz;
> +
> + if (csz > CHUNK_CHECK_LENGTH)
> + csz = CHUNK_CHECK_LENGTH;
> + memset(p + sz, SOME_JUNK, csz);
> + }
>   }
>  
>   } else {

Re: let PF to send challenge ack

[Alexander Bluhm]

On Fri, Sep 30, 2016 at 11:55:48PM +0200, Alexandr Nedvedicky wrote:
> The patch makes PF to send 'challenge ACK' for SYN packet, which matches
> session in established state.

The pf diff is OK bluhm@

> The patch also comes with test case. I've just learned it's bit tricky
> to use scapy for sending TCP packets. There are basically two pitfalls:

I have some remarks to the test.  First of all, I like it!

>   - the session established by scapy interferes with host's TCP stack,
>   where scapy is running. Host's TCP answers with RST to SYN-ACK,
>   which is sent as a response to scapy's SYN. Adding PF rule, which
>   blocks outbound RST solves the problem.

I have a better trick for that.  I use a non existing address on a
local net of the test machine.  As the packet will get lost after
unanswered arp requests, there is no RST.

In some tests I call it FAKE_ADDR in others FAKE_NET_ADDR depending
where I want the packet to be routed.  I will think about a solution
how to put you use case into my framework consistently.

>   - the scapy's sr() function (send and receive) is very smart not
>   to receive a challenge_ack sent by remote PF. Test case uses
>   sniffer to verify remote PF answers with challenge ACK. If you
>   are not interested in further details stop reading here.

Been there, seen that.

> I gave up and opted for poor man's solution to use a sniffer instead.

I also did this before.  Your solution with the thread is nicer
than the fork I do.  I will try wether it is useful for my tests.

>  run-regress-tcp: stamp-pfctl
>   @echo '\n $@ '
> + #
> + # The TCP handshake in challenge_ack.py is initiated by scapy,
> + # SRC's local TCP stack is very surprised to see SYN-ACK coming
> + # from ECO, so it answers with RST. The RST must not leave SRC,
> + # otherwise testing would be spoiled.
> + #
> + echo "block out on ${SRC_IF} proto tcp from ${SRC_OUT} to ${ECO_IN}\
> + port=echo flags R/R" | ${SUDO} pfctl -e -f -
> + ${SUDO} ${PYTHON}challenge_ack.py ${SRC_OUT} ${ECO_IN}
> + ${SUDO} pfctl -d -Fa
>  .for ip in ECO_IN ECO_OUT RDR_IN RDR_OUT RTT_IN
>   @echo Check tcp ${ip}:
>   ${SUDO} route -n delete -host -inet ${${ip}} || true

Do not reuse this target for your test.  I want many tests and
targets, so I can see what is failing.  The pf_forward test has
already reached a lot of complexity, I think it is better to create
a new subdir sys/net/pf_tcp where we can test properies of TCP
handling.  Copy the pf_forward Makefile and remove all the nat,
redirect and af-to stuff.  Everything in one test is not maintainable.

I will play with that idea together with the FAKE_ADDR.

> +srcaddr=sys.argv[1]
> +dstaddr=sys.argv[2]
> +port=os.getpid() & 0x
> +
> +ip=IP(src=srcaddr, dst=dstaddr)

Perhaps we can use FAKE_ADDR and ECO_IN directly.  I have to check
how other tests do it.

> +if sniffer.captured == None:
> + print "ERROR: no packet received"
> +exit(1)

> +if challenge_ack == None:
> + print "No ACK has been seen"
> +exit(1)

Here are tab vs. space errors.

bluhm

Re: splassert with if_run

[Alexander Bluhm]

On Mon, Oct 17, 2016 at 08:56:09PM +0200, egorenar-...@posteo.net wrote:
> i recently updated from 6.0  to a snapshot and
> since then i see splassert sometimes in my dmesg when i use if_run USB
> network device.
...
> splassert_check() at splassert_check+0x78
> sowakeup() at sowakeup+0x2c
> sorwakeup() at sorwakeup+0x84
> route_input() at route_input+0x284
> run_attach() at run_attach+0x322

This should fix it.

ok?

bluhm

Index: net/rtsock.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/rtsock.c,v
retrieving revision 1.207
diff -u -p -r1.207 rtsock.c
--- net/rtsock.c27 Sep 2016 18:41:11 -  1.207
+++ net/rtsock.c17 Oct 2016 20:46:47 -
@@ -339,7 +339,7 @@ route_input(struct mbuf *m0, ...)
struct routecb *rop;
struct rt_msghdr *rtm;
struct mbuf *m = m0;
-   int sockets = 0;
+   int s, sockets = 0;
struct socket *last = NULL;
va_list ap;
struct sockproto *proto;
@@ -419,6 +419,7 @@ route_input(struct mbuf *m0, ...)
if (last) {
struct mbuf *n;
if ((n = m_copym(m, 0, M_COPYALL, M_NOWAIT)) != NULL) {
+   s = splsoftnet();
if (sbspace(>so_rcv) < (2 * MSIZE) ||
sbappendaddr(>so_rcv, sosrc,
n, (struct mbuf *)NULL) == 0) {
@@ -435,11 +436,13 @@ route_input(struct mbuf *m0, ...)
sorwakeup(last);
sockets++;
}
+   splx(s);
}
}
last = rp->rcb_socket;
}
if (last) {
+   s = splsoftnet();
if (sbspace(>so_rcv) < (2 * MSIZE) ||
sbappendaddr(>so_rcv, sosrc,
m, (struct mbuf *)NULL) == 0) {
@@ -452,6 +455,7 @@ route_input(struct mbuf *m0, ...)
sorwakeup(last);
sockets++;
}
+   splx(s);
} else
m_freem(m);
 }

pf afto icmp checksum

[Alexander Bluhm]

Hi,

The checksum of a ICMP "need to frag" packet for TCP was wrong when
created from a ICMP6 "too big" packet.  The function pf_change_icmp_af()
has code to adjust the pseudo-header checksum in the ICMP6 case,
but pf_test_state_icmp() changed the proto before the case was
entered.

So call pf_change_icmp_af() before the pd->proto is converted in
the TCP and UDP payload case like it was already done for ICMP and
ICMP6 payload case.

This was found by the sys/net/pf_forward regress test.

My printf debugging revealed that dlen is positive and d may be
negative.  So declare the variables with the correct sign in
pf_change_icmp_af().

ok?

bluhm

Index: net/pf.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.989
diff -u -p -r1.989 pf.c
--- net/pf.c9 Oct 2016 18:01:57 -   1.989
+++ net/pf.c17 Oct 2016 20:00:41 -
@@ -2340,7 +2340,8 @@ pf_change_icmp_af(struct mbuf *m, int ip
struct mbuf *n = NULL;
struct ip   *ip4;
struct ip6_hdr  *ip6;
-   u_inthlen, ohlen, d;
+   u_inthlen, ohlen, dlen;
+   int  d;
 
if (af == naf || (af != AF_INET && af != AF_INET6) ||
(naf != AF_INET && naf != AF_INET6))
@@ -2417,7 +2418,7 @@ pf_change_icmp_af(struct mbuf *m, int ip
 
if (pd->proto == IPPROTO_ICMPV6) {
/* fixup pseudo-header */
-   int dlen = pd->tot_len - pd->off;
+   dlen = pd->tot_len - pd->off;
pf_cksum_fixup(pd->pcksum,
htons(dlen), htons(dlen + d), pd->proto);
}
@@ -5104,6 +5105,10 @@ pf_test_state_icmp(struct pf_pdesc *pd, 
m_copyback(pd->m, pd->off,
sizeof(struct icmp6_hdr),
pd->hdr.icmp6, M_NOWAIT);
+   if (pf_change_icmp_af(pd->m, ipoff2,
+   pd, , >addr[sidx],
+   >addr[didx], pd->af, nk->af))
+   return (PF_DROP);
if (nk->af == AF_INET)
pd->proto = IPPROTO_ICMP;
else
@@ -5117,11 +5122,6 @@ pf_test_state_icmp(struct pf_pdesc *pd, 
>addr[pd2.didx], nk->af);
pd->naf = nk->af;
 
-   if (pf_change_icmp_af(pd->m, ipoff2,
-   pd, , >addr[sidx],
-   >addr[didx], pd->af, nk->af))
-   return (PF_DROP);
-
pf_patch_16(pd,
_sport, nk->port[sidx]);
pf_patch_16(pd,
@@ -5220,6 +5220,10 @@ pf_test_state_icmp(struct pf_pdesc *pd, 
m_copyback(pd->m, pd->off,
sizeof(struct icmp6_hdr),
pd->hdr.icmp6, M_NOWAIT);
+   if (pf_change_icmp_af(pd->m, ipoff2,
+   pd, , >addr[sidx],
+   >addr[didx], pd->af, nk->af))
+   return (PF_DROP);
if (nk->af == AF_INET)
pd->proto = IPPROTO_ICMP;
else
@@ -5232,11 +5236,6 @@ pf_test_state_icmp(struct pf_pdesc *pd, 
PF_ACPY(>ndaddr,
>addr[pd2.didx], nk->af);
pd->naf = nk->af;
-
-   if (pf_change_icmp_af(pd->m, ipoff2,
-   pd, , >addr[sidx],
-   >addr[didx], pd->af, nk->af))
-   return (PF_DROP);
 
pf_patch_16(pd,
_sport, nk->port[sidx]);

splassert with if_run

[egorenar-dev]

Hi,
i recently updated from 6.0  to a snapshot and
since then i see splassert sometimes in my dmesg when i use if_run USB 
network device.


--
Regards

Alexander Egorenkov




OpenBSD 6.0-current (obj) #0: Sat Oct 15 18:53:54 CEST 2016

egorenar@mbp-openbsd.local:/usr/src/sys/arch/amd64/compile/MYKERNEL/obj
RTC BIOS diagnostic error 
ff

real mem = 17032024064 (16243MB)
avail mem = 16511303680 (15746MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.7 @ 0x78f8b000 (33 entries)
bios0: vendor Apple Inc. version "MBP114.88Z.0172.B09.1602151732" date 
02/15/2016

bios0: Apple Inc. MacBookPro11,5
acpi0 at bios0: rev 2
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP HPET APIC SBST ECDT SSDT SSDT SSDT SSDT SSDT 
SSDT SSDT SSDT SSDT SSDT MCFG DMAR VFCT
acpi0: wakeup devices PEG0(S3) GFX0(S3) PEG1(S3) PEG2(S3) EC__(S3) 
GMUX(S3) HDEF(S3) RP03(S4) ARPT(S4) RP04(S4) XHC1(S3) ADP1(S3) LID0(S3)

acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpihpet0 at acpi0: 14318179 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Core(TM) i7-4980HQ CPU @ 2.80GHz, 3791.98 MHz
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,SENSOR,ARAT

cpu0: 256KB 64b/line 8-way L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 99MHz
cpu0: mwait min=64, max=64, C-substates=0.2.1.2.4, IBE
cpu1 at mainbus0: apid 2 (application processor)
cpu1: Intel(R) Core(TM) i7-4980HQ CPU @ 2.80GHz, 3791.23 MHz
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,SENSOR,ARAT

cpu1: 256KB 64b/line 8-way L2 cache
cpu1: smt 0, core 1, package 0
cpu2 at mainbus0: apid 4 (application processor)
cpu2: Intel(R) Core(TM) i7-4980HQ CPU @ 2.80GHz, 3791.23 MHz
cpu2: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,SENSOR,ARAT

cpu2: 256KB 64b/line 8-way L2 cache
cpu2: smt 0, core 2, package 0
cpu3 at mainbus0: apid 6 (application processor)
cpu3: Intel(R) Core(TM) i7-4980HQ CPU @ 2.80GHz, 3791.23 MHz
cpu3: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,SENSOR,ARAT

cpu3: 256KB 64b/line 8-way L2 cache
cpu3: smt 0, core 3, package 0
cpu4 at mainbus0: apid 1 (application processor)
cpu4: Intel(R) Core(TM) i7-4980HQ CPU @ 2.80GHz, 3791.23 MHz
cpu4: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,SENSOR,ARAT

cpu4: 256KB 64b/line 8-way L2 cache
cpu4: smt 1, core 0, package 0
cpu5 at mainbus0: apid 3 (application processor)
cpu5: Intel(R) Core(TM) i7-4980HQ CPU @ 2.80GHz, 3791.23 MHz
cpu5: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,SENSOR,ARAT

cpu5: 256KB 64b/line 8-way L2 cache
cpu5: smt 1, core 1, package 0
cpu6 at mainbus0: apid 5 (application processor)
cpu6: Intel(R) Core(TM) i7-4980HQ CPU @ 2.80GHz, 3791.23 MHz
cpu6: 

cc -Wshadow: don't warn about variable-vs-global-function

[Philip Guenther]

On Sun, 16 Oct 2016, Philip Guenther wrote:
> So let's fix that and make our gcc a bit more like new ones.  Written 
> without peeking at the new ones and tested against the .c file at bottom 
> to verify that it doesn't fail or crash on some weird combo of 
> shadowing.
> 
> oks?

Updated diff that *keeps* the shadow warning for a function parameter 
which is a pointer-to-function and which shadows a global function, e.g.:

#include 
void
foo( void (*err)(void) )
{
err();
}

Previous diff suppressed the warning on the 'err' parameter; this revised 
diff keeps it.

oks?

Philip

Index: gnu/gcc/gcc/c-decl.c
===
RCS file: /cvs/src/gnu/gcc/gcc/c-decl.c,v
retrieving revision 1.4
diff -u -p -r1.4 c-decl.c
--- gnu/gcc/gcc/c-decl.c10 Sep 2015 10:56:35 -  1.4
+++ gnu/gcc/gcc/c-decl.c17 Oct 2016 18:18:36 -
@@ -1946,8 +1946,20 @@ warn_if_shadowing (tree new_decl)
  warning (OPT_Wshadow, "declaration of %q+D shadows a parameter",
   new_decl);
else if (DECL_FILE_SCOPE_P (old_decl))
- warning (OPT_Wshadow, "declaration of %q+D shadows a global "
-  "declaration", new_decl);
+ {
+   /* Don't warn about shadowing a global function unless the local
+  variable or parameter is a pointer to a function */
+   if (TREE_CODE (old_decl) == FUNCTION_DECL
+   && TREE_CODE (new_decl) != FUNCTION_DECL
+   && ((TREE_CODE (new_decl) != VAR_DECL
+&& TREE_CODE (new_decl) != PARM_DECL)
+   || !POINTER_TYPE_P (TREE_TYPE (new_decl))
+   || TREE_CODE (TREE_TYPE (TREE_TYPE (new_decl)))
+  != FUNCTION_TYPE))
+ break;
+   warning (OPT_Wshadow, "declaration of %q+D shadows a global "
+"declaration", new_decl);
+ }
else if (TREE_CODE (old_decl) == FUNCTION_DECL
 && DECL_BUILT_IN (old_decl))
  {

Re: ksh vi mode: fix region deletion and yanking

[Ingo Schwarze]

Hi Theo,

Theo Buehler wrote on Fri, Oct 14, 2016 at 09:09:34PM +0200:

> The patch reads fine to me and behaves well in some testing.
> It is ok tb@ as is.

Committed, thanks for checking.

> I wonder if for the sake of consistency it would be worth merging the
> 'b' and 'B' cases as well as the 'w' and 'W' cases the same way as you
> did merge the 'e' and 'E' cases below. The simplification is minimal,
> but it would read slightly better, I think.

Made sense to me, too.
Eight lines of code less and more similar to the other cases...

So i committed that too.

Yours,
  Ingo

Re: FAQ entry for vmm

[Edd Barrett]

Hi Reyk,

On Mon, Oct 17, 2016 at 06:20:25PM +0200, Reyk Floeter wrote:
> Sorry for the late reply - thanks for doing this.  No objections, OK
> from me as well.

Thanks. The general consensus was to try to reduce this a lot before
commit though. I will probably only include the wireless setup in the
FAQ.

> It is a good place to explain things like networking in more detail
> that cannot be done in the manpages (but the vm.conf(5) manpage also
> needs improvement in the EXAMPLES section).

I posted an improvement to vm.conf(5) yesterday, albeit not to the
examples section. Still looking for an OK on that.

> One very minor nit: why re0? ;-) I think modern vmm-compatible
> machines with VMX/VT-d/EPT will have em0 in most cases ...

It's just what my test system had in it. We can easily switch to em.

-- 
Best Regards
Edd Barrett

http://www.theunixzoo.co.uk

Re: FAQ entry for vmm

[Reyk Floeter]

On Sat, Oct 15, 2016 at 05:11:49PM +0100, Edd Barrett wrote:
> Hi,
> 
> Here's an intial stab at a FAQ entry for vmm.
> 
> It covers two common setups:
>   * a vmm guest with network access via the host's wired network
>   * a vmm guest with network access via the host's wireless network
> 
> Please critique. Once we have the details right, I can run it past tj.
> 
> Some other thoughts that arose as a result of writing this:
> 
>  * Unless I am wrong there is no way to start a single VM which is
>defined in vm.conf. Up until now I have been doing `vmctl reload`,
>which is not quite the same. (there is an XXX in the diff about
>this).
> 
>  * Should `vmctl status` list all the VMs defined in vm.conf (and any
>manually started VMs), indicating whether they are powered up or
>down?
> 
>  * rebooting a guest does not work.
> 
>  * attaching to a console by name would be nice touch.
> 

Sorry for the late reply - thanks for doing this.  No objections, OK
from me as well.

It is a good place to explain things like networking in more detail
that cannot be done in the manpages (but the vm.conf(5) manpage also
needs improvement in the EXAMPLES section).

One very minor nit: why re0? ;-) I think modern vmm-compatible
machines with VMX/VT-d/EPT will have em0 in most cases ...

Reyk

> Thanks
> 
> 
> Index: faq/faq10.html
> ===
> RCS file: /home/edd/cvsync/www/faq/faq10.html,v
> retrieving revision 1.253
> diff -u -p -r1.253 faq10.html
> --- faq/faq10.html2 Oct 2016 21:19:04 -   1.253
> +++ faq/faq10.html15 Oct 2016 16:08:48 -
> @@ -43,6 +43,11 @@
>Setting up a YP client
>  
>  Keeping OpenBSD up to date
> +Virtual machines with vmm(4)
> +
> +  Ths simplest vmm(4) setup
> +  Using vmm(4) when the host uses wireless 
> networking
> +
>  
>  
>  
> @@ -895,5 +900,280 @@ At other times, will require recompiling
>  patched library.
>  
>  
> +
> +Virtual machines with vmm(4)
> +
> +
> +OpenBSD ships with a virtual machine monitor,
> +http://man.openbsd.org/vmm;>vmm(4), which is capable of hosting
> +OpenBSD guests.
> +This section shows how to set up the common use-cases.
> +
> +The simplest vmm(4) setup
> +
> +
> +Suppose that we have a machine connected to the internet via a wired
> +re0 network interface, and that we wish to install an OpenBSD guest 
> VM
> +with internet access.
> +Suppose that re0 gets its network address via DHCP, and that we want
> +the guest machine to use that same DHCP server.
> +
> +
> +First we make a directory to hold the disk image and kernel:
> +
> +
> +# mkdir -p /vms/my_vm
> +# cd /vms/my_vm
> +
> +
> +
> +Next we make a disk image:
> +
> +
> +vmctl create disk.img -s 4.5G
> +
> +
> +The -s argument specifies the size of the disk image.
> +Note that the image is lazily allocated.
> +
> +
> +Next we need an OpenBSD kernel to boot.
> +We are going to run the OpenBSD installer, so we need a bsd.rd (at
> +least initially).
> +For the sake of example, let's use the bsd.rd from the host:
> +
> +
> +# cp /bsd.rd .
> +
> +
> +
> +Now we have to tell the host machine about the configuration of the VM.
> +In this FAQ we will do this using a
> +http://man.openbsd.org/vm.conf;vm.conf(5), so as to 
> bring
> +the VM up at boot time (but note that the VM configuration could be specified
> +when starting a VM manually with
> +http://man.openbsd.org/vmctl;>vmctl(8)).
> +
> +
> +In /etc/vm.conf, put the following:
> +
> +
> +switch "my_switch" {
> +interface bridge0
> +add re0
> +}
> +
> +vm "my_vm" {
> +memory 512M
> +disk "/vms/my_vm/disk.img"
> +kernel "/vms/my_vm/bsd.rd"
> +interface tap { switch "my_switch" }
> +}
> +
> +
> +
> +This should be mostly self explanatory, but the switch configuration perhaps
> +requires some more discussion.
> +When the VM starts, a
> +http://man.openbsd.org/tap;>tap(4) network interface 
> will
> +be created.
> +This interface corresponds to the
> +http://man.openbsd.org/vio;>vio(4) network interface
> +inside the guest.
> +By defining a "switch", and assigning it to our VM,
> +http://man.openbsd.org/vmd;>vmd(8) will later add the
> +host-side http://man.openbsd.org/tap;>tap(4) interface 
> to
> +an (automatically created)
> +http://man.openbsd.org/bridge;>bridge(4) interface.
> +By specifying add re0 in the switch definition, we also add the 
> host's
> +wired interface into the bridge, thus granting internet access to the guest.
> +
> +
> +Now let's bring the guest up:
> +
> +
> +# rcctl enable vmd # start VMs at boot
> +# rcctl start vmd
> +
> +
> +
> +If all went to plan,
> +http://man.openbsd.org/rcctl;>rcctl(8) should tell us 
> the
> +VM is up:
> +
> +
> +   ID   PID VCPUSMAXMEMCURMEM  TTY NAME
> +1 73539 1 512MB  91MB   /dev/ttyp7 my_vm
> +
> +
> +
> +If the guest is not listed, look in /var/log/daemon for clues as to
> +what went wrong.
> +
> +
> +Assuming all is well, we can now connect to 

Re: vmd: simplify fatal/fatalx errno handling

[Reyk Floeter]

On Sun, Oct 16, 2016 at 02:55:39PM -0700, Philip Guenther wrote:
> 
> Instead of using errno as a hidden argument to vfatal(), make it an 
> _actual_ argument named 'code', ala the errc/warnc family, and rename it 
> to vfatalc() to match the pattern set.
> 
> ok?
> 

OK, makes sense

please sync as mentioned by benno and me.

> 
> Index: log.c
> ===
> RCS file: /data/src/openbsd/src/usr.sbin/vmd/log.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 log.c
> --- log.c 12 Oct 2016 11:47:34 -  1.3
> +++ log.c 16 Oct 2016 21:17:40 -
> @@ -165,11 +165,10 @@ log_debug(const char *emsg, ...)
>  }
>  
>  static void
> -vfatal(const char *emsg, va_list ap)
> +vfatalc(int code, const char *emsg, va_list ap)
>  {
>   static char s[BUFSIZ];
>   const char  *sep;
> - int  saved_errno = errno;
>  
>   if (emsg != NULL) {
>   (void)vsnprintf(s, sizeof(s), emsg, ap);
> @@ -178,9 +177,9 @@ vfatal(const char *emsg, va_list ap)
>   s[0] = '\0';
>   sep = "";
>   }
> - if (saved_errno)
> + if (code)
>   logit(LOG_CRIT, "%s: %s%s%s",
> - log_procname, s, sep, strerror(saved_errno));
> + log_procname, s, sep, strerror(code));
>   else
>   logit(LOG_CRIT, "%s%s%s", log_procname, sep, s);
>  }
> @@ -191,7 +190,7 @@ fatal(const char *emsg, ...)
>   va_list ap;
>  
>   va_start(ap, emsg);
> - vfatal(emsg, ap);
> + vfatalc(errno, emsg, ap);
>   va_end(ap);
>   exit(1);
>  }
> @@ -201,9 +200,8 @@ fatalx(const char *emsg, ...)
>  {
>   va_list ap;
>  
> - errno = 0;
>   va_start(ap, emsg);
> - vfatal(emsg, ap);
> + vfatalc(0, emsg, ap);
>   va_end(ap);
>   exit(1);
>  }
> 

-- 

Re: vmd: simplify fatal/fatalx errno handling

[Reyk Floeter]

On Sun, Oct 16, 2016 at 08:03:05PM -0600, Theo de Raadt wrote:
> > On Sun, Oct 16, 2016 at 02:55:39PM -0700, Philip Guenther wrote:
> > > 
> > > Instead of using errno as a hidden argument to vfatal(), make it an 
> > > _actual_ argument named 'code', ala the errc/warnc family, and rename it 
> > > to vfatalc() to match the pattern set.
> > > 
> > > ok?
> > > 
> > > Philip Guenther
> > > 
> > 
> > Isn't this code used elsewhere too?
> 
> benno is in the process of unifying all the log.c versions, as much
> as possible.
> 

benno is working on a totally different variant of log.c that is found
in the routing daemons.  I talked with him at g2k16 about it and we'll
eventually try to sync them all together once he is done with that,
but he will first move all the daemon-/protocol-specific stuff out of
the other log.c's.

I unified my versions long ago, and there identical in at least:

usr.sbin/httpd
usr.sbin/ntpd
usr.sbin/relayd
usr.sbin/snmpd
usr.sbin/switchd
usr.sbin/vmd
sbin/iked

$ diff -Nup -I'\$OpenBSD' usr.sbin/vmd/log.c usr.sbin/ntpd/log.c

I think Philip's code is OK and syncing it with the other daemons is
as easy as copying vmd's updated version to all these directories and
committing them at once.

Reyk

Re: minor diff for ldapd.conf.5

[Jeremie Courreges-Anglas]

Rob Pierce  writes:

> Fix a couple of grammar mistakes, remove a redundant word, and add a FILES
> reference for the /etc/ldap/certs directory.

Committed, thanks.  I adjusted the FILES entries so that ldapd.conf is
still listed first, and so that the description of /etc/ldap/certs is
more generic (one can put TLS certs out of /etc/ldap/certs).

The description for `listen on' could be improved, based on this
(looking at the code):
- if no certificate name is provided, the interface name is used
  instead
- if the certificate name is not absolute, the cert & key files are
  looked up from /etc/ldap/certs
- the cert file is retrieved by appending .crt to the certificate name,
  the key file by appending .key

Right now the description is a bit unclear.  What happens if the
certificate name isn't an absolute name?  If anyone wants to reword it
in a nice way, be my guest.

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: ffs/msdosfs: Flush cache when updating mount to R/O

[Alexander Bluhm]

On Sun, Oct 16, 2016 at 10:24:06AM +0200, Stefan Fritsch wrote:
> On Sun, 16 Oct 2016, Stefan Fritsch wrote:
> > > * When a R/W mount is updated to R/O. I will send patches for this in a 
> > > separate mail.
> 
> Part 2: Use it
> 
> 
> msdosfs & ffs: flush cache if updating mount to r/o
> 
> Other filesystems can be changed later.
> 
> ok?

OK bluhm@

> + /* may be not supported, ignore error */
> + VOP_IOCTL(pmp->pm_devvp, DIOCCACHESYNC, , 
>  FWRITE, FSCRED, p);

This line is too long.

Re: Add ioctl for disk cache flush

[Alexander Bluhm]

On Sun, Oct 16, 2016 at 10:19:53AM +0200, Stefan Fritsch wrote:
> On Sun, 16 Oct 2016, Stefan Fritsch wrote:
> > * When a R/W mount is updated to R/O. I will send patches for this in a 
> > separate mail.
> 
> Part 1: Add an ioctl:
> 
> 
> add DIOCCACHESYNC
> 
> Add an ioctl to tell storage devices to flush their internal caches.
> Ported from netbsd by pedro@
> 
> OK?

Some remarks inline

> @@ -848,6 +848,10 @@ wdioctl(dev_t dev, u_long xfer, caddr_t addr, int flag, 
> struct proc *p)
>   }
>  #endif
>  
> + /* XXX pedro: should set AT_WAIT according to force flag */

Does this comment help?  Either do (*(int *)addr ?  AT_WAIT : 0)
or just pass AT_WAIT.  But don't force all people reading the code
to think about it.

I think the AT_WAIT code is correct and the comment is wrong.  In
sdioctl() the force flag overrides the SDF_DIRTY flag.  This is
different from setting AT_WAIT.  Just remove the pedro comment.
Then we have the NetBSD code.

> + case DIOCCACHESYNC:
> + return wd_flushcache(wd, AT_WAIT);
> +

All other cases have a "goto exit".  With the return we miss a
device_unref().

>   default:
>   error = wdc_ioctl(wd->drvp, xfer, addr, flag, p);
>   goto exit;


> @@ -1067,13 +1071,13 @@ wd_get_params(struct wd_softc *wd, u_int8_t flags, 
> struct ataparams *params)
>   }
>  }
>  
> -void
> +int
>  wd_flushcache(struct wd_softc *wd, int flags)
>  {
>   struct wdc_command wdc_c;
>  
>   if (wd->drvp->ata_vers < 4) /* WDCC_FLUSHCACHE is here since ATA-4 */
> - return;
> + return EIO;
>   bzero(_c, sizeof(struct wdc_command));
>   wdc_c.r_command = (wd->sc_flags & WDF_LBA48 ? WDCC_FLUSHCACHE_EXT :
>   WDCC_FLUSHCACHE);
> @@ -1088,20 +1092,18 @@ wd_flushcache(struct wd_softc *wd, int flags)
>   if (wdc_exec_command(wd->drvp, _c) != WDC_COMPLETE) {
>   printf("%s: flush cache command didn't complete\n",
>   wd->sc_dev.dv_xname);
> + return EIO;
>   }
> - if (wdc_c.flags & AT_TIMEOU) {
> - printf("%s: flush cache command timeout\n",
> - wd->sc_dev.dv_xname);
> + if (wdc_c.flags & AT_ERROR) {
> + if (wdc_c.r_error == WDCE_ABRT) /* command not supported */
> + return ENODEV;
>   }
> - if (wdc_c.flags & AT_DF) {
> - printf("%s: flush cache command: drive fault\n",
> + if (wdc_c.flags & (AT_ERROR | AT_TIMEOU | AT_DF)) {
> + printf("%s: flush cache command timeout\n",
>   wd->sc_dev.dv_xname);

NetBSD prints the flags here.  If we merge the cases, debugging
will be harder if don't print the relevant bits.

> + return EIO;
>   }
> - /*
> -  * Ignore error register, it shouldn't report anything else
> -  * than COMMAND ABORTED, which means the device doesn't support
> -  * flush cache
> -  */
> + return 0;
>  }

Instead of merging the cases, which was done independently in NetBSD,
I would perfer to take their original commit.

http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/ata/wd.c.diff?r1=1.275=1.276_with_tag=MAIN=h

Also I think their EIO/ENODEV choices are better.

> @@ -999,6 +999,15 @@ sdioctl(dev_t dev, u_long cmd, caddr_t addr, int flag, 
> struct proc *p)
>   error = sd_ioctl_cache(sc, cmd, (struct dk_cache *)addr);
>   goto exit;
>  
> + case DIOCCACHESYNC:
> + if (!ISSET(flag, FWRITE)) {
> + error = EBADF;
> + goto exit;
> + }
> + if ((sc->flags & SDF_DIRTY) != 0 || *(int *)addr != 0)
> + error = sd_flush(sc, 0);
> + return (error);

You need a goto exit to call device_unref().

> +
>   default:
>   if (part != RAW_PART) {
>   error = ENOTTY;

bluhm

Re: vmd: simplify fatal/fatalx errno handling

[Sebastian Benoit]

Theo de Raadt(dera...@openbsd.org) on 2016.10.16 20:03:05 -0600:
> > On Sun, Oct 16, 2016 at 02:55:39PM -0700, Philip Guenther wrote:
> > > 
> > > Instead of using errno as a hidden argument to vfatal(), make it an 
> > > _actual_ argument named 'code', ala the errc/warnc family, and rename it 
> > > to vfatalc() to match the pattern set.
> > > 
> > > ok?

i dont consider the use of errno to be a problem here, the same is done in
log_warn() and its kind of consistent with warn(3) etc.

> > > Philip Guenther
> > > 
> > 
> > Isn't this code used elsewhere too?

this variant of log.c is in sync in iked, httpd, relayd, snmpd, vmd and
quite different to the other ones.

if this change goes in here, please sync all these.

> benno is in the process of unifying all the log.c versions, as much
> as possible.

i'll keep a pointer to this thread to consider later.

Re: landisk: simply asid lookup

[Jeremie Courreges-Anglas]

Philip Guenther  writes:

> It used to be that access to a process's vmspace (and thus its pmap where 
> the ASID can be found) was only via a thread's p_vmspace member.  Now that 
> the process itself has a copy of that, the loops in the sh code that 
> iterated across threads to find a valid pointer can be eliminated.
>
> ok?

Looks fine to me fwiw, I don't have a landisk machine.

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

opencvs - show branch revision in status

[Joris Vink]

Hi,

Let's bring status a bit more inline with its GNU cvs counter part.

This diff adds the branch revision for the sticky tag if set.

.joris

Index: status.c
===
RCS file: /cvs/src/usr.bin/cvs/status.c,v
retrieving revision 1.96
diff -u -p -r1.96 status.c
--- status.c4 Apr 2015 14:20:11 -   1.96
+++ status.c17 Oct 2016 09:06:12 -
@@ -16,6 +16,7 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -119,7 +120,7 @@ void
 cvs_status_local(struct cvs_file *cf)
 {
size_t len;
-   RCSNUM *head;
+   RCSNUM *head, *brev;
const char *status;
struct rcs_delta *rdp;
char buf[PATH_MAX + CVS_REV_BUFSZ + 128];
@@ -223,11 +224,23 @@ cvs_status_local(struct cvs_file *cf)
}
 
if (cf->file_ent != NULL) {
-   if (cf->file_ent->ce_tag != NULL)
-   cvs_printf("   Sticky Tag:\t\t%s\n",
-   cf->file_ent->ce_tag);
-   else if (verbosity > 0)
+   if (cf->file_ent->ce_tag != NULL) {
+   if ((brev = rcs_sym_getrev(cf->file_rcs,
+   cf->file_ent->ce_tag)) == NULL) {
+   (void)strlcpy(buf, "- MISSING from RCS file!",
+   sizeof(buf));
+   } else {
+   rcsnum_tostr(brev, revbuf, sizeof(revbuf));
+   (void)xsnprintf(buf, sizeof(buf),
+   "(branch: %s)", revbuf);
+   free(brev);
+   }
+
+   cvs_printf("   Sticky Tag:\t\t%s %s\n",
+   cf->file_ent->ce_tag, buf);
+   } else if (verbosity > 0) {
cvs_printf("   Sticky Tag:\t\t(none)\n");
+   }
 
if (cf->file_ent->ce_date != -1) {
struct tm datetm;

Re: ifconfig(8): fix set switch(4) datapath id

[Reyk Floeter]

> Am 17.10.2016 um 14:33 schrieb Rafael Zalamena :
> 
>> On Mon, Oct 17, 2016 at 02:30:41PM +0400, Reyk Floeter wrote:
>> 
>>> Am 17.10.2016 um 14:16 schrieb Rafael Zalamena :
>>> 
>>> There are two inconsistencies with the ifconfig(8) switch(4) configuring:
>>> 1) Datapath ID is an unsigned 64 bit integer, not a signed one;
>> 
>> I think we should use strtonum instead of strtoull here.
> 
> If we use strtonum() we lose the ability to express the datapath id in
> hexadecimal numbers and we'll use only decimals. I think we should keep
> the strtoull().

Makes sense.

So OK for the diff.

Reyk

Re: ifconfig(8): fix set switch(4) datapath id

[Reyk Floeter]

> Am 17.10.2016 um 14:16 schrieb Rafael Zalamena :
> 
> There are two inconsistencies with the ifconfig(8) switch(4) configuring:
> 1) Datapath ID is an unsigned 64 bit integer, not a signed one;

I think we should use strtonum instead of strtoull here.

> 2) ifconfig(8) man pages says that the parameter is "datapath" not
>   "datapathid";
> 

Yes, this chunk is OK.

Reyk

> This diff fixes both problems and let us configure the datapath id
> correctly.
> 
> ok?
> 
> 
> Index: brconfig.c
> ===
> RCS file: /home/obsdcvs/src/sbin/ifconfig/brconfig.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 brconfig.c
> --- brconfig.c3 Sep 2016 17:13:48 -1.11
> +++ brconfig.c17 Oct 2016 10:11:25 -
> @@ -1051,7 +1051,7 @@ switch_datapathid(const char *arg, int d
>char *endptr;
> 
>errno = 0;
> -newdpid = strtoll(arg, , 0);
> +newdpid = strtoull(arg, , 0);
>if (arg[0] == '\0' || endptr[0] != '\0' || errno == ERANGE)
>errx(1, "invalid arg for datapath-id: %s", arg);
> 
> Index: ifconfig.c
> ===
> RCS file: /home/obsdcvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.330
> diff -u -p -r1.330 ifconfig.c
> --- ifconfig.c3 Sep 2016 13:46:57 -1.330
> +++ ifconfig.c17 Oct 2016 09:48:33 -
> @@ -517,7 +517,7 @@ const structcmd {
>{ "-roaming",0,0,umb_roaming },
>{ "patch",NEXTARG,0,setpair },
>{ "-patch",1,0,unsetpair },
> -{ "datapathid",NEXTARG,0,switch_datapathid },
> +{ "datapath",NEXTARG,0,switch_datapathid },
>{ "portno",NEXTARG2,0,NULL, switch_portno },
>{ "addlocal",NEXTARG,0,addlocal },
> #else /* SMALL */
> 

ifconfig(8): fix set switch(4) datapath id

[Rafael Zalamena]

There are two inconsistencies with the ifconfig(8) switch(4) configuring:
1) Datapath ID is an unsigned 64 bit integer, not a signed one;
2) ifconfig(8) man pages says that the parameter is "datapath" not
   "datapathid";

This diff fixes both problems and let us configure the datapath id
correctly.

ok?


Index: brconfig.c
===
RCS file: /home/obsdcvs/src/sbin/ifconfig/brconfig.c,v
retrieving revision 1.11
diff -u -p -r1.11 brconfig.c
--- brconfig.c  3 Sep 2016 17:13:48 -   1.11
+++ brconfig.c  17 Oct 2016 10:11:25 -
@@ -1051,7 +1051,7 @@ switch_datapathid(const char *arg, int d
char *endptr;
 
errno = 0;
-   newdpid = strtoll(arg, , 0);
+   newdpid = strtoull(arg, , 0);
if (arg[0] == '\0' || endptr[0] != '\0' || errno == ERANGE)
errx(1, "invalid arg for datapath-id: %s", arg);
 
Index: ifconfig.c
===
RCS file: /home/obsdcvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.330
diff -u -p -r1.330 ifconfig.c
--- ifconfig.c  3 Sep 2016 13:46:57 -   1.330
+++ ifconfig.c  17 Oct 2016 09:48:33 -
@@ -517,7 +517,7 @@ const structcmd {
{ "-roaming",   0,  0,  umb_roaming },
{ "patch",  NEXTARG,0,  setpair },
{ "-patch", 1,  0,  unsetpair },
-   { "datapathid", NEXTARG,0,  switch_datapathid },
+   { "datapath",   NEXTARG,0,  switch_datapathid },
{ "portno", NEXTARG2,   0,  NULL, switch_portno },
{ "addlocal",   NEXTARG,0,  addlocal },
 #else /* SMALL */

Re: make obj in config

[Marc Espie]

On Sun, Oct 16, 2016 at 11:52:45AM +0200, Martin Natano wrote:
> We don't need to re-run 'make obj' when obj exists. Ok?
> 
> natano
> 
> 
> Index: main.c
> ===
> RCS file: /cvs/src/usr.sbin/config/main.c,v
> retrieving revision 1.55
> diff -u -p -r1.55 main.c
> --- main.c16 Oct 2016 09:36:46 -  1.55
> +++ main.c16 Oct 2016 09:46:55 -
> @@ -703,9 +703,9 @@ setupdirs(void)
>   }
>   fclose(fp);
>  
> -reconfig:
>   if (system("make obj") != 0)
>   exit(2);
> +reconfig:
>   if (system("make config") != 0)
>   exit(2);
>   exit(0);

You could even tail-recurse directly ?

execlp("make", "make", "config", NULL);
exit(2);

will do the same thing with less processes (unless you care about "2" 
as an error code).

Re: use x2apic if it is enabled by BIOS

[Mark Kettenis]

> Date: Sat, 15 Oct 2016 09:55:05 +0200 (CEST)
> From: s...@openbsd.org
> 
> On Fri, 14 Oct 2016, Mike Larkin wrote:
> 
> > On Fri, Oct 14, 2016 at 04:49:31PM +0900, YASUOKA Masahiko wrote:
> > > Hi,
> > > 
> > > I'm working on NEC Express5800/R110h-1 (dmesg is attached).  On this
> > > machine, our kernel panics with following message.
> > > 
> > >   cpu0 at mainbus0panic: cpu at apic id 0 already attached?
> > > 
> > > This seems to happen since x2APIC on the machine is enabled by BIOS
> > > and the kernel doesn't assume that.  The diff makes the kernel use
> > > x2APIC if it is enabled by BIOS.
> > > 
> > > ok?
> > > 
> > 
> > This should go in snaps, or wait for reports from tech@ with test results
> > before it should be committed, IMO. We don't have full support for x2apic,
> > and blindly enabling it like this hoping that the bios set everything up
> > right is bound to be a bad assumption on at least one machine out there.
> 
> As I understand the code, it should only change behavior on systems where 
> x2apic is enabled by the bios. And currently on such systems openbsd will 
> not work anyway, because we then try to use xapic mode without disabling 
> x2apic mode. Or am I missing something?

No, I think you're right and the risk of the diff is low.

sparc64: delete old "traptrace" bits

[Philip Guenther]

In 2003(!) a pile of code was deleted from sparc64's locore.s:

revision 1.35
date: 2003/05/17 07:09:08;  author: art;  state: Exp;  lines: +1 -957;
Get rid of lots of hairy ifdefs that we'll most likely never use.
TRAPTRACE, TRAPSTATS, FLTTRACE and SCHED_DEBUG.

mdw@ henric@ ok.


That was the code that actually put data into the trap_trace[] array, 
rendering the remaining trap_trace* code useless.  How about we delete 
those bits?

This built and the box booted with the resulting kernel.

ok?

Philip Guenther

Index: sparc64/autoconf.c
===
RCS file: /cvs/src/sys/arch/sparc64/sparc64/autoconf.c,v
retrieving revision 1.126
diff -u -p -r1.126 autoconf.c
--- sparc64/autoconf.c  8 Jun 2016 17:24:44 -   1.126
+++ sparc64/autoconf.c  17 Oct 2016 06:25:36 -
@@ -586,11 +586,6 @@ bootpath_build(void)
 #else
printf("kernel has no debugger\n");
 #endif
-   } else if (*cp == 't') {
-   /* turn on traptrace w/o breaking into kdb */
-   extern int trap_trace_dis;
-
-   trap_trace_dis = 0;
}
}
 }
Index: sparc64/db_interface.c
===
RCS file: /cvs/src/sys/arch/sparc64/sparc64/db_interface.c,v
retrieving revision 1.45
diff -u -p -r1.45 db_interface.c
--- sparc64/db_interface.c  8 Oct 2016 05:49:09 -   1.45
+++ sparc64/db_interface.c  17 Oct 2016 06:25:37 -
@@ -73,17 +73,6 @@ db_regs_tddb_regs;   /* register state */
 
 extern void OF_enter(void);
 
-extern struct traptrace {
-   unsigned short tl:3,/* Trap level */
-   ns:4,   /* PCB nsaved */
-   tt:9;   /* Trap type */
-   unsigned short pid; /* PID */
-   u_int tstate;   /* tstate */
-   u_int tsp;  /* sp */
-   u_int tpc;  /* pc */
-   u_int tfault;   /* MMU tag access */
-} trap_trace[], trap_trace_end[];
-
 static long nil;
 
 static int
@@ -231,14 +220,12 @@ void db_pmap_kernel(db_expr_t, int, db_e
 void db_pload_cmd(db_expr_t, int, db_expr_t, char *);
 void db_pmap_cmd(db_expr_t, int, db_expr_t, char *);
 void db_lock(db_expr_t, int, db_expr_t, char *);
-void db_traptrace(db_expr_t, int, db_expr_t, char *);
 void db_dump_buf(db_expr_t, int, db_expr_t, char *);
 void db_dump_espcmd(db_expr_t, int, db_expr_t, char *);
 void db_watch(db_expr_t, int, db_expr_t, char *);
 void db_xir(db_expr_t, int, db_expr_t, char *);
 
 static void db_dump_pmap(struct pmap*);
-static void db_print_trace_entry(struct traptrace *, int);
 
 #ifdef MULTIPROCESSOR
 void db_cpuinfo_cmd(db_expr_t, int, db_expr_t, char *);
@@ -272,9 +259,6 @@ db_ktrap(type, tf)
struct trapstate *ts = _regs.ddb_ts[0];
extern int savetstate(struct trapstate *ts);
extern void restoretstate(int tl, struct trapstate *ts);
-   extern int trap_trace_dis;
-
-   trap_trace_dis++;
 
 #if NTDA > 0
tda_full_blast();
@@ -336,7 +320,6 @@ db_ktrap(type, tf)
*(struct frame *)tf->tf_out[6] = ddb_regs.ddb_fr;
 #endif
*tf = ddb_regs.ddb_tf;
-   trap_trace_dis--;
 
 #ifdef MULTIPROCESSOR
if (!db_switch_cpu)
@@ -1098,78 +1081,6 @@ db_setpcb(addr, have_addr, count, modif)
db_printf("PID %ld not found.\n", addr);
 }
 
-static void
-db_print_trace_entry(te, i)
-   struct traptrace *te;
-   int i;
-{
-   db_printf("%d:%d p:%d tt:%d:%llx:%llx %llx:%llx ", i,
- (int)te->tl, (int)te->pid,
- (int)te->tt, (unsigned long long)te->tstate,
- (unsigned long long)te->tfault, (unsigned long long)te->tsp,
- (unsigned long long)te->tpc);
-   db_printsym((u_long)te->tpc, DB_STGY_PROC, db_printf);
-   db_printf(": ");
-   if ((te->tpc && !(te->tpc&0x3)) &&
-   curproc &&
-   (curproc->p_pid == te->pid)) {
-   db_disasm((u_long)te->tpc, 0);
-   } else db_printf("\n");
-}
-
-void
-db_traptrace(addr, have_addr, count, modif)
-   db_expr_t addr;
-   int have_addr;
-   db_expr_t count;
-   char *modif;
-{
-   int i, start = 0, full = 0, reverse = 0;
-   struct traptrace *end;
-
-   start = 0;
-   end = _trace_end[0];
-
-   {
-   register char c, *cp = modif;
-   if (modif)
-   while ((c = *cp++) != 0) {
-   if (c == 'f')
-   full = 1;
-   if (c == 'r')
-   reverse = 1;
-   }
-   }
-
-   if (have_addr) {
-   start = addr / (sizeof (struct traptrace));
-   if (_trace[start] > _trace_end[0]) {
-   db_printf("Address out of