From patchwork Fri Apr 18 07:55:13 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 882754 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4F593171D2; Fri, 18 Apr 2025 07:56:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744962967; cv=none; b=k2g+PTNpsjmZOcQPbiC/nVKEoh6dphavXdyXykBg+2mJ0r8l9pt8uUWy/wJWuc3mqC/Cw4gN2zntnRd/CH+ziY6ZCf6FsPRgvmD5zNNWko0ma2kkHxZYz7mBFIqiTBocL2p4Zf/e4ON9ywiGX5DE5+QFUUs1Ma+PmjA+XWM5uYA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744962967; c=relaxed/simple; bh=Pd0q18P/a3uTz10uUBdmoKLjD8pYs76ReXf8d9zqkpc=; h=From:To:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=qH7Z4hcRPx8zLMIze2Hu3ue4dp1KECOii3iLzXeFJeg45teIsFfwP20nbWni6ayPwN3//XRJ6AZZ9zSawOxGl6nd4n9Bpa947BC7wpcrlstlstEqQQ8PJp2tRRSk+SP9jOXGrd5+KlMgQjsyk2sQpzPILNx9cl3sRog+tOzz2SA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=WLKD3PTH; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="WLKD3PTH" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1520AC4CEEC; Fri, 18 Apr 2025 07:56:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1744962966; bh=Pd0q18P/a3uTz10uUBdmoKLjD8pYs76ReXf8d9zqkpc=; h=From:To:Subject:Date:In-Reply-To:References:From; b=WLKD3PTHq2WvQ3pODLz33XwH4QmP7L98FBvXc680VECph7eO+915P0ISucxLOVduy dg1l0Z6t3ycc9XGcrp9HWADauRmiqO/FuU+jTiNavjy+j25Q5oIYwjICX3TCabxKfi SDiSHw/PPgaDrVvBLWDT/XRFSFgshT5NqWwMlm6voImhaKZOi5sLs0elunQFxaRrgX 6o/nDI69HVuLDudMMzO01MwaK8hc79KhJ/zeme69HcgNzjbgq8rCJ1CSRyJ2SgBnXv rpWuK0Q1u1fTXa0SEPOLSoiJE1CtLt9zxi0CkgP3hJtps+dYHF62CA1RFGRuYOxCCp EThzjeCFrAPkw== From: Damien Le Moal To: linux-ide@vger.kernel.org, Niklas Cassel , linux-scsi@vger.kernel.org, "Martin K . Petersen" Subject: [PATCH v2 1/5] ata: libata-scsi: Fix ata_mselect_control_ata_feature() return type Date: Fri, 18 Apr 2025 16:55:13 +0900 Message-ID: <20250418075517.369098-2-dlemoal@kernel.org> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250418075517.369098-1-dlemoal@kernel.org> References: <20250418075517.369098-1-dlemoal@kernel.org> Precedence: bulk X-Mailing-List: linux-scsi@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 The function ata_mselect_control_ata_feature() has a return type defined as unsigned int but this function may return negative error codes, which are correctly propagated up the call chain as integers. Fix ata_mselect_control_ata_feature() to have the correct int return type. While at it, also fix a typo in this function description comment. Fixes: df60f9c64576 ("scsi: ata: libata: Add ATA feature control sub-page translation") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal --- drivers/ata/libata-scsi.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 2796c0da8257..24e662c837e3 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3886,12 +3886,11 @@ static int ata_mselect_control_spg0(struct ata_queued_cmd *qc, } /* - * Translate MODE SELECT control mode page, sub-pages f2h (ATA feature mode + * Translate MODE SELECT control mode page, sub-page f2h (ATA feature mode * page) into a SET FEATURES command. */ -static unsigned int ata_mselect_control_ata_feature(struct ata_queued_cmd *qc, - const u8 *buf, int len, - u16 *fp) +static int ata_mselect_control_ata_feature(struct ata_queued_cmd *qc, + const u8 *buf, int len, u16 *fp) { struct ata_device *dev = qc->dev; struct ata_taskfile *tf = &qc->tf; From patchwork Fri Apr 18 07:55:14 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 882545 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 696F0171D2; Fri, 18 Apr 2025 07:56:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744962968; cv=none; b=Gk1cjGKUCcCaxg0BnvEhWvj+Z4rq1ltH5yvz4YwXZQadOnFmE9AV0d2Jr89JVYYD+StGoF9cKxRL0wImNBaDic2hO3fzsM6gV8hqtAtsWSE+aEI5PP8rTVCiwBnXI9A843RycXqCz2O+kKnRxEKmrUGj0Lhd1Xo4P3CRhsySNNc= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744962968; c=relaxed/simple; bh=BAsegvms7haop65f0DlfFwI/oQc+5+ACN6HlGOF5fq0=; h=From:To:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=Ygo04RA/KWJ5/ty6l/+ff9CraRuotpSwRFXpHW7qCHK092s3knc6/wjpa+meGYzzphXRD3MY9EZCoqlyZVXTJG3xvT52BFzKcLSTtbvN3dFZcaTGGYpiCp2ghzLZd4TYWdnfvfu3ZVvzMcuy0A+7yjXpiS7JOwKhgVUdhMGp8NU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=GhR/h1vf; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="GhR/h1vf" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2D9E0C4CEED; Fri, 18 Apr 2025 07:56:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1744962967; bh=BAsegvms7haop65f0DlfFwI/oQc+5+ACN6HlGOF5fq0=; h=From:To:Subject:Date:In-Reply-To:References:From; b=GhR/h1vfyNRoKc05dEJDKRSLisWlctytplVOx3sRBGX+8RXPYMMZkzDsZMfBVEUcj Q8tPuMMHA9UudSgFQs6ijUxIgkqgjyE8qHoaoNQyDvcv6SWaYBMUNuNauEz7GAfd2C pduIZZMpQGHK5yOQ5CYo44xMb3FnKbc3cGLhQ5DA6+5jk4L8UbsYUnfuo4d+27F/af 8HrVCPeLrKnv/X57HOmh65Xu9GKSlFVvnGrxwWIKPD1d8QoFmSsm1CDo2ONotv8ISC RiKcK1x3ul2yVPtEDAnA2UE6+1hGUBzxaKM6hkyLtYeAZEMFwR/n8iz7LaA0c59fbh dvcHNEBV0sjGw== From: Damien Le Moal To: linux-ide@vger.kernel.org, Niklas Cassel , linux-scsi@vger.kernel.org, "Martin K . Petersen" Subject: [PATCH v2 2/5] ata: libata-scsi: Fail MODE SELECT for unsupported mode pages Date: Fri, 18 Apr 2025 16:55:14 +0900 Message-ID: <20250418075517.369098-3-dlemoal@kernel.org> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250418075517.369098-1-dlemoal@kernel.org> References: <20250418075517.369098-1-dlemoal@kernel.org> Precedence: bulk X-Mailing-List: linux-scsi@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 For devices that do not support CDL, the subpage F2h of the control mode page 0Ah should not be supported. However, the function ata_mselect_control_ata_feature() does not fail for a device that does not have the ATA_DFLAG_CDL device flag set, which can lead to an invalid SET FEATURES command (which will be failed by the device) to be issued. Modify ata_mselect_control_ata_feature() to return -EOPNOTSUPP if it is executed for a device without CDL support. This error code is checked by ata_scsi_mode_select_xlat() (through ata_mselect_control()) to fail the MODE SELECT command immediately with an ILLEGAL REQUEST / INVALID FIELD IN CDB asc/ascq as mandated by the SPC specifications for unsupported mode pages. Fixes: df60f9c64576 ("scsi: ata: libata: Add ATA feature control sub-page translation") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal --- drivers/ata/libata-scsi.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 24e662c837e3..15661b05cb48 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3896,6 +3896,15 @@ static int ata_mselect_control_ata_feature(struct ata_queued_cmd *qc, struct ata_taskfile *tf = &qc->tf; u8 cdl_action; + /* + * The sub-page f2h should only be supported for devices that support + * the T2A and T2B command duration limits mode pages (note here the + * "should" which is what SAT-6 defines). So fail this command if the + * device does not support CDL. + */ + if (!(dev->flags & ATA_DFLAG_CDL)) + return -EOPNOTSUPP; + /* * The first four bytes of ATA Feature Control mode page are a header, * so offsets in mpage are off by 4 compared to buf. Same for len. @@ -4101,6 +4110,8 @@ static unsigned int ata_scsi_mode_select_xlat(struct ata_queued_cmd *qc) case CONTROL_MPAGE: ret = ata_mselect_control(qc, spg, p, pg_len, &fp); if (ret < 0) { + if (ret == -EOPNOTSUPP) + goto invalid_fld; fp += hdr_len + bd_len; goto invalid_param; } From patchwork Fri Apr 18 07:55:15 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 882753 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2C48426AA93; Fri, 18 Apr 2025 07:56:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744962969; cv=none; b=p9zaeCk1GcdDbWteqJ09t8kQf9kDeSuLyiu66rSccsDg4frarXACdbXsDELexvoezReERwSlIbxyPyBcW77RwfrlmEa3XjArLu4aSlbPNDrsBShO0AdFUkM+AV8x5WGe16yuatvXxF0JJDAemseYlH05PjkThSq65ah+1M71XfQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744962969; c=relaxed/simple; bh=G6q4Hp+wvfNJ64ikYUG8gWnNsxL66dJDVyKDX0EVSLI=; h=From:To:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=ne71uUDfYgG7Z9Vp3LS22XBK707CPFkiARkm2zEfIY00RHLiSxRWZm3T7wXLUXqvPSy6YbpPtMtqQt8RXzFd3Qh6XjuPlH40HwCgvik/jhIHw9wgc2Koe7uMPrhPaw+NMhuADkJEVx5bexfArfH4fwEpz1Hi1Sl2IAHTM+nRe9k= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=Gzp7/2OW; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="Gzp7/2OW" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4E7A0C4CEE2; Fri, 18 Apr 2025 07:56:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1744962969; bh=G6q4Hp+wvfNJ64ikYUG8gWnNsxL66dJDVyKDX0EVSLI=; h=From:To:Subject:Date:In-Reply-To:References:From; b=Gzp7/2OWBY3RA4D0XxezJZ0epxxfAs/Qvme9CSrl3ZkzK6tJBYMChvgc02qjzi81D T+zMmEm7pz877TBBNleAQyNzDihc5nC2GcP/azjt1CpxM31EYP5BB4JgSvG/JOXCOY rqal5TU1c+6dP5QfoQZgclEb7KbLQQH1WEAfn0zWIDn+geKrehq/AdrCLT7lLkwYmH 9uP6Es03E5SK+k2E0lOo4i1ZPNZlFDCtpgOQF9WzC8bxmYNTElOFfhqgYbHBiF2CVC k2SBOjvkD/miXUuvuaTv6CVWizSqxpeS5jz0yqUNlmRNaf8z+qTjWJ36HStcRmdNX6 DBSPzENnTAu1A== From: Damien Le Moal To: linux-ide@vger.kernel.org, Niklas Cassel , linux-scsi@vger.kernel.org, "Martin K . Petersen" Subject: [PATCH v2 3/5] ata: libata-scsi: Fix ata_msense_control_ata_feature() Date: Fri, 18 Apr 2025 16:55:15 +0900 Message-ID: <20250418075517.369098-4-dlemoal@kernel.org> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250418075517.369098-1-dlemoal@kernel.org> References: <20250418075517.369098-1-dlemoal@kernel.org> Precedence: bulk X-Mailing-List: linux-scsi@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 For the ATA features subpage of the control mode page, the T10 SAT-6 specifications state that: For a MODE SENSE command, the SATL shall return the CDL_CTRL field value that was last set by an application client. However, the function ata_msense_control_ata_feature() always sets the CDL_CTRL field to the 0x02 value to indicate support for the CDL T2A and T2B pages. This is thus incorrect and the value 0x02 must be reported only after the user enables the CDL feature, which is indicated with the ATA_DFLAG_CDL_ENABLED device flag. When this flag is not set, the CDL_CTRL field of the ATA feature subpage of the control mode page must report a value of 0x00. Fix ata_msense_control_ata_feature() to report the correct values for the CDL_CTRL field, according to the enable/disable state of the device CDL feature. Fixes: df60f9c64576 ("scsi: ata: libata: Add ATA feature control sub-page translation") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal Reviewed-by: Niklas Cassel Reviewed-by: Igor Pylypiv --- drivers/ata/libata-scsi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 15661b05cb48..f54e8a3dc46d 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2453,8 +2453,8 @@ static unsigned int ata_msense_control_ata_feature(struct ata_device *dev, */ put_unaligned_be16(ATA_FEATURE_SUB_MPAGE_LEN - 4, &buf[2]); - if (dev->flags & ATA_DFLAG_CDL) - buf[4] = 0x02; /* Support T2A and T2B pages */ + if (dev->flags & ATA_DFLAG_CDL_ENABLED) + buf[4] = 0x02; /* T2A and T2B pages enabled */ else buf[4] = 0; From patchwork Fri Apr 18 07:55:16 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 882544 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AD0E526AA93; Fri, 18 Apr 2025 07:56:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744962970; cv=none; b=Frw809sYWawxKtMJ2G5+zZPni3ls2okp9nhpB6VB+jE+klqmY6T3ZbdHz8Gp+t7DOzftAgXlaMy23QzOzpMVomKC3GV4bvJLfj8qMbMdj7yT+Q/Yb6o076I1rKATNZigCee+9mUj/gIkZJNcK+1Ki4IaN7Oq/CDQtvnUbcpQA3Y= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744962970; c=relaxed/simple; bh=8UnRA0IbWyrZMKkMnJdWzmhwS4iXZiwv9j4AZ1nFMGU=; h=From:To:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=Wjnyz7d7LdJ2j5MSPDPU4PcgxzkYNASUgtXRpHsN2pl5WzjHtNVjaeZ1OUqnudTE0ceYpbXSL3Mv+K2WsOwDccgeUg7oTC3qRASI9mwJUKKAwnSQIbkjZ7x4/AEpGRdxYsW/PKuk+twTZhhISciupRf5YUv1LcAHOoI7bsn9N20= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=WnLK1EFY; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="WnLK1EFY" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 68758C4CEE2; Fri, 18 Apr 2025 07:56:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1744962970; bh=8UnRA0IbWyrZMKkMnJdWzmhwS4iXZiwv9j4AZ1nFMGU=; h=From:To:Subject:Date:In-Reply-To:References:From; b=WnLK1EFYaIj+AxqS6J7b8pZUCZFzhw83qCJG0brnEca9Z1lWNg2gy3V+CTrJ/2F1u UZ605qMkaAAkq5Lk09NUvsH6JuJJ6SIMnQNUzgoJSWkl9v+yEUglc/r7RrNdWrVTaW owmKor3DAVwc+SH2uTgnGDR8843iT96alr9dVyrcW2Qa0UAsj3khjnRvIiH7Ue+P8T 10MSg6M0osTYPNru9qMrF6+2IDvsvbK/k95DPhrFQgQ2WM3U4h63Ku0NrOCnm0vkAe CuqMH+vmKAhQ+FmxQvCrGYvY3aoCfzTyldwc8grc17v8iXSm3Baypcm5DDi6KvycKE 6Shnx5U4NPY3A== From: Damien Le Moal To: linux-ide@vger.kernel.org, Niklas Cassel , linux-scsi@vger.kernel.org, "Martin K . Petersen" Subject: [PATCH v2 4/5] ata: libata-scsi: Improve CDL control Date: Fri, 18 Apr 2025 16:55:16 +0900 Message-ID: <20250418075517.369098-5-dlemoal@kernel.org> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250418075517.369098-1-dlemoal@kernel.org> References: <20250418075517.369098-1-dlemoal@kernel.org> Precedence: bulk X-Mailing-List: linux-scsi@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 With ATA devices supporting the CDL feature, using CDL requires that the feature be enabled with a SET FEATURES command. This command is issued as the translated command for the MODE SELECT command issued by scsi_cdl_enable() when the user enables CDL through the device cdl_enable sysfs attribute. Currently, ata_mselect_control_ata_feature() always translates a MODE SELECT command for the ATA features subpage of the control mode page to a SET FEATURES command to enable or disable CDL based on the cdl_ctrl field. However, there is no need to issue the SET FEATURES command if: 1) The MODE SELECT command requests disabling CDL and CDL is already disabled. 2) The MODE SELECT command requests enabling CDL and CDL is already enabled. Fix ata_mselect_control_ata_feature() to issue the SET FEATURES command only when necessary. Since enabling CDL also implies a reset of the CDL statistics log page, avoiding useless CDL enable operations also avoids clearing the CDL statistics log. Also add debug messages to clearly signal when CDL is being enabled or disabled using a SET FEATURES command. Fixes: df60f9c64576 ("scsi: ata: libata: Add ATA feature control sub-page translation") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal Reviewed-by: Niklas Cassel Reviewed-by: Igor Pylypiv --- drivers/ata/libata-scsi.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index f54e8a3dc46d..589dc0df63e9 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3917,17 +3917,27 @@ static int ata_mselect_control_ata_feature(struct ata_queued_cmd *qc, /* Check cdl_ctrl */ switch (buf[0] & 0x03) { case 0: - /* Disable CDL */ + /* Disable CDL if it is enabled */ + if (!(dev->flags & ATA_DFLAG_CDL_ENABLED)) + return 0; + ata_dev_dbg(dev, "Disabling CDL\n"); cdl_action = 0; dev->flags &= ~ATA_DFLAG_CDL_ENABLED; break; case 0x02: - /* Enable CDL T2A/T2B: NCQ priority must be disabled */ + /* + * Enable CDL if not already enabled. Since this is mutually + * exclusive with NCQ priority, allow this only if NCQ priority + * is disabled. + */ + if (dev->flags & ATA_DFLAG_CDL_ENABLED) + return 0; if (dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLED) { ata_dev_err(dev, "NCQ priority must be disabled to enable CDL\n"); return -EINVAL; } + ata_dev_dbg(dev, "Enabling CDL\n"); cdl_action = 1; dev->flags |= ATA_DFLAG_CDL_ENABLED; break; From patchwork Fri Apr 18 07:55:17 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 882752 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C7DB11ADC8D; Fri, 18 Apr 2025 07:56:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744962971; cv=none; b=roQi1Nd/W7Mc9yKa43BKkDMJ9iabzhimN5Ht5h7MfYhlVoq7cDU2UzxBvQ3PzoSKM2DmC4QQavGNHkKvk8VAtpBErXgAbb9EzJcP1p75UWJDg8eI/AfWiHxQTbp9SSxUzA+85Fq5TCtxHPldqGanPequaSpqL25KsEyfnf8s2PI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744962971; c=relaxed/simple; bh=CrRCKqz/JA5HsPhH6mB3lb5rhNmWhPIYyZNe/G7Iz0Y=; h=From:To:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=IdVpBlX8+K6Mpz9CdNKN7n0Ao1/GyTEaz7huwTn1+67SWzj/mHWza0L9jn+yziE/oOEwVu4sq4x5vIMnOkDpV5GjY4fbR6JI+gnEfsjtKX+QBqTI21TAuvSJyDLAxwmKyy4M5XYzMLAQHrrkdPuI0uQiRjxrm7oyhaq8IyZo7ZE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=nkK0jWcP; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="nkK0jWcP" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8A2E0C4CEEC; Fri, 18 Apr 2025 07:56:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1744962971; bh=CrRCKqz/JA5HsPhH6mB3lb5rhNmWhPIYyZNe/G7Iz0Y=; h=From:To:Subject:Date:In-Reply-To:References:From; b=nkK0jWcP6ZMIrW0BQWYExDSAnBlZlKGJLcUeZ2utXdnwUmaCvUELvdKbZ/Y7Wud6H UdMtZ6utU/NEMRycM06ZcXH2ydUCK8cJ3Lpl3aunZxfy1+/YJwN8XfhvFpr7Hvtb0R RCnMCFCKcNipOK9l89RTLRk84tIOfwUFWeYOq2zqf5eXXiuiiTAJluCmfLl5FqYIYi PCYIr21iUVOB7c+Jk3rNKvL6aNrN2SjuMzrLgimb1Yg7bOR9fXRO+FVQdf7haiSGLu ahi50KMHJDRc+N7/NETjHu+4W9U+Fy9hf3dQJLku+Dz0M7FH3IZKnUsi9fu6FnMsM4 bCSpLruY7fOIg== From: Damien Le Moal To: linux-ide@vger.kernel.org, Niklas Cassel , linux-scsi@vger.kernel.org, "Martin K . Petersen" Subject: [PATCH v2 5/5] scsi: Improve CDL control Date: Fri, 18 Apr 2025 16:55:17 +0900 Message-ID: <20250418075517.369098-6-dlemoal@kernel.org> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250418075517.369098-1-dlemoal@kernel.org> References: <20250418075517.369098-1-dlemoal@kernel.org> Precedence: bulk X-Mailing-List: linux-scsi@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 With ATA devices supporting the CDL feature, using CDL requires that the feature be enabled with a SET FEATURES command. This command is issued as the translated command for the MODE SELECT command issued by scsi_cdl_enable() when the user enables CDL through the device cdl_enable sysfs attribute. However, the implementation of scsi_cdl_enable() always issues a MODE SELECT command for ATA devices when the enable argument is true, even if CDL is already enabled on the device. While this does not cause any issue with using CDL descriptors with read/write commands (the CDL feature will be enabled on the drive), issuing the MODE SELECT command even when the device CDL feature is already enabled will cause a reset of the ATA device CDL statistics log page (as defined in ACS, any CDL enable action must reset the device statistics). Avoid this needless actions (and the implied statistics log page reset) by modifying scsi_cdl_enable() to issue the MODE SELECT command to enable CDL if and only if CDL is not reported as already enabled on the device. And while at it, simplify the initialization of the is_ata boolean variable and move the declaration of the scsi mode data and sense header variables to within the scope of ATA device handling. Fixes: 1b22cfb14142 ("scsi: core: Allow enabling and disabling command duration limits") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal Reviewed-by: Niklas Cassel Reviewed-by: Igor Pylypiv --- drivers/scsi/scsi.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index 53daf923ad8e..518a252eb6aa 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -707,26 +707,23 @@ void scsi_cdl_check(struct scsi_device *sdev) */ int scsi_cdl_enable(struct scsi_device *sdev, bool enable) { - struct scsi_mode_data data; - struct scsi_sense_hdr sshdr; - struct scsi_vpd *vpd; - bool is_ata = false; char buf[64]; + bool is_ata; int ret; if (!sdev->cdl_supported) return -EOPNOTSUPP; rcu_read_lock(); - vpd = rcu_dereference(sdev->vpd_pg89); - if (vpd) - is_ata = true; + is_ata = rcu_dereference(sdev->vpd_pg89); rcu_read_unlock(); /* * For ATA devices, CDL needs to be enabled with a SET FEATURES command. */ if (is_ata) { + struct scsi_mode_data data; + struct scsi_sense_hdr sshdr; char *buf_data; int len; @@ -735,16 +732,30 @@ int scsi_cdl_enable(struct scsi_device *sdev, bool enable) if (ret) return -EINVAL; - /* Enable CDL using the ATA feature page */ + /* Enable or disable CDL using the ATA feature page */ len = min_t(size_t, sizeof(buf), data.length - data.header_length - data.block_descriptor_length); buf_data = buf + data.header_length + data.block_descriptor_length; - if (enable) - buf_data[4] = 0x02; - else - buf_data[4] = 0; + + /* + * If we want to enable CDL and CDL is already enabled on the + * device, do nothing. This avoids needlessly resetting the CDL + * statistics on the device as that is implied by the CDL enable + * action. Similar to this, there is no need to do anything if + * we want to disable CDL and CDL is already disabled. + */ + if (enable) { + if ((buf_data[4] & 0x03) == 0x02) + goto out; + buf_data[4] &= ~0x03; + buf_data[4] |= 0x02; + } else { + if ((buf_data[4] & 0x03) == 0x00) + goto out; + buf_data[4] &= ~0x03; + } ret = scsi_mode_select(sdev, 1, 0, buf_data, len, 5 * HZ, 3, &data, &sshdr); @@ -756,6 +767,7 @@ int scsi_cdl_enable(struct scsi_device *sdev, bool enable) } } +out: sdev->cdl_enable = enable; return 0;