From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 04492C04EB8 for ; Tue, 4 Dec 2018 12:17:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 947DC20851 for ; Tue, 4 Dec 2018 12:17:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 947DC20851 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=huawei.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726266AbeLDMRM (ORCPT ); Tue, 4 Dec 2018 07:17:12 -0500 Received: from szxga05-in.huawei.com ([45.249.212.191]:15637 "EHLO huawei.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1725767AbeLDMRL (ORCPT ); Tue, 4 Dec 2018 07:17:11 -0500 Received: from DGGEMS403-HUB.china.huawei.com (unknown [172.30.72.58]) by Forcepoint Email with ESMTP id 8FDCE94B1310C; Tue, 4 Dec 2018 20:17:07 +0800 (CST) Received: from [127.0.0.1] (10.202.226.41) by DGGEMS403-HUB.china.huawei.com (10.3.19.203) with Microsoft SMTP Server id 14.3.408.0; Tue, 4 Dec 2018 20:16:59 +0800 From: John Garry Subject: Re: [PATCH v3 4/4] scsi: hisi_sas: Add support for DIF feature for v3 hw To: "Martin K. Petersen" References: <1543838994-30028-1-git-send-email-john.garry@huawei.com> <1543838994-30028-5-git-send-email-john.garry@huawei.com> CC: , , , , Xiang Chen Message-ID: <358aae82-394e-9df2-6b05-5af8e0f196b3@huawei.com> Date: Tue, 4 Dec 2018 12:16:54 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.202.226.41] X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 04/12/2018 04:12, Martin K. Petersen wrote: > > John, > Hi Martin, Thanks for checking. >> +static int hisi_sas_dif_dma_map(struct hisi_hba *hisi_hba, >> + int *n_elem_dif, struct sas_task *task) >> +{ >> + struct device *dev = hisi_hba->dev; >> + struct sas_ssp_task *ssp_task; >> + struct scsi_cmnd *scsi_cmnd; >> + int rc; >> + >> + if (task->num_scatter) { >> + ssp_task = &task->ssp_task; >> + scsi_cmnd = ssp_task->cmd; >> + >> + if (scsi_prot_sg_count(scsi_cmnd)) { >> + *n_elem_dif = dma_map_sg(dev, >> + scsi_prot_sglist(scsi_cmnd), >> + scsi_prot_sg_count(scsi_cmnd), >> + task->data_dir); >> + > > If you're only supporting DIF there is no DMA mapping or unmapping since > the PI is generated by or verified by the HBA. No additional data is > transferred to/from host memory. The protection scatterlist will be > NULL. So I was bit rushed in dropping DIX support... > >> + switch (prot_op) { >> + case SCSI_PROT_READ_INSERT: >> + prot->dw0 |= T10_INSRT_EN_MSK; >> + prot->lbrtgv = lbrt_chk_val; >> + break; >> + case SCSI_PROT_READ_STRIP: >> + prot->dw0 |= (T10_RMV_EN_MSK | T10_CHK_EN_MSK); >> + prot->lbrtcv = lbrt_chk_val; >> + if (prot_type == SCSI_PROT_DIF_TYPE1) >> + prot->dw4 |= (0xc << 16); >> + else if (prot_type == SCSI_PROT_DIF_TYPE3) >> + prot->dw4 |= (0xfc << 16); >> + break; >> + case SCSI_PROT_READ_PASS: >> + prot->dw0 |= T10_CHK_EN_MSK; >> + prot->lbrtcv = lbrt_chk_val; >> + if (prot_type == SCSI_PROT_DIF_TYPE1) >> + prot->dw4 |= (0xc << 16); >> + else if (prot_type == SCSI_PROT_DIF_TYPE3) >> + prot->dw4 |= (0xfc << 16); >> + break; >> + case SCSI_PROT_WRITE_INSERT: >> + prot->dw0 |= T10_INSRT_EN_MSK; >> + prot->lbrtgv = lbrt_chk_val; >> + break; >> + case SCSI_PROT_WRITE_STRIP: >> + prot->dw0 |= (T10_RMV_EN_MSK | T10_CHK_EN_MSK); >> + prot->lbrtcv = lbrt_chk_val; >> + break; >> + case SCSI_PROT_WRITE_PASS: >> + prot->dw0 |= T10_CHK_EN_MSK; >> + prot->lbrtcv = lbrt_chk_val; >> + if (prot_type == SCSI_PROT_DIF_TYPE1) >> + prot->dw4 |= (0xc << 16); >> + else if (prot_type == SCSI_PROT_DIF_TYPE3) >> + prot->dw4 |= (0xfc << 16); >> + break; >> + default: >> + WARN(1, "prot_op(0x%x) is not valid\n", prot_op); >> + break; >> + } > > DIF is WRITE_INSERT/READ_STRIP operations only. > as above >> + if ((prot_op == SCSI_PROT_READ_INSERT) || >> + (prot_op == SCSI_PROT_WRITE_INSERT) || >> + (prot_op == SCSI_PROT_WRITE_PASS) || >> + (prot_op == SCSI_PROT_READ_PASS)) { >> + unsigned int interval = scsi_prot_interval(scsi_cmnd); >> + unsigned int ilog2_interval = ilog2(interval); >> + >> + len = (task->total_xfer_len >> ilog2_interval) * 8; >> + } > > if (scmd->prot_flags & SCSI_PROT_TRANSFER_PI) { > >> + .sg_prot_tablesize = HISI_SAS_SGE_PAGE_CNT, > > Also not required if you're only doing DIF. There is no protection > scatterlist. and again. > > Anyway. Instead of throwing the DIX stuff away, let's figure out what > the problem is. > So I just did not want to upstream support for a feature which seems to not be working. Are you happy for us to make DIX support in this driver "hisi_sas DIX experimental" short term? Regardless, we can discuss figuring out any SCSI MQ vs DIX issue in "DIF/DIX issue related to config CONFIG_SCSI_MQ_DEFAULT". Cheers, John