I do not see why userspace needs the _OLD values exposed. I suggest either keep them under _KERNEL or move to .c files.

Ah, yes, I agree. Under _KERNEL is fine. I think it's useful to leave them in the header so it's clear that these values are not available for future operations.

Strictly speaking, the comment is wrong. Thread may use FPU if core dumping write path utilizes FPU in kernel. Think about things like geli.

Worse, if context switch happens after fpugetregs() but before bcopy() finished, and FPU state was still loaded into hw, the context switch would spill the state into the save area, overriding your mask.

I somewhat dislike this. My feel is that for ABI extensibility and stability it would be better to supply both return values from the same path, i.e. define some structute which contains both mask and ext_state size, and copy it out. Also, a padding for future extensions might be due.

Why copy of dst into buf is needed ?

Why ? buf is not used afterfard.

Use static const char freebsd[] = "FreeBSD" ? And then, you can just use the pointer to array and sizeof() of it, instead of magic '8' and strlen ?

Very nice cleanup, probably should be committed separately.

For the first bit, I'm worried that overwriting this will cause problems with the thread reloading the state from the save area. (i.e. the xrstor would fault due to the garbage at this location). Alternatively, I could use a gross hack to determine the offset into 'buf' and do this after elf64_populate_note(). It's just really gross to do (and I'm trying to avoid having to create another temporary buffer. For one, I can't return an error from here and the rest of the coredump code uses M_NOWAIT allocations).

So I think a temporary buffer is out. If we know for certain that xrstor won't happen after this point (the thread can't go back into userland to do a DNA trap, and presumably even if something like geli is used it will just do an xsave of the current state and then load in a kernel-side state, but it won't do an xrstor when it is done, it would leave fpcurthread clear and be waiting for a DNA trap?) then a critical section might be ok. Otherwise I will have to frob buf afterwards, which is gross, but doable I guess.

Yeah, I started out returning only one thing and then added xcr. I can just do a structure. I'm not sure what else I would add to it, but perhaps we could require 'data' to be the size of the structure being fetched to make it extendable in the future?

I did this (in all 3 versions) so that buf would be ready for a future MD note added in the future. However, I can drop that and someone else can add it in the future if needed.

This is actually copy and pasted from elsewhere in the file, but I would prefer this, yes. You might even be able to just use 'sizeof("FreeBSD")' directly. However, a 'static const char note_name[]' that the entire file used would be better.

There are two options.

One, which is harder, is to allow to compose note data from pieces.

Another, much simpler, is to drop state from hardware in advance. Like

critical_enter();
fpudrop();
critical_leave();
do your stuff;
restore the save area;

fpudrop() ensures that context switch does not need to do the save. Since using FPU in interrupt context is disabled, this should work.

Yes, I want the structure. And I think that requiring data be the size is good idea. ptrace(PT_LWPINFO) already does this.

Fine, keep it, but please add a comment.

Hmm, I ended up letting elfNN_populate_note() return a pointer to the start of the payload inside the note. I then use that to stash the saved copy of the mask. (So I never write into the thread's saved area directly). Let me know what you think of that approach vs the other.