Re: [PATCH] scsi_debug: implement IMMED bit

[Bart Van Assche <Bart.VanAssche@wdc.com>]

On Wed, 2018-01-31 at 15:26 -0500, Douglas Gilbert wrote:
> On 2018-01-31 12:06 PM, Bart Van Assche wrote:
> > On 01/29/18 21:54, Douglas Gilbert wrote:
> > > +static const struct opcode_info_t sync_cache_iarr[] = {
> > > +    {0, 0x91, 0, F_LONG_DELAY | F_M_ACCESS, resp_sync_cache, NULL,
> > > +        {16,  0x7, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> > 
> >                    ^^^
> > Can you clarify the choice of "0x7" in the above? After having had a look at 
> > SBC-4 I was expecting 0x2 (the mask for the IMMED bit) since all other bits in 
> > that command byte are either reserved or obsolete.
> 
> As a general rule when you see "obsolete" that means that field was used
> in an earlier standard (bit 0 was RELADDR and bit 2 was SYNC_NV). So
> application clients complying with earlier versions of SBC might set those
> bits. If they are set and the mask is being enforced I choose to not fail
> the command as an illegal request. Basically accept and ignore.

I agree with setting bits for obsolete flags. The reason I was asking about that
mask is because I found the following in SBC-4 for byte 1 of SYNCHRONIZE CACHE(16):
* Bits 7..3: reserved.
* Bit 2: obsolete.
* Bit 1: IMMED.
* Bit 0: reserved.

> > > -    return 0;
> > > +    return (cmd[1] & 0x1) ? SDEG_RES_IMMED_MASK : 0; /* check IMMED bit */
> > 
> > Shouldn't the mask 0x2 be used to check for the IMMED bit?
> 
> That comment needs a little more context:

Sorry, I confused byte 1 of the START STOP command with that of the SYNCHRONIZE
CACHE command so please ignore the above comment.

Bart.