Page MenuHomeFreeBSD

pcb: mark fields that should be preserved for kernel debuggers
ClosedPublic

Authored by minsoochoo0122_proton.me on Fri, Feb 6, 8:04 PM.
Tags
None
Referenced Files
F145242136: D55149.id171381.diff
Tue, Feb 17, 12:18 PM
Unknown Object (File)
Sun, Feb 15, 1:03 PM
Unknown Object (File)
Sun, Feb 15, 11:48 AM
Unknown Object (File)
Sat, Feb 14, 7:12 AM
Unknown Object (File)
Fri, Feb 13, 11:52 AM
Unknown Object (File)
Fri, Feb 13, 10:19 AM
Unknown Object (File)
Fri, Feb 13, 6:51 AM
Unknown Object (File)
Fri, Feb 13, 3:26 AM

Details

Summary

Signed-off-by: Minsoo Choo <minsoochoo0122@proton.me>

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

My impression is that the whole pcb have to be considered part of the contract, unfortunately.
IMO instead of copying the amd64-style of the comment, it is better to remove the *-marking and make a herald comment describing that.

In D55149#1260992, @kib wrote:

My impression is that the whole pcb have to be considered part of the contract, unfortunately.
IMO instead of copying the amd64-style of the comment, it is better to remove the *-marking and make a herald comment describing that.

Why should we preserve the whole pcb structure? Are there programs depend on this other than kernel debuggers?

In D55149#1260992, @kib wrote:

My impression is that the whole pcb have to be considered part of the contract, unfortunately.
IMO instead of copying the amd64-style of the comment, it is better to remove the *-marking and make a herald comment describing that.

Why should we preserve the whole pcb structure? Are there programs depend on this other than kernel debuggers?

I had to add and preserve the padding fields pcb_pad in the amd64 structure to be compatible with kgdb at least. This makes me sure that the whole pcb layout is fixed by ABI.

In D55149#1261002, @kib wrote:
In D55149#1260992, @kib wrote:

My impression is that the whole pcb have to be considered part of the contract, unfortunately.
IMO instead of copying the amd64-style of the comment, it is better to remove the *-marking and make a herald comment describing that.

Why should we preserve the whole pcb structure? Are there programs depend on this other than kernel debuggers?

I had to add and preserve the padding fields pcb_pad in the amd64 structure to be compatible with kgdb at least. This makes me sure that the whole pcb layout is fixed by ABI.

Sounds good to me. Since I'm not familiar with herald comment, could you write one for me?

sys/amd64/include/pcb.h
47–49
sys/amd64/include/pcb.h
47–49

Should I do the same for trapframes?

sys/amd64/include/pcb.h
47–49

Are trapframes directly accessed by debuggers?

I am not surprised, but I plan to work on something that definitely changes trapframe on amd64. Most likely, it would only add some stuff at the end of the structure, and this 'something' is defined by hardware. But still, it is formally ABI breakage which is inevitable in this case.

sys/amd64/include/pcb.h
47–49

https://cgit.freebsd.org/ports/tree/devel/gdb/files/kgdb/amd64fbsd-kern.c:

Trapframe offsets are hardcoded like pcb offsets, so reordering will break the ABI. Adding extra field should be fine unless you want to add it to kgdb.

sys/amd64/include/pcb.h
47–49

Then I suggest to talk about layout of existing fields in the comments, instead of the whole structure ABI.
I.e. the structure can be extended, but the layout (and semantic?) of existing fields must not change.

  • frame.h: mark struct trapframe to be preserved
kib added inline comments.
sys/x86/include/frame.h
122 ↗(On Diff #171381)
123 ↗(On Diff #171381)
This revision is now accepted and ready to land.Sat, Feb 7, 3:19 AM
This revision now requires review to proceed.Sat, Feb 7, 4:25 AM
sys/x86/include/frame.h
121 ↗(On Diff #171387)

I suggest two drop the trapframe part of the change, and commit the pcb comment, it is fine with me as is.

After that, the trapframe part can be ironed in the separate review. I do not think it is correct to say about it that it must not be modified, because parts of it, at least on x86, are defined by hw. Unlike struct pcb, which is pure software construct.

This revision is now accepted and ready to land.Sat, Feb 7, 3:26 PM
This revision was automatically updated to reflect the committed changes.