Page MenuHomeFreeBSD

loader: biosdisk interface should be able to cope with 4k sectors
ClosedPublic

Authored by tsoome on Jun 13 2017, 12:24 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Apr 22, 5:31 AM
Unknown Object (File)
Fri, Apr 12, 3:36 PM
Unknown Object (File)
Fri, Apr 12, 1:47 PM
Unknown Object (File)
Fri, Apr 12, 1:47 PM
Unknown Object (File)
Fri, Apr 12, 1:46 PM
Unknown Object (File)
Fri, Apr 12, 6:39 AM
Unknown Object (File)
Feb 20 2024, 1:22 AM
Unknown Object (File)
Jan 11 2024, 9:50 PM
Subscribers

Details

Summary

The 4kn support in current bios specific biosdisk.c is broken, as the code
is only implementing the support for the 512B sector size.

This work is building the support for custom size sectors, we still do assume
the requested data to be multiple of 512B blocks and we only do address the
biosdisk.c interface here.

For reference, see also:
https://www.illumos.org/issues/8303
https://www.illumos.org/rb/r/547

As the GELI is moved above biosdisk "layer", the GELI should just work

Test Plan

Need system(s) with 4kn disks - at this time, we only can test data disks because earlier stages only do support 512B sector size too. 512B sector disks are tested.

Diff Detail

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

Event Timeline

Looks good to me. What needs to be done for the GELI bits?

sys/boot/i386/libi386/biosdisk.c
573 ↗(On Diff #29536)

... the IO size is a multiple of ...

575 ↗(On Diff #29536)

assuming IO is a multiple of ...

576 ↗(On Diff #29536)

a multiple of ...

597 ↗(On Diff #29536)

requested read: %zu too large

Looks good to me. What needs to be done for the GELI bits?

As for the first round I essentially did move the bounce buffer usage from bd_io to strategy; the current geli does assume it is still in bd_io. But I think that is not too hard to work out. The another thing is that for 4kn, we actually do the same LBA/buffer adjustments, so I *think*, it probably is good idea to merge the current geli code in bd_read with the strategy bits, so the LBA/buffer manipulations would get done if it is either 4kn *or* GELI. So we wont duplicate that code. Hm, and I think that should pretty much cover it - of course the geli case would need to use the temporary buffer for decryption - but thats what we use anyhow. Hm, i think that would pretty much cover it. Hm, I wonder if we can make the bd_open() and geli taste a bit more generic too, so we could just use the "normal" strategy for probing...

tsoome marked 3 inline comments as done.

Comment wording as suggested by allanjude

tsoome edited the test plan for this revision. (Show Details)
imp requested changes to this revision.Aug 16 2018, 1:08 PM
imp added inline comments.
stand/i386/libi386/biosdisk.c
500 ↗(On Diff #46763)

The original comment is better. this one is more awkward English.

639 ↗(On Diff #46763)

where did the bouncing go?

705 ↗(On Diff #46763)

This looks like an unrelated change.

This revision now requires changes to proceed.Aug 16 2018, 1:08 PM

Comments updated.

stand/i386/libi386/biosdisk.c
705 ↗(On Diff #46763)

No, it was at line 510 before, but in a bit different form. Later on we will need to update it a bit more as there are ~3 different codes known to hint about missing media. But for this round, I want to keep the status quo as much as I can.

tsoome edited the test plan for this revision. (Show Details)

reverted comment, requested by imp.

stand/i386/libi386/biosdisk.c
639 ↗(On Diff #46763)

we set up and use bounce buffer (bbuf) in bd_realstrategy(). We set up buffer and establish what and how much to read/write, in bd_io() we perform the IO and handle the errors.

stand/i386/libi386/biosdisk.c
639 ↗(On Diff #46763)

but where do we test to see if we need to bounce or not?

stand/i386/libi386/biosdisk.c
639 ↗(On Diff #46763)

In this rev we do not test, we always bounce. We can skip bounce when the buffer for strategy() is allocated from BSS or data segment or stack, but in most cases we get buffer from heap (bcache). It is not hard to move the check over of course.

I generally like the latest version...
Modulo my one question about divide by zero.

stand/i386/libi386/biosdisk.c
263 ↗(On Diff #49461)

With no 'else' does sector_size remain 0?

472 ↗(On Diff #49461)

How do we know sectorsize != 0 here?

tsoome added inline comments.
stand/i386/libi386/biosdisk.c
263 ↗(On Diff #49461)

BIOSDISK_SECSIZE actually, it is set here (in bd_int13probe) at line 222.

472 ↗(On Diff #49461)

We have it set in bd_int13probe.

This revision was not accepted when it landed; it landed in state Needs Review.Oct 23 2018, 2:44 PM
This revision was automatically updated to reflect the committed changes.
tsoome marked 2 inline comments as done.