Re: [QUESTION]: Why did we clear the lowest bit of SCSI command's status in scsi_status_is_good

[James Bottomley <jejb@linux.ibm.com>]

On Mon, 2022-11-28 at 11:58 +0800, Wenchao Hao wrote:
> static inline bool scsi_status_is_good(int
> status)                                                              
>                                                                      
>                           
> {
>         if (status < 0)
>                 return false;
> 
>         if (host_byte(status) == DID_NO_CONNECT)
>                 return false;
> 
>         /*  
>          * FIXME: bit0 is listed as reserved in SCSI-2, but is
>          * significant in SCSI-3.  For now, we follow the SCSI-2
>          * behaviour and ignore reserved bits.
>          */
>         status &= 0xfe;
>         return ((status == SAM_STAT_GOOD) ||
>                 (status == SAM_STAT_CONDITION_MET) ||
>                 /* Next two "intermediate" statuses are obsolete in
> SAM-4 */
>                 (status == SAM_STAT_INTERMEDIATE) ||
>                 (status == SAM_STAT_INTERMEDIATE_CONDITION_MET) ||
>                 /* FIXME: this is obsolete in SAM-3 */
>                 (status == SAM_STAT_COMMAND_TERMINATED));
> }
> We have function defined in include/scsi/scsi.h as above, which is
> used to check if the status in result is good.
> 
> But the function cleared the lowest bit of SCSI command's status,
> which would translate an undefined status to good in some condition,
> for example the status is 0x1.
> 
> This code seems introduced in this patch:
> https://lore.kernel.org/all/1052403312.2097.35.camel@mulgrave/
> 
> Is anyone who knows why did we clear the lowest bit? 

It says why in the comment you quote above ... what is unclear about
it?

> We found some firmware or drivers would return status which did not
> defined in SAM. Now SCSI middle level do not have an uniform behavior
> for these undefined status. I want to change the logic to return
> error for all status which did not defined in SAM or define a method
> in host template to let drivers to judge what to do in this
> condition.

Why? The general rule of thumb is be strict in what you emit and
generous in what you receive (which is why reserved bits are ignored).
Is the drive you refer to above not working in some way, in which case
detail it so people can understand the actual problem.

James