Page MenuHomeFreeBSD
Differential D21655

efipart: use page-aligned buffer for BlockIO
AbandonedPublic
Actions

Authored by kevans on Sep 14 2019, 2:06 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Apr 18, 10:45 PM
Unknown Object (File)
Dec 22 2023, 9:44 PM
Unknown Object (File)
Dec 21 2023, 2:38 PM
Unknown Object (File)
Nov 4 2023, 9:14 AM
Unknown Object (File)
Jul 12 2023, 2:58 PM
Unknown Object (File)
Apr 8 2023, 12:31 AM
Unknown Object (File)
Mar 21 2023, 8:28 PM
Subscribers
bcran

Details

Reviewers
imp
tsoome
manu
bcran
Summary

Using non-page-aligned buffers should raise EFI_INVALID_PARAMETER according to the spec. I would suspect the efipart_readwrite call just above this is not necessarily kosher, but I have not thought about it too much.

PR: 240572

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 26491

Event Timeline

kevans created this revision.Sep 14 2019, 2:06 AM
Herald added a subscriber: bcran. · View Herald TranscriptSep 14 2019, 2:06 AM
Harbormaster completed remote builds in B26491: Diff 62082.Sep 14 2019, 2:06 AM
bcran accepted this revision.Sep 14 2019, 2:24 AM
This revision is now accepted and ready to land.Sep 14 2019, 2:24 AM
tsoome requested changes to this revision.Sep 14 2019, 9:00 AM

Yes, this fix is not complete. We should always use "bounce buffer" here and only use aligned memory. Of course the disk write is also broken here (unless I'm mistaken)...

This revision now requires changes to proceed.Sep 14 2019, 9:00 AM
kevans added a comment.Sep 14 2019, 1:45 PM
In D21655#472020, @tsoome wrote:

Yes, this fix is not complete. We should always use "bounce buffer" here and only use aligned memory. Of course the disk write is also broken here (unless I'm mistaken)...

Any chance you have a little bit for this? I've run out of time for loader stuff for now.

bcran added a comment.Sep 14 2019, 3:14 PM

Actually yes, this change isn't correct. See https://github.com/tianocore/edk2/blob/master/MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c for an example of how to allocate aligned memory (InternalAllocateAlignedPages etc.)

bcran requested changes to this revision.Sep 14 2019, 3:15 PM
tsoome added a comment.Sep 14 2019, 3:27 PM
In D21655#472098, @bcran wrote:

Actually yes, this change isn't correct. See https://github.com/tianocore/edk2/blob/master/MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c for an example of how to allocate aligned memory (InternalAllocateAlignedPages etc.)

It is not just about an allocation; we actually should check ioalign property...

kevans: Im not sure when exactly I can look on it, also I 'm not sure about testing - does the qemu help me there?

kevans abandoned this revision.Sep 17 2019, 2:50 PM

rS352446 should fix the alignment issues here.

Revision Contents
Changeset List

PathSize
stand/
efi/
libefi/
12 lines

Diff 62082

View Options

stand/efi/libefi/efipart.c

Loading...