RE: [PATCH 0/3] scsi: ufs-bsg: Add read descriptor
+Bean Thanks, Avri > -Original Message- > From: linux-scsi-ow...@vger.kernel.org > On Behalf Of Avri Altman > Sent: Friday, November 30, 2018 9:32 AM > To: James E.J. Bottomley ; Martin K. Petersen > ; linux-scsi@vger.kernel.org > Cc: Christoph Hellwig ; Avi Shchislowski > ; Alex Lemberg ; > Bart Van Assche ; Evan Green > ; Doug Anderson ; > Tomas Winkler ; adrian.hun...@intel.com; > Sayali Lokhande > Subject: RE: [PATCH 0/3] scsi: ufs-bsg: Add read descriptor > > A gentle ping. > > Cheers, > Avri > > > -Original Message- > > From: linux-scsi-ow...@vger.kernel.org ow...@vger.kernel.org> > > On Behalf Of Avri Altman > > Sent: Monday, November 26, 2018 11:03 AM > > To: James E.J. Bottomley ; Martin K. Petersen > > ; linux-scsi@vger.kernel.org > > Cc: Christoph Hellwig ; Bart Van Assche > > ; Avi Shchislowski > > ; Alex Lemberg ; > > Avri Altman > > Subject: [PATCH 0/3] scsi: ufs-bsg: Add read descriptor > > > > UFS Protocol Information Units (UPIU) are UFS packets that travel > > between the host and the device on the UniPro bus. Our previous series > > added the capability to send UPIUs to the ufs driver. It does not cover > > all the possible UPIU types - we are mainly focused on device > management, > > provisioning, testing and validation, so it covers UPIUs that falls in > > that box. > > > > Our intension is to publish ufs-utils soon - an open source user space > > utility that relies on that infrastructure to perform those tasks. > > This short series is adding one last functionality needed by ufs-utils > > that was somehow left behind - allowing reading descriptors as well. > > > > Avri Altman (3): > > bsg: Make job reply size different than SCSI_SENSE_BUFFERSIZE > > scsi: ufs: Allow reading descriptor via raw upiu > > scsi: ufs-bsg: Allow reading descriptors > > > > Documentation/scsi/ufs.txt | 8 > > block/bsg-lib.c| 4 ++-- > > drivers/scsi/ufs/ufs_bsg.c | 25 +++-- > > drivers/scsi/ufs/ufshcd.c | 20 ++-- > > include/linux/bsg-lib.h| 2 ++ > > 5 files changed, 41 insertions(+), 18 deletions(-) > > > > -- > > 1.9.1
Re: [PATCH v3 4/4] scsi: hisi_sas: Add support for DIF feature for v3 hw
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
Re: DIF/DIX issue related to config CONFIG_SCSI_MQ_DEFAULT
Hi John, > We have also noticed that if we just enable DIF in hisi_sas (with MQ), > and not DIX, then no issue. Enabling DIF doesn't really do anything on the kernel side other than setting PROTECT=1 in the READ/WRITE CDB and telling the driver which DIX protection operation the HBA should use. Since protection information is invisible to the kernel and only sent on the wire between initiator and target, enabling DIF doesn't really have the ability to interfere with anything on the kernel side. We're basically just setting flags asking HBA and storage to enable protected transfers. > I did also noticed mail "[PATCH v2 01/23] zfcp: make DIX experimental, > disabled, and independent of DIF", where DIX is made experimental. ...for the zfcp driver on zSeries. Just nitpicking on terminology here: T10 Protection Information (formerly known as DIF) describes how to generate and verify 8 bytes of extra information that's sent trailing each logical block on the wire between an initiator and target. The T10 PI spec is focused on the target device implementation of this and largely ignores the initiator side. DIX tries to remedy this deficiency. It is a spec that describes a set of logical operations an initiator must implement to facilitate sending and receiving the T10 protection information to/from host memory instead of terminating it at the HBA. The DIX spec isn't experimental, it's about a decade old and hasn't changed in years. The Linux kernel support for data integrity passthrough in the block layer and SCSI isn't experimental either. It's also a decade old and used extensively in production. So I object to the notion of "DIX being made experimental". An ASIC/firmware/driver implementation of DIX may be experimental. And of course I can't rule out regressions in the kernel block integrity implementation as a result of some of the recent MQ changes (will be happy to work with you guys to figure those out). But DIX isn't experimental, nor is the kernel support for passing protection information to an HBA. > For now we may not support DIX. It seems to have issues. We wanted to > try 3008 card on our system, but it does not seem to support DIX 0-3. For some reason Broadcom have not upstreamed their DIX support. It's supposedly available in their outbox driver. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v5 5/5] target: perform t10_wwn ID initialisation in target_alloc_device()
On Sat, 2018-12-01 at 15:59 +0100, Hannes Reinecke wrote:
> On 12/1/18 12:34 AM, David Disseldorp wrote:
> > Initialise the t10_wwn vendor, model and revision defaults when a
> > device is allocated instead of when it's enabled. This ensures that
> > custom vendor or model strings set prior to enablement are not later
> > overwritten with default values.
> >
> > Signed-off-by: David Disseldorp
> > ---
> > drivers/target/target_core_device.c | 34
> > +-
> > 1 file changed, 17 insertions(+), 17 deletions(-)
> > > diff --git a/drivers/target/target_core_device.c
>
> b/drivers/target/target_core_device.c
> > index 5512871f50e4..6318d59a1564 100644
> > --- a/drivers/target/target_core_device.c
> > +++ b/drivers/target/target_core_device.c
> > @@ -810,6 +810,23 @@ struct se_device *target_alloc_device(struct se_hba
> > *hba, const char *name)
> > mutex_init(&xcopy_lun->lun_tg_pt_md_mutex);
> > xcopy_lun->lun_tpg = &xcopy_pt_tpg;
> >
> > + /*
> > +* Preload the initial INQUIRY const values if we are doing
> > +* anything virtual (IBLOCK, FILEIO, RAMDISK), but not for TCM/pSCSI
> > +* passthrough because this is being provided by the backend LLD.
> > +*/
> > + BUILD_BUG_ON(sizeof(dev->t10_wwn.vendor) != INQUIRY_VENDOR_LEN + 1);
> > + BUILD_BUG_ON(sizeof(dev->t10_wwn.model) != INQUIRY_MODEL_LEN + 1);
> > + BUILD_BUG_ON(sizeof(dev->t10_wwn.revision) != INQUIRY_REVISION_LEN + 1);
> > + if (!(dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)) {
> > + strlcpy(dev->t10_wwn.vendor, "LIO-ORG",
> > + sizeof(dev->t10_wwn.vendor));
> > + strlcpy(dev->t10_wwn.model, dev->transport->inquiry_prod,
> > + sizeof(dev->t10_wwn.model));
> > + strlcpy(dev->t10_wwn.revision, dev->transport->inquiry_rev,
> > + sizeof(dev->t10_wwn.revision));
> > + }
> > +
> > return dev;
> > }
> >
>
> This is odd. I'd rather have it consistent across backends, ie either
> move the initialisation into the backends, or provide a means to check
> if the inquiry data has already been pre-filled.
> But this check really is awkward.
Agreed. I also would like to see that that if-condition is removed ...
Thanks,
Bart.
[PATCH] hpsa: add module parameter to disable irq affinity
The PCI_IRQ_AFFINITY flag prevents customers from
changing the smp_affinity and smp_affinity_list entries.
- add a module parameter to allow this flag to be turned
off.
- to turn off PCI_IRQ_AFFINITY:
flag hpsa_disable_irq_affinity=1
Reviewed-by: David Carroll
Reviewed-by: Scott Teel
Signed-off-by: Don Brace
---
drivers/scsi/hpsa.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index c9cccf35e9d7..0aa5aa66151f 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -87,6 +87,10 @@ static int hpsa_simple_mode;
module_param(hpsa_simple_mode, int, S_IRUGO|S_IWUSR);
MODULE_PARM_DESC(hpsa_simple_mode,
"Use 'simple mode' rather than 'performant mode'");
+static bool hpsa_disable_irq_affinity;
+module_param(hpsa_disable_irq_affinity, bool, S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(hpsa_disable_irq_affinity,
+ "Turn off managed irq affinity. Allows smp_affinity to be changed.");
/* define the PCI info for the cards we can control */
static const struct pci_device_id hpsa_pci_device_id[] = {
@@ -7389,7 +7393,7 @@ static void hpsa_setup_reply_map(struct ctlr_info *h)
*/
static int hpsa_interrupt_mode(struct ctlr_info *h)
{
- unsigned int flags = PCI_IRQ_LEGACY;
+ unsigned int flags;
int ret;
/* Some boards advertise MSI but don't really support it */
@@ -7400,17 +7404,20 @@ static int hpsa_interrupt_mode(struct ctlr_info *h)
case 0x40830E11:
break;
default:
+ flags = PCI_IRQ_MSIX;
+ if (!hpsa_disable_irq_affinity)
+ flags |= PCI_IRQ_AFFINITY;
ret = pci_alloc_irq_vectors(h->pdev, 1, MAX_REPLY_QUEUES,
- PCI_IRQ_MSIX | PCI_IRQ_AFFINITY);
+ flags);
if (ret > 0) {
h->msix_vectors = ret;
return 0;
}
- flags |= PCI_IRQ_MSI;
break;
}
+ flags = PCI_IRQ_LEGACY | PCI_IRQ_MSI;
ret = pci_alloc_irq_vectors(h->pdev, 1, 1, flags);
if (ret < 0)
return ret;
Re: DISABLE_CLUSTERING in scsi drivers
On Mon, 3 Dec 2018, Hannes Reinecke wrote: > As I said: I need to do PIO for the last two bytes of the data buffer. > For everything else DMA works nicely, it's just the last two bytes which > might be left over in the FIFO buffer under certain circumstances. I read the driver a few times already, thanks. > If you have an alternative suggestion without scsi_kmap_atomic_sg() I'd > be happy to convert it. I'm not trying to avoid scsi_kmap_atomic_sg(). Nevermind. -- > > Cheers, > > Hannes >
Re: [PATCH 01/41] scsi: BusLogic: mark expected switch fall-through
On 11/27/18 9:21 PM, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
>
> Addresses-Coverity-ID: 1056537 ("Missing break in switch")
> Signed-off-by: Gustavo A. R. Silva
> ---
> drivers/scsi/BusLogic.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
> index 9cee941f97d6..717eef3ee893 100644
> --- a/drivers/scsi/BusLogic.c
> +++ b/drivers/scsi/BusLogic.c
> @@ -2641,6 +2641,7 @@ static int blogic_resultcode(struct blogic_adapter
> *adapter,
> case BLOGIC_BAD_CMD_PARAM:
> blogic_warn("BusLogic Driver Protocol Error 0x%02X\n",
> adapter, adapter_status);
> + /* fall through */
> case BLOGIC_DATA_UNDERRUN:
> case BLOGIC_DATA_OVERRUN:
> case BLOGIC_NOEXPECT_BUSFREE:
>
Looks good.
Acked-by: Khalid Aziz
Re: [EXT] [PATCH] scsi: qla2xxx: NULL check before some freeing functions is not needed.
> On Dec 2, 2018, at 12:52 PM, Thomas Meyer wrote: > > External Email > > -- > External Email > > NULL check before some freeing functions is not needed. > > Signed-off-by: Thomas Meyer > --- > > diff -u -p a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c > --- a/drivers/scsi/qla2xxx/qla_os.c > +++ b/drivers/scsi/qla2xxx/qla_os.c > @@ -4191,12 +4191,10 @@ fail_free_nvram: >kfree(ha->nvram); >ha->nvram = NULL; > fail_free_ctx_mempool: > - if (ha->ctx_mempool) > - mempool_destroy(ha->ctx_mempool); > + mempool_destroy(ha->ctx_mempool); >ha->ctx_mempool = NULL; > fail_free_srb_mempool: > - if (ha->srb_mempool) > - mempool_destroy(ha->srb_mempool); > + mempool_destroy(ha->srb_mempool); >ha->srb_mempool = NULL; > fail_free_gid_list: >dma_free_coherent(&ha->pdev->dev, qla2x00_gid_list_size(ha), > @@ -4498,8 +4496,7 @@ qla2x00_mem_free(struct qla_hw_data *ha) >dma_free_coherent(&ha->pdev->dev, MCTP_DUMP_SIZE, > ha->mctp_dump, >ha->mctp_dump_dma); > > - if (ha->srb_mempool) > - mempool_destroy(ha->srb_mempool); > + mempool_destroy(ha->srb_mempool); > >if (ha->dcbx_tlv) >dma_free_coherent(&ha->pdev->dev, DCBX_TLV_DATA_SIZE, > @@ -4531,8 +4528,7 @@ qla2x00_mem_free(struct qla_hw_data *ha) >if (ha->async_pd) >dma_pool_free(ha->s_dma_pool, ha->async_pd, ha->async_pd_dma); > > - if (ha->s_dma_pool) > - dma_pool_destroy(ha->s_dma_pool); > + dma_pool_destroy(ha->s_dma_pool); > >if (ha->gid_list) >dma_free_coherent(&ha->pdev->dev, qla2x00_gid_list_size(ha), > @@ -4553,14 +4549,11 @@ qla2x00_mem_free(struct qla_hw_data *ha) >} >} > > - if (ha->dl_dma_pool) > - dma_pool_destroy(ha->dl_dma_pool); > + dma_pool_destroy(ha->dl_dma_pool); > > - if (ha->fcp_cmnd_dma_pool) > - dma_pool_destroy(ha->fcp_cmnd_dma_pool); > + dma_pool_destroy(ha->fcp_cmnd_dma_pool); > > - if (ha->ctx_mempool) > - mempool_destroy(ha->ctx_mempool); > + mempool_destroy(ha->ctx_mempool); > >qlt_mem_free(ha); > > @@ -7106,8 +7099,7 @@ qla2x00_module_exit(void) >qla2x00_release_firmware(); >kmem_cache_destroy(srb_cachep); >qlt_exit(); > - if (ctx_cachep) > - kmem_cache_destroy(ctx_cachep); > + kmem_cache_destroy(ctx_cachep); >fc_release_transport(qla2xxx_transport_template); >fc_release_transport(qla2xxx_transport_vport_template); > } Looks good. Acked-by: Himanshu Madhani Thanks, - Himanshu
Re: DISABLE_CLUSTERING in scsi drivers
On 12/2/18 11:13 PM, Finn Thain wrote: On Sun, 2 Dec 2018, Hannes Reinecke wrote: On 12/2/18 10:21 PM, Finn Thain wrote: On Sun, 2 Dec 2018, Hannes Reinecke wrote: Well, that lone 'kmap' is due to a quirk/errata in the datasheet; essentially we have to PIO a lone byte out of the FIFO to clear it up. And this byte is technically still part of the SCSI data, so we need to stuff it onto the end of the actual data sg list. Which is what the kmap() thingie does. So it really shouldn't be affected by the clustering algorithm. Sorry, I don't follow. If it's dead code, can it be removed Oh, it's not dead code. It's required as per datasheet. If it's not, does it require DISABLE_CLUSTERING? No, not really. It just affects the very last byte of the sglist, so I can't really see how it should be affected by clustering. AIUI, the scsi_kmap_atomic_sg() call which you added to esp_scsi.c assumes that the sg list elements are page sized and page aligned. DISABLE_CLUSTERING provides that guarantee, but am53c974.c doesn't use it. Is this not a bug? What am I missing? As I said: I need to do PIO for the last two bytes of the data buffer. For everything else DMA works nicely, it's just the last two bytes which might be left over in the FIFO buffer under certain circumstances. If you have an alternative suggestion without scsi_kmap_atomic_sg() I'd be happy to convert it. Cheers, Hannes -- Dr. Hannes ReineckezSeries & Storage h...@suse.com +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
[PATCH v3 3/4] scsi: hisi_sas: Make sg_tablesize consistent value
From: Xiang Chen
Sht->sg_tablesize is set in the driver, and it will be assigned to
shost->sg_tablesize in SCSI mid-layer. So it is not necessary to
assign shost->sg_table one more time in the driver.
In addition to the change, change each scsi_host_template.sg_tablesize
to HISI_SAS_SGE_PAGE_CNT instead of SG_ALL.
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas_main.c | 1 -
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 2 +-
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 2 +-
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 3 +--
4 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 95350fd..eed7fc5 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -2410,7 +2410,6 @@ int hisi_sas_probe(struct platform_device *pdev,
shost->max_lun = ~0;
shost->max_channel = 1;
shost->max_cmd_len = 16;
- shost->sg_tablesize = min_t(u16, SG_ALL, HISI_SAS_SGE_PAGE_CNT);
if (hisi_hba->hw->slot_index_alloc) {
shost->can_queue = hisi_hba->hw->max_command_entries;
shost->cmd_per_lun = hisi_hba->hw->max_command_entries;
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index 7186648..107f7c9 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -1816,7 +1816,7 @@ static int hisi_sas_v1_init(struct hisi_hba *hisi_hba)
.change_queue_depth = sas_change_queue_depth,
.bios_param = sas_bios_param,
.this_id= -1,
- .sg_tablesize = SG_ALL,
+ .sg_tablesize = HISI_SAS_SGE_PAGE_CNT,
.max_sectors= SCSI_DEFAULT_MAX_SECTORS,
.use_clustering = ENABLE_CLUSTERING,
.eh_device_reset_handler = sas_eh_device_reset_handler,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index 8580c71..8760987 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -3578,7 +3578,7 @@ static void wait_cmds_complete_timeout_v2_hw(struct
hisi_hba *hisi_hba,
.change_queue_depth = sas_change_queue_depth,
.bios_param = sas_bios_param,
.this_id= -1,
- .sg_tablesize = SG_ALL,
+ .sg_tablesize = HISI_SAS_SGE_PAGE_CNT,
.max_sectors= SCSI_DEFAULT_MAX_SECTORS,
.use_clustering = ENABLE_CLUSTERING,
.eh_device_reset_handler = sas_eh_device_reset_handler,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 59b5f64..44781e3 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -2231,7 +2231,7 @@ static ssize_t intr_coal_count_v3_hw_store(struct device
*dev,
.change_queue_depth = sas_change_queue_depth,
.bios_param = sas_bios_param,
.this_id= -1,
- .sg_tablesize = SG_ALL,
+ .sg_tablesize = HISI_SAS_SGE_PAGE_CNT,
.max_sectors= SCSI_DEFAULT_MAX_SECTORS,
.use_clustering = ENABLE_CLUSTERING,
.eh_device_reset_handler = sas_eh_device_reset_handler,
@@ -2373,7 +2373,6 @@ static ssize_t intr_coal_count_v3_hw_store(struct device
*dev,
shost->max_lun = ~0;
shost->max_channel = 1;
shost->max_cmd_len = 16;
- shost->sg_tablesize = min_t(u16, SG_ALL, HISI_SAS_SGE_PAGE_CNT);
shost->can_queue = hisi_hba->hw->max_command_entries -
HISI_SAS_RESERVED_IPTT_CNT;
shost->cmd_per_lun = hisi_hba->hw->max_command_entries -
--
1.9.1
[PATCH v3 4/4] scsi: hisi_sas: Add support for DIF feature for v3 hw
From: Xiang Chen
For v3 hw, we support DIF operation for SAS, but not SATA.
In addition, DIF CRC16 is supported.
This patchset adds the SW support for the described features. The main
components are as follows:
- Allocate memory for PI
- Fill PI fields
- Fill related to DIF in DQ and protection iu memories
DIX support seems to have some issue when enabled with SCSI MQ, so we'll
omit support for now. This was discusse here:
https://marc.info/?l=linux-scsi&m=154357719329297&w=2
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas.h | 18
drivers/scsi/hisi_sas/hisi_sas_main.c | 99 ++---
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 187 +++--
3 files changed, 286 insertions(+), 18 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index 912d234..6389b86 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -55,6 +55,11 @@
#define hisi_sas_sge_addr_mem(slot) hisi_sas_sge_addr(slot->buf)
#define hisi_sas_sge_addr_dma(slot) hisi_sas_sge_addr(slot->buf_dma)
+#define hisi_sas_sge_dif_addr(buf) \
+ (buf + offsetof(struct hisi_sas_slot_dif_buf_table, sge_dif_page))
+#define hisi_sas_sge_dif_addr_mem(slot) hisi_sas_sge_dif_addr(slot->buf)
+#define hisi_sas_sge_dif_addr_dma(slot) hisi_sas_sge_dif_addr(slot->buf_dma)
+
#define HISI_SAS_MAX_SSP_RESP_SZ (sizeof(struct ssp_frame_hdr) + 1024)
#define HISI_SAS_MAX_SMP_RESP_SZ 1028
#define HISI_SAS_MAX_STP_RESP_SZ 28
@@ -197,6 +202,7 @@ struct hisi_sas_slot {
struct sas_task *task;
struct hisi_sas_port*port;
u64 n_elem;
+ u64 n_elem_dif;
int dlvry_queue;
int dlvry_queue_slot;
int cmplt_queue;
@@ -268,6 +274,8 @@ struct hisi_hba {
struct pci_dev *pci_dev;
struct device *dev;
+ bool enable_dif;
+
void __iomem *regs;
void __iomem *sgpio_regs;
struct regmap *ctrl;
@@ -422,6 +430,11 @@ struct hisi_sas_sge_page {
struct hisi_sas_sge sge[HISI_SAS_SGE_PAGE_CNT];
} __aligned(16);
+#define HISI_SAS_SGE_DIF_PAGE_CNT SG_CHUNK_SIZE
+struct hisi_sas_sge_dif_page {
+ struct hisi_sas_sge sge[HISI_SAS_SGE_DIF_PAGE_CNT];
+} __aligned(16);
+
struct hisi_sas_command_table_ssp {
struct ssp_frame_hdr hdr;
union {
@@ -452,6 +465,11 @@ struct hisi_sas_slot_buf_table {
struct hisi_sas_sge_page sge_page;
};
+struct hisi_sas_slot_dif_buf_table {
+ struct hisi_sas_slot_buf_table slot_buf;
+ struct hisi_sas_sge_dif_page sge_dif_page;
+};
+
extern struct scsi_transport_template *hisi_sas_stt;
extern void hisi_sas_stop_phys(struct hisi_hba *hisi_hba);
extern int hisi_sas_alloc(struct hisi_hba *hisi_hba, struct Scsi_Host *shost);
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index eed7fc5..146fce3 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -252,14 +252,21 @@ void hisi_sas_slot_task_free(struct hisi_hba *hisi_hba,
struct sas_task *task,
task->lldd_task = NULL;
- if (!sas_protocol_ata(task->task_proto))
+ if (!sas_protocol_ata(task->task_proto)) {
+ struct sas_ssp_task *ssp_task = &task->ssp_task;
+ struct scsi_cmnd *scsi_cmnd = ssp_task->cmd;
+
if (slot->n_elem)
dma_unmap_sg(dev, task->scatter,
task->num_scatter,
task->data_dir);
+ if (slot->n_elem_dif)
+ dma_unmap_sg(dev, scsi_prot_sglist(scsi_cmnd),
+scsi_prot_sg_count(scsi_cmnd),
+task->data_dir);
+ }
}
-
spin_lock_irqsave(&dq->lock, flags);
list_del_init(&slot->entry);
spin_unlock_irqrestore(&dq->lock, flags);
@@ -380,6 +387,59 @@ static int hisi_sas_dma_map(struct hisi_hba *hisi_hba,
return rc;
}
+static void hisi_sas_dif_dma_unmap(struct hisi_hba *hisi_hba,
+ struct sas_task *task, int n_elem_dif)
+{
+ struct device *dev = hisi_hba->dev;
+
+ if (n_elem_dif) {
+ struct sas_ssp_task *ssp_task = &task->ssp_task;
+ struct scsi_cmnd *scsi_cmnd = ssp_task->cmd;
+
+ dma_unmap_sg(dev, scsi_prot_sglist(scsi_cmnd),
+scsi_prot_sg_count(scsi_cmnd),
+task->data_dir);
+ }
+}
+
+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_cm
[PATCH v3 1/4] scsi: hisi_sas: Fix warnings detected by sparse
This patchset fixes some warnings detected by the sparse tool, like these:
drivers/scsi/hisi_sas/hisi_sas_main.c:1469:52: warning: incorrect type in
assignment (different base types)
drivers/scsi/hisi_sas/hisi_sas_main.c:1469:52:expected unsigned short
[unsigned] [assigned] [usertype] tag_of_task_to_be_managed
drivers/scsi/hisi_sas/hisi_sas_main.c:1469:52:got restricted __le16
[usertype]
drivers/scsi/hisi_sas/hisi_sas_main.c:1723:52: warning: incorrect type in
assignment (different base types)
drivers/scsi/hisi_sas/hisi_sas_main.c:1723:52:expected unsigned short
[unsigned] [assigned] [usertype] tag_of_task_to_be_managed
drivers/scsi/hisi_sas/hisi_sas_main.c:1723:52:got restricted __le16
[usertype]
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas.h | 2 +-
drivers/scsi/hisi_sas/hisi_sas_main.c | 6 ++--
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 15
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 66 +++---
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 37 +++
5 files changed, 71 insertions(+), 55 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index 535c613..912d234 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -211,7 +211,7 @@ struct hisi_sas_slot {
/* Do not reorder/change members after here */
void*buf;
dma_addr_t buf_dma;
- int idx;
+ u16 idx;
};
struct hisi_sas_hw {
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 65dc749..c39c91c 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -1461,12 +1461,12 @@ static int hisi_sas_abort_task(struct sas_task *task)
if (task->lldd_task && task->task_proto & SAS_PROTOCOL_SSP) {
struct scsi_cmnd *cmnd = task->uldd_task;
struct hisi_sas_slot *slot = task->lldd_task;
- u32 tag = slot->idx;
+ u16 tag = slot->idx;
int rc2;
int_to_scsilun(cmnd->device->lun, &lun);
tmf_task.tmf = TMF_ABORT_TASK;
- tmf_task.tag_of_task_to_be_managed = cpu_to_le16(tag);
+ tmf_task.tag_of_task_to_be_managed = tag;
rc = hisi_sas_debug_issue_ssp_tmf(task->dev, lun.scsi_lun,
&tmf_task);
@@ -1720,7 +1720,7 @@ static int hisi_sas_query_task(struct sas_task *task)
int_to_scsilun(cmnd->device->lun, &lun);
tmf_task.tmf = TMF_QUERY_TASK;
- tmf_task.tag_of_task_to_be_managed = cpu_to_le16(tag);
+ tmf_task.tag_of_task_to_be_managed = tag;
rc = hisi_sas_debug_issue_ssp_tmf(device,
lun.scsi_lun,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index d24342b..7186648 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -510,6 +510,7 @@ static void setup_itct_v1_hw(struct hisi_hba *hisi_hba,
struct hisi_sas_itct *itct = &hisi_hba->itct[device_id];
struct asd_sas_port *sas_port = device->port;
struct hisi_sas_port *port = to_hisi_sas_port(sas_port);
+ u64 sas_addr;
memset(itct, 0, sizeof(*itct));
@@ -534,8 +535,8 @@ static void setup_itct_v1_hw(struct hisi_hba *hisi_hba,
itct->qw0 = cpu_to_le64(qw0);
/* qw1 */
- memcpy(&itct->sas_addr, device->sas_addr, SAS_ADDR_SIZE);
- itct->sas_addr = __swab64(itct->sas_addr);
+ memcpy(&sas_addr, device->sas_addr, SAS_ADDR_SIZE);
+ itct->sas_addr = cpu_to_le64(__swab64(sas_addr));
/* qw2 */
itct->qw2 = cpu_to_le64((500ULL << ITCT_HDR_IT_NEXUS_LOSS_TL_OFF) |
@@ -561,7 +562,7 @@ static void clear_itct_v1_hw(struct hisi_hba *hisi_hba,
reg_val &= ~CFG_AGING_TIME_ITCT_REL_MSK;
hisi_sas_write32(hisi_hba, CFG_AGING_TIME, reg_val);
- qw0 = cpu_to_le64(itct->qw0);
+ qw0 = le64_to_cpu(itct->qw0);
qw0 &= ~ITCT_HDR_VALID_MSK;
itct->qw0 = cpu_to_le64(qw0);
}
@@ -1102,7 +1103,7 @@ static void slot_err_v1_hw(struct hisi_hba *hisi_hba,
case SAS_PROTOCOL_SSP:
{
int error = -1;
- u32 dma_err_type = cpu_to_le32(err_record->dma_err_type);
+ u32 dma_err_type = le32_to_cpu(err_record->dma_err_type);
u32 dma_tx_err_type = ((dma_err_type &
ERR_HDR_DMA_TX_ERR_TYPE_MSK)) >>
ERR_HDR_DMA_TX_ERR_TYPE_OFF;
@@ -1110,9 +,9 @@ static void slot_err_v1_hw(struct hisi_hba *hisi_hba,
ERR_HDR_DMA_RX_ERR_TYPE_MSK)) >>
ERR_HDR_DMA_RX_ERR_TYPE_OFF;
u32 trans_tx_fail_t
[PATCH v3 2/4] scsi: hisi_sas: Relocate some code to reduce complexity
From: Xiang Chen
Relocate the codes related to dma_map/unmap in hisi_sas_task_prep()
to reduce complexity, with a view to add DIF/DIX support.
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas_main.c | 146 +-
1 file changed, 90 insertions(+), 56 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index c39c91c..95350fd 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -296,6 +296,90 @@ static void hisi_sas_task_prep_abort(struct hisi_hba
*hisi_hba,
device_id, abort_flag, tag_to_abort);
}
+static void hisi_sas_dma_unmap(struct hisi_hba *hisi_hba,
+ struct sas_task *task, int n_elem,
+ int n_elem_req, int n_elem_resp)
+{
+ struct device *dev = hisi_hba->dev;
+
+ if (!sas_protocol_ata(task->task_proto)) {
+ if (task->num_scatter) {
+ if (n_elem)
+ dma_unmap_sg(dev, task->scatter,
+task->num_scatter,
+task->data_dir);
+ } else if (task->task_proto & SAS_PROTOCOL_SMP) {
+ if (n_elem_req)
+ dma_unmap_sg(dev, &task->smp_task.smp_req,
+1, DMA_TO_DEVICE);
+ if (n_elem_resp)
+ dma_unmap_sg(dev, &task->smp_task.smp_resp,
+1, DMA_FROM_DEVICE);
+ }
+ }
+}
+
+static int hisi_sas_dma_map(struct hisi_hba *hisi_hba,
+ struct sas_task *task, int *n_elem,
+ int *n_elem_req, int *n_elem_resp)
+{
+ struct device *dev = hisi_hba->dev;
+ int rc;
+
+ if (sas_protocol_ata(task->task_proto)) {
+ *n_elem = task->num_scatter;
+ } else {
+ unsigned int req_len, resp_len;
+
+ if (task->num_scatter) {
+ *n_elem = dma_map_sg(dev, task->scatter,
+task->num_scatter, task->data_dir);
+ if (!*n_elem) {
+ rc = -ENOMEM;
+ goto prep_out;
+ }
+ } else if (task->task_proto & SAS_PROTOCOL_SMP) {
+ *n_elem_req = dma_map_sg(dev, &task->smp_task.smp_req,
+1, DMA_TO_DEVICE);
+ if (!*n_elem_req) {
+ rc = -ENOMEM;
+ goto prep_out;
+ }
+ req_len = sg_dma_len(&task->smp_task.smp_req);
+ if (req_len & 0x3) {
+ rc = -EINVAL;
+ goto err_out_dma_unmap;
+ }
+ *n_elem_resp = dma_map_sg(dev, &task->smp_task.smp_resp,
+ 1, DMA_FROM_DEVICE);
+ if (!*n_elem_resp) {
+ rc = -ENOMEM;
+ goto err_out_dma_unmap;
+ }
+ resp_len = sg_dma_len(&task->smp_task.smp_resp);
+ if (resp_len & 0x3) {
+ rc = -EINVAL;
+ goto err_out_dma_unmap;
+ }
+ }
+ }
+
+ if (*n_elem > HISI_SAS_SGE_PAGE_CNT) {
+ dev_err(dev, "task prep: n_elem(%d) > HISI_SAS_SGE_PAGE_CNT",
+ *n_elem);
+ rc = -EINVAL;
+ goto err_out_dma_unmap;
+ }
+ return 0;
+
+err_out_dma_unmap:
+ /* It would be better to call dma_unmap_sg() here, but it's messy */
+ hisi_sas_dma_unmap(hisi_hba, task, *n_elem,
+ *n_elem_req, *n_elem_resp);
+prep_out:
+ return rc;
+}
+
static int hisi_sas_task_prep(struct sas_task *task,
struct hisi_sas_dq **dq_pointer,
bool is_tmf, struct hisi_sas_tmf_task *tmf,
@@ -338,49 +422,10 @@ static int hisi_sas_task_prep(struct sas_task *task,
return -ECOMM;
}
- if (!sas_protocol_ata(task->task_proto)) {
- unsigned int req_len, resp_len;
-
- if (task->num_scatter) {
- n_elem = dma_map_sg(dev, task->scatter,
- task->num_scatter, task->data_dir);
- if (!n_elem) {
- rc = -ENOMEM;
- goto prep_out;
- }
- } else if (task->task_proto & SAS_PRO
[PATCH v3 0/3] hisi_sas: DIF support
This patchset introduces support to the driver for DIF (or PI -protection information). We are dropping DIX support for now, based on issues discussed in the following: https://marc.info/?l=linux-scsi&m=154357719329297&w=4 We will only support PI in v3 hw at the moment, even though previous hw versions also support it. The series is broken down as follows: - Fix pre-existing sparse warnings - Tidy sg table size config - Some tidy-up to accept PI support - Add components for PI support for main and v3 driver Difference v1->v2: - drop scsi_prot_op_normal() v2->v3: - fix sparse warnings - drop DIX support John Garry (1): scsi: hisi_sas: Fix warnings detected by sparse Xiang Chen (3): scsi: hisi_sas: Relocate some code to reduce complexity scsi: hisi_sas: Make sg_tablesize consistent value scsi: hisi_sas: Add support for DIF feature for v3 hw drivers/scsi/hisi_sas/hisi_sas.h | 20 ++- drivers/scsi/hisi_sas/hisi_sas_main.c | 248 +++-- drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 17 +-- drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 68 + drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 227 +++--- 5 files changed, 448 insertions(+), 132 deletions(-) -- 1.9.1
Re: [PATCH v5 5/5] target: perform t10_wwn ID initialisation in target_alloc_device()
On Sun, 2 Dec 2018 23:22:23 +0100, David Disseldorp wrote:
> > > + if (!(dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)) {
> > > + strlcpy(dev->t10_wwn.vendor, "LIO-ORG",
> > > + sizeof(dev->t10_wwn.vendor));
> > > + strlcpy(dev->t10_wwn.model, dev->transport->inquiry_prod,
> > > + sizeof(dev->t10_wwn.model));
> > > + strlcpy(dev->t10_wwn.revision, dev->transport->inquiry_rev,
> > > + sizeof(dev->t10_wwn.revision));
> > > + }
> > > +
> > > return dev;
> > > }
> > >
> > This is odd. I'd rather have it consistent across backends, ie either
> > move the initialisation into the backends, or provide a means to check
> > if the inquiry data has already been pre-filled.
> > But this check really is awkward.
>
> Not quite sure I follow here. I could the default setting to the
> target_backend_ops.alloc_device() code paths, but I don't think the
> duplication would make this much cleaner, if at all.
> I can look into this further if you like (target_backend_ops.inquiry_rev
> could be dropped that way),
Looking a little closer, I think we can drop the conditional completely
and set the vendor/model/rev defaults for all cases here:
- target_core_pscsi overwrites the defaults in the
pscsi_configure_device() callback.
+ the contents is then only used for configfs via
$pscsi_dev/info, $pscsi_dev/statistics/scsi_lu/vend, etc.
- target_core_user doesn't touch the defaults, nor are they used for
anything outside of configfs.
> but my preference would be to do so as a
> follow-up patch-set.
This is still my preference.
Cheers, David
