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