Page MenuHomeFreeBSD

Add SHA512, skein, large blocks support for loader zfs.
ClosedPublic

Authored by tsoome on Aug 4 2016, 6:16 PM.

Details

Summary

loader zfs reader is missing several features, which can be supported.

Updated sha512 from illumos.
Using skein from freebsd crypto tree.
Since loader itself is using 64MB memory for heap, updated zfsboot to
use same, and this also allows to support large blocks.

Note, adding additional features does increate zfsboot code, therefore
this update does increase zfsboot code to 128k, also I have ported gptldr.S
update to zfsldr.S to support 64k+ code.

Set copy count for gptldr/zfsldr blocks to 8 - this is more than enough for
quite some time.

With little effort it is possible to add edonr, gzip would need more updates,
so to keep the changes relatively simple, I'm not adding gzip this time.

Test Plan

I do not have facilities to test userboot nor sparc, it would be nice to have
confirmation for those.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

tsoome retitled this revision from to Add SHA512, skein, large blocks support for loader zfs..
tsoome updated this object.
tsoome edited the test plan for this revision. (Show Details)

I tested this patch. Code builds and boot1/loader work. But I can't test that new functionality works because "zfs" utility still does not allow me to set these properties on my boot pool.

I tested this patch. Code builds and boot1/loader work. But I can't test that new functionality works because "zfs" utility still does not allow me to set these properties on my boot pool.

If you are fine with the rest of not being able to boot the pool (ever again):

zpool set bootfs= pool
zfs set checksum=skein pool/dataset
zpool set bootfs=pool/ROOT/default pool

OK! Testing coming up! I got a new laptop that I didn't put anything worth retaining on so that I can test zpool things.

That boots for me!

$ df /
Filesystem 1K-blocks Used Avail Capacity Mounted on
tank/ROOT/zfs-boot-test 477224124 3837044 473387080 1% /
$ zfs get checksum tank
NAME PROPERTY VALUE SOURCE
tank checksum skein local
$ zpool get bootfs tank
NAME PROPERTY VALUE SOURCE
tank bootfs tank/ROOT/zfs-boot-test local

Updated to revision 304018.

That boots for me!

$ df /
Filesystem 1K-blocks Used Avail Capacity Mounted on
tank/ROOT/zfs-boot-test 477224124 3837044 473387080 1% /
$ zfs get checksum tank
NAME PROPERTY VALUE SOURCE
tank checksum skein local
$ zpool get bootfs tank
NAME PROPERTY VALUE SOURCE
tank bootfs tank/ROOT/zfs-boot-test local

Thanks. Did you also check sha512? Also, remember the checksum setting change has immediate effect only on new writes, so if no files were touched, only some metadata got new checksum (which does prove the bootability anyhow;)

This does work with sha512. Is there any way we can get this into 11.0-RELEASE?

This does work with sha512. Is there any way we can get this into 11.0-RELEASE?

I don't think so:

"This is a reminder that commit requests to releng/11.0 at this point
should be for critical fixes only. It is still unclear if 11.0-RC3 will
be necessary, but we will not allow anything intrusive enough to require it."

I would be nice to have, but I don't think critical;)

The change looks good to me overall, thanks for working on it!

I haven't yet got a chance to actually build the boot1.efi image, which must be smaller than 128K due to limitation of the current template (but in that case, the build would fail, so I assume it worked for you?)

Add Ed as a reviewer in the meantime.

delphij added a reviewer: delphij.

Looks good to me.

This revision is now accepted and ready to land.Aug 15 2016, 12:02 AM

The change looks good to me overall, thanks for working on it!

I haven't yet got a chance to actually build the boot1.efi image, which must be smaller than 128K due to limitation of the current template (but in that case, the build would fail, so I assume it worked for you?)

Add Ed as a reviewer in the meantime.

120832 bytes for boot1.efi - it is close, the current limit is 131072 bytes. I think we should actually rise the limit in template, with all zfs features included (gzip), I have bootia32.efi 211456 bytes and bootx64.efi 169472 bytes in illumos port (UEFI32 is there to support boards with 32bit UEFI but capable of running 64bit kernel).

The change looks good to me overall, thanks for working on it!

I haven't yet got a chance to actually build the boot1.efi image, which must be smaller than 128K due to limitation of the current template (but in that case, the build would fail, so I assume it worked for you?)

Add Ed as a reviewer in the meantime.

120832 bytes for boot1.efi - it is close, the current limit is 131072 bytes. I think we should actually rise the limit in template, with all zfs features included (gzip), I have bootia32.efi 211456 bytes and bootx64.efi 169472 bytes in illumos port (UEFI32 is there to support boards with 32bit UEFI but capable of running 64bit kernel).

We should make boot1.efi have a limit of 256kb. There's nothing really limiting it. And our boot1.efifat already has 800k (which really should be like 20M these days because of the need to land efi images on the boot partitions for vendor firmware upgrades and the like).

In D7418#155796, @imp wrote:

The change looks good to me overall, thanks for working on it!

I haven't yet got a chance to actually build the boot1.efi image, which must be smaller than 128K due to limitation of the current template (but in that case, the build would fail, so I assume it worked for you?)

Add Ed as a reviewer in the meantime.

120832 bytes for boot1.efi - it is close, the current limit is 131072 bytes. I think we should actually rise the limit in template, with all zfs features included (gzip), I have bootia32.efi 211456 bytes and bootx64.efi 169472 bytes in illumos port (UEFI32 is there to support boards with 32bit UEFI but capable of running 64bit kernel).

We should make boot1.efi have a limit of 256kb. There's nothing really limiting it. And our boot1.efifat already has 800k (which really should be like 20M these days because of the need to land efi images on the boot partitions for vendor firmware upgrades and the like).

The size of efifat (or efi system partition( is actually a bit more complex issue, and should belong into another topic, but just to give quick known facts...

  1. the uefi standard by itself does not really set limit on fat type, except it does mention that to support removable media the fat12/fat16 should be supported.
  1. people have found cases where system does not boot unless system partition is using fat32. (google did point to some ubuntu support forum).
  1. fat32 by itself means minimum 33MB with 512B sector size (or was it 34MB).
  1. Microsoft is requiring 128MB I think - they have way more complex boot logic and need to store early modules.
  1. minimum fat32 with 4k sector size is 256MB, which is probably why Oracle is using this size with Oracle Solaris.

Obviously for single OS setup even going after fat32 will give quite some playground, if it really is enough for possible scenarios, is another question and outside of the scope of this issue here:)

For what it's worth, I was able to install boot1.efifat to my 800K ESP during the testing.
Can we MFC this to stable/11 so that it's available in 11.1?

imp added a reviewer: imp.
This revision was automatically updated to reflect the committed changes.