Skip to content

Commit

Permalink
Add extra RestoreTpl() call in DiskIo to maintain TPL raise/restore s…
Browse files Browse the repository at this point in the history
…ymmetry (#230)

Adds a call to RestoreTpl() in DiskIo2ReadWriteDisk(). While the current
implementation does not technically violate spec on raise/restore TPL,
this extra call ensures symmetry between RaiseTpl and RestoreTpl calls,
which makes analysis of TPL correctness simpler and permits certain
non-standard TPL usages that some platforms require.

- [ ] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

Booted a system using this driver for disk access, observed that
symmetry between Raise/Restore TPL was maintained as expected.

N/A

Remove unnecessary TPL restore call (#229)

Comments out a redundant call to RestoreTpl(). While this does not
technically violate spec on raise/restore TPL, TPL should already be at
the specified level. This extra call introduces an asymmetry between
RaiseTpl and RestoreTpl calls, which makes analysis of TPL correctness
more difficult and hampers certain non-standard TPL usages that some
platforms require.

- [ ] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

Booted and observed no un-intended side effects w.r.t. TPL with this
modification. Added test instrumentation to verify that TPL is always
already at the desired state prior to this call being executed.

N/A

---------

Co-authored-by: Michael Kubacki <michael.kubacki@microsoft.com>
  • Loading branch information
2 people authored and kenlautner committed May 9, 2023
1 parent 69c9f03 commit 946e51b
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 2 deletions.
5 changes: 4 additions & 1 deletion MdeModulePkg/Core/Dxe/Image/Image.c
Original file line number Diff line number Diff line change
Expand Up @@ -1768,7 +1768,10 @@ CoreStartImage (
// Image has completed. Verify the tpl is the same
//
ASSERT (Image->Tpl == gEfiCurrentTpl);
CoreRestoreTpl (Image->Tpl);
if (Image->Tpl != gEfiCurrentTpl) {
// MU_CHANGE - reduce TPL restore
CoreRestoreTpl (Image->Tpl);
} // MU_CHANGE - reduce TPL restore

CoreFreePool (Image->JumpBuffer);

Expand Down
4 changes: 3 additions & 1 deletion MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIo.c
Original file line number Diff line number Diff line change
Expand Up @@ -847,6 +847,7 @@ DiskIo2ReadWriteDisk (
DISK_IO_SUBTASK *Subtask;
DISK_IO2_TASK *Task;
EFI_TPL OldTpl;
EFI_TPL OldTpl1; // MU_CHANGE - TPL Symmetry
BOOLEAN Blocking;
BOOLEAN SubtaskBlocking;
LIST_ENTRY *SubtasksPtr;
Expand Down Expand Up @@ -977,7 +978,7 @@ DiskIo2ReadWriteDisk (
}
}

gBS->RaiseTPL (TPL_NOTIFY);
OldTpl1 = gBS->RaiseTPL (TPL_NOTIFY); // MU_CHANGE - TPL symmetry

//
// Remove all the remaining subtasks when failure.
Expand Down Expand Up @@ -1012,6 +1013,7 @@ DiskIo2ReadWriteDisk (
FreePool (Task);
}

gBS->RestoreTPL (OldTpl1); // MU_CHANGE TPL Symmetry
gBS->RestoreTPL (OldTpl);

return Status;
Expand Down

0 comments on commit 946e51b

Please sign in to comment.