[SLOF] [PATCH v2] scsi: implement READ (16) command

[Thomas Huth]

On 12.10.2016 05:42, Nikunj A Dadhania wrote:
> For disks bigger than 2TB(512B sector size), read-10 would fail as it is
> limited by the block address(4bytes). Add and use SCSI command READ(16)
> when max blocks is more than 0xFFFFFFFF.
> 
> Signed-off-by: Nikunj A Dadhania <nikunj at linux.vnet.ibm.com>
> ---
>  slof/fs/scsi-disk.fs    |  7 ++++++-
>  slof/fs/scsi-support.fs | 29 +++++++++++++++++++++++++++++
>  2 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/slof/fs/scsi-disk.fs b/slof/fs/scsi-disk.fs
> index 2f7d740..bcaa19e 100644
> --- a/slof/fs/scsi-disk.fs
> +++ b/slof/fs/scsi-disk.fs
> @@ -82,7 +82,12 @@ CREATE cdb 10 allot
>      >r rot r> 			                ( block# #blocks addr len )
>      2swap                                       ( addr len block# #blocks )
>      dup >r
> -    cdb scsi-build-read-10                      ( addr len )
> +    cdb                                         ( addr len block# #blocks cdb )
> +    max-block-num FFFFFFFF > IF
> +        scsi-build-read-16                      ( addr len )
> +    ELSE
> +        scsi-build-read-10                      ( addr len )
> +    THEN
>      r> -rot                                     ( #blocks addr len )
>      scsi-dir-read cdb scsi-param-size 10
>      retry-scsi-command
> diff --git a/slof/fs/scsi-support.fs b/slof/fs/scsi-support.fs
> index 3e65c87..202d43f 100644
> --- a/slof/fs/scsi-support.fs
> +++ b/slof/fs/scsi-support.fs
> @@ -525,6 +525,35 @@ CONSTANT scsi-length-read-12
>  ;
>  
>  \ ***************************************************************************
> +\ SCSI-Command: READ (16)
> +\         Type: Block Command
> +\ ***************************************************************************
> +\ Forth Word:   scsi-build-read-16  ( block# #blocks cdb -- )
> +\ ***************************************************************************
> +\ command code
> +88 CONSTANT scsi-cmd-read-16
> +
> +\ CDB structure
> +STRUCT
> +   /c FIELD read-16>operation-code     \ code: 88
> +   /c FIELD read-16>protect            \ RDPROTECT, DPO, FUA, FUA_NV
> +   /n	FIELD read-16>block-address      \ lba

If we'd ever do a 32-bit version of SLOF again, the /n would be wrong
here. It'd be better to use /x here instead.
Additionally, there's a stray TAB character in the line, messsing up the
indentation.

> +   /l FIELD read-16>length             \ transfer length (32bits)
> +   /c FIELD read-16>group              \ group number
> +   /c FIELD read-16>control
> +CONSTANT scsi-length-read-16
> +
> +: scsi-build-read-16                         ( block# #blocks cdb -- )
> +   >r                                        ( block# #blocks )  ( R: -- cdb )
> +   r@ scsi-length-read-16 erase              \ 16 bytes CDB
> +	scsi-cmd-read-16 r@ read-16>operation-code c! ( block# #blocks )

Yet another stray TAB character.

> +   r@ read-16>length l!                      ( block# )
> +   r@ read-16>block-address !                (  )

As with the "/n" vs. "/x" above, it would be nicer here to use "x!"
instead of "!"

> +   scsi-param-control r> read-16>control c!  ( R: cdb -- )
> +   scsi-length-read-16 to scsi-param-size    \ update CDB length
> +;

Apart from the cosmetic nits, the patch looks fine to me now, so once
you've fixed them, feel free to add my:

Reviewed-by: Thomas Huth <thuth at redhat.com>