Douglas,

On 2018/08/06 9:12, Douglas Gilbert wrote:
> ...
> So perhaps struct scsi_host_template and scsi_host need a bounds_check:1
> flag so certain "SCSI" hosts (e.g. ATA and USB mass storage but not UAS(P))
> could set it by default. So the SATA host (libata ?) could set the bit in
> its template.
> 
> Would the block layer also want to tweak bounds_check?

Hmm... I do not think so. The block device capacity is set by the device driver
when the device queue is created and that value is used to check and validate
the bios/requests being built/issues by the device user. I think that check
needs to stay as it is since it is common to many different block devices that
may not represent a single HW device (e.g. device mappers, md, etc).

>>> The block layer does almost all its block handling in units of 512 byte
>>> blocks whereas SCSI disks may have other logical block sizes (e.g. 4096
>>> byte logical blocks). So when the block sizes are different it is
>>> possible that there is an alignment issue. But if that occurs, it
>>> would be a logic issue in the block layer. Note that the current
>>> mainline code does this check even when it is not needed (e.g. when
>>> the logical block size of the disk is 512 bytes).
>>
>> Of note here is that there is a special case that is not being handled: ZBC
>> drives with 512B logical and 4K physical blocks will accept any 512B aligned
>> read commands (that is any read command), but will require 4K aligned write
>> commands for sequential write preferred zones (writes to conventional zones 
>> can
>> be 512B aligned). Since there is currently no differentiation between read 
>> and
>> write commands for the alignment check, much less based on the target zone 
>> type
>> of these commands, checks are incomplete for ZBC & ZAC disks in the generic
>> block layer. These checks could be added to SD though now that a bitmap is
>> attached to the device request queue to indicate if a zone is conventional or
>> sequential. For such test to be effective, sdkp->bounds_check would need to 
>> be
>> forced to 1 for host managed zoned disks.
> 
> Like the "out of range" case I guess that compliant ZBC & ZAC disks are 
> required
> to fail this case and send back an indicative error message. But yes, extra 
> code
> could be added to do that. If there are enough ZBC/ZAC special cases (like the
> protect code) we could split the submission side into three fast paths on
> entry to sd_init_command(): one for protect, one for ZBC/ZAC and one for the
> rest (the most common case, I assume). That leaves open the question can a ZBC
> disk also have protection information :-)

I do not know of any such drive on the market. The standards certainly do not
prevent it, so that is something that we should have as a possibility.

Best regards.

-- 
Damien Le Moal
Western Digital Research

Reply via email to