qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 3/6] scsi: fix parsing of allocation length f


From: Kevin Wolf
Subject: Re: [Qemu-devel] [PATCH v2 3/6] scsi: fix parsing of allocation length field
Date: Fri, 18 Nov 2011 14:07:48 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:7.0) Gecko/20110927 Thunderbird/7.0

Am 14.11.2011 14:31, schrieb Paolo Bonzini:
> - several MMC commands were parsed wrong by QEMU because their allocation
> length/parameter list length is placed in a non-standard position in
> the CDB (i.e. it is different from most commands with the same value in
> bits 5-7).
> 
> - SEND VOLUME TAG length was multiplied by 40 which is not in SMC.  The
> parameter list length is between 32 and 40 bytes.  Same for MEDIUM SCAN
> (spec found at http://ldkelley.com/SCSI2/SCSI2-16.html but not in any of
> the PDFs I have here).
> 
> - READ_POSITION (SSC) conflicts with PRE_FETCH (SBC).  READ_POSITION's
> transfer length is not hardcoded to 20 in SSC; for PRE_FETCH cmd->xfer
> should be 0.  Both fixed.
> 
> - FORMAT MEDIUM (the SSC name for FORMAT UNIT) was missing.  The FORMAT
> UNIT command is still somewhat broken for block devices because its
> parameter list length is not in the CDB.  However it works for CD/DVD
> drives, which mandate the length of the payload.
> 
> - fixed wrong sign-extensions for 32-bit fields (for the LBA field,
> this affects disks >1 TB).
> 
> - several other SBC or SSC commands were missing or parsed wrong.
> 
> - some commands were not in the list of "write" commands.
> 
> Reported-by: Thomas Schmitt <address@hidden>
> Tested-by: Thomas Schmitt <address@hidden> (MMC bits only)
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  hw/scsi-bus.c |   95 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 files changed, 86 insertions(+), 9 deletions(-)

> @@ -671,11 +696,11 @@ static int scsi_req_length(SCSICommand *cmd, SCSIDevice 
> *dev, uint8_t *buf)
>          cmd->len = 10;
>          break;
>      case 4:
> -        cmd->xfer = ldl_be_p(&buf[10]);
> +        cmd->xfer = ldl_be_p(&buf[10]) & 0xffffffffULL;

Makes me wonder why we don't have an unsigned version of ldl_be_p...

I'll apply this anyway, we can still clean it up on top if we like.

Kevin



reply via email to

[Prev in Thread] Current Thread [Next in Thread]