Page MenuHomeFreeBSD
Differential D3667

Re-check for ast after ast handler
ClosedPublic
Actions

Authored by kib on Sep 14 2015, 10:23 PM.
Tags
Referenced Files
Unknown Object (File)
Sat, Dec 21, 2:02 AM
Unknown Object (File)
Thu, Dec 5, 5:29 AM
Unknown Object (File)
Sun, Dec 1, 2:04 AM
Unknown Object (File)
Oct 5 2024, 9:55 AM
Unknown Object (File)
Oct 2 2024, 12:00 PM
Unknown Object (File)
Oct 1 2024, 10:48 PM
Unknown Object (File)
Oct 1 2024, 7:59 PM
Unknown Object (File)
Sep 23 2024, 4:21 PM
View All 84 Files
Subscribers
emaste
imp

Details

Reviewers
andrew
Commits
rS288115: Re-check for new ast after ast was handled. We should not return to
Summary

MD code must recheck for new ast scheduled after the existing ast was handled. It seems that the branch was missed as result of typo, the code is correctly structured to reenter the loop.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kib updated this revision to Diff 8760.Sep 14 2015, 10:23 PM
kib retitled this revision from to Re-check for ast after ast handler.
kib updated this object.
kib edited the test plan for this revision. (Show Details)
kib added a reviewer: andrew.
kib set the repository for this revision to rS FreeBSD src repository - subversion.
kib added a project: arm64.
Herald added subscribers: emaste, imp. · View Herald TranscriptSep 14 2015, 10:23 PM
emaste added a comment.Sep 15 2015, 9:34 PM
  • Seems like it ought to have a comment
  • Lines 123-125 are the same as 101-103, no?
kib added a comment.Sep 17 2015, 1:21 AM

Indeed, it seems that placing the 1: label earlier would allow to remove the daif read duplication.

kib updated this revision to Diff 8809.Sep 17 2015, 1:22 AM

Remove duplication; add comment.

andrew added inline comments.Sep 18 2015, 1:08 PM
sys/arm64/arm64/exception.S
101 ↗(On Diff #8809)

I think you could move this to the daifset instruction below. Unless there is a case where interrupts are left dis/enabled after calling ast this shouldn't change.

Along with this the restore interrupts code below can also be removed, the next instruction after do_ast disables interrupts as we will be modifying x18, which may be in an inconsistent state.

kib added inline comments.Sep 18 2015, 5:54 PM
sys/arm64/arm64/exception.S
101 ↗(On Diff #8809)

I moved the '1' label to msr instruction. x19 is callee-saved.

Do you propose to not restore the the interrupts state around the call to ast ? If yes, I do not think this is correct. Ast handling should be done with interrupts enabled. This way, I even think that we should not restore the interrupts, but explicitely enable them. But the patch is probably fine for now.

kib updated this revision to Diff 8836.Sep 18 2015, 5:55 PM

Updates after last comment.

andrew accepted this revision.Sep 22 2015, 2:41 PM
andrew edited edge metadata.
This revision is now accepted and ready to land.Sep 22 2015, 2:41 PM
Closed by commit rS288115: Re-check for new ast after ast was handled. We should not return to (authored by kib). · Explain WhySep 22 2015, 4:30 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
head/
sys/
arm64/
arm64/
8 lines

Diff 8887

View Options

head/sys/arm64/arm64/exception.S

Loading...