Page MenuHomeFreeBSD

Replace EFI part devices.
ClosedPublic

Authored by tsoome on Nov 19 2016, 5:03 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Jul 17, 7:00 PM
Unknown Object (File)
Mon, Jul 15, 11:03 AM
Unknown Object (File)
Mon, Jul 1, 1:20 PM
Unknown Object (File)
Mon, Jul 1, 8:57 AM
Unknown Object (File)
Mon, Jul 1, 6:35 AM
Unknown Object (File)
Sun, Jun 30, 9:58 AM
Unknown Object (File)
Sun, Jun 30, 9:04 AM
Unknown Object (File)
Sun, Jun 30, 9:00 AM

Details

Summary

partX: devices are unfortunately not so informative, so this patch is an
attempt to use existing partitioning code to present the devices in more
user friendly way.

Test Plan

I have tested this code on illumos (where it was originally developed), with
usb/cd/disk boot. However, the same series of tests should be
performed on freebsd as well.

Diff Detail

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

Event Timeline

tsoome retitled this revision from to Replace EFI part devices..
tsoome updated this object.
tsoome edited the test plan for this revision. (Show Details)
tsoome added reviewers: allanjude, imp.
sys/boot/common/disk.c
283 ↗(On Diff #22351)

why would a disk_ioctl return NOTTY?

There's another review out that goes a bit in the other direction. It creates a UEFI UFS driver for easier booting. Perhaps you can reconcile the two?

sys/boot/common/disk.c
283 ↗(On Diff #22351)

because ENOTTY means "ioctl not supported"

sys/boot/common/disk.c
283 ↗(On Diff #22351)

Thats traditional way (esp. in stacked drivers) to say "I don't implement this stuff", so the next layer can answer instead. So when some another command is added, disk_ioctl will return ENOTTY and block driver can respond if it does implement it.

Note the disk_ioctl implementation this way is kind of hack, I did not want to add up just random command code - as in fbsd the headers are actually shared between kernel and loader, but this way I can get the partition size, and it [the size of the partition] can be really helpful in different scenarios.

lidl_pix.net added inline comments.
sys/boot/efi/libefi/efipart.c
463 ↗(On Diff #22351)

Indentation needed.

469 ↗(On Diff #22351)

This looks like a leftover debugging printf().

In D8581#178131, @imp wrote:

There's another review out that goes a bit in the other direction. It creates a UEFI UFS driver for easier booting. Perhaps you can reconcile the two?

you mean the one implementing efi file system layer? I dont think those two should necessarily be even conflicting, because as an human, you still would like to know what devices you have and where from you are booting from. This work does not touch the file systems, just the block devices behind the scenes.

tsoome marked 2 inline comments as done.
tsoome edited edge metadata.

cstyle and leftover debug printf.

validate od pointer in disk_ioctl.

Filter out nested partitions. At least AMI does create device tree like:
../Sata(..)/HD(1,GPT,...)/HD(1,MBR,...) for PMBR entry.

We only need the disk device handles and first level partition to help
us to identify the disk, we can just ignore nested partitions.

Adjust the device name printout on verbose lsdev to make it easier to read.

Updated to revision 311995.

imp edited edge metadata.

I think this is fine. Nothing is leaping out at me

This revision is now accepted and ready to land.Jan 12 2017, 9:07 PM
tsoome edited edge metadata.

rebase on r312374, updated the NULL checks on efi_devpath_last_node()
and efi_devpath_trim().

This revision now requires review to proceed.Jan 18 2017, 8:37 AM
tsoome edited edge metadata.

svn update to 313329

allanjude edited edge metadata.

Approved for commit

This revision is now accepted and ready to land.Feb 6 2017, 9:06 AM
This revision was automatically updated to reflect the committed changes.