[PATCH 3/3] scsi: sync cache on write

[Caleb Connolly]

On 3/25/25 14:33, Neil Armstrong wrote:
> On 25/03/2025 14:02, Caleb Connolly wrote:
>> We don't have a mechanism to safely shutdown block devices prior to a
>> baord reset or driver removal. Prevent data loss by synchronizing the
>> SCSI cache after every write.
>>
>> In particular this solves the issue of capsule updates looping on some
>> devices because the board resets immediately after deleting the capsule
>> file and this write wouldn't be flushed in time.
>>
>> This may impact NAND wear, but should be negligible given the usecases
>> for disk write in U-Boot.
>>
>> Signed-off-by: Caleb Connolly <caleb.connolly at linaro.org>
>> ---
>>   drivers/scsi/scsi.c | 28 ++++++++++++++++++++++++++++
>>   1 file changed, 28 insertions(+)
>>
>> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
>> index 
>> 1aa7fbdbb5278e387de72a3c3e73d19ea0342fff..1b71e580b89b100395ea132a4976366a713cba8a 100644
>> --- a/drivers/scsi/scsi.c
>> +++ b/drivers/scsi/scsi.c
>> @@ -77,8 +77,25 @@ static void scsi_setup_inquiry(struct scsi_cmd *pccb)
>>       pccb->cmdlen = 6;
>>       pccb->msgout[0] = SCSI_IDENTIFY; /* NOT USED */
>>   }
>> +static void scsi_setup_sync_cache(struct scsi_cmd *pccb, lbaint_t start,
>> +                  unsigned short blocks)
>> +{
>> +    pccb->cmd[0] = SCSI_SYNC_CACHE;
>> +    pccb->cmd[1] = 0;
>> +    pccb->cmd[2] = (unsigned char)(start >> 24) & 0xff;
>> +    pccb->cmd[3] = (unsigned char)(start >> 16) & 0xff;
>> +    pccb->cmd[4] = (unsigned char)(start >> 8) & 0xff;
>> +    pccb->cmd[5] = (unsigned char)start & 0xff;
>> +    pccb->cmd[6] = 0;
>> +    pccb->cmd[7] = (unsigned char)(blocks >> 8) & 0xff;
>> +    pccb->cmd[8] = (unsigned char)blocks & 0xff;
>> +    pccb->cmd[9] = 0;
>> +    pccb->cmdlen = 10;
>> +    pccb->msgout[0] = SCSI_IDENTIFY; /* NOT USED */
>> +}
>> +
>>   static void scsi_setup_read_ext(struct scsi_cmd *pccb, lbaint_t start,
>>                   unsigned short blocks)
>>   {
>>       pccb->cmd[0] = SCSI_READ10;
>> @@ -235,8 +252,19 @@ static ulong scsi_write(struct udevice *dev, 
>> lbaint_t blknr, lbaint_t blkcnt,
>>               blkcnt -= blks;
>>               break;
>>           }
>> +        /*
>> +         * Ensure the writes are flushed to disk so we don't lose data
>> +         * on board reset.
>> +         * FIXME: this ought to be in a SCSI shutdown path instead
>> +         */
>> +        scsi_setup_sync_cache(pccb, start, smallblks);
> 
> Why can't this be done at the end of scsi_write() ?

We have the same limitation that we can only flush SCSI_MAX_BLK blocks 
at once. It could be done in a second loop at the end but I figured it's 
more sensible to just interleave them.
> 
>> +        if (scsi_exec(bdev, pccb)) {
>> +            scsi_print_error(pccb);
>> +            break;
>> +        }
>> +
>>           if (blks > max_blks) {
>>               start += max_blks;
>>               blks -= max_blks;
>>           } else {
>>
> 
> And perhaps a sync subcommand of the scsi command would be nice to have !

Good idea, that would tie in nicely if/when we get a .sync() op for 
block devices and call it prior to board reset.
> 
> Neil
> 

-- 
Caleb (they/them)