Humm, that's not just a cosmetic error. cmci_monitor() assumes it is not being run concurrently on multiple CPUs so that if a given MC# bank is shared, only one CPU monitors it. It seems this is a very old bug of mine from the original CMCI commit. :( Arguably mca_init() should perhaps lock the mca_lock, but that is a bit trickier perhaps. The lock isn't initialized when mca_init() is called for the BSP. While I would prefer to do locking in mca.c for clarity reasons it would be a bit messier. This is ok, but perhaps add a comment to mca_init() saying we rely on the ap_boot_mtx to serialize it (just the boot-time mca_init(), not the resume mca_resume()).

In D15147#319396, @jmg wrote:

I have an spare BBB, and I can test this if you send me an image + test case. I'm not setup for building images right now.

The code looks correct, sans all the type changes. I'd recommend committing the type changes separately from fcmpset change.

This looks sensible to me.

• Use howmany() and pointer subtraction for auxargs.

Ping?

There is also ddb.4 though many commands are not documented in it.

Not sure if this will add conflicts for any future merges, but I don't think there's an upstream for our current top. (I think any upstream that might exist is probably quite a bit diverged already)

Test and change look good, just some style suggestions.

So I know that debuggers expect that after a PT_STEP stepping is disabled when the resulting SIGTRAP is reported. I suspect this will break gdb at least. That is, the debugger expects to do something like:

I think the #ifdef just makes it look like the code is in the middle of the block. I think it would be clearer to read if the #ifdef were reversed so that the CAPSICUM code came first as then you'd have the rest of the vars right after the #ifdef and only a bit of code in the #else.

I'm going to commit the current patch for now to unbreak the non-xtoolchain gcc ports, but I'm happy to refine the condition later if there is a better one to use.

Andriy, looks sane to me too. Thanks! Peter, are you ok with this now?

One other possibility is to define a helper struct that contains the structure sizes and has function pointer callbacks (kevent and aio took this approach), but this is probably fine.

Any thoughts?

Thanks.

I think it would be fine to prefer the sysctl to the kenv, but imagine booting a kernel with 'hint.acpi.0.disabled=1'. In that case I think you would want acpidump to honor the kenv hint from the loader's memory scan (or from an EFI table) rather than doing the memory scan in acpidump? But also, I don't think the commit log is accurate to say that the hint can be wrong. We currently assume it is correct if set.

• Changes to use libc++ instead of libstc++.
• Define CROSS_DIRECTORY_STRUCTURE always.

In D14466#313304, @avg wrote:

In D14466#313263, @jhb wrote:

Andriy, does that mean you were able to test the debug server itself on AMD?

I've only reviewed it.
I can do a test if you'd like, just give me some instructions to get started and a couple of days.

Andriy, does that mean you were able to test the debug server itself on AMD?

We've only done direct commits to stable when it was a "oops, I've already removed the driver from head and forgot to do the deprecation notice". In general we should just follow the rule of HEAD first.

Seems ok to me.

Hmm, what are the cases when the hint is wrong? The kernel always prefers the hint for the acpi0 driver (see sys/x86/acpica/OsdEnvironment.c:AcpiOsGetRootPointer). The hint name is just an implementation detail, it is only set by the loader if the loader actually finds the root pointer via the memory scan or from EFI's system tables.

Bare function pointers is a bit hackish. A couple of thoughts come to mind:

In D14466#310258, @grehan wrote:

Anything available I could use to test this on AMD ?

In testing I found that it needed some additional changes from devel/powerpc64-gcc for libc++. Those appear to have worked, but when I tried a related fix for amd64-gcc I got a weird build error in an amd64 buildworld I haven't resolved yet.

Agreed that the driver is dubious at best. I'm not sure if it's even really used. I don't think it was a commodity part.

I don't think we need an explicit shim between TOE drivers and the kernel to try to insulate from changes to 'tcp_info'. 'tcp_info' is already part of the userland ABI, so changes to its layout are already going to require compat shims, etc. (probably a new socket option enum for the new layout, etc.) such that we should be hesitant to change it very often. For KBI breakage you would only have to worry about altering the layout of existing fields in a stable branch (whereas ABI breakage spans multiple stable branches). We already have padding fields to permit adding new fields in our current stable branches. Do we think there are future changes to 'tcp_info' that can't be accommodated by the existing padding fields?