Re: [PATCH 0/7] SCSI: cleanup debugfs usage

2019-01-23 Thread John Garry

On 22/01/2019 15:08, Greg Kroah-Hartman wrote:

When calling debugfs code, there is no need to ever check the return
value of the call, as no logic should ever change if a call works
properly or not.  Fix up a bunch of x86-specific code to not care about
the results of debugfs.

Greg Kroah-Hartman (7):
  scsi: bfa: no need to check return value of debugfs_create functions
  scsi: csiostor: no need to check return value of debugfs_create
functions
  scsi: fnic: no need to check return value of debugfs_create functions
  scsi: snic: no need to check return value of debugfs_create functions
  scsi: lpfc: no need to check return value of debugfs_create functions
  scsi: qlogic: no need to check return value of debugfs_create
functions
  scsi: qla2xxx: no need to check return value of debugfs_create
functions



JFYI, Martin has some more debugfs usage queued for 5.1 here:
https://kernel.googlesource.com/pub/scm/linux/kernel/git/mkp/scsi/+log/5.1/scsi-queue/drivers/scsi/hisi_sas

I'll send a patch to tidy-up, as above. It would not be x86 specific, 
but I am not sure what above is.


Thanks,
John




 drivers/scsi/bfa/bfad_debugfs.c   |  17 ---
 drivers/scsi/csiostor/csio_init.c |   6 +-
 drivers/scsi/fnic/fnic_debugfs.c  |  88 +---
 drivers/scsi/fnic/fnic_main.c |   7 +-
 drivers/scsi/fnic/fnic_stats.h|   2 +-
 drivers/scsi/fnic/fnic_trace.c|  17 +--
 drivers/scsi/fnic/fnic_trace.h|   4 +-
 drivers/scsi/lpfc/lpfc_debugfs.c  | 170 --
 drivers/scsi/qedf/qedf_debugfs.c  |  18 +---
 drivers/scsi/qedi/qedi_debugfs.c  |  17 +--
 drivers/scsi/qla2xxx/qla_dfs.c|  43 +---
 drivers/scsi/snic/snic_debugfs.c  | 133 +--
 drivers/scsi/snic/snic_main.c |  14 +--
 drivers/scsi/snic/snic_stats.h|   2 +-
 drivers/scsi/snic/snic_trc.c  |  12 +--
 drivers/scsi/snic/snic_trc.h  |   4 +-
 16 files changed, 48 insertions(+), 506 deletions(-)






[PATCH] scsi: dpt_i2o: clean up indentation issues, remove spaces

2019-01-23 Thread Colin King
From: Colin Ian King 

There are several lines where the indentation has an extra space,
fix this by removing the spaces.

Signed-off-by: Colin Ian King 
---
 drivers/scsi/dpt_i2o.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index 70d1a18278af..f9560a183c12 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -877,8 +877,8 @@ static void adpt_i2o_sys_shutdown(void)
adpt_hba *pHba, *pNext;
struct adpt_i2o_post_wait_data *p1, *old;
 
-printk(KERN_INFO"Shutting down Adaptec I2O controllers.\n");
-printk(KERN_INFO"   This could take a few minutes if there are many 
devices attached\n");
+   printk(KERN_INFO "Shutting down Adaptec I2O controllers.\n");
+   printk(KERN_INFO "   This could take a few minutes if there are many 
devices attached\n");
/* Delete all IOPs from the controller chain */
/* They should have already been released by the
 * scsi-core
@@ -901,7 +901,7 @@ static void adpt_i2o_sys_shutdown(void)
 // spin_unlock_irqrestore(&adpt_post_wait_lock, flags);
adpt_post_wait_queue = NULL;
 
-printk(KERN_INFO "Adaptec I2O controllers down.\n");
+   printk(KERN_INFO "Adaptec I2O controllers down.\n");
 }
 
 static int adpt_install_hba(struct scsi_host_template* sht, struct pci_dev* 
pDev)
@@ -3427,7 +3427,7 @@ static int adpt_i2o_issue_params(int cmd, adpt_hba* pHba, 
int tid,
return -((res[1] >> 16) & 0xFF); /* -BlockStatus */
}
 
-return 4 + ((res[1] & 0x) << 2); /* bytes used in resblk */ 
+   return 4 + ((res[1] & 0x) << 2); /* bytes used in resblk */
 }
 
 
@@ -3500,8 +3500,8 @@ static int adpt_i2o_enable_hba(adpt_hba* pHba)
 
 static int adpt_i2o_systab_send(adpt_hba* pHba)
 {
-u32 msg[12];
-int ret;
+   u32 msg[12];
+   int ret;
 
msg[0] = I2O_MESSAGE_SIZE(12) | SGL_OFFSET_6;
msg[1] = I2O_CMD_SYS_TAB_SET<<24 | HOST_TID<<12 | ADAPTER_TID;
-- 
2.19.1



[PATCH] scsi: qlogicfas408: clean up a couple of indentation issues

2019-01-23 Thread Colin King
From: Colin Ian King 

An if statement is indented correctly and an outb statement has
a redundant empty comment and incorrect indentation. Fix these.

Signed-off-by: Colin Ian King 
---
 drivers/scsi/qlogicfas408.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/qlogicfas408.c b/drivers/scsi/qlogicfas408.c
index 8b471a925b43..136681ad18a5 100644
--- a/drivers/scsi/qlogicfas408.c
+++ b/drivers/scsi/qlogicfas408.c
@@ -139,7 +139,7 @@ static int ql_pdma(struct qlogicfas408_priv *priv, int 
phase, char *request, int
} else {/* out */
 #if QL_TURBO_PDMA
rtrc(4)
-   if (reqlen >= 128 && inb(qbase + 8) & 0x10) {   /* 
empty */
+   if (reqlen >= 128 && inb(qbase + 8) & 0x10) {   /* empty */
outsl(qbase + 4, request, 32);
reqlen -= 128;
request += 128;
@@ -240,7 +240,7 @@ static void ql_icmd(struct scsi_cmnd *cmd)
outb(0x40 | qlcfg8 | priv->qinitid, qbase + 8);
outb(qlcfg7, qbase + 7);
outb(qlcfg6, qbase + 6);
-/**/ outb(qlcfg5, qbase + 5);  /* select timer */
+   outb(qlcfg5, qbase + 5);/* select timer */
outb(qlcfg9 & 7, qbase + 9);/* prescaler */
 /* outb(0x99, qbase + 5);  */
outb(scmd_id(cmd), qbase + 4);
-- 
2.19.1



Re: [PATCH for-5.0] scsi: communicate max segment size to the DMA mapping code

2019-01-23 Thread Steffen Maier

On 01/16/2019 05:12 PM, Christoph Hellwig wrote:

When a host driver sets a maximum segment size we should not only
propagate that setting to the block layer, which can merge segments,
but also to the DMA mapping layer which can merge segments as well.

Fixes: 50c2e9107f ("scsi: introduce a max_segment_size host_template 
parameters")
Signed-off-by: Christoph Hellwig 
---
  drivers/ata/pata_macio.c |  9 -
  drivers/ata/sata_inic162x.c  | 22 +-
  drivers/firewire/sbp2.c  |  5 +
  drivers/scsi/aacraid/linit.c |  9 -
  drivers/scsi/scsi_lib.c  |  4 ++--
  5 files changed, 20 insertions(+), 29 deletions(-)



diff --git a/drivers/firewire/sbp2.c b/drivers/firewire/sbp2.c
index 09b845e90114..a785ffd5af89 100644
--- a/drivers/firewire/sbp2.c
+++ b/drivers/firewire/sbp2.c
@@ -1144,10 +1144,6 @@ static int sbp2_probe(struct fw_unit *unit, const struct 
ieee1394_device_id *id)
if (device->is_local)
return -ENODEV;

-   if (dma_get_max_seg_size(device->card->device) > SBP2_MAX_SEG_SIZE)
-   WARN_ON(dma_set_max_seg_size(device->card->device,
-SBP2_MAX_SEG_SIZE));
-
shost = scsi_host_alloc(&scsi_driver_template, sizeof(*tgt));
if (shost == NULL)
return -ENOMEM;
@@ -1610,6 +1606,7 @@ static struct scsi_host_template scsi_driver_template = {
.eh_abort_handler   = sbp2_scsi_abort,
.this_id= -1,
.sg_tablesize   = SG_ALL,
+   .max_segment_size   = SBP2_MAX_SEG_SIZE,
.can_queue  = 1,
.sdev_attrs = sbp2_scsi_sysfs_attrs,
  };



diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index b13cc9288ba0..6d65ac584eba 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1842,8 +1842,8 @@ void __scsi_init_queue(struct Scsi_Host *shost, struct 
request_queue *q)
blk_queue_segment_boundary(q, shost->dma_boundary);
dma_set_seg_boundary(dev, shost->dma_boundary);

-   blk_queue_max_segment_size(q,
-   min(shost->max_segment_size, dma_get_max_seg_size(dev)));
+   blk_queue_max_segment_size(q, shost->max_segment_size);
+   dma_set_max_seg_size(dev, shost->max_segment_size);


Zfcp can only have max_segment_size of one page (4kB). Officially announced 
through dev.dma_parms since v2.6.35 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/s390/scsi?id=683229845f1780b10041ee7a1043fc8f10061455.


With Martin's 5.0/scsi-fixes which includes your above patch, I now get 64kB 
instead of 4kB for the respective queue limit because __scsi_ini_queue 
overwrites the dma parameter:


[root@host:/sys/bus/ccw/drivers/zfcp/0.0.3c40/host5/rport-5:0-4/target5:0:4/5:0:4:10/block/sdi/queue](0)# 
cat max_segment_size

65536

To my surprise, I don't get IO errors with zfcp. Maybe my IO pattern does not 
cause too large segments to be created. I would have expected the FCP channel 
complaining rightly so if we pass segments larger than one page. Maybe the 
additional dma_boundary of pagesize-1 helped the too large max_segment_size to 
not become effective?


A quick attempt to adapt zfcp to your patch would be to set 
scsi_host_template.max_segment_size = ZFCP_QDIO_SBALE_LEN.


Ideas?

--
Mit freundlichen Gruessen / Kind regards
Steffen Maier

Linux on IBM Z Development

https://www.ibm.com/privacy/us/en/
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294



[Bug 199435] HPSA + P420i resetting logical Direct-Access never complete

2019-01-23 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=199435

--- Comment #28 from Anthony Hausman (anthonyhaussm...@gmail.com) ---
I have actually compiled the hpsa driver 3.4.20.141 into the kernel 4.19.13.

I still have the same behavior, a heavy load (3000) and all disk of the
controller unavailable.

But this time, it's not the reset who trigger the bug, here the log that I
have.

First of all, one disk returns a lot of critical medium error:
```
[Wed Jan 23 15:55:34 2019] print_req_error: critical medium error, dev sdt,
sector 13836632
[Wed Jan 23 15:55:34 2019] sd 3:1:0:19: [sdt] Unaligned partial completion
(resid=52, sector_sz=512)
[Wed Jan 23 15:55:35 2019] sd 3:1:0:19: [sdt] Unaligned partial completion
(resid=48, sector_sz=512)
[Wed Jan 23 15:55:35 2019] sd 3:1:0:19: [sdt] Unaligned partial completion
(resid=32, sector_sz=512)
[Wed Jan 23 15:55:35 2019] sd 3:1:0:19: [sdt] Unaligned partial completion
(resid=52, sector_sz=512)
[Wed Jan 23 15:55:52 2019] sd 3:1:0:19: [sdt] Unaligned partial completion
(resid=32, sector_sz=512)
[Wed Jan 23 15:55:52 2019] scsi_io_completion_action: 5 callbacks suppressed
[Wed Jan 23 15:55:52 2019] sd 3:1:0:19: [sdt] tag#23 FAILED Result:
hostbyte=DID_OK driverbyte=DRIVER_SENSE
[Wed Jan 23 15:55:52 2019] sd 3:1:0:19: [sdt] tag#23 Sense Key : Medium Error
[current] 
[Wed Jan 23 15:55:52 2019] sd 3:1:0:19: [sdt] tag#23 Add. Sense: Unrecovered
read error
[Wed Jan 23 15:55:52 2019] sd 3:1:0:19: [sdt] tag#23 CDB: Read(16) 88 00 00 00
00 00 00 d3 21 58 00 00 00 08 00 00
[Wed Jan 23 15:55:52 2019] print_req_error: 5 callbacks suppressed
[Wed Jan 23 15:55:52 2019] print_req_error: critical medium error, dev sdt,
sector 13836632
```

After this, hpsa show sone failed inquiry:
```
[Wed Jan 23 15:57:07 2019] hpsa :08:00.0: aborted: NULL_SDEV_PTR
TAG:0x:0770 LUN:00c03901
CDB:12003100
[Wed Jan 23 15:57:07 2019] hpsa :08:00.0: hpsa_update_device_info: inquiry
failed, device will be skipped.
[Wed Jan 23 15:57:08 2019] hpsa :08:00.0:   removed scsi 3:0:50:0:
Direct-Access ATA  MB4000GCWDC  PHYS DRV SSDSmartPathCap- En- Exp=0
qd=14
[Wed Jan 23 15:57:31 2019] hpsa :08:00.0: aborted: NULL_SDEV_PTR
TAG:0x:15c0 LUN:00c03901
CDB:12003100
[Wed Jan 23 15:57:31 2019] hpsa :08:00.0: hpsa_update_device_info: inquiry
failed, device will be skipped.
[Wed Jan 23 15:57:54 2019] hpsa :08:00.0: aborted: NULL_SDEV_PTR
TAG:0x:0e70 LUN:00c03901
CDB:12003100
[Wed Jan 23 15:57:54 2019] hpsa :08:00.0: hpsa_update_device_info: inquiry
failed, device will be skipped.

[Wed Jan 23 15:59:04 2019] hpsa :08:00.0: aborted: NULL_SDEV_PTR
TAG:0x:2650 LUN:00c03901
CDB:12003100
[Wed Jan 23 15:59:04 2019] hpsa :08:00.0: hpsa_update_device_info: inquiry
failed, device will be skipped.
[Wed Jan 23 15:59:28 2019] hpsa :08:00.0: aborted: NULL_SDEV_PTR
TAG:0x:1400 LUN:00c03901
CDB:12003100
[Wed Jan 23 15:59:28 2019] hpsa :08:00.0: hpsa_update_device_info: inquiry
failed, device will be skipped.
[Wed Jan 23 15:59:51 2019] hpsa :08:00.0: aborted: NULL_SDEV_PTR
TAG:0x:1400 LUN:00c03901
CDB:12003100
[Wed Jan 23 15:59:51 2019] hpsa :08:00.0: hpsa_update_device_info: inquiry
failed, device will be skipped.
```


And following this Call Trace:
```
Wed Jan 23 16:00:19 2019] INFO: task task:12406 blocked for more than 120
seconds.
[Wed Jan 23 16:00:19 2019]   Not tainted 4.19.13-dailymotion #1
[Wed Jan 23 16:00:19 2019] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
disables this message.
[Wed Jan 23 16:00:19 2019] taskD0 12406  12384 0x
[Wed Jan 23 16:00:19 2019] Call Trace:
[Wed Jan 23 16:00:19 2019]  ? __schedule+0x2b7/0x880
[Wed Jan 23 16:00:19 2019]  ? bit_wait+0x50/0x50
[Wed Jan 23 16:00:19 2019]  schedule+0x28/0x80
[Wed Jan 23 16:00:19 2019]  io_schedule+0x12/0x40
[Wed Jan 23 16:00:19 2019]  bit_wait_io+0xd/0x50
[Wed Jan 23 16:00:19 2019]  __wait_on_bit+0x44/0x80
[Wed Jan 23 16:00:19 2019]  out_of_line_wait_on_bit+0x91/0xb0
[Wed Jan 23 16:00:19 2019]  ? init_wait_var_entry+0x40/0x40
[Wed Jan 23 16:00:19 2019]  __ext4_get_inode_loc+0x1a4/0x3f0
[Wed Jan 23 16:00:19 2019]  ext4_iget+0x8f/0xbb0
[Wed Jan 23 16:00:19 2019]  ? d_alloc_parallel+0x9d/0x4a0
[Wed Jan 23 16:00:19 2019]  ext4_lookup+0xda/0x200
[Wed Jan 23 16:00:19 2019]  __lookup_slow+0x97/0x150
[Wed Jan 23 16:00:19 2019]  lookup_slow+0x35/0x50
[Wed Jan 23 16:00:19 2019]  walk_component+0x1c4/0x340
[Wed Jan 23 16:00:19 2019]  link_path_walk.part.33+0x2a6/0x510
[Wed Jan 23 16:00:19 2019]  ? path_init+0x190/0x310
[Wed Jan 23 16:00:19 2019]  path_openat+0xdd/0x1540
[Wed Jan 23 16:00:19 2019]  ? get_futex_key+0x2ed/0x3d0
[Wed Jan 23 16:00:19 2019]  do_filp_open+0x9b/0x110
[Wed Jan 23 16:00:19 2019]  ? __check_object_size+

Re: [PATCH 0/7] Fix handling of bidi commands

2019-01-23 Thread Bart Van Assche
On Tue, 2019-01-22 at 23:49 -0500, Douglas Gilbert wrote:
> On 2019-01-22 7:56 p.m., Bart Van Assche wrote:
> > On Tue, 2019-01-22 at 18:30 -0500, Douglas Gilbert wrote:
> > > This patchset needs something like the following if UAS (USB Attached
> > > SCSI) is configured in your kernel.
> > > 
> > > Beware of tabs/spaces/line_wraps as this is a cut and paste:
> > > 
> > > diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
> > > index 36742e8e7edc..24f3f95917a5 100644
> > > --- a/drivers/usb/storage/uas.c
> > > +++ b/drivers/usb/storage/uas.c
> > > @@ -401,9 +401,9 @@ static void uas_data_cmplt(struct urb *urb)
> > >   if (status != -ENOENT && status != -ECONNRESET && 
> > > status !=
> > > -ESHUTDOWN)
> > >   uas_log_cmd_state(cmnd, "data cmplt err", 
> > > status);
> > >   /* error: no data transfered */
> > > -   sdb->resid = sdb->length;
> > > +   scsi_in_set_resid(cmnd, sdb->length);
> > >   } else {
> > > -   sdb->resid = sdb->length - urb->actual_length;
> > > +   scsi_in_set_resid(cmnd, sdb->length - urb->actual_length);
> > >   }
> > >   uas_try_complete(cmnd, __func__);
> > >out:
> > 
> > Thanks Doug! I will fold a slightly modified version of this patch in. BTW,
> > does this mean that you have been able to test this patch series?
> 
> Yes, but I didn't get far. With my sg v4 driver, scsi_debug and my sg_tst_bidi
> utility and this patchset after a short while I get a NULL pointer dereference
> in scsi_mq_prep_fn() inside the bidi conditional, probably the memset().
> 
> RIP ---> 0x2ce7
> 
> 
>  if (blk_bidi_rq(req)) {
>  2c9a:   49 8b 85 20 01 00 00mov0x120(%r13),%rax
>  2ca1:   48 85 c0test   %rax,%rax
>  2ca4:   74 60   je 2d06 
>  memset(&scsi_in_cmd(cmd)->sdb, 0,
>  2ca6:   48 c7 80 28 02 00 00movq   $0x0,0x228(%rax)
>  2cad:   00 00 00 00
>  2cb1:   48 c7 80 30 02 00 00movq   $0x0,0x230(%rax)
>  2cb8:   00 00 00 00
>  2cbc:   48 c7 80 38 02 00 00movq   $0x0,0x238(%rax)
>  2cc3:   00 00 00 00
>  2cc7:   49 8b 85 60 02 00 00mov0x260(%r13),%rax
>  2cce:   48 8b 90 20 01 00 00mov0x120(%rax),%rdx
>  2cd5:   48 8d 82 28 01 00 00lea0x128(%rdx),%rax
>  2cdc:   48 85 d2test   %rdx,%rdx
>  2cdf:   49 0f 44 c6 cmove  %r14,%rax
>  cmd->device->host->hostt->cmd_size;
>  2ce3:   48 8b 50 38 mov0x38(%rax),%rdx
>  2ce7:   48 8b 12mov(%rdx),%rdx   <==
>  2cea:   48 8b 92 98 00 00 00mov0x98(%rdx),%rdx
>  2cf1:   8b 92 30 01 00 00   mov0x130(%rdx),%edx
>  cmd->sdb.table.sgl = (void *)cmd + sizeof(struct scsi_cmnd) +
>  2cf7:   48 8d 94 10 b0 01 00lea0x1b0(%rax,%rdx,1),%rdx
>  2cfe:   00
>  2cff:   48 89 90 00 01 00 00mov%rdx,0x100(%rax)
>  blk_mq_start_request(req)
> .
> 
> It might not be your patchset, but the location does look suspicious.

Hi Doug,

I think this means that the scsi_in_cmd(cmd)->device pointer is NULL. I will
address this in the next version of this patch series.

Thanks,

Bart.


Re: [PATCH for-5.0] scsi: communicate max segment size to the DMA mapping code

2019-01-23 Thread Christoph Hellwig
On Wed, Jan 23, 2019 at 02:45:49PM +0100, Steffen Maier wrote:
> Zfcp can only have max_segment_size of one page (4kB). Officially announced 
> through dev.dma_parms since v2.6.35 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/s390/scsi?id=683229845f1780b10041ee7a1043fc8f10061455.
>
> With Martin's 5.0/scsi-fixes which includes your above patch, I now get 
> 64kB instead of 4kB for the respective queue limit because __scsi_ini_queue 
> overwrites the dma parameter:

Well, had the driver usd the proper dma_set_max_seg_size API instead
of handcoding it I would have converted it..

> To my surprise, I don't get IO errors with zfcp. Maybe my IO pattern does 
> not cause too large segments to be created. I would have expected the FCP 
> channel complaining rightly so if we pass segments larger than one page. 
> Maybe the additional dma_boundary of pagesize-1 helped the too large 
> max_segment_size to not become effective?

The driver already sets the dma_boundary field, which prevents from
merging I/Os that span multiple pages, and thus limits each segment
to a page or less.

> A quick attempt to adapt zfcp to your patch would be to set 
> scsi_host_template.max_segment_size = ZFCP_QDIO_SBALE_LEN.

I think we can just drop the dma_parms max_segment_size without
replacement due to the dma_boundary.

You do however still need to set up the dma_parms structure itself, as
that is also used to communicate the boundary to the IOMMU.  If would
however be great if you moved that setup into the bus code instead
of the driver, like we do for all other major hardware busses.


[PATCH] libsas: Remove scsi_to_u32()

2019-01-23 Thread Bart Van Assche
Since the function scsi_to_u32() is identical to get_unaligned_be32(),
change all scsi_to_u32() calls into get_unaligned_be32() calls.

Cc: Jian Luo 
Cc: John Garry 
Signed-off-by: Bart Van Assche 
---
 drivers/scsi/libsas/sas_expander.c | 9 +
 include/scsi/scsi.h| 6 --
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c 
b/drivers/scsi/libsas/sas_expander.c
index 83e3715d30e4..b13b245791b0 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "sas_internal.h"
 
@@ -696,10 +697,10 @@ int sas_smp_get_phy_events(struct sas_phy *phy)
if (res)
goto out;
 
-   phy->invalid_dword_count = scsi_to_u32(&resp[12]);
-   phy->running_disparity_error_count = scsi_to_u32(&resp[16]);
-   phy->loss_of_dword_sync_count = scsi_to_u32(&resp[20]);
-   phy->phy_reset_problem_count = scsi_to_u32(&resp[24]);
+   phy->invalid_dword_count = get_unaligned_be32(&resp[12]);
+   phy->running_disparity_error_count = get_unaligned_be32(&resp[16]);
+   phy->loss_of_dword_sync_count = get_unaligned_be32(&resp[20]);
+   phy->phy_reset_problem_count = get_unaligned_be32(&resp[24]);
 
  out:
kfree(req);
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index eb7853c1a23b..5339baadc082 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -274,10 +274,4 @@ static inline int scsi_is_wlun(u64 lun)
 /* Used to obtain the PCI location of a device */
 #define SCSI_IOCTL_GET_PCI 0x5387
 
-/* Pull a u32 out of a SCSI message (using BE SCSI conventions) */
-static inline __u32 scsi_to_u32(__u8 *ptr)
-{
-   return (ptr[0]<<24) + (ptr[1]<<16) + (ptr[2]<<8) + ptr[3];
-}
-
 #endif /* _SCSI_SCSI_H */
-- 
2.20.1.321.g9e740568ce-goog



[PATCH v2 4/7] Introduce scsi_out_cmd()

2019-01-23 Thread Bart Van Assche
This patch does not change any functionality.

Cc: Douglas Gilbert 
Cc: Hannes Reinecke 
Cc: Christoph Hellwig 
Signed-off-by: Bart Van Assche 
---
 include/scsi/scsi_cmnd.h | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 78183e851a0d..213404163993 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -230,9 +230,14 @@ static inline struct scsi_data_buffer *scsi_in(struct 
scsi_cmnd *cmd)
return &scsi_in_cmd(cmd)->sdb;
 }
 
+static inline struct scsi_cmnd *scsi_out_cmd(struct scsi_cmnd *cmd)
+{
+   return cmd;
+}
+
 static inline struct scsi_data_buffer *scsi_out(struct scsi_cmnd *cmd)
 {
-   return &cmd->sdb;
+   return &scsi_out_cmd(cmd)->sdb;
 }
 
 static inline int scsi_sg_copy_from_buffer(struct scsi_cmnd *cmd,
-- 
2.20.1.321.g9e740568ce-goog



[PATCH v2 2/7] Change scsi_cmnd.prot_sdb from a pointer into a regular member

2019-01-23 Thread Bart Van Assche
This patch slightly increases the size of struct scsi_cmnd if data
protection is disabled, decreases the size of the data appended at
the end of struct scsi_cmnd if data protection is enabled and
simplifies the SCSI core.

Cc: Douglas Gilbert 
Cc: Hannes Reinecke 
Cc: Christoph Hellwig 
Signed-off-by: Bart Van Assche 
---
 drivers/scsi/scsi_lib.c  | 33 -
 include/scsi/scsi_cmnd.h |  8 
 2 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 94842b104bcc..6bfbe50ef38e 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -566,7 +566,7 @@ static void scsi_mq_free_sgtables(struct scsi_cmnd *cmd)
sg_free_table_chained(&sdb->table, true);
}
if (scsi_prot_sg_count(cmd))
-   sg_free_table_chained(&cmd->prot_sdb->table, true);
+   sg_free_table_chained(&cmd->prot_sdb.table, true);
 }
 
 static void scsi_mq_uninit_cmd(struct scsi_cmnd *cmd)
@@ -1065,10 +1065,10 @@ blk_status_t scsi_init_io(struct scsi_cmnd *cmd)
}
 
if (blk_integrity_rq(rq)) {
-   struct scsi_data_buffer *prot_sdb = cmd->prot_sdb;
+   struct scsi_data_buffer *prot_sdb = &cmd->prot_sdb;
int ivecs, count;
 
-   if (WARN_ON_ONCE(!prot_sdb)) {
+   if (WARN_ON_ONCE(!scsi_host_get_prot(cmd->device->host))) {
/*
 * This can happen if someone (e.g. multipath)
 * queues a command to a device on an adapter
@@ -1091,8 +1091,7 @@ blk_status_t scsi_init_io(struct scsi_cmnd *cmd)
BUG_ON(count > ivecs);
BUG_ON(count > queue_max_integrity_segments(rq->q));
 
-   cmd->prot_sdb = prot_sdb;
-   cmd->prot_sdb->table.nents = count;
+   prot_sdb->table.nents = count;
}
 
return BLK_STS_OK;
@@ -1156,7 +1155,6 @@ void scsi_del_cmd_from_list(struct scsi_cmnd *cmd)
 void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd)
 {
void *buf = cmd->sense_buffer;
-   void *prot = cmd->prot_sdb;
struct request *rq = blk_mq_rq_from_pdu(cmd);
unsigned int flags = cmd->flags & SCMD_PRESERVED_FLAGS;
unsigned long jiffies_at_alloc;
@@ -1175,7 +1173,6 @@ void scsi_init_command(struct scsi_device *dev, struct 
scsi_cmnd *cmd)
 
cmd->device = dev;
cmd->sense_buffer = buf;
-   cmd->prot_sdb = prot;
cmd->flags = flags;
INIT_DELAYED_WORK(&cmd->abort_work, scmd_eh_abort_handler);
cmd->jiffies_at_alloc = jiffies_at_alloc;
@@ -1617,12 +1614,13 @@ static blk_status_t scsi_mq_prep_fn(struct request *req)
sg = (void *)cmd + sizeof(struct scsi_cmnd) + shost->hostt->cmd_size;
cmd->sdb.table.sgl = sg;
 
-   if (scsi_host_get_prot(shost)) {
-   memset(cmd->prot_sdb, 0, sizeof(struct scsi_data_buffer));
-
-   cmd->prot_sdb->table.sgl =
-   (struct scatterlist *)(cmd->prot_sdb + 1);
-   }
+   /*
+* Always initialize cmd->prot_sdb.nents such that
+* scsi_prot_sg_count() does not have to call scsi_host_get_prot().
+*/
+   memset(&cmd->prot_sdb, 0, sizeof(struct scsi_data_buffer));
+   if (scsi_host_get_prot(shost))
+   cmd->prot_sdb.table.sgl = (void *)&sg + scsi_mq_sgl_size(shost);
 
if (blk_bidi_rq(req)) {
struct request *next_rq = req->next_rq;
@@ -1781,7 +1779,6 @@ static int scsi_mq_init_request(struct blk_mq_tag_set 
*set, struct request *rq,
struct Scsi_Host *shost = set->driver_data;
const bool unchecked_isa_dma = shost->unchecked_isa_dma;
struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
-   struct scatterlist *sg;
 
if (unchecked_isa_dma)
cmd->flags |= SCMD_UNCHECKED_ISA_DMA;
@@ -1791,12 +1788,6 @@ static int scsi_mq_init_request(struct blk_mq_tag_set 
*set, struct request *rq,
return -ENOMEM;
cmd->req.sense = cmd->sense_buffer;
 
-   if (scsi_host_get_prot(shost)) {
-   sg = (void *)cmd + sizeof(struct scsi_cmnd) +
-   shost->hostt->cmd_size;
-   cmd->prot_sdb = (void *)sg + scsi_mq_sgl_size(shost);
-   }
-
return 0;
 }
 
@@ -1893,7 +1884,7 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost)
sgl_size = scsi_mq_sgl_size(shost);
cmd_size = sizeof(struct scsi_cmnd) + shost->hostt->cmd_size + sgl_size;
if (scsi_host_get_prot(shost))
-   cmd_size += sizeof(struct scsi_data_buffer) + sgl_size;
+   cmd_size += sgl_size;
 
memset(&shost->tag_set, 0, sizeof(shost->tag_set));
shost->tag_set.ops = &scsi_mq_ops;
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index d85e6befa26b..0406c0fbee3e 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd

[PATCH v2 3/7] Fix bidi handling

2019-01-23 Thread Bart Van Assche
Some code in the SCSI core interprets blk_mq_rq_to_pdu(cmd->request->next_rq)
as a struct scsi_data_buffer, e.g. scsi_mq_prep_fn(). Other code in the SCSI
core interprets the same data structure as a struct scsi_request, e.g.
scsi_io_completion(). Avoid this confusion by using the SCSI data buffer
associated with "next_rq" for bidi requests. This patch avoids that
submitting a bidi command triggers a NULL pointer dereference.

Reported-by: Douglas Gilbert 
Cc: Douglas Gilbert 
Cc: Hannes Reinecke 
Cc: Christoph Hellwig 
Fixes: d285203cf647 ("scsi: add support for a blk-mq based I/O path.") # v3.17
Signed-off-by: Bart Van Assche 
---
 drivers/scsi/scsi_lib.c  | 35 +++
 include/scsi/scsi_cmnd.h | 13 +
 2 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 6bfbe50ef38e..bcbf266e4172 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -556,15 +556,10 @@ static void scsi_uninit_cmd(struct scsi_cmnd *cmd)
 
 static void scsi_mq_free_sgtables(struct scsi_cmnd *cmd)
 {
-   struct scsi_data_buffer *sdb;
-
if (cmd->sdb.table.nents)
sg_free_table_chained(&cmd->sdb.table, true);
-   if (cmd->request->next_rq) {
-   sdb = cmd->request->next_rq->special;
-   if (sdb)
-   sg_free_table_chained(&sdb->table, true);
-   }
+   if (scsi_bidi_cmnd(cmd))
+   sg_free_table_chained(&scsi_in(cmd)->table, true);
if (scsi_prot_sg_count(cmd))
sg_free_table_chained(&cmd->prot_sdb.table, true);
 }
@@ -1059,7 +1054,7 @@ blk_status_t scsi_init_io(struct scsi_cmnd *cmd)
return ret;
 
if (blk_bidi_rq(rq)) {
-   ret = scsi_init_sgtable(rq->next_rq, rq->next_rq->special);
+   ret = scsi_init_sgtable(rq->next_rq, scsi_in(cmd));
if (ret)
goto out_free_sgtables;
}
@@ -1595,12 +1590,17 @@ static unsigned int scsi_mq_sgl_size(struct Scsi_Host 
*shost)
sizeof(struct scatterlist);
 }
 
+static void scsi_init_sdb(struct Scsi_Host *shost, struct scsi_cmnd *cmd)
+{
+   cmd->sdb.table.sgl = (void *)cmd + sizeof(struct scsi_cmnd) +
+   shost->hostt->cmd_size;
+}
+
 static blk_status_t scsi_mq_prep_fn(struct request *req)
 {
struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(req);
struct scsi_device *sdev = req->q->queuedata;
struct Scsi_Host *shost = sdev->host;
-   struct scatterlist *sg;
 
scsi_init_command(sdev, cmd);
 
@@ -1611,8 +1611,7 @@ static blk_status_t scsi_mq_prep_fn(struct request *req)
cmd->tag = req->tag;
cmd->prot_op = SCSI_PROT_NORMAL;
 
-   sg = (void *)cmd + sizeof(struct scsi_cmnd) + shost->hostt->cmd_size;
-   cmd->sdb.table.sgl = sg;
+   scsi_init_sdb(shost, cmd);
 
/*
 * Always initialize cmd->prot_sdb.nents such that
@@ -1620,17 +1619,13 @@ static blk_status_t scsi_mq_prep_fn(struct request *req)
 */
memset(&cmd->prot_sdb, 0, sizeof(struct scsi_data_buffer));
if (scsi_host_get_prot(shost))
-   cmd->prot_sdb.table.sgl = (void *)&sg + scsi_mq_sgl_size(shost);
+   cmd->prot_sdb.table.sgl = (void *)&cmd->sdb +
+   scsi_mq_sgl_size(shost);
 
if (blk_bidi_rq(req)) {
-   struct request *next_rq = req->next_rq;
-   struct scsi_data_buffer *bidi_sdb = blk_mq_rq_to_pdu(next_rq);
-
-   memset(bidi_sdb, 0, sizeof(struct scsi_data_buffer));
-   bidi_sdb->table.sgl =
-   (struct scatterlist *)(bidi_sdb + 1);
-
-   next_rq->special = bidi_sdb;
+   memset(&scsi_in_cmd(cmd)->sdb, 0,
+  sizeof(scsi_in_cmd(cmd)->sdb));
+   scsi_init_sdb(shost, scsi_in_cmd(cmd));
}
 
blk_mq_start_request(req);
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 0406c0fbee3e..78183e851a0d 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -215,14 +215,19 @@ static inline int scsi_get_resid(struct scsi_cmnd *cmd)
 
 static inline int scsi_bidi_cmnd(struct scsi_cmnd *cmd)
 {
-   return blk_bidi_rq(cmd->request) &&
-   (cmd->request->next_rq->special != NULL);
+   return blk_bidi_rq(cmd->request);
+}
+
+static inline struct scsi_cmnd *scsi_in_cmd(struct scsi_cmnd *cmd)
+{
+   if (likely(!scsi_bidi_cmnd(cmd)))
+   return cmd;
+   return blk_mq_rq_to_pdu(cmd->request->next_rq);
 }
 
 static inline struct scsi_data_buffer *scsi_in(struct scsi_cmnd *cmd)
 {
-   return scsi_bidi_cmnd(cmd) ?
-   cmd->request->next_rq->special : &cmd->sdb;
+   return &scsi_in_cmd(cmd)->sdb;
 }
 
 static inline struct scsi_data_buffer *scsi_out(struct scsi_cmnd *cmd)
-- 
2.20.1.321.g9e740568ce-goog



[PATCH v2 0/7] Fix handling of bidi commands

2019-01-23 Thread Bart Van Assche
Hi Martin,

Recently Doug Gilbert reported that handling of bidi commands is broken
in the scsi-mq code. This patch series fixes that bug and also simplifies
bidi command handling. Please consider these patches for kernel v5.1.

Thanks,

Bart.

Changes compared to v1:
- Fixed a NULL pointer dereference in scsi_init_sdb().

Bart Van Assche (7):
  Introduce the bidi_supported flag in the host template structure
  Change scsi_cmnd.prot_sdb from a pointer into a regular member
  Fix bidi handling
  Introduce scsi_out_cmd()
  Move several function definitions in 
  Introduce scsi_in_[sg]et_resid() and scsi_out_[sg]et_resid()
  Move the resid member from struct scsi_data_buffer into struct
scsi_cmnd

 drivers/scsi/iscsi_tcp.c   |  8 +---
 drivers/scsi/libiscsi.c| 12 ++---
 drivers/scsi/scsi_debug.c  | 20 +++--
 drivers/scsi/scsi_lib.c| 70 --
 drivers/scsi/sd.c  |  4 +-
 drivers/scsi/virtio_scsi.c |  4 +-
 drivers/target/loopback/tcm_loop.c |  8 +---
 drivers/usb/storage/uas.c  |  8 ++--
 include/scsi/scsi_cmnd.h   | 67 
 include/scsi/scsi_host.h   |  2 +
 10 files changed, 101 insertions(+), 102 deletions(-)

-- 
2.20.1.321.g9e740568ce-goog



[PATCH v2 1/7] Introduce the bidi_supported flag in the host template structure

2019-01-23 Thread Bart Van Assche
This patch does not change any functionality but makes the drivers
that support bidirectional commands more compact.

Cc: Douglas Gilbert 
Cc: Hannes Reinecke 
Cc: Christoph Hellwig 
Cc: Lee Duncan 
Cc: Chris Leech 
Signed-off-by: Bart Van Assche 
---
 drivers/scsi/iscsi_tcp.c   |  8 +---
 drivers/scsi/scsi_debug.c  | 11 +--
 drivers/scsi/scsi_lib.c|  2 ++
 drivers/target/loopback/tcm_loop.c |  8 +---
 include/scsi/scsi_host.h   |  2 ++
 5 files changed, 7 insertions(+), 24 deletions(-)

diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index cae6368ebb98..8c09e9e45a62 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -952,12 +952,6 @@ static umode_t iscsi_sw_tcp_attr_is_visible(int 
param_type, int param)
return 0;
 }
 
-static int iscsi_sw_tcp_slave_alloc(struct scsi_device *sdev)
-{
-   blk_queue_flag_set(QUEUE_FLAG_BIDI, sdev->request_queue);
-   return 0;
-}
-
 static int iscsi_sw_tcp_slave_configure(struct scsi_device *sdev)
 {
struct iscsi_sw_tcp_host *tcp_sw_host = iscsi_host_priv(sdev->host);
@@ -985,7 +979,7 @@ static struct scsi_host_template iscsi_sw_tcp_sht = {
.eh_device_reset_handler= iscsi_eh_device_reset,
.eh_target_reset_handler = iscsi_eh_recover_target,
.dma_boundary   = PAGE_SIZE - 1,
-   .slave_alloc= iscsi_sw_tcp_slave_alloc,
+   .bidi_supported = true,
.slave_configure= iscsi_sw_tcp_slave_configure,
.target_alloc   = iscsi_target_alloc,
.proc_name  = "iscsi_tcp",
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 661512bec3ac..e253c0129b40 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -3948,15 +3948,6 @@ static struct sdebug_dev_info 
*find_build_dev_info(struct scsi_device *sdev)
return open_devip;
 }
 
-static int scsi_debug_slave_alloc(struct scsi_device *sdp)
-{
-   if (sdebug_verbose)
-   pr_info("slave_alloc <%u %u %u %llu>\n",
-  sdp->host->host_no, sdp->channel, sdp->id, sdp->lun);
-   blk_queue_flag_set(QUEUE_FLAG_BIDI, sdp->request_queue);
-   return 0;
-}
-
 static int scsi_debug_slave_configure(struct scsi_device *sdp)
 {
struct sdebug_dev_info *devip =
@@ -5834,7 +5825,7 @@ static struct scsi_host_template sdebug_driver_template = 
{
.proc_name =sdebug_proc_name,
.name = "SCSI DEBUG",
.info = scsi_debug_info,
-   .slave_alloc =  scsi_debug_slave_alloc,
+   .bidi_supported =   true,
.slave_configure =  scsi_debug_slave_configure,
.slave_destroy =scsi_debug_slave_destroy,
.ioctl =scsi_debug_ioctl,
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 00cd365fb7d2..94842b104bcc 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1881,6 +1881,8 @@ struct request_queue *scsi_mq_alloc_queue(struct 
scsi_device *sdev)
sdev->request_queue->queuedata = sdev;
__scsi_init_queue(sdev->host, sdev->request_queue);
blk_queue_flag_set(QUEUE_FLAG_SCSI_PASSTHROUGH, sdev->request_queue);
+   if (sdev->host->hostt->bidi_supported)
+   blk_queue_flag_set(QUEUE_FLAG_BIDI, sdev->request_queue);
return sdev->request_queue;
 }
 
diff --git a/drivers/target/loopback/tcm_loop.c 
b/drivers/target/loopback/tcm_loop.c
index 7bd7c0c0db6f..e54a5c57a8bb 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -304,12 +304,6 @@ static int tcm_loop_target_reset(struct scsi_cmnd *sc)
return FAILED;
 }
 
-static int tcm_loop_slave_alloc(struct scsi_device *sd)
-{
-   blk_queue_flag_set(QUEUE_FLAG_BIDI, sd->request_queue);
-   return 0;
-}
-
 static struct scsi_host_template tcm_loop_driver_template = {
.show_info  = tcm_loop_show_info,
.proc_name  = "tcm_loopback",
@@ -325,7 +319,7 @@ static struct scsi_host_template tcm_loop_driver_template = 
{
.cmd_per_lun= 1024,
.max_sectors= 0x,
.dma_boundary   = PAGE_SIZE - 1,
-   .slave_alloc= tcm_loop_slave_alloc,
+   .bidi_supported = true,
.module = THIS_MODULE,
.track_queue_depth  = 1,
 };
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 6ca954e9f752..384b50992a56 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -430,6 +430,8 @@ struct scsi_host_template {
/* True if the low-level driver supports blk-mq only */
unsigned force_blk_mq:1;
 
+   unsigned bidi_supported:1;
+
/*
 * Countdown for host blocking with no commands outstanding.
 */
-- 
2.20.1.321.g9e740568ce-goog



[PATCH v2 7/7] Move the resid member from struct scsi_data_buffer into struct scsi_cmnd

2019-01-23 Thread Bart Van Assche
This patch does not change any functionality but reduces the size of
struct scsi_cmnd.

Cc: Oliver Neukum 
Cc: Douglas Gilbert 
Cc: Hannes Reinecke 
Cc: Christoph Hellwig 
Cc: linux-...@vger.kernel.org
Signed-off-by: Bart Van Assche 
---
 drivers/scsi/scsi_lib.c   | 2 --
 drivers/usb/storage/uas.c | 8 +---
 include/scsi/scsi_cmnd.h  | 9 -
 3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index bcbf266e4172..4feba3b5aff1 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -945,14 +945,12 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned 
int good_bytes)
 * scsi_result_to_blk_status may have reset the host_byte
 */
scsi_req(req)->result = cmd->result;
-   scsi_req(req)->resid_len = scsi_get_resid(cmd);
 
if (unlikely(scsi_bidi_cmnd(cmd))) {
/*
 * Bidi commands Must be complete as a whole,
 * both sides at once.
 */
-   scsi_req(req->next_rq)->resid_len = scsi_in(cmd)->resid;
if (scsi_end_request(req, BLK_STS_OK, blk_rq_bytes(req),
blk_rq_bytes(req->next_rq)))
WARN_ONCE(true,
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 36742e8e7edc..ea40fd78e6ff 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -365,7 +365,7 @@ static void uas_stat_cmplt(struct urb *urb)
 
 static void uas_data_cmplt(struct urb *urb)
 {
-   struct scsi_cmnd *cmnd = urb->context;
+   struct scsi_cmnd *cmnd = urb->context, *cmpl_cmd = NULL;
struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;
struct uas_dev_info *devinfo = (void *)cmnd->device->hostdata;
struct scsi_data_buffer *sdb = NULL;
@@ -375,10 +375,12 @@ static void uas_data_cmplt(struct urb *urb)
spin_lock_irqsave(&devinfo->lock, flags);
 
if (cmdinfo->data_in_urb == urb) {
+   cmpl_cmd = scsi_in_cmd(cmnd);
sdb = scsi_in(cmnd);
cmdinfo->state &= ~DATA_IN_URB_INFLIGHT;
cmdinfo->data_in_urb = NULL;
} else if (cmdinfo->data_out_urb == urb) {
+   cmpl_cmd = scsi_out_cmd(cmnd);
sdb = scsi_out(cmnd);
cmdinfo->state &= ~DATA_OUT_URB_INFLIGHT;
cmdinfo->data_out_urb = NULL;
@@ -401,9 +403,9 @@ static void uas_data_cmplt(struct urb *urb)
if (status != -ENOENT && status != -ECONNRESET && status != 
-ESHUTDOWN)
uas_log_cmd_state(cmnd, "data cmplt err", status);
/* error: no data transfered */
-   sdb->resid = sdb->length;
+   cmpl_cmd->req.resid_len = sdb->length;
} else {
-   sdb->resid = sdb->length - urb->actual_length;
+   cmpl_cmd->req.resid_len = sdb->length - urb->actual_length;
}
uas_try_complete(cmnd, __func__);
 out:
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 8f3ed55a5ee5..9035c760cae0 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -35,7 +35,6 @@ struct scsi_driver;
 struct scsi_data_buffer {
struct sg_table table;
unsigned length;
-   int resid;
 };
 
 /* embedded in scsi_cmnd */
@@ -229,22 +228,22 @@ static inline unsigned scsi_bufflen(struct scsi_cmnd *cmd)
 
 static inline void scsi_in_set_resid(struct scsi_cmnd *cmd, int resid)
 {
-   scsi_in_cmd(cmd)->sdb.resid = resid;
+   scsi_in_cmd(cmd)->req.resid_len = resid;
 }
 
 static inline int scsi_in_get_resid(struct scsi_cmnd *cmd)
 {
-   return scsi_in_cmd(cmd)->sdb.resid;
+   return scsi_in_cmd(cmd)->req.resid_len;
 }
 
 static inline void scsi_out_set_resid(struct scsi_cmnd *cmd, int resid)
 {
-   cmd->sdb.resid = resid;
+   cmd->req.resid_len = resid;
 }
 
 static inline int scsi_out_get_resid(struct scsi_cmnd *cmd)
 {
-   return cmd->sdb.resid;
+   return cmd->req.resid_len;
 }
 
 static inline void scsi_set_resid(struct scsi_cmnd *cmd, int resid)
-- 
2.20.1.321.g9e740568ce-goog



[PATCH v2 6/7] Introduce scsi_in_[sg]et_resid() and scsi_out_[sg]et_resid()

2019-01-23 Thread Bart Van Assche
This patch does not change any functionality.

Cc: Douglas Gilbert 
Cc: Hannes Reinecke 
Cc: Christoph Hellwig 
Cc: Lee Duncan 
Cc: Chris Leech 
Cc: Paolo Bonzini 
Signed-off-by: Bart Van Assche 
---
 drivers/scsi/libiscsi.c| 12 ++--
 drivers/scsi/scsi_debug.c  |  9 +
 drivers/scsi/sd.c  |  4 ++--
 drivers/scsi/virtio_scsi.c |  4 ++--
 include/scsi/scsi_cmnd.h   | 24 ++--
 5 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index b8d325ce8754..fe5b3371553c 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -650,8 +650,8 @@ static void fail_scsi_task(struct iscsi_task *task, int err)
if (!scsi_bidi_cmnd(sc))
scsi_set_resid(sc, scsi_bufflen(sc));
else {
-   scsi_out(sc)->resid = scsi_out(sc)->length;
-   scsi_in(sc)->resid = scsi_in(sc)->length;
+   scsi_out_set_resid(sc, scsi_out(sc)->length);
+   scsi_in_set_resid(sc, scsi_in(sc)->length);
}
 
/* regular RX path uses back_lock */
@@ -912,7 +912,7 @@ static void iscsi_scsi_cmd_rsp(struct iscsi_conn *conn, 
struct iscsi_hdr *hdr,
if (scsi_bidi_cmnd(sc) && res_count > 0 &&
(rhdr->flags & ISCSI_FLAG_CMD_BIDI_OVERFLOW ||
 res_count <= scsi_in(sc)->length))
-   scsi_in(sc)->resid = res_count;
+   scsi_in_set_resid(sc, res_count);
else
sc->result = (DID_BAD_TARGET << 16) | rhdr->cmd_status;
}
@@ -962,7 +962,7 @@ iscsi_data_in_rsp(struct iscsi_conn *conn, struct iscsi_hdr 
*hdr,
if (res_count > 0 &&
(rhdr->flags & ISCSI_FLAG_CMD_OVERFLOW ||
 res_count <= scsi_in(sc)->length))
-   scsi_in(sc)->resid = res_count;
+   scsi_in_set_resid(sc, res_count);
else
sc->result = (DID_BAD_TARGET << 16) | rhdr->cmd_status;
}
@@ -1807,8 +1807,8 @@ int iscsi_queuecommand(struct Scsi_Host *host, struct 
scsi_cmnd *sc)
if (!scsi_bidi_cmnd(sc))
scsi_set_resid(sc, scsi_bufflen(sc));
else {
-   scsi_out(sc)->resid = scsi_out(sc)->length;
-   scsi_in(sc)->resid = scsi_in(sc)->length;
+   scsi_out_set_resid(sc, scsi_out(sc)->length);
+   scsi_in_set_resid(sc, scsi_in(sc)->length);
}
sc->scsi_done(sc);
return 0;
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index e253c0129b40..822aed12ca31 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1019,7 +1019,7 @@ static int fill_from_dev_buffer(struct scsi_cmnd *scp, 
unsigned char *arr,
 
act_len = sg_copy_from_buffer(sdb->table.sgl, sdb->table.nents,
  arr, arr_len);
-   sdb->resid = scsi_bufflen(scp) - act_len;
+   scsi_in_set_resid(scp, scsi_bufflen(scp) - act_len);
 
return 0;
 }
@@ -1044,9 +1044,10 @@ static int p_fill_from_dev_buffer(struct scsi_cmnd *scp, 
const void *arr,
act_len = sg_pcopy_from_buffer(sdb->table.sgl, sdb->table.nents,
   arr, arr_len, skip);
pr_debug("%s: off_dst=%u, scsi_bufflen=%u, act_len=%u, resid=%d\n",
-__func__, off_dst, scsi_bufflen(scp), act_len, sdb->resid);
+__func__, off_dst, scsi_bufflen(scp), act_len,
+scsi_in_get_resid(scp));
n = (int)scsi_bufflen(scp) - ((int)off_dst + act_len);
-   sdb->resid = min(sdb->resid, n);
+   scsi_in_set_resid(scp, min(scsi_in_get_resid(scp), n));
return 0;
 }
 
@@ -2774,7 +2775,7 @@ static int resp_read_dt0(struct scsi_cmnd *scp, struct 
sdebug_dev_info *devip)
if (unlikely(ret == -1))
return DID_ERROR << 16;
 
-   scsi_in(scp)->resid = scsi_bufflen(scp) - ret;
+   scsi_in_set_resid(scp, scsi_bufflen(scp) - ret);
 
if (unlikely(sqcp)) {
if (sqcp->inj_recovered) {
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index b0eb83526c54..2219ba81f0b3 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -844,7 +844,7 @@ static blk_status_t sd_setup_unmap_cmnd(struct scsi_cmnd 
*cmd)
cmd->allowed = SD_MAX_RETRIES;
cmd->transfersize = data_len;
rq->timeout = SD_TIMEOUT;
-   scsi_req(rq)->resid_len = data_len;
+   scsi_set_resid(cmd, data_len);
 
return scsi_init_io(cmd);
 }
@@ -908,7 +908,7 @@ static blk_status_t sd_setup_write_same10_cmnd(struct 
scsi_cmnd *cmd,
cmd->allowed = SD_MAX_RETRIES;
cmd->transfersize = data_len;
rq->timeout = unmap ? SD_TIMEOUT : SD_WRITE_SAME_TIMEOUT;
-   scsi_req(rq)->resid_len = data_len;
+   scsi_set_resid(cmd, data_len);
 
return scsi_init_io(cmd);
 }
diff -

[PATCH v2 5/7] Move several function definitions in

2019-01-23 Thread Bart Van Assche
Since the next patch will call scsi_out_cmd() from scsi_[gs]et_resid(),
reorder the function definitions in .

Cc: Douglas Gilbert 
Cc: Hannes Reinecke 
Cc: Christoph Hellwig 
Signed-off-by: Bart Van Assche 
---
 include/scsi/scsi_cmnd.h | 50 
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 213404163993..673980ed7db6 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -185,61 +185,61 @@ static inline int scsi_dma_map(struct scsi_cmnd *cmd) { 
return -ENOSYS; }
 static inline void scsi_dma_unmap(struct scsi_cmnd *cmd) { }
 #endif /* !CONFIG_SCSI_DMA */
 
-static inline unsigned scsi_sg_count(struct scsi_cmnd *cmd)
+static inline int scsi_bidi_cmnd(struct scsi_cmnd *cmd)
 {
-   return cmd->sdb.table.nents;
+   return blk_bidi_rq(cmd->request);
 }
 
-static inline struct scatterlist *scsi_sglist(struct scsi_cmnd *cmd)
+static inline struct scsi_cmnd *scsi_in_cmd(struct scsi_cmnd *cmd)
 {
-   return cmd->sdb.table.sgl;
+   if (likely(!scsi_bidi_cmnd(cmd)))
+   return cmd;
+   return blk_mq_rq_to_pdu(cmd->request->next_rq);
 }
 
-static inline unsigned scsi_bufflen(struct scsi_cmnd *cmd)
+static inline struct scsi_data_buffer *scsi_in(struct scsi_cmnd *cmd)
 {
-   return cmd->sdb.length;
+   return &scsi_in_cmd(cmd)->sdb;
 }
 
-static inline void scsi_set_resid(struct scsi_cmnd *cmd, int resid)
+static inline struct scsi_cmnd *scsi_out_cmd(struct scsi_cmnd *cmd)
 {
-   cmd->sdb.resid = resid;
+   return cmd;
 }
 
-static inline int scsi_get_resid(struct scsi_cmnd *cmd)
+static inline struct scsi_data_buffer *scsi_out(struct scsi_cmnd *cmd)
 {
-   return cmd->sdb.resid;
+   return &scsi_out_cmd(cmd)->sdb;
 }
 
-#define scsi_for_each_sg(cmd, sg, nseg, __i)   \
-   for_each_sg(scsi_sglist(cmd), sg, nseg, __i)
-
-static inline int scsi_bidi_cmnd(struct scsi_cmnd *cmd)
+static inline unsigned scsi_sg_count(struct scsi_cmnd *cmd)
 {
-   return blk_bidi_rq(cmd->request);
+   return cmd->sdb.table.nents;
 }
 
-static inline struct scsi_cmnd *scsi_in_cmd(struct scsi_cmnd *cmd)
+static inline struct scatterlist *scsi_sglist(struct scsi_cmnd *cmd)
 {
-   if (likely(!scsi_bidi_cmnd(cmd)))
-   return cmd;
-   return blk_mq_rq_to_pdu(cmd->request->next_rq);
+   return cmd->sdb.table.sgl;
 }
 
-static inline struct scsi_data_buffer *scsi_in(struct scsi_cmnd *cmd)
+static inline unsigned scsi_bufflen(struct scsi_cmnd *cmd)
 {
-   return &scsi_in_cmd(cmd)->sdb;
+   return cmd->sdb.length;
 }
 
-static inline struct scsi_cmnd *scsi_out_cmd(struct scsi_cmnd *cmd)
+static inline void scsi_set_resid(struct scsi_cmnd *cmd, int resid)
 {
-   return cmd;
+   cmd->sdb.resid = resid;
 }
 
-static inline struct scsi_data_buffer *scsi_out(struct scsi_cmnd *cmd)
+static inline int scsi_get_resid(struct scsi_cmnd *cmd)
 {
-   return &scsi_out_cmd(cmd)->sdb;
+   return cmd->sdb.resid;
 }
 
+#define scsi_for_each_sg(cmd, sg, nseg, __i)   \
+   for_each_sg(scsi_sglist(cmd), sg, nseg, __i)
+
 static inline int scsi_sg_copy_from_buffer(struct scsi_cmnd *cmd,
   void *buf, int buflen)
 {
-- 
2.20.1.321.g9e740568ce-goog



[PATCH] sd: Protect against submitting READ(6) or WRITE(6) with 256 logical blocks

2019-01-23 Thread Bart Van Assche
Since the READ(6) and WRITE(6) commands interpret a zero in the transfer
length field in the CDB as 256 logical blocks, avoid submitting such
commands.

Cc: Douglas Gilbert 
Cc: Hannes Reinecke 
Cc: Christoph Hellwig 
Reported-by: Douglas Gilbert 
Signed-off-by: Bart Van Assche 
---
 drivers/scsi/sd.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 4e69f182a1e5..b0eb83526c54 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1129,6 +1129,10 @@ static blk_status_t sd_setup_rw6_cmnd(struct scsi_cmnd 
*cmd, bool write,
  sector_t lba, unsigned int nr_blocks,
  unsigned char flags)
 {
+   /* Avoid that 0 blocks gets translated into 256 blocks. */
+   if (WARN_ON_ONCE(nr_blocks == 0))
+   return BLK_STS_IOERR;
+
if (unlikely(flags & 0x8)) {
/*
 * This happens only if this drive failed 10byte rw
-- 
2.20.1.321.g9e740568ce-goog



Re: [PATCH] sd: Protect against submitting READ(6) or WRITE(6) with 256 logical blocks

2019-01-23 Thread Douglas Gilbert

On 2019-01-23 2:12 p.m., Bart Van Assche wrote:

Since the READ(6) and WRITE(6) commands interpret a zero in the transfer
length field in the CDB as 256 logical blocks, avoid submitting such
commands.

Cc: Douglas Gilbert 
Cc: Hannes Reinecke 
Cc: Christoph Hellwig 
Reported-by: Douglas Gilbert 
Signed-off-by: Bart Van Assche 


Reviewed-by: Douglas Gilbert 


---
  drivers/scsi/sd.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 4e69f182a1e5..b0eb83526c54 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1129,6 +1129,10 @@ static blk_status_t sd_setup_rw6_cmnd(struct scsi_cmnd 
*cmd, bool write,
  sector_t lba, unsigned int nr_blocks,
  unsigned char flags)
  {
+   /* Avoid that 0 blocks gets translated into 256 blocks. */
+   if (WARN_ON_ONCE(nr_blocks == 0))
+   return BLK_STS_IOERR;
+
if (unlikely(flags & 0x8)) {
/*
 * This happens only if this drive failed 10byte rw





[PATCH v2 0/9] phy: qcom-ufs: Enable regulators to be off in suspend

2019-01-23 Thread Evan Green
The goal with this series is to enable shutting off regulators that power
UFS during system suspend.

In "the good life" version of this, we'd just disable the regulators
in phy_poweroff and be done with it. Unfortunately, that's not symmetric,
as regulators are not enabled during phy_poweron. Ok, so you might think
we could just move the regulator enable and anything else that needs to
come along into phy_poweron, so that we can then undo it all in
phy_poweroff. That's where things get tricky.

The qcom-qmp-phy overloaded the phy_init and phy_poweron callbacks,
basically to mean "init phase 1" and "init phase 2". There are two phases
because they have this phy_reset bit outside of the phy (in the UFS
controller registers), and they need to make sure this bit is toggled at
specific points in the phy init sequence. So there's this implicit
sequence in the init dance between ufs-qcom.c and phy-qcom-qmp.c:
1) ufs-qcom asserts the PHY reset bit.
2) phy-qcom-qmp phy_init does most of its initialization, but exits early.
3) ufs-qcom deasserts the PHY reset bit.
4) phy-qcom-qmp phy_poweron finishes its initialization.

This init dance is very difficult to follow in the code (since it's split
between two drivers and not spelled out well), and arguably represents a
deficiency in the hardware description of these devices.

In this series I'm proposing tweaking the bindings for the Qualcomm
UFS controller and PHY. In it we expose a reset controller from the
UFS controller, that is then picked up and used from the PHY code.
With this, the phy code can be reorganized to complete its initialization
in a single function, removing the implicit two-phase overloading.

Then I can move most of the phy initialization, including enabling
the regulators, into phy_poweron. Now, when phy_poweroff is called,
the phy actually powers off. This finally disables the regulators
and allows me to save power in system suspend.

Because the UFS PHY reset bit is now toggled in the PHY, rather
than in ufs-qcom, this also percolated to all other PHYs using
ufs-qcom, which from what I can see is just 8996.

There are a couple of tradeoffs in this series that I'd welcome feedback
on. First, it breaks compatibility with device trees that don't expose
this new reset controller. Making this work with older device trees
would be pretty ugly in the code, and given that the SDM845 UFS DT nodes
aren't accepted upstream yet, the breakage seemed worth it. I'm not as
sure about 8996.

Second, I removed the calls to phy_poweroff during clock gating. This
was originally dialing down a clock or two, while leaving the phy powered.
I've now changed the semantics of phy_poweroff to, well, actually power off.
This works great for userlands that have set UFS's spm_lvl to 5 (off) like
I have, but maybe changes power consumption for devices that have spm_lvl
set to 3. I could try to use phy_init and phy_poweron as the two different
possible transitions (fully off, and clocks off respectively), but I'm not
sure if it actually matters, and I like the idea that phy_poweroff really
does power the thing off.

Also, I don't have an 8996 device to test. If someone is able to test this
out and perhaps point out any (hopefully obvious) bugs in the 8996 portion,
I'd be grateful.

This patch is based atop phy-next, plus the UFS DT nodes, which are now
patch 3, 4, 5 of [1].

[1] https://lore.kernel.org/lkml/20181210192826.241350-1-evgr...@chromium.org/

Changes in v2:
- Added resets to example (Stephen).
- Remove include of reset.h (Stephen)
- Fix error print of phy_power_on (Stephen)
- Comment for reset controller warnings on id != 0 (Stephen)
- Add static to ufs_qcom_reset_ops (Stephen).
- Use devm_* to get the reset (Stephen)
- Clear ufs_reset on error getting it
- Remove needless error print (Stephen)
- Removed whitespace changes (Stephen)
- Use devm_ to get the reset (Stephen)

Evan Green (9):
  dt-bindings: ufs: Add #reset-cells for Qualcomm controllers
  dt-bindings: phy-qcom-qmp: Add UFS PHY reset
  dt-bindings: phy: qcom-ufs: Add resets property
  arm64: dts: sdm845: Add UFS PHY reset
  arm64: dts: msm8996: Add UFS PHY reset controller
  scsi: ufs: qcom: Expose the reset controller for PHY
  phy: qcom-qmp: Utilize UFS reset controller
  phy: qcom-qmp: Move UFS phy to phy_poweron/off
  phy: qcom-ufs: Refactor all init steps into phy_poweron

 .../devicetree/bindings/phy/qcom-qmp-phy.txt  |   6 +-
 .../devicetree/bindings/ufs/ufs-qcom.txt  |   5 +-
 .../devicetree/bindings/ufs/ufshcd-pltfrm.txt |   3 +
 arch/arm64/boot/dts/qcom/msm8996.dtsi |   4 +-
 arch/arm64/boot/dts/qcom/sdm845.dtsi  |   3 +
 drivers/phy/qualcomm/phy-qcom-qmp.c   | 122 ++
 drivers/phy/qualcomm/phy-qcom-ufs-i.h |   5 +-
 drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c  |  25 +---
 drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c  |  25 +---
 drivers/phy/qualcomm/phy-qcom-ufs.c   |  57 ++--
 drivers/scsi/ufs/Kconfig  |   1 +

[PATCH v2 6/9] scsi: ufs: qcom: Expose the reset controller for PHY

2019-01-23 Thread Evan Green
Expose a reset controller that the phy can use to perform its
initialization in a single callback.

Also, change the use of the phy functions from ufs-qcom such that
phy_poweron actually fires up the phy, and phy_poweroff actually
powers it down.

Signed-off-by: Evan Green 

---
Note: This change depends on the remaining changes in this series,
since UFS PHY reset now needs to be done by the PHY driver.

Changes in v2:
- Remove include of reset.h (Stephen)
- Fix error print of phy_power_on (Stephen)
- Comment for reset controller warnings on id != 0 (Stephen)
- Add static to ufs_qcom_reset_ops (Stephen).

 drivers/scsi/ufs/Kconfig|   1 +
 drivers/scsi/ufs/ufs-qcom.c | 111 ++--
 drivers/scsi/ufs/ufs-qcom.h |   4 ++
 3 files changed, 72 insertions(+), 44 deletions(-)

diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
index 2ddbb26d9c265..63c5c4115981f 100644
--- a/drivers/scsi/ufs/Kconfig
+++ b/drivers/scsi/ufs/Kconfig
@@ -100,6 +100,7 @@ config SCSI_UFS_QCOM
tristate "QCOM specific hooks to UFS controller platform driver"
depends on SCSI_UFSHCD_PLATFORM && ARCH_QCOM
select PHY_QCOM_UFS
+   select RESET_CONTROLLER
help
  This selects the QCOM specific additions to UFSHCD platform driver.
  UFS host on QCOM needs some vendor specific configuration before
diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 3aeadb14aae1e..277ed6ad71c9b 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -49,6 +49,11 @@ static void ufs_qcom_get_default_testbus_cfg(struct 
ufs_qcom_host *host);
 static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba *hba,
   u32 clk_cycles);
 
+static struct ufs_qcom_host *rcdev_to_ufs_host(struct reset_controller_dev 
*rcd)
+{
+   return container_of(rcd, struct ufs_qcom_host, rcdev);
+}
+
 static void ufs_qcom_dump_regs_wrapper(struct ufs_hba *hba, int offset, int 
len,
   const char *prefix, void *priv)
 {
@@ -255,11 +260,6 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
if (is_rate_B)
phy_set_mode(phy, PHY_MODE_UFS_HS_B);
 
-   /* Assert PHY reset and apply PHY calibration values */
-   ufs_qcom_assert_reset(hba);
-   /* provide 1ms delay to let the reset pulse propagate */
-   usleep_range(1000, 1100);
-
/* phy initialization - calibrate the phy */
ret = phy_init(phy);
if (ret) {
@@ -268,15 +268,6 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
goto out;
}
 
-   /* De-assert PHY reset and start serdes */
-   ufs_qcom_deassert_reset(hba);
-
-   /*
-* after reset deassertion, phy will need all ref clocks,
-* voltage, current to settle down before starting serdes.
-*/
-   usleep_range(1000, 1100);
-
/* power on phy - start serdes and phy's power and clocks */
ret = phy_power_on(phy);
if (ret) {
@@ -290,7 +281,6 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
return 0;
 
 out_disable_phy:
-   ufs_qcom_assert_reset(hba);
phy_exit(phy);
 out:
return ret;
@@ -554,21 +544,10 @@ static int ufs_qcom_suspend(struct ufs_hba *hba, enum 
ufs_pm_op pm_op)
ufs_qcom_disable_lane_clks(host);
phy_power_off(phy);
 
-   /* Assert PHY soft reset */
-   ufs_qcom_assert_reset(hba);
-   goto out;
-   }
-
-   /*
-* If UniPro link is not active, PHY ref_clk, main PHY analog power
-* rail and low noise analog power rail for PLL can be switched off.
-*/
-   if (!ufs_qcom_is_link_active(hba)) {
+   } else if (!ufs_qcom_is_link_active(hba)) {
ufs_qcom_disable_lane_clks(host);
-   phy_power_off(phy);
}
 
-out:
return ret;
 }
 
@@ -578,21 +557,26 @@ static int ufs_qcom_resume(struct ufs_hba *hba, enum 
ufs_pm_op pm_op)
struct phy *phy = host->generic_phy;
int err;
 
-   err = phy_power_on(phy);
-   if (err) {
-   dev_err(hba->dev, "%s: failed enabling regs, err = %d\n",
-   __func__, err);
-   goto out;
-   }
+   if (ufs_qcom_is_link_off(hba)) {
+   err = phy_power_on(phy);
+   if (err) {
+   dev_err(hba->dev, "%s: failed PHY power on: %d\n",
+   __func__, err);
+   return err;
+   }
 
-   err = ufs_qcom_enable_lane_clks(host);
-   if (err)
-   goto out;
+   err = ufs_qcom_enable_lane_clks(host);
+   if (err)
+   return err;
 
-   hba->is_sys_suspended = false;
+   } else if (!ufs_qcom_is_link_active(hba)) {
+   err = ufs_qcom_enable_lane_clk

[PATCH] Fix sense handling in __scsi_execute()

2019-01-23 Thread Bart Van Assche
Since blk_execute_rq() no longer allocates a sense buffer and no longer
initializes the sense pointer the callers of blk_execute_rq() have to do
initialize the sense pointer. Hence this patch that initializes rq->sense
and that removes a superfluous memcpy() statement.

Cc: Christoph Hellwig 
Cc: Douglas Gilbert 
Cc:  # v4.11+
Fixes: 82ed4db499b8 ("block: split scsi_request out of struct request")
Signed-off-by: Bart Van Assche 
---
 drivers/scsi/scsi_lib.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 4feba3b5aff1..8b9f4b1bca35 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -271,6 +271,7 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned 
char *cmd,
rq->cmd_len = COMMAND_SIZE(cmd[0]);
memcpy(rq->cmd, cmd, rq->cmd_len);
rq->retries = retries;
+   rq->sense = sense;
req->timeout = timeout;
req->cmd_flags |= flags;
req->rq_flags |= rq_flags | RQF_QUIET;
@@ -291,8 +292,6 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned 
char *cmd,
 
if (resid)
*resid = rq->resid_len;
-   if (sense && rq->sense_len)
-   memcpy(sense, rq->sense, SCSI_SENSE_BUFFERSIZE);
if (sshdr)
scsi_normalize_sense(rq->sense, rq->sense_len, sshdr);
ret = rq->result;
-- 
2.20.1.321.g9e740568ce-goog



Re: Kernel v5.0, scsi_debug and libiscsi

2019-01-23 Thread Bart Van Assche
On Fri, 2019-01-11 at 13:01 -0500, Douglas Gilbert wrote:
> On 2019-01-10 6:22 p.m., Bart Van Assche wrote:
> > Hi Doug,
> > 
> > Have you ever tried to run the libiscsi conformance tests against
> > the scsi_debug driver? I tried the following:
> > 
> > modprobe scsi_debug delay=0 max_luns=3
> > dev=$(for f in 
> > /sys/bus/pseudo/drivers/scsi_debug/adapter*/host*/target*/[0-9]*/block/*; 
> > do echo $f; break; done)
> > dev=/dev/$(basename $dev)
> > libiscsi/test-tool/iscsi-test-cu --dataloss --allow-sanitize "$dev"
> > 
> > That test triggers the following output:
> > 
> > BUG: unable to handle kernel paging request at a8d741235e00
> > PGD 13b141067 P4D 13b141067 PUD 13b146067 PMD 6fc5a067 PTE 0
> > Oops: 0002 [#1] SMP PTI
> > CPU: 3 PID: 4967 Comm: iscsi-test-cu Not tainted 4.18.0-13-generic 
> > #14-Ubuntu
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 
> > 04/01/2014
> > RIP: 0010:memcpy_erms+0x6/0x10
> 
> Since memory corruption errors have been found elsewhere in
> lk 5.0-rc1 and a fix looks like it is pending, I will leave this
> one alone as I can't replicate it.

Hi Doug,

I can replicate this crash easily. I also noticed that this crash only occurs if
the scsi_debug driver is loaded with fake_rw=0. It does not occur with 
fake_rw=1.
It seems like the following code in resp_write_same() assumes that fake_storep 
!= NULL?

/* if ndob then zero 1 logical block, else fetch 1 logical block */
if (ndob) {
memset(fake_storep + lba_off, 0, sdebug_sector_size);
ret = 0;
} else
ret = fetch_to_dev_buffer(scp, fake_storep + lba_off,
  sdebug_sector_size);

Bart.


Re: [PATCH] Fix sense handling in __scsi_execute()

2019-01-23 Thread Bart Van Assche
On Wed, 2019-01-23 at 14:42 -0800, Bart Van Assche wrote:
> Since blk_execute_rq() no longer allocates a sense buffer and no longer
> initializes the sense pointer the callers of blk_execute_rq() have to do
> initialize the sense pointer. Hence this patch that initializes rq->sense
> and that removes a superfluous memcpy() statement.
> 
> Cc: Christoph Hellwig 
> Cc: Douglas Gilbert 
> Cc:  # v4.11+
> Fixes: 82ed4db499b8 ("block: split scsi_request out of struct request")
> Signed-off-by: Bart Van Assche 
> ---
>  drivers/scsi/scsi_lib.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 4feba3b5aff1..8b9f4b1bca35 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -271,6 +271,7 @@ int __scsi_execute(struct scsi_device *sdev, const 
> unsigned char *cmd,
>   rq->cmd_len = COMMAND_SIZE(cmd[0]);
>   memcpy(rq->cmd, cmd, rq->cmd_len);
>   rq->retries = retries;
> + rq->sense = sense;
>   req->timeout = timeout;
>   req->cmd_flags |= flags;
>   req->rq_flags |= rq_flags | RQF_QUIET;
> @@ -291,8 +292,6 @@ int __scsi_execute(struct scsi_device *sdev, const 
> unsigned char *cmd,
>  
>   if (resid)
>   *resid = rq->resid_len;
> - if (sense && rq->sense_len)
> - memcpy(sense, rq->sense, SCSI_SENSE_BUFFERSIZE);
>   if (sshdr)
>   scsi_normalize_sense(rq->sense, rq->sense_len, sshdr);
>   ret = rq->result;

Please ignore this patch - I just realized that this is not the right way to
fix the issue I ran into.

Bart.


[LSF/MM TOPIC] SCSI Error Handling and HBA Recovery

2019-01-23 Thread Bart Van Assche
Several SCSI low-level drivers need to suspend .queuecommand() calls while
HBA or transport layer recovery happens. The iSCSI and SRP initiator drivers
use scsi_target_block() to block new .queuecommand() calls while recovery
happens. scsi_target_block() prevents that the block layer core triggers new
.queuecommand() calls but does not prevent that the SCSI error handler calls
.queuecommand(). SCSI LLD authors have the choice of either hoping that
.queuecommand() calls from the SCSI error handler won't happen while transport
layer recovery is in progress or to add code in the .queuecommand() function
that detects from which context that call comes and to delay such
.queuecommand() calls. In the SRP initiator driver that code looks as follows:

const bool in_scsi_eh = !in_interrupt() && current == shost->ehandler;

/*
 * The SCSI EH thread is the only context from which srp_queuecommand()
 * can get invoked for blocked devices (SDEV_BLOCK /
 * SDEV_CREATED_BLOCK). Avoid racing with srp_reconnect_rport() by
 * locking the rport mutex if invoked from inside the SCSI EH.
 */
if (in_scsi_eh)
mutex_lock(&rport->mutex);

In my opinion the SCSI core should make it easy for LLD authors to prevent that
the error handler calls .queuecommand() while transport layer recovery is in
progress. So considerable time ago I posted several patches that modify the SCSI
error handler and that avoid that SCSI LLDs have to detect the context a
.queuecommand() call comes from. None of these patches were accepted and no 
alternative approach was proposed. Hence the proposal to discuss this topic in
person during LSF/MM.

See also "[PATCH 1/2] RDMA/srp: Avoid calling mutex_lock() from inside
scsi_queue_rq()" (https://www.spinics.net/lists/linux-rdma/msg73842.html).

Thanks,

Bart.


Re: [PATCH v2 0/7] Fix handling of bidi commands

2019-01-23 Thread Douglas Gilbert

On 2019-01-23 2:10 p.m., Bart Van Assche wrote:

Hi Martin,

Recently Doug Gilbert reported that handling of bidi commands is broken
in the scsi-mq code. This patch series fixes that bug and also simplifies
bidi command handling. Please consider these patches for kernel v5.1.

Thanks,

Bart.

Changes compared to v1:
- Fixed a NULL pointer dereference in scsi_init_sdb().

Bart Van Assche (7):
   Introduce the bidi_supported flag in the host template structure
   Change scsi_cmnd.prot_sdb from a pointer into a regular member
   Fix bidi handling
   Introduce scsi_out_cmd()
   Move several function definitions in 
   Introduce scsi_in_[sg]et_resid() and scsi_out_[sg]et_resid()
   Move the resid member from struct scsi_data_buffer into struct
 scsi_cmnd

  drivers/scsi/iscsi_tcp.c   |  8 +---
  drivers/scsi/libiscsi.c| 12 ++---
  drivers/scsi/scsi_debug.c  | 20 +++--
  drivers/scsi/scsi_lib.c| 70 --
  drivers/scsi/sd.c  |  4 +-
  drivers/scsi/virtio_scsi.c |  4 +-
  drivers/target/loopback/tcm_loop.c |  8 +---
  drivers/usb/storage/uas.c  |  8 ++--
  include/scsi/scsi_cmnd.h   | 67 
  include/scsi/scsi_host.h   |  2 +
  10 files changed, 101 insertions(+), 102 deletions(-)


I have been running v2 against the scsi_debug driver mainly doing
XDWRITEREAD(10)s with the odd INQUIRY on Martin's 5.1/scsi-queue
branch with my sg v4 driver (20190118 version) for 2 hours. So far
no sign of problems or memory usage expansion.

A script is running this command:
   sg_tst_bidi -d=4 -q=64 -N -vv -l=0x123 -Q -R=64 /dev/sg1

every 3 seconds was the test. That utility can be found in sg3_utils-1.45
beta rev 808 (testing directory) linked at the top of:
   http://sg.danny.cz/sg/

It is queuing up 64 SG_IOSUBMITs (nearly all XDWRITEREAD(10)s) then
reading their responses back and checking for errors. That is repeated
64 times (-R=64). That was the test that crashed v1 of this patch.

Also Boaz Harrosh confirmed to me that without this patchset (or v1)
exofs tests "blew up" when SCSI errors were injected (as predicted).
So Boaz, could you apply this patchset and try those tests again.

So:
  Tested-by: Douglas Gilbert 

Please apply this to all patches in this set.


[PATCH] libfc free skb when receiving invalid flogi resp

2019-01-23 Thread ming . lu . mlu
From: Ming Lu 

The issue to be fixed in this commit is when libfc found it received a
invalid FLOGI response from FC switch, it would return without freeing
the fc frame, which is just the skb data. This would cause memory leak
if FC switch keeps sending invalid FLOGI responses.

This fix is just to make it execute `fc_frame_free(fp)` before returning
from function `fc_lport_flogi_resp`.

Signed-off-by: Ming Lu 
---
 drivers/scsi/libfc/fc_lport.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c
index be83590ed955..ff943f477d6f 100644
--- a/drivers/scsi/libfc/fc_lport.c
+++ b/drivers/scsi/libfc/fc_lport.c
@@ -1726,14 +1726,14 @@ void fc_lport_flogi_resp(struct fc_seq *sp, struct 
fc_frame *fp,
fc_frame_payload_op(fp) != ELS_LS_ACC) {
FC_LPORT_DBG(lport, "FLOGI not accepted or bad response\n");
fc_lport_error(lport, fp);
-   goto err;
+   goto out;
}
 
flp = fc_frame_payload_get(fp, sizeof(*flp));
if (!flp) {
FC_LPORT_DBG(lport, "FLOGI bad response\n");
fc_lport_error(lport, fp);
-   goto err;
+   goto out;
}
 
mfs = ntohs(flp->fl_csp.sp_bb_data) &
@@ -1743,7 +1743,7 @@ void fc_lport_flogi_resp(struct fc_seq *sp, struct 
fc_frame *fp,
FC_LPORT_DBG(lport, "FLOGI bad mfs:%hu response, "
 "lport->mfs:%hu\n", mfs, lport->mfs);
fc_lport_error(lport, fp);
-   goto err;
+   goto out;
}
 
if (mfs <= lport->mfs) {
-- 
2.17.1



Re: Kernel v5.0, scsi_debug and libiscsi

2019-01-23 Thread Douglas Gilbert

On 2019-01-23 5:56 p.m., Bart Van Assche wrote:

On Fri, 2019-01-11 at 13:01 -0500, Douglas Gilbert wrote:

On 2019-01-10 6:22 p.m., Bart Van Assche wrote:

Hi Doug,

Have you ever tried to run the libiscsi conformance tests against
the scsi_debug driver? I tried the following:

modprobe scsi_debug delay=0 max_luns=3
dev=$(for f in 
/sys/bus/pseudo/drivers/scsi_debug/adapter*/host*/target*/[0-9]*/block/*; do 
echo $f; break; done)
dev=/dev/$(basename $dev)
libiscsi/test-tool/iscsi-test-cu --dataloss --allow-sanitize "$dev"

That test triggers the following output:

BUG: unable to handle kernel paging request at a8d741235e00
PGD 13b141067 P4D 13b141067 PUD 13b146067 PMD 6fc5a067 PTE 0
Oops: 0002 [#1] SMP PTI
CPU: 3 PID: 4967 Comm: iscsi-test-cu Not tainted 4.18.0-13-generic #14-Ubuntu
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
RIP: 0010:memcpy_erms+0x6/0x10


Since memory corruption errors have been found elsewhere in
lk 5.0-rc1 and a fix looks like it is pending, I will leave this
one alone as I can't replicate it.


Hi Doug,

I can replicate this crash easily. I also noticed that this crash only occurs if
the scsi_debug driver is loaded with fake_rw=0. It does not occur with 
fake_rw=1.
It seems like the following code in resp_write_same() assumes that fake_storep 
!= NULL?

/* if ndob then zero 1 logical block, else fetch 1 logical block */
if (ndob) {
memset(fake_storep + lba_off, 0, sdebug_sector_size);
ret = 0;
} else
ret = fetch_to_dev_buffer(scp, fake_storep + lba_off,
  sdebug_sector_size);


It is table driven. It shouldn't call that function if FF_MEDIA_IO is part of
that command's flag and fake_storep is NULL. Both WS10 and WS16 have that flag.

But there is a problem if virtual_gb > 0 .

Could you try the attached patch, it should wrap cleanly in the virtual_gb > 0
case.

Doug Gilbert

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 661512bec3ac..b190277d945c 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -735,7 +735,7 @@ static inline bool scsi_debug_lbp(void)
 		(sdebug_lbpu || sdebug_lbpws || sdebug_lbpws10);
 }
 
-static void *fake_store(unsigned long long lba)
+static void *lba2fake_store(unsigned long long lba)
 {
 	lba = do_div(lba, sdebug_store_sectors);
 
@@ -2514,8 +2514,8 @@ static int do_device_access(struct scsi_cmnd *scmd, u32 sg_skip, u64 lba,
 	return ret;
 }
 
-/* If fake_store(lba,num) compares equal to arr(num), then copy top half of
- * arr into fake_store(lba,num) and return true. If comparison fails then
+/* If lba2fake_store(lba,num) compares equal to arr(num), then copy top half of
+ * arr into lba2fake_store(lba,num) and return true. If comparison fails then
  * return false. */
 static bool comp_write_worker(u64 lba, u32 num, const u8 *arr)
 {
@@ -2643,7 +2643,7 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec,
 		if (sdt->app_tag == cpu_to_be16(0x))
 			continue;
 
-		ret = dif_verify(sdt, fake_store(sector), sector, ei_lba);
+		ret = dif_verify(sdt, lba2fake_store(sector), sector, ei_lba);
 		if (ret) {
 			dif_errors++;
 			return ret;
@@ -3261,10 +3261,12 @@ static int resp_write_scat(struct scsi_cmnd *scp,
 static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num,
 			   u32 ei_lba, bool unmap, bool ndob)
 {
+	int ret;
 	unsigned long iflags;
 	unsigned long long i;
-	int ret;
-	u64 lba_off;
+	u32 lb_size = sdebug_sector_size;
+	u64 block, lbaa;
+	u8 *fs1p;
 
 	ret = check_device_access_params(scp, lba, num);
 	if (ret)
@@ -3276,31 +3278,30 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num,
 		unmap_region(lba, num);
 		goto out;
 	}
-
-	lba_off = lba * sdebug_sector_size;
+	lbaa = lba;
+	block = do_div(lbaa, sdebug_store_sectors);
 	/* if ndob then zero 1 logical block, else fetch 1 logical block */
+	fs1p = fake_storep + (block * lb_size);
 	if (ndob) {
-		memset(fake_storep + lba_off, 0, sdebug_sector_size);
+		memset(fs1p, 0, lb_size);
 		ret = 0;
 	} else
-		ret = fetch_to_dev_buffer(scp, fake_storep + lba_off,
-	  sdebug_sector_size);
+		ret = fetch_to_dev_buffer(scp, fs1p, lb_size);
 
 	if (-1 == ret) {
 		write_unlock_irqrestore(&atomic_rw, iflags);
 		return DID_ERROR << 16;
-	} else if (sdebug_verbose && !ndob && (ret < sdebug_sector_size))
+	} else if (sdebug_verbose && !ndob && (ret < lb_size))
 		sdev_printk(KERN_INFO, scp->device,
 			"%s: %s: lb size=%u, IO sent=%d bytes\n",
-			my_name, "write same",
-			sdebug_sector_size, ret);
+			my_name, "write same", lb_size, ret);
 
 	/* Copy first sector to remaining blocks */
-	for (i = 1 ; i < num ; i++)
-		memcpy(fake_storep + ((lba + i) * sdebug_sector_size),
-		   fake_storep + lba_off,
-		   sdebug_sector_size);
-
+	for (i = 1 ; i < num ; i++) {
+		lbaa = lba + i;
+		block = do_div(lbaa, sdebug_store_sectors);
+	

[ 0/1] Add support for bus voting in UFS

2019-01-23 Thread Asutosh Das
This patch adds bus voting support using the new interconnect framework.
This is dependent on the below mentioned patch series:
https://patchwork.kernel.org/project/linux-arm-msm/list/?series=67417

This was tested using Qualcomm SDM 845 interconnect service
provider patch.

Asutosh Das (1):
  scsi: qcom-ufs: Add support for bus voting using ICB framework

 .../devicetree/bindings/ufs/ufshcd-pltfrm.txt  |  12 ++
 drivers/scsi/ufs/ufs-qcom.c| 234 -
 drivers/scsi/ufs/ufs-qcom.h|  20 ++
 3 files changed, 218 insertions(+), 48 deletions(-)

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
Foundation Collaborative Project.



[ 1/1] scsi: qcom-ufs: Add support for bus voting using ICB framework

2019-01-23 Thread Asutosh Das
Adapt to the new ICB framework for bus bandwidth voting.

This requires the source/destination port ids.
Also this requires a tuple of values.

The tuple is for two different paths - from UFS master
to BIMC slave. The other is from CPU master to UFS slave.
This tuple consists of the average and peak bandwidth.

Signed-off-by: Asutosh Das 
---
 .../devicetree/bindings/ufs/ufshcd-pltfrm.txt  |  12 ++
 drivers/scsi/ufs/ufs-qcom.c| 234 -
 drivers/scsi/ufs/ufs-qcom.h|  20 ++
 3 files changed, 218 insertions(+), 48 deletions(-)

diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt 
b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
index a99ed55..94249ef 100644
--- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
+++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
@@ -45,6 +45,18 @@ Optional properties:
 Note: If above properties are not defined it can be assumed that the supply
 regulators or clocks are always on.
 
+* Following bus parameters are required:
+interconnects
+interconnect-names
+- Please refer to Documentation/devicetree/bindings/interconnect/
+  for more details on the above.
+qcom,msm-bus,name - string describing the bus path
+qcom,msm-bus,num-cases - number of configurations in which ufs can operate in
+qcom,msm-bus,num-paths - number of paths to vote for
+qcom,msm-bus,vectors-KBps - Takes a tuple ,  (2 tuples for 2 
num-paths)
+   The number of these entries *must* be same as
+   num-cases.
+
 Example:
ufshc@0xfc598000 {
compatible = "jedec,ufs-1.1";
diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 2b38db2..213e975 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "ufshcd.h"
@@ -27,6 +28,10 @@
 #define UFS_QCOM_DEFAULT_DBG_PRINT_EN  \
(UFS_QCOM_DBG_PRINT_REGS_EN | UFS_QCOM_DBG_PRINT_TEST_BUS_EN)
 
+#define UFS_DDR "ufs-ddr"
+#define CPU_UFS "cpu-ufs"
+
+
 enum {
TSTBUS_UAWM,
TSTBUS_UARM,
@@ -714,7 +719,6 @@ static int ufs_qcom_get_pwr_dev_param(struct 
ufs_qcom_dev_params *qcom_param,
return 0;
 }
 
-#ifdef CONFIG_MSM_BUS_SCALING
 static int ufs_qcom_get_bus_vote(struct ufs_qcom_host *host,
const char *speed_mode)
 {
@@ -768,24 +772,83 @@ static void ufs_qcom_get_speed_mode(struct 
ufs_pa_layer_attr *p, char *result)
}
 }
 
+static int ufs_qcom_get_ib_ab(struct ufs_qcom_host *host, int index,
+ struct qcom_bus_vectors *ufs_ddr_vec,
+ struct qcom_bus_vectors *cpu_ufs_vec)
+{
+   struct qcom_bus_path *usecase;
+
+   if (!host->qbsd)
+   return -EINVAL;
+
+   if (index > host->qbsd->num_usecase)
+   return -EINVAL;
+
+   usecase = host->qbsd->usecase;
+
+   /*
+*
+* usecase:0  usecase:0
+* ufs->ddr   cpu->ufs
+* |vec[0&1] | vec[2&3]|
+* +++++
+* | ab | ib | ab | ib |
+* |++++
+* .
+* .
+* .
+* usecase:n  usecase:n
+* ufs->ddr   cpu->ufs
+* |vec[0&1] | vec[2&3]|
+* +++++
+* | ab | ib | ab | ib |
+* |++++
+*/
+
+   /* index refers to offset in usecase */
+   ufs_ddr_vec->ab = usecase[index].vec[0].ab;
+   ufs_ddr_vec->ib = usecase[index].vec[0].ib;
+
+   cpu_ufs_vec->ab = usecase[index].vec[1].ab;
+   cpu_ufs_vec->ib = usecase[index].vec[1].ib;
+
+   return 0;
+}
+
 static int ufs_qcom_set_bus_vote(struct ufs_qcom_host *host, int vote)
 {
int err = 0;
+   struct qcom_bus_scale_data *d = host->qbsd;
+   struct qcom_bus_vectors path0, path1;
+   struct device *dev = host->hba->dev;
 
-   if (vote != host->bus_vote.curr_vote) {
-   err = msm_bus_scale_client_update_request(
-   host->bus_vote.client_handle, vote);
-   if (err) {
-   dev_err(host->hba->dev,
-   "%s: msm_bus_scale_client_update_request() 
failed: bus_client_handle=0x%x, vote=%d, err=%d\n",
-   __func__, host->bus_vote.client_handle,
-   vote, err);
-   goto out;
-   }
+   err = ufs_qcom_get_ib_ab(host, vote, &path0, &path1);
+   if (err) {
+   dev_err(dev, "Error: failed (%d) to get ib/ab\n",
+   err);
+   return err;
+   }
 
-   host->bus_vote.curr_vote = vote;
+   dev_dbg(dev, "Setting vote: %d: ufs-ddr: ab: %llu ib: %llu\n", vote,
+   path0.ab, path0.ib);
+   err = icc_set_bw(d->ufs_ddr, path0.ab, path0.ib);
+   if (err) {
+   dev_er

Re: [PATCH] libfc free skb when receiving invalid flogi resp

2019-01-23 Thread Hannes Reinecke

On 1/24/19 6:25 AM, ming.lu@gmail.com wrote:

From: Ming Lu 

The issue to be fixed in this commit is when libfc found it received a
invalid FLOGI response from FC switch, it would return without freeing
the fc frame, which is just the skb data. This would cause memory leak
if FC switch keeps sending invalid FLOGI responses.

This fix is just to make it execute `fc_frame_free(fp)` before returning
from function `fc_lport_flogi_resp`.

Signed-off-by: Ming Lu 
---
  drivers/scsi/libfc/fc_lport.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c
index be83590ed955..ff943f477d6f 100644
--- a/drivers/scsi/libfc/fc_lport.c
+++ b/drivers/scsi/libfc/fc_lport.c
@@ -1726,14 +1726,14 @@ void fc_lport_flogi_resp(struct fc_seq *sp, struct 
fc_frame *fp,
fc_frame_payload_op(fp) != ELS_LS_ACC) {
FC_LPORT_DBG(lport, "FLOGI not accepted or bad response\n");
fc_lport_error(lport, fp);
-   goto err;
+   goto out;
}
  
  	flp = fc_frame_payload_get(fp, sizeof(*flp));

if (!flp) {
FC_LPORT_DBG(lport, "FLOGI bad response\n");
fc_lport_error(lport, fp);
-   goto err;
+   goto out;
}
  
  	mfs = ntohs(flp->fl_csp.sp_bb_data) &

@@ -1743,7 +1743,7 @@ void fc_lport_flogi_resp(struct fc_seq *sp, struct 
fc_frame *fp,
FC_LPORT_DBG(lport, "FLOGI bad mfs:%hu response, "
 "lport->mfs:%hu\n", mfs, lport->mfs);
fc_lport_error(lport, fp);
-   goto err;
+   goto out;
}
  
  	if (mfs <= lport->mfs) {



Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


blk-mq private tags for SCSI

2019-01-23 Thread Hannes Reinecke

Hi all,

blk-mq has the concept of 'private' tags to handle driver-internal commands.
Also quite some SCSI HBAs use internal commands for configuration, event 
handling etc.
But sadly no interface to use the 'private' tags from the block layer 
exists, so quite some drivers like megaraid_sas, aacraid, or hpsa have 
to implement their own management of internal commands.


I have made several prototypes on how private tags could be used in the 
SCSI context, but neither attempts turned out to be utterly convincing:


a) create an abstraction in the block layer to get private tags directly
   from the tag map. Problem is that the tag allocation mechanism is
   deeply intertwined with request queues, so desegregating them would
   involve quite some churn in the block layer.

b) create a separate admin queue for private commands. Problem is that
   then we always will have a SCSI command as the first payload, which
   in most cases isn't required as most of the internal commands are
   transport or HBA specific, not SCSI CDBs. Also for some drivers we
   need to send commands to figure out the configuration, which then
   influences the size of the tag space, leaving us with a catch-22
   situation.

I would like to discuss at LSF/MM if using private tags in the SCSI 
layer is something to be attempted, and if so what would be the best 
direction to take.


Cheers,

Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH] SCSI: fcoe: remove unneeded fcoe_ctlr_destroy_store export

2019-01-23 Thread Hannes Reinecke

On 1/22/19 3:28 PM, Greg Kroah-Hartman wrote:

There's no need to export fcoe_ctlr_destroy_store as a symbol, so remove
the EXPORT_SYMBOL() line for it.

Cc: Johannes Thumshirn 
Cc: "James E.J. Bottomley" 
Cc: "Martin K. Petersen" 
Signed-off-by: Greg Kroah-Hartman 
---
  drivers/scsi/fcoe/fcoe_transport.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/fcoe/fcoe_transport.c 
b/drivers/scsi/fcoe/fcoe_transport.c
index f4909cd206d3..6d3949b687ef 100644
--- a/drivers/scsi/fcoe/fcoe_transport.c
+++ b/drivers/scsi/fcoe/fcoe_transport.c
@@ -855,7 +855,6 @@ ssize_t fcoe_ctlr_destroy_store(struct bus_type *bus,
mutex_unlock(&ft_mutex);
return rc;
  }
-EXPORT_SYMBOL(fcoe_ctlr_destroy_store);
  
  /**

   * fcoe_transport_create() - Create a fcoe interface


Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)