Signed-off-by: Minsoo Choo <minsoochoo0122@proton.me>
Details
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.
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. | |
| 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. |