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

Review Request #547 — Created May 29, 2017 and submitted

tsoome
illumos-gate
8303
577
c2ea3ab...
general

loader: 4k block io

This patch is really just about "custom" sector size support in biosdisk.c, with intention to support the sector sizes 512, 2048, 4096, so we could merge biosdisk and bioscd interfaces to reduce the code duplication.

The actual merge shall be implemented as separate patch.

The 4kn disk boot support will additionally need pmbr code update, as the current implementation is assuming 512B sectors.

This code is currently tested on 2 systems, one with 6TB disks and another with 8TB disks and the fix is confirmed (details are in issue 8303).

  • 0
  • 0
  • 9
  • 1
  • 10
Description From Last Updated
tsoome
tsoome
yuripv
  1. 
      
  2. explicit != 0 check

  3. explicit != 0 check

  4. what's 0x20?

    1. controller not ready (usually happening with floppy devices when there is no floppy), but there was one report with other device - likely the sd card reader or like. Of course this will need comment.

  5. 
      
alp
  1. 
      
  2. Shouldn't BIOSDISK_SECSIZE constant be used here instead of '512'?

  3. 
      
tsoome
rm
  1. 
      
  2. is a multiple of 512 bytes?
    1. yes, as those reads are coming from file system layer, we do get the IO based on multiple of 512B blocks, so this check below is really just kind of safeguard.

    2. I meant the comment should read that. 'First make sure the IO is a multiple of 512 bytes.'

  3. Given tha we end up handling partial disk reads at +486, why do we still maintain this restriction based on BIOSDISK_SECSIZE?
  4. Since this is an int64_t * 512 should we worry about overflow? Why do we use BIOSDISK_SECSIZE when we use the device's sector size elsewhere?

    1. the 512 and the multiplication is actually coming from the fact that both libstand read and bcache block number is calculated based on 512, so we actually do get the disk block number in 512B blocks (from bcache and libstand read()). So we are just converting back to bytes and then calculating the "real" disk block based on actual media block size. Also for the same reason, we can not overflow.

  5. So, if blkoff is the offset into a block that we want to write and bd_io only operates in terms of blocks, doesn't this mean that part of bbuf is uninitialized or am I misunderstanding how this is expected to work?
    1. right, this one seems to be buggy as for first write, we actually need to read the block to get bbuf filled up, then apply partial change (if the blkoff != 0) and then write. Followup writes are already full writes except for last one... so yea, this write is just broken. I'll think about it and make an update later.

    2. I think it is better now:)

  6. 
      
tsoome
tsoome
tsoome
rm
  1. I think the updates to write look correct now.

  2. 
      
tsoome
tsoome
rm
  1. Ship It!
  2. 
      
tsoome
Review request changed

Status: Closed (submitted)

Loading...