All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Martin K. Petersen" <martin.petersen@oracle.com>
To: John Garry <john.garry@huawei.com>
Cc: jejb@linux.vnet.ibm.com, martin.petersen@oracle.com,
	linuxarm@huawei.com, linux-kernel@vger.kernel.org,
	linux-scsi@vger.kernel.org,
	Xiang Chen <chenxiang66@hisilicon.com>
Subject: Re: [PATCH v3 4/4] scsi: hisi_sas: Add support for DIF feature for v3 hw
Date: Mon, 03 Dec 2018 23:12:39 -0500	[thread overview]
Message-ID: <yq1ftveszew.fsf@oracle.com> (raw)
In-Reply-To: <1543838994-30028-5-git-send-email-john.garry@huawei.com> (John Garry's message of "Mon, 3 Dec 2018 20:09:54 +0800")


John,

> +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.

> +	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.

> +		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.

Anyway. Instead of throwing the DIX stuff away, let's figure out what
the problem is.

-- 
Martin K. Petersen	Oracle Linux Engineering

WARNING: multiple messages have this Message-ID (diff)
From: "Martin K. Petersen" <martin.petersen@oracle.com>
To: John Garry <john.garry@huawei.com>
Cc: <jejb@linux.vnet.ibm.com>, <martin.petersen@oracle.com>,
	<linuxarm@huawei.com>, <linux-kernel@vger.kernel.org>,
	<linux-scsi@vger.kernel.org>,
	Xiang Chen <chenxiang66@hisilicon.com>
Subject: Re: [PATCH v3 4/4] scsi: hisi_sas: Add support for DIF feature for v3 hw
Date: Mon, 03 Dec 2018 23:12:39 -0500	[thread overview]
Message-ID: <yq1ftveszew.fsf@oracle.com> (raw)
In-Reply-To: <1543838994-30028-5-git-send-email-john.garry@huawei.com> (John Garry's message of "Mon, 3 Dec 2018 20:09:54 +0800")


John,

> +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.

> +	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.

> +		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.

Anyway. Instead of throwing the DIX stuff away, let's figure out what
the problem is.

-- 
Martin K. Petersen	Oracle Linux Engineering

  reply	other threads:[~2018-12-04  4:12 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-03 12:09 [PATCH v3 0/3] hisi_sas: DIF support John Garry
2018-12-03 12:09 ` John Garry
2018-12-03 12:09 ` [PATCH v3 1/4] scsi: hisi_sas: Fix warnings detected by sparse John Garry
2018-12-03 12:09   ` John Garry
2018-12-03 12:09 ` [PATCH v3 2/4] scsi: hisi_sas: Relocate some code to reduce complexity John Garry
2018-12-03 12:09   ` John Garry
2018-12-03 12:09 ` [PATCH v3 3/4] scsi: hisi_sas: Make sg_tablesize consistent value John Garry
2018-12-03 12:09   ` John Garry
2018-12-03 12:09 ` [PATCH v3 4/4] scsi: hisi_sas: Add support for DIF feature for v3 hw John Garry
2018-12-03 12:09   ` John Garry
2018-12-04  4:12   ` Martin K. Petersen [this message]
2018-12-04  4:12     ` Martin K. Petersen
2018-12-04 12:16     ` John Garry
2018-12-04 12:16       ` John Garry
2018-12-05  2:14       ` Martin K. Petersen
2018-12-05  2:14         ` Martin K. Petersen
2018-12-05 15:12         ` John Garry
2018-12-05 15:12           ` John Garry
2018-12-06  4:17           ` Martin K. Petersen
2018-12-06  4:17             ` Martin K. Petersen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=yq1ftveszew.fsf@oracle.com \
    --to=martin.petersen@oracle.com \
    --cc=chenxiang66@hisilicon.com \
    --cc=jejb@linux.vnet.ibm.com \
    --cc=john.garry@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=linuxarm@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.