Page MenuHomeFreeBSD
Differential D55189

frame.h: mark struct trapframe to be preserved
ClosedPublic
Actions

Authored by minsoochoo0122_proton.me on Sun, Feb 8, 11:07 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Feb 13, 12:52 PM
Unknown Object (File)
Fri, Feb 13, 3:06 AM
Unknown Object (File)
Thu, Feb 12, 2:39 AM
Unknown Object (File)
Wed, Feb 11, 6:56 PM
Unknown Object (File)
Wed, Feb 11, 2:55 PM
Unknown Object (File)
Wed, Feb 11, 12:26 PM
Unknown Object (File)
Wed, Feb 11, 9:17 AM
Unknown Object (File)
Wed, Feb 11, 3:03 AM
View All 9 Files
Subscribers
emaste
imp
jhibbits
kib
riscv

Details

Reviewers
andrew
manu
Commits
rG2088e742013b: x86: Note that trapframe is used by kernel debuggers
Summary

There are programs that depend on this structure (e.g. kernel debuggers)
that breaks when the ABI changes.

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

minsoochoo0122_proton.me created this revision.Sun, Feb 8, 11:07 PM
Herald added a reviewer: andrew. · View Herald TranscriptSun, Feb 8, 11:07 PM
Herald added a reviewer: andrew. · View Herald Transcript
Herald added a reviewer: manu. · View Herald Transcript
Herald added subscribers: riscv, jhibbits, emaste, imp. · View Herald Transcript
minsoochoo0122_proton.me requested review of this revision.Sun, Feb 8, 11:07 PM
Harbormaster completed remote builds in B70531: Diff 171471.Sun, Feb 8, 11:07 PM
minsoochoo0122_proton.me added a subscriber: kib.Sun, Feb 8, 11:08 PM
kib added inline comments.Mon, Feb 9, 12:08 AM
sys/x86/include/frame.h
50

It is enough to have one instance of this in x86/frame.h. Move one comment out of #ifdef, and remove another.

Then, the wording must be completely different, at least for x86, because the layout is defined by hardware, and its changes in somewhat compatible way.

So I think it would be fine to have sentence like 'struct trapframe is known to kernel debuggers.' But not more.

minsoochoo0122_proton.me updated this revision to Diff 171477.Mon, Feb 9, 1:06 AM

cleanup x86

minsoochoo0122_proton.me marked an inline comment as done.Mon, Feb 9, 1:06 AM
Harbormaster completed remote builds in B70534: Diff 171477.Mon, Feb 9, 1:06 AM
kib added inline comments.Mon, Feb 9, 1:09 AM
sys/x86/include/frame.h
41

Remove this blank line.

Then I suggest you to commit the x86 part.

minsoochoo0122_proton.me updated this revision to Diff 171478.Mon, Feb 9, 1:21 AM

remove blank line and separate commit for x86

Harbormaster completed remote builds in B70535: Diff 171478.Mon, Feb 9, 1:21 AM
minsoochoo0122_proton.me marked an inline comment as done.Mon, Feb 9, 1:21 AM
kib added inline comments.Mon, Feb 9, 1:31 AM
sys/x86/include/frame.h
47

No, move this line to the top-level comment, applicable to both i386 and amd64.

minsoochoo0122_proton.me updated this revision to Diff 171480.Mon, Feb 9, 1:37 AM

update misplaced comment

Harbormaster completed remote builds in B70536: Diff 171480.Mon, Feb 9, 1:37 AM
minsoochoo0122_proton.me marked an inline comment as done.Mon, Feb 9, 1:37 AM
kib added a comment.Mon, Feb 9, 1:41 AM

Please commit the x86 change.

minsoochoo0122_proton.me added a comment.Mon, Feb 9, 1:45 AM
In D55189#1261892, @kib wrote:

Please commit the x86 change.

I did in 728adff455a3. Or do you mean submit only x86 change and discard others?

kib added a comment.Mon, Feb 9, 1:51 AM
In D55189#1261893, @minsoochoo0122_proton.me wrote:
In D55189#1261892, @kib wrote:

Please commit the x86 change.

I did in 728adff455a3. Or do you mean submit only x86 change and discard others?

I do not see this revision in the FreeBSD git repo. I mean literally commit the sys/x86/include part, leaving other arches for their maintainers to review.

minsoochoo0122_proton.me added a comment.Mon, Feb 9, 1:55 AM
In D55189#1261894, @kib wrote:
In D55189#1261893, @minsoochoo0122_proton.me wrote:
In D55189#1261892, @kib wrote:

Please commit the x86 change.

I did in 728adff455a3. Or do you mean submit only x86 change and discard others?

I do not see this revision in the FreeBSD git repo. I mean literally commit the sys/x86/include part, leaving other arches for their maintainers to review.

For 728adff455a3, I meant under commits tab in this revision. I don't understand what you mean by commit because I don't have commit bit. Sorry if I'm causing miscommunication here, but I'm confused.

kib added a comment.Mon, Feb 9, 1:58 AM
In D55189#1261895, @minsoochoo0122_proton.me wrote:
In D55189#1261894, @kib wrote:
In D55189#1261893, @minsoochoo0122_proton.me wrote:
In D55189#1261892, @kib wrote:

Please commit the x86 change.

I did in 728adff455a3. Or do you mean submit only x86 change and discard others?

I do not see this revision in the FreeBSD git repo. I mean literally commit the sys/x86/include part, leaving other arches for their maintainers to review.

For 728adff455a3, I meant under commits tab in this revision. I don't understand what you mean by commit because I don't have commit bit. Sorry if I'm causing miscommunication here, but I'm confused.

Ok, send me the git-format patch for the x86 part, with the right author, commit message, and all the metadata. I will push it for you.

This revision was not accepted when it landed; it landed in state Needs Review.Mon, Feb 9, 2:59 AM
Closed by commit rG2088e742013b: x86: Note that trapframe is used by kernel debuggers (authored by minsoochoo0122_proton.me, committed by kib). · Explain Why
This revision was automatically updated to reflect the committed changes.
kib added a commit: rG2088e742013b: x86: Note that trapframe is used by kernel debuggers.

Revision Contents
Changeset List

PathSize
sys/
x86/
include/
1 line

Diff 171481

View Options

sys/x86/include/frame.h

Loading...