[PATCH v1] scsi: Check the SPC version in sd_read_cpr

[PATCH v1] scsi: Check the SPC version in sd_read_cpr

[Yohan Joung]

Add SPC version verification to avoid unnecessary inquiry command

Signed-off-by: Yohan Joung <yohan.joung@sk.com>
---
 drivers/scsi/sd.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 6203915945a4..9d71ad24d8e3 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3413,11 +3413,16 @@ static inline sector_t sd64_to_sectors(struct scsi_disk *sdkp, u8 *buf)
 static void sd_read_cpr(struct scsi_disk *sdkp)
 {
 	struct blk_independent_access_ranges *iars = NULL;
+	struct scsi_device *sdev = sdkp->device;
 	unsigned char *buffer = NULL;
 	unsigned int nr_cpr = 0;
 	int i, vpd_len, buf_len = SD_BUF_SIZE;
 	u8 *desc;
 
+	/* Support for CPR was defined in SPC-5. */
+	if (sdev->scsi_level < SCSI_SPC_5)
+		return;
+
 	/*
 	 * We need to have the capacity set first for the block layer to be
 	 * able to check the ranges.
-- 
2.25.1

Re: [PATCH v1] scsi: Check the SPC version in sd_read_cpr

[Martin K. Petersen]

Yohan,

> Add SPC version verification to avoid unnecessary inquiry command

Are you experiencing an actual error condition as a result of this
INQUIRY operation?

Devices often adopt new protocol features prior to the spec being
finalized. Therefore we generally use INQUIRY to discover capabilities
rather than relying on the reported spec compliance.

> +	/* Support for CPR was defined in SPC-5. */
> +	if (sdev->scsi_level < SCSI_SPC_5)
> +		return;
> +

I'm OK with the change but I'll defer to Damien as to whether this is an
acceptable heuristic.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v1] scsi: Check the SPC version in sd_read_cpr

[Damien Le Moal]

On 7/23/24 09:49, Martin K. Petersen wrote:
> 
> Yohan,
> 
>> Add SPC version verification to avoid unnecessary inquiry command
> 
> Are you experiencing an actual error condition as a result of this
> INQUIRY operation?
> 
> Devices often adopt new protocol features prior to the spec being
> finalized. Therefore we generally use INQUIRY to discover capabilities
> rather than relying on the reported spec compliance.
> 
>> +	/* Support for CPR was defined in SPC-5. */
>> +	if (sdev->scsi_level < SCSI_SPC_5)
>> +		return;
>> +
> 
> I'm OK with the change but I'll defer to Damien as to whether this is an
> acceptable heuristic.

If this is solving an issue with an ATA device managed by libata, we can handle
that in libata. Otherwise, if this is an ATA device connected to a SAS HBA or a
SAS drive, then the issue could be with the HBA.

I missed this patch so I am not aware of the actual issue this is trying to solve...

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v1] scsi: Check the SPC version in sd_read_cpr

[Damien Le Moal]

On 7/23/24 09:49, Martin K. Petersen wrote:
> 
> Yohan,
> 
>> Add SPC version verification to avoid unnecessary inquiry command
> 
> Are you experiencing an actual error condition as a result of this
> INQUIRY operation?
> 
> Devices often adopt new protocol features prior to the spec being
> finalized. Therefore we generally use INQUIRY to discover capabilities
> rather than relying on the reported spec compliance.
> 
>> +	/* Support for CPR was defined in SPC-5. */
>> +	if (sdev->scsi_level < SCSI_SPC_5)
>> +		return;
>> +
> 
> I'm OK with the change but I'll defer to Damien as to whether this is an
> acceptable heuristic.

By the way, that may not be adequate as ATA devices that implement CPR may
advertise an ACS version that does not translate to SPC-5, which is exactly the
point you said.

-- 
Damien Le Moal
Western Digital Research