Re: [PATCH 0/2] storvsc: changes for scsi next

[Martin K. Petersen]

Stephen,

> These are refactoring changes to the Hyper-V scsi driver.

Applied to 4.13/scsi-queue. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH v2] scsi: csiostor: add support for Chelsio T6 adapters

[Martin K. Petersen]

Varun,

> Enable probe for T6 adapters, add code to flash T6 firmware and
> firmware config file, use T6 specific macros.

Applied to 4.13/scsi-queue. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: smartpqi: mark PM functions as __maybe_unused

[Martin K. Petersen]

Arnd,

> I notice that today's linux-next no longer contains the patch that
> introduced the warning.

I had tagged my 4.12 fixes branch with for-next. It should be back to
4.13 material shortly.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH v2] scsi: zero per-cmd private driver data for each MQ I/O

[Martin K. Petersen]

Long,

> In lower layer driver's (LLD) scsi_host_template, the driver may
> optionally ask SCSI to allocate its private driver memory for each
> command, by specifying cmd_size. This memory is allocated at the end
> of scsi_cmnd by SCSI.  Later when SCSI queues a command, the LLD can
> use scsi_cmd_priv to get to its private data.
>  
> Some LLD, e.g. hv_storvsc, doesn't clear its private data before
> use. In this case, the LLD may get to stale or uninitialized data in
> its private driver memory. This may result in unexpected driver and
> hardware behavior.
>
> Fix this problem by also zeroing the private driver memory before
> passing them to LLD.

Applied to 4.12/scsi-fixes. Thank you!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: libiscsi: use kvzalloc for iscsi_pool_init

[Martin K. Petersen]

Kyle,

> Use kvzalloc for iscsi_pool in iscsi_pool_init.

Applied to 4.13/scsi-queue. Thank you!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: csiostor: fix use after free in csio_hw_use_fwconfig()

[Martin K. Petersen]

Varun,

> mbp pointer is passed to csio_hw_validate_caps() so call
> mempool_free() after calling csio_hw_validate_caps().

Applied to 4.12/scsi-fixes. Thank you!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: libcxgbi: fix skb use after free

[Martin K. Petersen]

> skb->data is assigned to task->hdr in cxgbi_conn_alloc_pdu(),
> skb gets freed after tx but task->hdr is still dereferenced in
> iscsi_tcp_task_xmit() to avoid this call skb_get() after allocating
> skb and free the skb in cxgbi_cleanup_task() or before allocating new
> skb in cxgbi_conn_alloc_pdu().

Somebody please review!

https://patchwork.kernel.org/patch/9729239/

Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] csiostor: Avoid content leaks and casts

[Martin K. Petersen]

Varun,

You weren't CC:ed on this patch. Please review. Thanks!

> When copying attributes, the len argument was padded out and the
> resulting memcpy() would copy beyond the end of the source buffer.
> Avoid this, and use size_t for val_len to avoid all the
> casts. Similarly, avoid source buffer casts and use void *.
>
> Additionally enforces val_len can be represented by u16 and that the
> DMA buffer was not overflowed. Fixes the size of mfa, which is not
> FC_FDMI_PORT_ATTR_MAXFRAMESIZE_LEN (but it will be padded up to
> 4). This was noticed by the future CONFIG_FORTIFY_SOURCE checks.

https://patchwork.kernel.org/patch/9718995/

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: ufs: Clean up some rpm/spm level SysFS nodes upon remove

[Martin K. Petersen]

Michal,

> When reloading module these two attributes aren't cleaned up properly
> and they persist causing warnings when trying to load module
> again. Additionally they are not recreated properly due to that.

Applied to 4.12/scsi-fixes, thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH blktests 0/3] Add SCSI generic test group

[Omar Sandoval]

On Thu, May 18, 2017 at 03:29:45PM +0200, Johannes Thumshirn wrote:
> On 05/18/2017 03:19 PM, Christoph Hellwig wrote:
> > All SG_IO test should also apply to block device nodes that support
> > the ioctl..
> > 
> 
> But these are not necessarily SG_IO tests, are they?
> 
> The test included is doesn't hit the SG_IO path in the sg driver, but
> the sg_read path.
> 
> Of cause we can make a generic sg_io tool but IMHO it's quite convenient
> to just throw in the reproducers we get.
> 
> And as it exercises the /dev/sg character device I decided to make a new
> group.
> 
>   Johannes

Looking at this some more, it seems like the syzkaller reproducer always
bangs on /dev/sg0. How hard would it be to adapt it to run on the sg
device for every test device instead?

[PATCH v2] scsi: zero per-cmd private driver data for each MQ I/O

[Long Li]

From: Long Li 

In lower layer driver's (LLD) scsi_host_template, the driver may optionally 
ask SCSI to allocate its private driver memory for each command, by 
specifying cmd_size. This memory is allocated at the end of scsi_cmnd by SCSI. 
Later when SCSI queues a command, the LLD can use scsi_cmd_priv to get to its 
private data.
 
Some LLD, e.g. hv_storvsc, doesn't clear its private data before use. In
this case, the LLD may get to stale or uninitialized data in its private 
driver memory. This may result in unexpected driver and hardware behavior.

Fix this problem by also zeroing the private driver memory before passing
them to LLD.

Signed-off-by: Long Li 
Reviewed-by: Bart Van Assche 
Reviewed-by: KY Srinivasan 
Reviewed-by: Christoph Hellwig 
CC: sta...@vger.kernel.org

---
 drivers/scsi/scsi_lib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 19125d7..a821593 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1850,7 +1850,7 @@ static int scsi_mq_prep_fn(struct request *req)
 
/* zero out the cmd, except for the embedded scsi_request */
memset((char *)cmd + sizeof(cmd->req), 0,
-   sizeof(*cmd) - sizeof(cmd->req));
+   sizeof(*cmd) - sizeof(cmd->req) + shost->hostt->cmd_size);
 
req->special = cmd;
 
-- 
2.7.4

Re: [PATCH blktests 2/3] tests/sg: add SCSI generic test grouop

[Omar Sandoval]

On Thu, May 18, 2017 at 02:06:20PM -0700, Omar Sandoval wrote:
> On Thu, May 18, 2017 at 02:13:07PM +0200, Johannes Thumshirn wrote:
> > Add a test group for tests of the SCSI generic driver and and
> > functions common to the SCSI generic driver and it's test cases.
> > 
> > Signed-off-by: Johannes Thumshirn 
> > ---
> >  common/sg  | 22 ++
> >  tests/sg/group | 40 
> >  2 files changed, 62 insertions(+)
> >  create mode 100644 common/sg
> >  create mode 100644 tests/sg/group
> > 
> > diff --git a/common/sg b/common/sg
> > new file mode 100644
> > index ..30b5089c68f7
> > --- /dev/null
> > +++ b/common/sg
> > +# TODO: if this test group has extra requirements for what devices it can 
> > be
> > +# run on, it should define a group_device_requires() function. If tests in 
> > this
> > +# group can be run on the test device, it should return zero. Otherwise, it
> > +# should return non-zero and set the $SKIP_REASON variable. $TEST_DEV is 
> > the
> > +# full path of the block device (e.g., /dev/nvme0n1 or /dev/sda1), and
> > +# $TEST_DEV_SYSFS is the sysfs path of the disk (not the partition, e.g.,
> > +# /sys/block/nvme0n1 or /sys/block/sda).
> > +#
> > +# Usually, group_device_requires() just needs to check that the test 
> > device is
> > +# the right type of hardware or supports any necessary features using the
> > +# _test_dev_foo helpers. If group_device_requires() returns non-zero, all 
> > tests
> > +# in this group will be skipped on that device.
> > +# group_device_requires() {
> > +#  _test_dev_is_foo && _test_dev_supports_bar
> > +# }
> 
> Leftover TODO, I'll remove it when applying. If we add an sg test that
> runs on an actual device, we can define group_device_requires().

On second though, since I had some comments for patch 3, just fix this
up when you resend.

Re: [PATCH blktests 3/3] sg/001: add regression test for syzcaller generated GPF

[Omar Sandoval]

On Thu, May 18, 2017 at 02:13:08PM +0200, Johannes Thumshirn wrote:
> Add a regression test for commit 48ae8484e9fc ("scsi: sg: don't return
> bogus Sg_requests"). This is a general protection fault triggered by
> syzcaller.
> 
> Signed-off-by: Johannes Thumshirn 
> ---
>  tests/sg/001 | 48 
>  tests/sg/001.out |  2 ++
>  2 files changed, 50 insertions(+)
>  create mode 100755 tests/sg/001
>  create mode 100644 tests/sg/001.out
> 
> diff --git a/tests/sg/001 b/tests/sg/001
> new file mode 100755
> index ..3a72931d5748
> --- /dev/null
> +++ b/tests/sg/001
> @@ -0,0 +1,48 @@
> +#!/bin/bash
> +#
> +# Regression test for commit 48ae8484e9fc ("scsi: sg: don't return bogus 
> Sg_requests")
> +#
> +# Copyright (C) 2017 Johannes Thumshirn 
> +#
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation, either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see .
> +
> +. common/sg
> +. common/scsi_debug
> +
> +DESCRIPTION="Regression test for commit 48ae8484e9fc (\"scsi: sg: don't 
> return bogus Sg_requests\")"

This description doesn't really say what the test does. If this test
fails, the person running it should be able to know what the test is
doing without having to decode the syzkaller reproducer. I'd prefer
something like:

DESCRIPTION="do bogus sg reads and writes"

Or something like that, that's my best attempt at understanding the
reproducer :)

> +TIMED=1
> +QUICK=1
> +
> +requires() {
> + _have_program src/sg-001 \
> + _have_program timeout \

timeout is part of coreutils so this isn't necessary.

> + && _have_scsi_debug \
> + && _have_scsi_generic
> +}
> +
> +
> +test() {
> + echo "Running ${TEST_NAME}"
> +
> + if ! _get_scsi_debug_dev; then
> + return 1
> + fi
> +
> + _divide_timeout 2

You don't use $TIMEOUT in this test, so remove this and the TIMED=1. The
arbitrary timeout is fine for this kind of test, I think.

> + timeout -s INT 10s ./src/sg-001
> +
> + _put_scsi_debug_dev
> + 
> + echo "Test complete"
> +}
> diff --git a/tests/sg/001.out b/tests/sg/001.out
> new file mode 100644
> index ..beb4c437dd28
> --- /dev/null
> +++ b/tests/sg/001.out
> @@ -0,0 +1,2 @@
> +Running sg/001
> +Test complete
> -- 
> 2.12.0
> 

Re: [PATCH blktests 2/3] tests/sg: add SCSI generic test grouop

[Omar Sandoval]

On Thu, May 18, 2017 at 02:13:07PM +0200, Johannes Thumshirn wrote:
> Add a test group for tests of the SCSI generic driver and and
> functions common to the SCSI generic driver and it's test cases.
> 
> Signed-off-by: Johannes Thumshirn 
> ---
>  common/sg  | 22 ++
>  tests/sg/group | 40 
>  2 files changed, 62 insertions(+)
>  create mode 100644 common/sg
>  create mode 100644 tests/sg/group
> 
> diff --git a/common/sg b/common/sg
> new file mode 100644
> index ..30b5089c68f7
> --- /dev/null
> +++ b/common/sg
> +# TODO: if this test group has extra requirements for what devices it can be
> +# run on, it should define a group_device_requires() function. If tests in 
> this
> +# group can be run on the test device, it should return zero. Otherwise, it
> +# should return non-zero and set the $SKIP_REASON variable. $TEST_DEV is the
> +# full path of the block device (e.g., /dev/nvme0n1 or /dev/sda1), and
> +# $TEST_DEV_SYSFS is the sysfs path of the disk (not the partition, e.g.,
> +# /sys/block/nvme0n1 or /sys/block/sda).
> +#
> +# Usually, group_device_requires() just needs to check that the test device 
> is
> +# the right type of hardware or supports any necessary features using the
> +# _test_dev_foo helpers. If group_device_requires() returns non-zero, all 
> tests
> +# in this group will be skipped on that device.
> +# group_device_requires() {
> +#_test_dev_is_foo && _test_dev_supports_bar
> +# }

Leftover TODO, I'll remove it when applying. If we add an sg test that
runs on an actual device, we can define group_device_requires().

Re: Need help with handling failed ATA pass-through command and sense data

[Ewan D. Milne]

On Thu, 2017-05-18 at 13:37 -0400, Alan Stern wrote:
> 
> I had completely forgotten about this code.  :-(
> 
> Looks like you put your finger on the source of the problem.  So if the 
> device sends back essentially empty sense data (SK = No Sense, ASC = 
> ASCQ = 0), but the USB transport indicates command failure, how should 
> we inform the SCSI core in a way that won't cause infinite retries or 
> obnoxious log messages?
> 
> Should we be doing a better job of detecting empty sense data -- that 
> is, do we need to check for non-empty ATA status?
> 
> Or has the SCSI core improved so that it no longer does infinite
> retries (see commit f1a0743bc0e7 "USB: storage: When a device returns
> no sense data, call it a Hardware Error" and Bugzilla entry #14118),
> meaning that this code can be removed entirely?
> 
> Alan Stern

We added:

commit ee60b2c52ec8ecdcbcd2f85cc117b525f649441f
Author: Eiichi Tsukata 
Date:   Tue Feb 11 14:29:52 2014 +0900

[SCSI] Add timeout to avoid infinite command retry

but this may not give you the behavior you want, because it bounds
the execution time to (# of retries + 1) * timeout.  So if you get
an immediate error return it could still take a while for this code
to give up retrying, i.e. it does not have the same properties as
your commit f1a0743bc0e7.

I suppose you could decode the ATA Status Return sense data descriptor
but I don't know how good the compliance is among all the ATA devices.
Table 177 in section 1.2.2.8 of SAT-4 r06 seems to say that most of
the fields in the sense data are unspecified for ATA PASS-THROUGH
commands, so this probably explains why you see nothing else useful.
Perhaps the logging should be delegated to the USB or ATA code for
these commands, since they are not really part of SCSI?

I have seen a case of a Fibre Channel array returning all zeroes
in the sense data, but this was because it was malfunctioning.

-Ewan

[RFC v2 2/2] scsi: add rate limit to scsi sense code uevent

[Song Liu]

This patch adds rate limits to SCSI sense code uevets. Currently,
the rate limit is hard-coded to 16 events per second.

The code tracks nano second time of latest 16 events in a circular
buffer. When a new event arrives, the time is compared against the
latest time in the buffer. If the difference is smaller than one
second, the new event is dropped.

Signed-off-by: Song Liu 
---
 drivers/scsi/scsi_error.c  | 15 +++
 drivers/scsi/scsi_scan.c   |  4 
 include/scsi/scsi_device.h | 12 +++-
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index a49c63a..91e6b0a 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -436,11 +436,26 @@ static void scsi_send_sense_uevent(struct scsi_device 
*sdev,
 #ifdef CONFIG_SCSI_SENSE_UEVENT
struct scsi_event *evt;
unsigned char sb_len;
+   unsigned long flags;
+   u64 time_ns;
 
if (!test_bit(sshdr->sense_key & 0xf,
  >sense_event_filter))
return;
evt = sdev_evt_alloc(SDEV_EVT_SCSI_SENSE, GFP_ATOMIC);
+
+   time_ns = ktime_to_ns(ktime_get());
+   spin_lock_irqsave(>latest_event_lock, flags);
+   if (time_ns - sdev->latest_event_times[sdev->latest_event_idx] <
+   NSEC_PER_SEC) {
+   spin_unlock_irqrestore(>latest_event_lock, flags);
+   return;
+   }
+   sdev->latest_event_times[sdev->latest_event_idx] = time_ns;
+   sdev->latest_event_idx = (sdev->latest_event_idx + 1) %
+   MAX_SENSE_EVENT_PER_SECOND;
+   spin_unlock_irqrestore(>latest_event_lock, flags);
+
if (!evt)
return;
 
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 6f7128f..67b3f71 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -301,6 +301,10 @@ static struct scsi_device *scsi_alloc_sdev(struct 
scsi_target *starget,
}
}
 
+#ifdef CONFIG_SCSI_SENSE_UEVENT
+   spin_lock_init(>latest_event_lock);
+#endif
+
return sdev;
 
 out_device_destroy:
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 5b6f06d..9217dae 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -214,12 +214,22 @@ struct scsi_device {
atomic_t ioerr_cnt;
 
 #ifdef CONFIG_SCSI_SENSE_UEVENT
+#define MAX_SENSE_EVENT_PER_SECOND 16
/*
 * filter of sense code uevent
 * setting bit X (0x00 - 0x0e) of sense_event_filter enables
 * uevent for sense key X
 */
-   unsigned long   sense_event_filter;
+   unsigned long   sense_event_filter;
+   /*
+* To rate limit uevents to MAX_SENSE_EVENT_PER_SECOND, we track
+* nano second time of MAX_SENSE_EVENT_PER_SECOND most recent
+* events. If there are already MAX_SENSE_EVENT_PER_SECOND in the
+* past seconds, new event is dropped.
+*/
+   u64 latest_event_times[MAX_SENSE_EVENT_PER_SECOND];
+   int latest_event_idx;
+   spinlock_t  latest_event_lock;
 #endif
 
struct device   sdev_gendev,
-- 
2.9.3

[RFC v2 1/2] scsi: generate uevent for SCSI sense code

[Song Liu]

This patch adds capability for SCSI layer to generate uevent for SCSI
sense code. The feature is gated by CONFIG_SCSI_SENSE_UEVENT.

We can configure which sense keys generate uevent for each device
through sysfs entry sense_event_filter, which is a bitmap of
"sense keys to generate uevent" For example, the following enables
uevent for MEDIUM_ERROR (0x03) and HARDWARE_ERROR (0x04) on scsi
drive sdc:

echo 0x000c > /sys/block/sdc/device/sense_event_filter

Here is an example output captured by udevadm:

KERNEL[587.353177] change   /devices/pci:00/XX
ACTION=change
CDB=\x88\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x08\x00\x00
DEVPATH=/devices/pci:00/:00:01.0/:01:00.0/host6/XX
DEVTYPE=scsi_device
DRIVER=sd
MODALIAS=scsi:t-0x00
SDEV_SENSE=1
SENSE_BUFFER=\x72\x03\x11\x14\x00\x00\x00\x34\x00\x0a\x80 
SENSE_CODE=3/11/14
SEQNUM=4796
SUBSYSTEM=scsi

Signed-off-by: Song Liu 
---
 drivers/scsi/Kconfig   | 14 +++
 drivers/scsi/scsi_error.c  | 43 ++
 drivers/scsi/scsi_lib.c| 58 +-
 drivers/scsi/scsi_sysfs.c  | 51 
 include/scsi/scsi_common.h |  6 +
 include/scsi/scsi_device.h | 27 -
 6 files changed, 197 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 3c52867..fd25e14 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -237,6 +237,20 @@ config SCSI_LOGGING
  there should be no noticeable performance impact as long as you have
  logging turned off.
 
+config SCSI_SENSE_UEVENT
+   bool "SCSI sense code logging"
+   depends on SCSI
+   default n
+   ---help---
+ This turns on uevent for SCSI sense code.
+
+ You can configure which sense keys generate uevent for each device
+ through sysfs entry sense_event_filter. For example, the following
+ enables uevent for MEDIUM_ERROR (0x03) and HARDWARE_ERROR (0x04)
+ on scsi drive sdc:
+
+ echo 0x000c > /sys/block/sdc/device/sense_event_filter
+
 config SCSI_SCAN_ASYNC
bool "Asynchronous SCSI scanning"
depends on SCSI
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index ecc07da..a49c63a 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -426,6 +426,48 @@ static void scsi_report_sense(struct scsi_device *sdev,
}
 }
 
+/*
+ * generate uevent when receiving sense code from device
+ */
+static void scsi_send_sense_uevent(struct scsi_device *sdev,
+  struct scsi_cmnd *scmd,
+  struct scsi_sense_hdr *sshdr)
+{
+#ifdef CONFIG_SCSI_SENSE_UEVENT
+   struct scsi_event *evt;
+   unsigned char sb_len;
+
+   if (!test_bit(sshdr->sense_key & 0xf,
+ >sense_event_filter))
+   return;
+   evt = sdev_evt_alloc(SDEV_EVT_SCSI_SENSE, GFP_ATOMIC);
+   if (!evt)
+   return;
+
+   evt->sense_evt_data.cmnd = kzalloc(scmd->cmd_len, GFP_ATOMIC);
+   if (!evt->sense_evt_data.cmnd)
+   goto alloc_cmd_fail;
+
+   sb_len = scsi_sense_data_length(scmd->sense_buffer);
+
+   evt->sense_evt_data.sense_buffer = kzalloc(sb_len, GFP_ATOMIC);
+   if (!evt->sense_evt_data.sense_buffer)
+   goto alloc_sense_fail;
+
+   evt->sense_evt_data.cmd_len = scmd->cmd_len;
+   evt->sense_evt_data.sb_len = sb_len;
+   memcpy(evt->sense_evt_data.cmnd, scmd->cmnd, scmd->cmd_len);
+   memcpy(evt->sense_evt_data.sense_buffer, scmd->sense_buffer, sb_len);
+
+   sdev_evt_send(sdev, evt);
+   return;
+alloc_sense_fail:
+   kfree(evt->sense_evt_data.cmnd);
+alloc_cmd_fail:
+   kfree(evt);
+#endif
+}
+
 /**
  * scsi_check_sense - Examine scsi cmd sense
  * @scmd:  Cmd to have sense checked.
@@ -446,6 +488,7 @@ int scsi_check_sense(struct scsi_cmnd *scmd)
return FAILED;  /* no valid sense data */
 
scsi_report_sense(sdev, );
+   scsi_send_sense_uevent(sdev, scmd, );
 
if (scsi_sense_is_deferred())
return NEEDS_RETRY;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index e31f1cc..37b3b7e 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2660,7 +2660,15 @@ EXPORT_SYMBOL(scsi_device_set_state);
 static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt)
 {
int idx = 0;
-   char *envp[3];
+   char *envp[5];  /* SDEV_EVT_SCSI_SENSE needs most entries (4) */
+#ifdef CONFIG_SCSI_SENSE_UEVENT
+   char *buf = NULL;
+   int i;
+   int buf_size;
+   int offset;
+   struct scsi_sense_hdr sshdr;
+#endif
+
 
switch (evt->evt_type) {
case SDEV_EVT_MEDIA_CHANGE:
@@ -2685,6 +2693,49 @@ static void scsi_evt_emit(struct scsi_device *sdev, 
struct scsi_event *evt)
case 

[RFC v2 0/2] generate uevent for SCSI sense code

[Song Liu]

This change is to follow up our discussion on event log for media
management during LSF/MM 2017.

I have included feedbacks from Hannes Reinecke, Ewan D. Milne, and
Benjamin Block.

Please kindly let me know your thoughts on this.

Thanks,
Song

Song Liu (2):
  scsi: generate uevent for SCSI sense code
  scsi: add rate limit to scsi sense code uevent

 drivers/scsi/Kconfig   | 14 +++
 drivers/scsi/scsi_error.c  | 58 ++
 drivers/scsi/scsi_lib.c| 58 +-
 drivers/scsi/scsi_scan.c   |  4 
 drivers/scsi/scsi_sysfs.c  | 51 
 include/scsi/scsi_common.h |  6 +
 include/scsi/scsi_device.h | 37 -
 7 files changed, 226 insertions(+), 2 deletions(-)

--
2.9.3

[Bug 195285] qla2xxx FW immediatly crashing after target start

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=195285

--- Comment #8 from lober...@redhat.com ---
- Original Message -
> From: bugzilla-dae...@bugzilla.kernel.org
> To: linux-s...@kernel.org
> Sent: Thursday, May 18, 2017 2:09:51 PM
> Subject: [Bug 195285] qla2xxx FW immediatly crashing after target start
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=195285
> 
> --- Comment #6 from himanshu.madh...@cavium.com (himanshu.madh...@qlogic.com)
> ---
> Hi Anthony, Laurence,
> 
> Can you try attached patch to see if it works for you?
> 
> if Yes, I'll send out to SCSI mailing list to be included into upstream.
> 
> Thanks,
> Himanshu
> 
> --
> You are receiving this mail because:
> You are watching the assignee of the bug.
> 
Absolutely, and thanks
Regards
Laurence

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 195285] qla2xxx FW immediatly crashing after target start

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=195285

--- Comment #7 from himanshu.madh...@cavium.com (himanshu.madh...@qlogic.com) 
---
Created attachment 256619
  --> https://bugzilla.kernel.org/attachment.cgi?id=256619=edit
Patch to address target configuration for ISP25xx

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 195285] qla2xxx FW immediatly crashing after target start

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=195285

--- Comment #6 from himanshu.madh...@cavium.com (himanshu.madh...@qlogic.com) 
---
Hi Anthony, Laurence, 

Can you try attached patch to see if it works for you? 

if Yes, I'll send out to SCSI mailing list to be included into upstream. 

Thanks,
Himanshu

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

Re: [PATCH] scsi: ufs: Clean up some rpm/spm level SysFS nodes upon remove

[Subhash Jadavani]

On 2017-05-11 23:36, Michal Potomski wrote:

From: Michał Potomski 

When reloading module these two attributes aren't
cleaned up properly and they persist causing warnings
when trying to load module again. Additionally they are
not recreated properly due to that.

Signed-off-by: Michał Potomski 
---
 drivers/scsi/ufs/ufshcd.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index abc7e87..ffe8d86 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7698,6 +7698,12 @@ static inline void
ufshcd_add_sysfs_nodes(struct ufs_hba *hba)
ufshcd_add_spm_lvl_sysfs_nodes(hba);
 }

+static inline void ufshcd_remove_sysfs_nodes(struct ufs_hba *hba)
+{
+   device_remove_file(hba->dev, >rpm_lvl_attr);
+   device_remove_file(hba->dev, >spm_lvl_attr);
+}
+
 /**
  * ufshcd_shutdown - shutdown routine
  * @hba: per adapter instance
@@ -7735,6 +7741,7 @@ int ufshcd_shutdown(struct ufs_hba *hba)
  */
 void ufshcd_remove(struct ufs_hba *hba)
 {
+   ufshcd_remove_sysfs_nodes(hba);
scsi_remove_host(hba->host);
/* disable interrupts */
ufshcd_disable_intr(hba, hba->intr_mask);



Looks good to me.
Reviewed-by: Subhash Jadavani 


--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] scsi: remove useless variable assignment

[Gustavo A. R. Silva]

Hi James,

Quoting James Bottomley :


On Wed, 2017-05-17 at 19:30 -0500, Gustavo A. R. Silva wrote:

Remove this assignment once the value stored in variable _k_ is
overwritten after a few lines.

Addresses-Coverity-ID: 1226927
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/scsi/qlogicfas408.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/qlogicfas408.c
b/drivers/scsi/qlogicfas408.c
index c3a9151..269440a 100644
--- a/drivers/scsi/qlogicfas408.c
+++ b/drivers/scsi/qlogicfas408.c
@@ -329,7 +329,6 @@ static unsigned int ql_pcmd(struct scsi_cmnd
*cmd)
 */
if ((k = ql_wai(priv)))
return (k << 16);
-   k = inb(qbase + 5); /* should be 0x10, bus
service */


That doesn't look right to me.  inb() is a statement which has an
effect on the I/O device regardless of whether the returned value is
used or discarded.  In this case I think it's being used to clear
pending interrupts, so removing it will likely cause a phase error.



You are right, I get it.

In this case I think a patch to ignore the return value could be applied:

index c3a9151..8f5339a 100644
--- a/drivers/scsi/qlogicfas408.c
+++ b/drivers/scsi/qlogicfas408.c
@@ -329,7 +329,7 @@ static unsigned int ql_pcmd(struct scsi_cmnd *cmd)
 */
if ((k = ql_wai(priv)))
return (k << 16);
-   k = inb(qbase + 5); /* should be 0x10, bus service */
+   inb(qbase + 5); /* should be 0x10, bus service */
}

What do you think?

Thank you for the clarification.
--
Gustavo A. R. Silva

Re: Need help with handling failed ATA pass-through command and sense data

[Alan Stern]

On Thu, 18 May 2017, Ewan D. Milne wrote:

> On Thu, 2017-05-18 at 11:34 -0400, Ewan D. Milne wrote:
> > On Thu, 2017-05-18 at 10:35 -0400, Alan Stern wrote:
> > > This is in reference to
> > > 
> > >   https://bugzilla.redhat.com/show_bug.cgi?id=1351305
> > > 
> > > The problem is that some program (probably udisks2) periodically sends
> > > the following ATA pass-through command to a USB-ATA bridge attached to
> > > a Western Digital drive:
> > > 
> > >   85062000   e500
> > 
> > According to T10 SAT-4 this is an ATA PASS-THROUGH(16) command, with
> > PROTOCOL=3 (non-data), CK_COND=1, COMMAND=E5h (the ATA command).
> > 
> > > 
> > > I don't know what this command does (some sort of reset?).  The command
> > > fails and the device returns the following sense data:
> > > 
> > >   7200 000e 090c 00ff  0050
> > > 
> > > I don't know how to decode this -- I don't have copies of the relevant
> > > documents.  Can anybody decode this for me?
> > 
> > This is a current descriptor format sense data wih a single
> > ATA Status Return sense data descriptor (beginning at 8 bytes into
> > the sense data buffer 090c...)  The relevant fields are COUNT=255 LBA=0
> > and STATUS=50h (which I suspect is what is the interesting part).
> > 
> > > 
> > > Anyway, the SCSI core treats it as a Hardware Error and puts warning
> > > messages in the kernel log:
> > > 
> > > [17244.280612] sd 7:0:0:0: [sdd] tag#0 FAILED Result: hostbyte=DID_ERROR 
> > > driverbyte=DRIVER_SENSE
> > > [17244.280614] sd 7:0:0:0: [sdd] tag#0 Sense Key : Hardware Error 
> > > [current] [descriptor] 
> > > [17244.280616] sd 7:0:0:0: [sdd] tag#0 Add. Sense: No additional sense 
> > > information
> > > [17244.280618] sd 7:0:0:0: [sdd] tag#0 CDB: ATA command pass through(16) 
> > > 85 06 20 00 00 00 00 00 00 00 00 00 00 00 e5 00
> > > 
> > > Is this really the right thing to do?  Could it be that we are failing 
> > > to interpret this sense data correctly?
> > 
> > With the 7200 000e 090c 00ff  0050 sense data
> > buffer above scsi_sense_key_string() should have returned "No Sense" as
> > the array value is 0.  Even if we somehow managed to fail to correctly
> > interpret descriptor format sense vs. fixed format sense.
> > 
> > > 
> > > Other commands provoke similar responses from the device, but without 
> > > obnoxious log messages.  For example, the command:
> > > 
> > >   85082e00 0100  ec00
> > > 
> > > fails with the following sense data:
> > > 
> > >   7200 000e 090c   0050
> > > 
> > > and no output to the log.  I don't know why the behavior is different.  
> > > There are other similar examples, with and without log messages.
> > 
> > It seems more likely that somehow there is a path where the wrong or
> > uninitialized sshdr structure is being passed to the logging routine.
> > 
> > -Ewan
> 
> Oh, wait.  You said USB-ATA bridge.
> 
> There is this code in drivers/usb/storage/transport.c:
> Perhaps this is responsible.  It is forcing the sense key to
> HARDWARE_ERROR under certain conditions.  That would cause the
> logging message you see to be output.
> 
> /*
>   
>
>  * We often get empty sense data.  This could indicate that   
>   
>
>  * everything worked or that there was an unspecified 
>   
>
>  * problem.  We have to decide which. 
>   
>
>  */
> if (sshdr.sense_key == 0 && sshdr.asc == 0 && sshdr.ascq == 0 
> &&
> fm_ili == 0) {
> /*
>   
>
>  * If things are really okay, then let's show that.   
>   
>
>  * Zero out the sense buffer so the higher layers 
>   
>
>  * won't realize we did an unsolicited auto-sense.
> 

Re: [PATCH v2 net-next] qed: Utilize FW 8.20.0.0

[David Miller]

From: Yuval Mintz 
Date: Thu, 18 May 2017 19:41:04 +0300

> This pushes qed [and as result, all qed* drivers] into using 8.20.0.0
> firmware. The changes are mostly contained in qed with minor changes
> to qedi due to some HSI changes.
> 
> Content-wise, the firmware contains fixes to various issues exposed
> since the release of the previous firmware, including:
>  - Corrects iSCSI fast retransmit when data digest is enabled.
>  - Stop draining packets when receiving several consecutive PFCs.
>  - Prevent possible assertion when consecutively opening/closing
>many connections.
>  - Prevent possible assertion due to too long BDQ fetch time.
> 
> In addition, the new firmware would allow us to later add iWARP support
> in qed and qedr.
> 
> Changes from previous version
> -
>  - V2: Fix warning in qed_debug.c 
> 
> Signed-off-by: Chad Dupuis 
> Signed-off-by: Ram Amrani 
> Signed-off-by: Tomer Tayar 
> Signed-off-by: Manish Rangankar 
> Signed-off-by: Yuval Mintz 

Applied, hopefully this one goes more smoothly.

Thanks.

Re: Need help with handling failed ATA pass-through command and sense data

[Ewan D. Milne]

On Thu, 2017-05-18 at 11:34 -0400, Ewan D. Milne wrote:
> On Thu, 2017-05-18 at 10:35 -0400, Alan Stern wrote:
> > This is in reference to
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1351305
> > 
> > The problem is that some program (probably udisks2) periodically sends
> > the following ATA pass-through command to a USB-ATA bridge attached to
> > a Western Digital drive:
> > 
> > 85062000   e500
> 
> According to T10 SAT-4 this is an ATA PASS-THROUGH(16) command, with
> PROTOCOL=3 (non-data), CK_COND=1, COMMAND=E5h (the ATA command).
> 
> > 
> > I don't know what this command does (some sort of reset?).  The command
> > fails and the device returns the following sense data:
> > 
> > 7200 000e 090c 00ff  0050
> > 
> > I don't know how to decode this -- I don't have copies of the relevant
> > documents.  Can anybody decode this for me?
> 
> This is a current descriptor format sense data wih a single
> ATA Status Return sense data descriptor (beginning at 8 bytes into
> the sense data buffer 090c...)  The relevant fields are COUNT=255 LBA=0
> and STATUS=50h (which I suspect is what is the interesting part).
> 
> > 
> > Anyway, the SCSI core treats it as a Hardware Error and puts warning
> > messages in the kernel log:
> > 
> > [17244.280612] sd 7:0:0:0: [sdd] tag#0 FAILED Result: hostbyte=DID_ERROR 
> > driverbyte=DRIVER_SENSE
> > [17244.280614] sd 7:0:0:0: [sdd] tag#0 Sense Key : Hardware Error [current] 
> > [descriptor] 
> > [17244.280616] sd 7:0:0:0: [sdd] tag#0 Add. Sense: No additional sense 
> > information
> > [17244.280618] sd 7:0:0:0: [sdd] tag#0 CDB: ATA command pass through(16) 85 
> > 06 20 00 00 00 00 00 00 00 00 00 00 00 e5 00
> > 
> > Is this really the right thing to do?  Could it be that we are failing 
> > to interpret this sense data correctly?
> 
> With the 7200 000e 090c 00ff  0050 sense data
> buffer above scsi_sense_key_string() should have returned "No Sense" as
> the array value is 0.  Even if we somehow managed to fail to correctly
> interpret descriptor format sense vs. fixed format sense.
> 
> > 
> > Other commands provoke similar responses from the device, but without 
> > obnoxious log messages.  For example, the command:
> > 
> > 85082e00 0100  ec00
> > 
> > fails with the following sense data:
> > 
> > 7200 000e 090c   0050
> > 
> > and no output to the log.  I don't know why the behavior is different.  
> > There are other similar examples, with and without log messages.
> 
> It seems more likely that somehow there is a path where the wrong or
> uninitialized sshdr structure is being passed to the logging routine.
> 
> -Ewan

Oh, wait.  You said USB-ATA bridge.

There is this code in drivers/usb/storage/transport.c:
Perhaps this is responsible.  It is forcing the sense key to
HARDWARE_ERROR under certain conditions.  That would cause the
logging message you see to be output.

/*  

   
 * We often get empty sense data.  This could indicate that 

   
 * everything worked or that there was an unspecified   

   
 * problem.  We have to decide which.   

   
 */
if (sshdr.sense_key == 0 && sshdr.asc == 0 && sshdr.ascq == 0 &&
fm_ili == 0) {
/*  

   
 * If things are really okay, then let's show that. 

   
 * Zero out the sense buffer so the higher layers   

   
 * won't realize we did an unsolicited auto-sense.  

   
 */
if (result == USB_STOR_TRANSPORT_GOOD) {
srb->result = 

RE: [PATCH] scsi: zero per-cmd driver data for each MQ I/O

[Long Li]

> -Original Message-
> From: Bart Van Assche [mailto:bart.vanass...@sandisk.com]
> Sent: Thursday, May 18, 2017 8:52 AM
> To: h...@infradead.org
> Cc: j...@linux.vnet.ibm.com; linux-scsi@vger.kernel.org; linux-
> ker...@vger.kernel.org; Stephen Hemminger ; KY
> Srinivasan ; Long Li ;
> martin.peter...@oracle.com
> Subject: Re: [PATCH] scsi: zero per-cmd driver data for each MQ I/O
> 
> On Wed, 2017-05-17 at 23:54 -0700, Christoph Hellwig wrote:
> > On Wed, May 17, 2017 at 11:05:18PM +, Bart Van Assche wrote:
> > > Thank you for the feedback. I'm working on a patch series that
> > > merges the scsi-sq and scsi-mq code paths for command initialization
> > > and that should fix the bug you encountered.
> >
> > While that sounds great (I tried it a while ago but gave up due to
> > priorities) I think we should merge this patch as-is and backport it
> > to stable for now and rebase your series on top of it.
> 
> Hello Christoph,
> 
> I will rebase my patch series on top of Long Li's patch. Long, please repost 
> your
> patch with a more detailed description and with the Reviewed-by and Cc:
> stable tags added.

Will do that.

> 
> Thanks,
> 
> Bart.

[PATCH 0/2] storvsc: changes for scsi next

[Stephen Hemminger]

These are refactoring changes to the Hyper-V scsi driver.

Stephen Hemminger (2):
  storvsc: use in place iterator function
  storvsc: remove unnecessary channel inbound lock

 drivers/hv/channel_mgmt.c  |  1 -
 drivers/scsi/storvsc_drv.c | 52 --
 include/linux/hyperv.h |  1 -
 3 files changed, 18 insertions(+), 36 deletions(-)

-- 
2.11.0

[PATCH 1/2] storvsc: use in place iterator function

[Stephen Hemminger]

In 4.12-rc1, new functions were added to support iterating
over elements in the vmbus event ring. This patch uses them to
simplify the ring buffer handling in virtual SCSI driver as well.

Signed-off-by: Stephen Hemminger 
---
 drivers/scsi/storvsc_drv.c | 44 +++-
 1 file changed, 15 insertions(+), 29 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index ae966dc3bbc5..f8a1649e4c63 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1149,13 +1149,9 @@ static void storvsc_on_receive(struct storvsc_device 
*stor_device,
 static void storvsc_on_channel_callback(void *context)
 {
struct vmbus_channel *channel = (struct vmbus_channel *)context;
+   const struct vmpacket_descriptor *desc;
struct hv_device *device;
struct storvsc_device *stor_device;
-   u32 bytes_recvd;
-   u64 request_id;
-   unsigned char packet[ALIGN(sizeof(struct vstor_packet), 8)];
-   struct storvsc_cmd_request *request;
-   int ret;
 
if (channel->primary_channel != NULL)
device = channel->primary_channel->device_obj;
@@ -1166,32 +1162,22 @@ static void storvsc_on_channel_callback(void *context)
if (!stor_device)
return;
 
-   do {
-   ret = vmbus_recvpacket(channel, packet,
-  ALIGN((sizeof(struct vstor_packet) -
-vmscsi_size_delta), 8),
-  _recvd, _id);
-   if (ret == 0 && bytes_recvd > 0) {
-
-   request = (struct storvsc_cmd_request *)
-   (unsigned long)request_id;
-
-   if ((request == _device->init_request) ||
-   (request == _device->reset_request)) {
-
-   memcpy(>vstor_packet, packet,
-  (sizeof(struct vstor_packet) -
-   vmscsi_size_delta));
-   complete(>wait_event);
-   } else {
-   storvsc_on_receive(stor_device,
-   (struct vstor_packet *)packet,
-   request);
-   }
+   foreach_vmbus_pkt(desc, channel) {
+   void *packet = hv_pkt_data(desc);
+   struct storvsc_cmd_request *request;
+
+   request = (struct storvsc_cmd_request *)
+   ((unsigned long)desc->trans_id);
+
+   if (request == _device->init_request ||
+   request == _device->reset_request) {
+   memcpy(>vstor_packet, packet,
+  (sizeof(struct vstor_packet) - 
vmscsi_size_delta));
+   complete(>wait_event);
} else {
-   break;
+   storvsc_on_receive(stor_device, packet, request);
}
-   } while (1);
+   }
 }
 
 static int storvsc_connect_to_vsp(struct hv_device *device, u32 ring_size,
-- 
2.11.0

[PATCH 2/2] storvsc: remove unnecessary channel inbound lock

[Stephen Hemminger]

In storvsc driver, inbound messages do not go through inbound lock.
The only effect of this lock was is to provide a barrier for
connect and remove logic.

Signed-off-by: Stephen Hemminger 
---
 drivers/hv/channel_mgmt.c  | 1 -
 drivers/scsi/storvsc_drv.c | 8 +++-
 include/linux/hyperv.h | 1 -
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 735f9363f2e4..685572bae1f0 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -332,7 +332,6 @@ static struct vmbus_channel *alloc_channel(void)
if (!channel)
return NULL;
 
-   spin_lock_init(>inbound_lock);
spin_lock_init(>lock);
 
INIT_LIST_HEAD(>sc_list);
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index f8a1649e4c63..8d955db6424f 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1206,13 +1206,13 @@ static int storvsc_connect_to_vsp(struct hv_device 
*device, u32 ring_size,
 static int storvsc_dev_remove(struct hv_device *device)
 {
struct storvsc_device *stor_device;
-   unsigned long flags;
 
stor_device = hv_get_drvdata(device);
 
-   spin_lock_irqsave(>channel->inbound_lock, flags);
stor_device->destroy = true;
-   spin_unlock_irqrestore(>channel->inbound_lock, flags);
+
+   /* Make sure flag is set before waiting */
+   wmb();
 
/*
 * At this point, all outbound traffic should be disable. We
@@ -1229,9 +1229,7 @@ static int storvsc_dev_remove(struct hv_device *device)
 * we have drained - to drain the outgoing packets, we need to
 * allow incoming packets.
 */
-   spin_lock_irqsave(>channel->inbound_lock, flags);
hv_set_drvdata(device, NULL);
-   spin_unlock_irqrestore(>channel->inbound_lock, flags);
 
/* Close the channel */
vmbus_close(device->channel);
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index e09fc8290c2f..b7d7bbec74e0 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -744,7 +744,6 @@ struct vmbus_channel {
u32 ringbuffer_pagecount;
struct hv_ring_buffer_info outbound;/* send to parent */
struct hv_ring_buffer_info inbound; /* receive from parent */
-   spinlock_t inbound_lock;
 
struct vmbus_close_msg close_msg;
 
-- 
2.11.0

Re: [PATCH] scsi: zero per-cmd driver data for each MQ I/O

[Bart Van Assche]

On Wed, 2017-05-17 at 23:54 -0700, Christoph Hellwig wrote:
> On Wed, May 17, 2017 at 11:05:18PM +, Bart Van Assche wrote:
> > Thank you for the feedback. I'm working on a patch series that merges the 
> > scsi-sq
> > and scsi-mq code paths for command initialization and that should fix the 
> > bug you
> > encountered.
> 
> While that sounds great (I tried it a while ago but gave up due to
> priorities) I think we should merge this patch as-is and backport it
> to stable for now and rebase your series on top of it.

Hello Christoph,

I will rebase my patch series on top of Long Li's patch. Long, please
repost your patch with a more detailed description and with the Reviewed-by
and Cc: stable tags added.

Thanks,

Bart.

Re: Need help with handling failed ATA pass-through command and sense data

[Ewan D. Milne]

On Thu, 2017-05-18 at 10:35 -0400, Alan Stern wrote:
> This is in reference to
> 
>   https://bugzilla.redhat.com/show_bug.cgi?id=1351305
> 
> The problem is that some program (probably udisks2) periodically sends
> the following ATA pass-through command to a USB-ATA bridge attached to
> a Western Digital drive:
> 
>   85062000   e500

According to T10 SAT-4 this is an ATA PASS-THROUGH(16) command, with
PROTOCOL=3 (non-data), CK_COND=1, COMMAND=E5h (the ATA command).

> 
> I don't know what this command does (some sort of reset?).  The command
> fails and the device returns the following sense data:
> 
>   7200 000e 090c 00ff  0050
> 
> I don't know how to decode this -- I don't have copies of the relevant
> documents.  Can anybody decode this for me?

This is a current descriptor format sense data wih a single
ATA Status Return sense data descriptor (beginning at 8 bytes into
the sense data buffer 090c...)  The relevant fields are COUNT=255 LBA=0
and STATUS=50h (which I suspect is what is the interesting part).

> 
> Anyway, the SCSI core treats it as a Hardware Error and puts warning
> messages in the kernel log:
> 
> [17244.280612] sd 7:0:0:0: [sdd] tag#0 FAILED Result: hostbyte=DID_ERROR 
> driverbyte=DRIVER_SENSE
> [17244.280614] sd 7:0:0:0: [sdd] tag#0 Sense Key : Hardware Error [current] 
> [descriptor] 
> [17244.280616] sd 7:0:0:0: [sdd] tag#0 Add. Sense: No additional sense 
> information
> [17244.280618] sd 7:0:0:0: [sdd] tag#0 CDB: ATA command pass through(16) 85 
> 06 20 00 00 00 00 00 00 00 00 00 00 00 e5 00
> 
> Is this really the right thing to do?  Could it be that we are failing 
> to interpret this sense data correctly?

With the 7200 000e 090c 00ff  0050 sense data
buffer above scsi_sense_key_string() should have returned "No Sense" as
the array value is 0.  Even if we somehow managed to fail to correctly
interpret descriptor format sense vs. fixed format sense.

> 
> Other commands provoke similar responses from the device, but without 
> obnoxious log messages.  For example, the command:
> 
>   85082e00 0100  ec00
> 
> fails with the following sense data:
> 
>   7200 000e 090c   0050
> 
> and no output to the log.  I don't know why the behavior is different.  
> There are other similar examples, with and without log messages.

It seems more likely that somehow there is a path where the wrong or
uninitialized sshdr structure is being passed to the logging routine.

-Ewan

> 
> Any help would be appreciated.
> 
> Alan Stern
> 

RE: [PATCH net-next] qed: Utilize FW 8.20.0.0

[Mintz, Yuval]

> >> This pushes qed [and as result, all qed* drivers] into using 8.20.0.0
> >> firmware. The changes are mostly contained in qed with minor changes
> >> to qedi due to some HSI changes.
> >>
> >> Content-wise, the firmware contains fixes to various issues exposed
> >> since the release of the previous firmware, including:
> >>  - Corrects iSCSI fast retransmit when data digest is enabled.
> >>  - Stop draining packets when receiving several consecutive PFCs.
> >>  - Prevent possible assertion when consecutively opening/closing
> >>many connections.
> >>  - Prevent possible assertion due to too long BDQ fetch time.
> >>
> >> In addition, the new firmware would allow us to later add iWARP
> >> support in qed and qedr.
> >>
> >> Signed-off-by: Chad Dupuis 
> >> Signed-off-by: Ram Amrani 
> >> Signed-off-by: Tomer Tayar 
> >> Signed-off-by: Manish Rangankar 
> >> Signed-off-by: Yuval Mintz 
> >
> > Applied.
> 
> Actually I had to revert.  Please look at the compiler output before
> submitting changes:
> 
> drivers/net/ethernet/qlogic/qed/qed_debug.c: In function ‘qed_grc_dump’:
> drivers/net/ethernet/qlogic/qed/qed_debug.c:2425:6: warning: ‘addr’ may
> be used uninitialized in this function [-Wmaybe-uninitialized]
>   u32 byte_addr = DWORDS_TO_BYTES(addr), offset = 0, i;
>   ^
> drivers/net/ethernet/qlogic/qed/qed_debug.c:3534:7: note: ‘addr’ was
> declared here
>u32 addr, size = RSS_REG_RSS_RAM_DATA_SIZE;
>^
> 
> 'addr' is never, ever, assigned a value, yet it is passed into a function as 
> an
> argument.

Sorry about that. Will send v2 [hopefully] later today.

[PATCH v2] scsi: csiostor: add support for Chelsio T6 adapters

[Varun Prakash]

Enable probe for T6 adapters, add code to flash T6
firmware and firmware config file, use T6 specific macros.

v2: updated commit message

Signed-off-by: Varun Prakash 
---
 drivers/scsi/csiostor/csio_hw.c  | 79 ++--
 drivers/scsi/csiostor/csio_hw_chip.h | 14 +++
 drivers/scsi/csiostor/csio_hw_t5.c   | 29 +
 drivers/scsi/csiostor/csio_init.c|  6 ++-
 drivers/scsi/csiostor/csio_wr.c  |  4 +-
 5 files changed, 88 insertions(+), 44 deletions(-)

diff --git a/drivers/scsi/csiostor/csio_hw.c b/drivers/scsi/csiostor/csio_hw.c
index 622bdab..5ca4099 100644
--- a/drivers/scsi/csiostor/csio_hw.c
+++ b/drivers/scsi/csiostor/csio_hw.c
@@ -794,18 +794,24 @@ csio_hw_dev_ready(struct csio_hw *hw)
 {
uint32_t reg;
int cnt = 6;
+   int src_pf;
 
while (((reg = csio_rd_reg32(hw, PL_WHOAMI_A)) == 0x) &&
   (--cnt != 0))
mdelay(100);
 
-   if ((cnt == 0) && (((int32_t)(SOURCEPF_G(reg)) < 0) ||
-  (SOURCEPF_G(reg) >= CSIO_MAX_PFN))) {
+   if (csio_is_t5(hw->pdev->device & CSIO_HW_CHIP_MASK))
+   src_pf = SOURCEPF_G(reg);
+   else
+   src_pf = T6_SOURCEPF_G(reg);
+
+   if ((cnt == 0) && (((int32_t)(src_pf) < 0) ||
+  (src_pf >= CSIO_MAX_PFN))) {
csio_err(hw, "PL_WHOAMI returned 0x%x, cnt:%d\n", reg, cnt);
return -EIO;
}
 
-   hw->pfn = SOURCEPF_G(reg);
+   hw->pfn = src_pf;
 
return 0;
 }
@@ -1581,10 +1587,16 @@ csio_hw_flash_config(struct csio_hw *hw, u32 
*fw_cfg_param, char *path)
unsigned int mtype = 0, maddr = 0;
uint32_t *cfg_data;
int value_to_add = 0;
+   const char *fw_cfg_file;
+
+   if (csio_is_t5(pci_dev->device & CSIO_HW_CHIP_MASK))
+   fw_cfg_file = FW_CFG_NAME_T5;
+   else
+   fw_cfg_file = FW_CFG_NAME_T6;
 
-   if (request_firmware(, FW_CFG_NAME_T5, dev) < 0) {
+   if (request_firmware(, fw_cfg_file, dev) < 0) {
csio_err(hw, "could not find config file %s, err: %d\n",
-FW_CFG_NAME_T5, ret);
+fw_cfg_file, ret);
return -ENOENT;
}
 
@@ -1623,9 +1635,8 @@ csio_hw_flash_config(struct csio_hw *hw, u32 
*fw_cfg_param, char *path)
ret = csio_memory_write(hw, mtype, maddr + size, 4, );
}
if (ret == 0) {
-   csio_info(hw, "config file upgraded to %s\n",
- FW_CFG_NAME_T5);
-   snprintf(path, 64, "%s%s", "/lib/firmware/", FW_CFG_NAME_T5);
+   csio_info(hw, "config file upgraded to %s\n", fw_cfg_file);
+   snprintf(path, 64, "%s%s", "/lib/firmware/", fw_cfg_file);
}
 
 leave:
@@ -1883,6 +1894,19 @@ static struct fw_info fw_info_array[] = {
.intfver_iscsi = FW_INTFVER(T5, ISCSI),
.intfver_fcoe = FW_INTFVER(T5, FCOE),
},
+   }, {
+   .chip = CHELSIO_T6,
+   .fs_name = FW_CFG_NAME_T6,
+   .fw_mod_name = FW_FNAME_T6,
+   .fw_hdr = {
+   .chip = FW_HDR_CHIP_T6,
+   .fw_ver = __cpu_to_be32(FW_VERSION(T6)),
+   .intfver_nic = FW_INTFVER(T6, NIC),
+   .intfver_vnic = FW_INTFVER(T6, VNIC),
+   .intfver_ri = FW_INTFVER(T6, RI),
+   .intfver_iscsi = FW_INTFVER(T6, ISCSI),
+   .intfver_fcoe = FW_INTFVER(T6, FCOE),
+   },
}
 };
 
@@ -1999,6 +2023,7 @@ csio_hw_flash_fw(struct csio_hw *hw, int *reset)
struct device *dev = _dev->dev ;
const u8 *fw_data = NULL;
unsigned int fw_size = 0;
+   const char *fw_bin_file;
 
/* This is the firmware whose headers the driver was compiled
 * against
@@ -2011,9 +2036,14 @@ csio_hw_flash_fw(struct csio_hw *hw, int *reset)
return -EINVAL;
}
 
-   if (request_firmware(, FW_FNAME_T5, dev) < 0) {
+   if (csio_is_t5(pci_dev->device & CSIO_HW_CHIP_MASK))
+   fw_bin_file = FW_FNAME_T5;
+   else
+   fw_bin_file = FW_FNAME_T6;
+
+   if (request_firmware(, fw_bin_file, dev) < 0) {
csio_err(hw, "could not find firmware image %s, err: %d\n",
-FW_FNAME_T5, ret);
+fw_bin_file, ret);
} else {
fw_data = fw->data;
fw_size = fw->size;
@@ -2238,9 +2268,14 @@ static void
 csio_hw_intr_enable(struct csio_hw *hw)
 {
uint16_t vec = (uint16_t)csio_get_mb_intr_idx(csio_hw_to_mbm(hw));
-   uint32_t pf = SOURCEPF_G(csio_rd_reg32(hw, PL_WHOAMI_A));
+   u32 pf = 0;
uint32_t pl = csio_rd_reg32(hw, PL_INT_ENABLE_A);
 
+   if (csio_is_t5(hw->pdev->device & CSIO_HW_CHIP_MASK))
+   

Re: [PATCH net-next] qed: Utilize FW 8.20.0.0

[David Miller]

From: David Miller 
Date: Thu, 18 May 2017 10:34:28 -0400 (EDT)

> From: Yuval Mintz 
> Date: Wed, 17 May 2017 22:38:40 +0300
> 
>> This pushes qed [and as result, all qed* drivers] into using 8.20.0.0
>> firmware. The changes are mostly contained in qed with minor changes
>> to qedi due to some HSI changes.
>> 
>> Content-wise, the firmware contains fixes to various issues exposed
>> since the release of the previous firmware, including:
>>  - Corrects iSCSI fast retransmit when data digest is enabled.
>>  - Stop draining packets when receiving several consecutive PFCs.
>>  - Prevent possible assertion when consecutively opening/closing
>>many connections.
>>  - Prevent possible assertion due to too long BDQ fetch time.
>> 
>> In addition, the new firmware would allow us to later add iWARP support
>> in qed and qedr.
>> 
>> Signed-off-by: Chad Dupuis 
>> Signed-off-by: Ram Amrani 
>> Signed-off-by: Tomer Tayar 
>> Signed-off-by: Manish Rangankar 
>> Signed-off-by: Yuval Mintz 
> 
> Applied.

Actually I had to revert.  Please look at the compiler output before
submitting changes:

drivers/net/ethernet/qlogic/qed/qed_debug.c: In function ‘qed_grc_dump’:
drivers/net/ethernet/qlogic/qed/qed_debug.c:2425:6: warning: ‘addr’ may be used 
uninitialized in this function [-Wmaybe-uninitialized]
  u32 byte_addr = DWORDS_TO_BYTES(addr), offset = 0, i;
  ^
drivers/net/ethernet/qlogic/qed/qed_debug.c:3534:7: note: ‘addr’ was declared 
here
   u32 addr, size = RSS_REG_RSS_RAM_DATA_SIZE;
   ^

'addr' is never, ever, assigned a value, yet it is passed into a function as an
argument.

Need help with handling failed ATA pass-through command and sense data

[Alan Stern]

This is in reference to

https://bugzilla.redhat.com/show_bug.cgi?id=1351305

The problem is that some program (probably udisks2) periodically sends
the following ATA pass-through command to a USB-ATA bridge attached to
a Western Digital drive:

85062000   e500

I don't know what this command does (some sort of reset?).  The command
fails and the device returns the following sense data:

7200 000e 090c 00ff  0050

I don't know how to decode this -- I don't have copies of the relevant
documents.  Can anybody decode this for me?

Anyway, the SCSI core treats it as a Hardware Error and puts warning
messages in the kernel log:

[17244.280612] sd 7:0:0:0: [sdd] tag#0 FAILED Result: hostbyte=DID_ERROR 
driverbyte=DRIVER_SENSE
[17244.280614] sd 7:0:0:0: [sdd] tag#0 Sense Key : Hardware Error [current] 
[descriptor] 
[17244.280616] sd 7:0:0:0: [sdd] tag#0 Add. Sense: No additional sense 
information
[17244.280618] sd 7:0:0:0: [sdd] tag#0 CDB: ATA command pass through(16) 85 06 
20 00 00 00 00 00 00 00 00 00 00 00 e5 00

Is this really the right thing to do?  Could it be that we are failing 
to interpret this sense data correctly?

Other commands provoke similar responses from the device, but without 
obnoxious log messages.  For example, the command:

85082e00 0100  ec00

fails with the following sense data:

7200 000e 090c   0050

and no output to the log.  I don't know why the behavior is different.  
There are other similar examples, with and without log messages.

Any help would be appreciated.

Alan Stern

Re: [PATCH net-next] qed: Utilize FW 8.20.0.0

[David Miller]

From: Yuval Mintz 
Date: Wed, 17 May 2017 22:38:40 +0300

> This pushes qed [and as result, all qed* drivers] into using 8.20.0.0
> firmware. The changes are mostly contained in qed with minor changes
> to qedi due to some HSI changes.
> 
> Content-wise, the firmware contains fixes to various issues exposed
> since the release of the previous firmware, including:
>  - Corrects iSCSI fast retransmit when data digest is enabled.
>  - Stop draining packets when receiving several consecutive PFCs.
>  - Prevent possible assertion when consecutively opening/closing
>many connections.
>  - Prevent possible assertion due to too long BDQ fetch time.
> 
> In addition, the new firmware would allow us to later add iWARP support
> in qed and qedr.
> 
> Signed-off-by: Chad Dupuis 
> Signed-off-by: Ram Amrani 
> Signed-off-by: Tomer Tayar 
> Signed-off-by: Manish Rangankar 
> Signed-off-by: Yuval Mintz 

Applied.

Re: [PATCH] lpfc: Fix NULL pointer dereference during PCI error recovery

[Guilherme G. Piccoli]

On 05/17/2017 09:21 PM, Martin K. Petersen wrote:
> 
> Guilherme,
> 
>> Recent commit on patchset "lpfc updates for 11.2.0.14" fixed an issue
>> about dereferencing a NULL pointer on port reset. The specific commit,
>> named "lpfc: Fix system crash when port is reset.", is missing a check
>> against NULL pointer on lpfc_els_flush_cmd() though.
>>
>> Since we destroy the queues on adapter resets, like in PCI error
>> recovery path, we need the validation present on this patch in order
>> to avoid a NULL pointer dereference when trying to flush commands of
>> ELS wq, after it has been destroyed (which would lead to a kernel
>> oops).
> 
> Applied to 4.12/scsi-fixes. Thank you!
> 

Thanks Martin!

RE: [PATCH] scsi: zero per-cmd driver data for each MQ I/O

[KY Srinivasan]

> -Original Message-
> From: Christoph Hellwig [mailto:h...@infradead.org]
> Sent: Wednesday, May 17, 2017 11:55 PM
> To: Bart Van Assche 
> Cc: j...@linux.vnet.ibm.com; linux-scsi@vger.kernel.org; linux-
> ker...@vger.kernel.org; Long Li ;
> martin.peter...@oracle.com; Stephen Hemminger
> ; KY Srinivasan 
> Subject: Re: [PATCH] scsi: zero per-cmd driver data for each MQ I/O
> 
> On Wed, May 17, 2017 at 11:05:18PM +, Bart Van Assche wrote:
> > Thank you for the feedback. I'm working on a patch series that merges the
> scsi-sq
> > and scsi-mq code paths for command initialization and that should fix the
> bug you
> > encountered.
> 
> While that sounds great (I tried it a while ago but gave up due to
> priorities) I think we should merge this patch as-is and backport it
> to stable for now and rebase your series on top of it.
Thanks Christoph.

K. Y

Re: [PATCH blktests 0/3] Add SCSI generic test group

[Johannes Thumshirn]

On 05/18/2017 03:19 PM, Christoph Hellwig wrote:
> All SG_IO test should also apply to block device nodes that support
> the ioctl..
> 

But these are not necessarily SG_IO tests, are they?

The test included is doesn't hit the SG_IO path in the sg driver, but
the sg_read path.

Of cause we can make a generic sg_io tool but IMHO it's quite convenient
to just throw in the reproducers we get.

And as it exercises the /dev/sg character device I decided to make a new
group.

Johannes

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH blktests 0/3] Add SCSI generic test group

[Christoph Hellwig]

All SG_IO test should also apply to block device nodes that support
the ioctl..

[PATCH blktests 1/3] Add ability to build test-cases

[Johannes Thumshirn]

Add the ability to build test cases from C files. This is handy for
things like syzcaller reproducers and all other kinds of test
binaries.

Signed-off-by: Johannes Thumshirn 
---
 Makefile   |  26 +++-
 src/.gitignore |   1 +
 src/Makefile   |  14 ++
 src/sg-001.c   | 430 +
 4 files changed, 470 insertions(+), 1 deletion(-)
 create mode 100644 src/.gitignore
 create mode 100644 src/Makefile
 create mode 100644 src/sg-001.c

diff --git a/Makefile b/Makefile
index 3a0f0100232c..a70623dd4a52 100644
--- a/Makefile
+++ b/Makefile
@@ -1,7 +1,31 @@
+ifeq ("$(origin V)", "command line")
+   BUILD_VERBOSE = $(V)
+else
+   BUILD_VERBOSE = 0
+endif
+ifndef BUILD_VERBOSE
+   BUILD_VERBOSE = 0
+endif
+
+ifeq ($(BUILD_VERBOSE),1)
+   Q =
+else
+   Q = @
+endif
+
+MAKEOPTS = --no-print-directory Q=$(Q)
+
+SUBDIRS = src
+default:
+   $(Q)$(MAKE) $(MAKEOPTS) -C $(SUBDIRS)
+
+clean:
+   $(Q)$(MAKE) $(MAKEOPTS) -C $(SUBDIRS) clean
+
 all:
@echo "Please read README.md"
 
 shellcheck:
shellcheck -x -f gcc check new common/* tests/*/[0-9]*[0-9]
 
-.PHONY: all shellcheck
+.PHONY: all shellcheck clean
diff --git a/src/.gitignore b/src/.gitignore
new file mode 100644
index ..f543ddb9280f
--- /dev/null
+++ b/src/.gitignore
@@ -0,0 +1 @@
+sg-001
diff --git a/src/Makefile b/src/Makefile
new file mode 100644
index ..8012c2e542c0
--- /dev/null
+++ b/src/Makefile
@@ -0,0 +1,14 @@
+CC = gcc
+CFLAGS = -O2
+
+TARGETS = sg-001
+
+FILES = $(TARGETS:=.c)
+
+$(TARGETS):
+   @echo "[CC]$@"
+   $(Q)$(CC) $@.c -o $@ $(CFLAGS)
+
+clean:
+   @echo "[CLEAN]  $(notdir $(CURDIR))"
+   $(Q)rm -f $(TARGETS) *.o
diff --git a/src/sg-001.c b/src/sg-001.c
new file mode 100644
index ..ace85af523b3
--- /dev/null
+++ b/src/sg-001.c
@@ -0,0 +1,430 @@
+// autogenerated by syzkaller (http://github.com/google/syzkaller)
+
+#ifndef __NR_read
+#define __NR_read 0
+#endif
+#ifndef __NR_mmap
+#define __NR_mmap 9
+#endif
+#ifndef __NR_syz_open_dev
+#define __NR_syz_open_dev 102
+#endif
+#ifndef __NR_write
+#define __NR_write 1
+#endif
+
+#define _GNU_SOURCE
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+const int kFailStatus = 67;
+const int kErrorStatus = 68;
+const int kRetryStatus = 69;
+
+__attribute__((noreturn)) void doexit(int status)
+{
+  volatile unsigned i;
+  syscall(__NR_exit_group, status);
+  for (i = 0;; i++) {
+  }
+}
+
+__attribute__((noreturn)) void fail(const char* msg, ...)
+{
+  int e = errno;
+  fflush(stdout);
+  va_list args;
+  va_start(args, msg);
+  vfprintf(stderr, msg, args);
+  va_end(args);
+  fprintf(stderr, " (errno %d)\n", e);
+  doexit((e == ENOMEM || e == EAGAIN) ? kRetryStatus : kFailStatus);
+}
+
+__attribute__((noreturn)) void exitf(const char* msg, ...)
+{
+  int e = errno;
+  fflush(stdout);
+  va_list args;
+  va_start(args, msg);
+  vfprintf(stderr, msg, args);
+  va_end(args);
+  fprintf(stderr, " (errno %d)\n", e);
+  doexit(kRetryStatus);
+}
+
+static int flag_debug;
+
+void debug(const char* msg, ...)
+{
+  if (!flag_debug)
+return;
+  va_list args;
+  va_start(args, msg);
+  vfprintf(stdout, msg, args);
+  va_end(args);
+  fflush(stdout);
+}
+
+__thread int skip_segv;
+__thread jmp_buf segv_env;
+
+static void segv_handler(int sig, siginfo_t* info, void* uctx)
+{
+  uintptr_t addr = (uintptr_t)info->si_addr;
+  const uintptr_t prog_start = 1 << 20;
+  const uintptr_t prog_end = 100 << 20;
+  if (__atomic_load_n(_segv, __ATOMIC_RELAXED) &&
+  (addr < prog_start || addr > prog_end)) {
+debug("SIGSEGV on %p, skipping\n", addr);
+_longjmp(segv_env, 1);
+  }
+  debug("SIGSEGV on %p, exiting\n", addr);
+  doexit(sig);
+  for (;;) {
+  }
+}
+
+static void install_segv_handler()
+{
+  struct sigaction sa;
+  memset(, 0, sizeof(sa));
+  sa.sa_sigaction = segv_handler;
+  sa.sa_flags = SA_NODEFER | SA_SIGINFO;
+  sigaction(SIGSEGV, , NULL);
+  sigaction(SIGBUS, , NULL);
+}
+
+#define NONFAILING(...)\
+  {\
+__atomic_fetch_add(_segv, 1, __ATOMIC_SEQ_CST);   \
+if (_setjmp(segv_env) == 0) {  \
+  __VA_ARGS__; \
+}  \
+__atomic_fetch_sub(_segv, 1, __ATOMIC_SEQ_CST);   \
+  }
+
+#define BITMASK_LEN(type, bf_len) (type)((1ull << (bf_len)) - 1)
+
+#define 

[PATCH blktests 0/3] Add SCSI generic test group

[Johannes Thumshirn]

Add a test group for the SCSI generic driver and one syzcaller
reproducer for this group.

The reprodcuer is distributed as a C program, so the makefile is
amended to build C files to be used in the test.

I didn't get the TIMEOUT to work (not even with block/001) so I
decided to just require the 'timeout' program in my testcase.

Johannes Thumshirn (3):
  Add ability to build test-cases
  tests/sg: add SCSI generic test grouop
  sg/001: add regression test for syzcaller generated GPF

 Makefile |  26 +++-
 common/sg|  22 +++
 src/.gitignore   |   1 +
 src/Makefile |  14 ++
 src/sg-001.c | 430 +++
 tests/sg/001 |  48 +++
 tests/sg/001.out |   2 +
 tests/sg/group   |  40 ++
 8 files changed, 582 insertions(+), 1 deletion(-)
 create mode 100644 common/sg
 create mode 100644 src/.gitignore
 create mode 100644 src/Makefile
 create mode 100644 src/sg-001.c
 create mode 100755 tests/sg/001
 create mode 100644 tests/sg/001.out
 create mode 100644 tests/sg/group

-- 
2.12.0

[PATCH blktests 2/3] tests/sg: add SCSI generic test grouop

[Johannes Thumshirn]

Add a test group for tests of the SCSI generic driver and and
functions common to the SCSI generic driver and it's test cases.

Signed-off-by: Johannes Thumshirn 
---
 common/sg  | 22 ++
 tests/sg/group | 40 
 2 files changed, 62 insertions(+)
 create mode 100644 common/sg
 create mode 100644 tests/sg/group

diff --git a/common/sg b/common/sg
new file mode 100644
index ..30b5089c68f7
--- /dev/null
+++ b/common/sg
@@ -0,0 +1,22 @@
+#!/bin/bash
+#
+# SCSI generic helper functions.
+#
+# Copyright (C) 2017 Johannes Thumshirn
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+
+_have_scsi_generic() {
+   _have_module sg
+}
diff --git a/tests/sg/group b/tests/sg/group
new file mode 100644
index ..55af9a2730eb
--- /dev/null
+++ b/tests/sg/group
@@ -0,0 +1,40 @@
+#!/bin/bash
+#
+# Regression tests for SCSI generic device
+#
+# Copyright (C) 2017 Johannes Thumshirn 
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+
+. common/sg
+
+group_requires() {
+   _have_scsi_generic
+}
+
+# TODO: if this test group has extra requirements for what devices it can be
+# run on, it should define a group_device_requires() function. If tests in this
+# group can be run on the test device, it should return zero. Otherwise, it
+# should return non-zero and set the $SKIP_REASON variable. $TEST_DEV is the
+# full path of the block device (e.g., /dev/nvme0n1 or /dev/sda1), and
+# $TEST_DEV_SYSFS is the sysfs path of the disk (not the partition, e.g.,
+# /sys/block/nvme0n1 or /sys/block/sda).
+#
+# Usually, group_device_requires() just needs to check that the test device is
+# the right type of hardware or supports any necessary features using the
+# _test_dev_foo helpers. If group_device_requires() returns non-zero, all tests
+# in this group will be skipped on that device.
+# group_device_requires() {
+#  _test_dev_is_foo && _test_dev_supports_bar
+# }
-- 
2.12.0

[PATCH blktests 3/3] sg/001: add regression test for syzcaller generated GPF

[Johannes Thumshirn]

Add a regression test for commit 48ae8484e9fc ("scsi: sg: don't return
bogus Sg_requests"). This is a general protection fault triggered by
syzcaller.

Signed-off-by: Johannes Thumshirn 
---
 tests/sg/001 | 48 
 tests/sg/001.out |  2 ++
 2 files changed, 50 insertions(+)
 create mode 100755 tests/sg/001
 create mode 100644 tests/sg/001.out

diff --git a/tests/sg/001 b/tests/sg/001
new file mode 100755
index ..3a72931d5748
--- /dev/null
+++ b/tests/sg/001
@@ -0,0 +1,48 @@
+#!/bin/bash
+#
+# Regression test for commit 48ae8484e9fc ("scsi: sg: don't return bogus 
Sg_requests")
+#
+# Copyright (C) 2017 Johannes Thumshirn 
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+
+. common/sg
+. common/scsi_debug
+
+DESCRIPTION="Regression test for commit 48ae8484e9fc (\"scsi: sg: don't return 
bogus Sg_requests\")"
+TIMED=1
+QUICK=1
+
+requires() {
+   _have_program src/sg-001 \
+   _have_program timeout \
+   && _have_scsi_debug \
+   && _have_scsi_generic
+}
+
+
+test() {
+   echo "Running ${TEST_NAME}"
+
+   if ! _get_scsi_debug_dev; then
+   return 1
+   fi
+
+   _divide_timeout 2
+   timeout -s INT 10s ./src/sg-001
+
+   _put_scsi_debug_dev
+   
+   echo "Test complete"
+}
diff --git a/tests/sg/001.out b/tests/sg/001.out
new file mode 100644
index ..beb4c437dd28
--- /dev/null
+++ b/tests/sg/001.out
@@ -0,0 +1,2 @@
+Running sg/001
+Test complete
-- 
2.12.0

[PATCH] scsi: lpfc: make a couple of functions static

[Colin King]

From: Colin Ian King 

functions lpfc_nvmet_cleanup_io_context and lpfc_nvmet_setup_io_context
can be made static as they do not need to be in global scope.

Cleans up sparse warnings:
  "warning: symbol 'lpfc_nvmet_cleanup_io_context' was not declared.
   Should it be static?"
  "warning: symbol 'lpfc_nvmet_setup_io_context' was not declared.
   Should it be static?"

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

diff --git a/drivers/scsi/lpfc/lpfc_nvmet.c b/drivers/scsi/lpfc/lpfc_nvmet.c
index f94294b77b7b..469958e3d50a 100644
--- a/drivers/scsi/lpfc/lpfc_nvmet.c
+++ b/drivers/scsi/lpfc/lpfc_nvmet.c
@@ -828,7 +828,7 @@ static struct nvmet_fc_target_template lpfc_tgttemplate = {
.target_priv_sz = sizeof(struct lpfc_nvmet_tgtport),
 };
 
-void
+static void
 lpfc_nvmet_cleanup_io_context(struct lpfc_hba *phba)
 {
struct lpfc_nvmet_ctxbuf *ctx_buf, *next_ctx_buf;
@@ -858,7 +858,7 @@ lpfc_nvmet_cleanup_io_context(struct lpfc_hba *phba)
}
 }
 
-int
+static int
 lpfc_nvmet_setup_io_context(struct lpfc_hba *phba)
 {
struct lpfc_nvmet_ctxbuf *ctx_buf;
-- 
2.11.0

Re: [PATCH 09/22] scsi: hisi_sas: retrieve SAS address for pci-based controller

[John Garry]

Hi Arnd,

Currently there is no pci device listed in the ACPI tables.

What I am doing is declaring a fake device in the root of the System bus
tree of the ACPI tables, and in the kernel driver finding it by matching the
name. It is not the ACPI companion for the pci device.

So I think that we can define the pci device under the pci bus in the ACPI
tables, and define the ADR and DSD. Then we would have an ACPI companion for
the device, and from that get the SAS address.


Yes, that would be much better, and allow us to use the device properties
interface directly. An additional advantage is that the property definition
can be exactly the same as for the v1/v2 platform_device properties
for anything that might be needed across all versions.  sas-addr already
fits in there, and there might be additional properties you need in the
future.



Hi Arnd,

OK, we'll try this.

As for other properties, phy and queue count are common to all versions.

However, I need to consider more if I should add these - maybe it is ok. 
The reason we did not hardcode these for v1/v2 was that these were 
variable for controllers inter and intra SoC.


hip08 only has 1 controller, and I have been told that each controller 
will have a unique pci device id that for future SoCs, so it's viable to 
hardcode.



An alternative to this ACPI device method is for UEFI to write the SAS
address to a defined free location in device's pci config space, which the
driver can read.


That sounds ok as well, it would be nice to not rely on firmware data here,
but I'd have to see how the implementation ends up: IIRC you should
not just put the data at a fixed location in the config space but instead use
the 'extended capabilities' infrastructure to find the data.
PCI_EXT_CAP_ID_VNDR might be the right one here, but I don't
know enough about this, so please consult with someone who does
(or the PCIe specification).



Right, we'll keep ACPI table description as plan A.


  Arnd



Much appreciated,
John


.

Re: [PATCH] scsi: smartpqi: mark PM functions as __maybe_unused

[Arnd Bergmann]

On Thu, May 18, 2017 at 10:32 AM, Arnd Bergmann  wrote:
> The newly added suspend/resume support causes harmless warnings
> when CONFIG_PM is disabled:
>
> smartpqi/smartpqi_init.c:5147:12: error: 'pqi_ctrl_wait_for_pending_io' 
> defined but not used [-Werror=unused-function]
> smartpqi/smartpqi_init.c:2019:13: error: 'pqi_wait_until_lun_reset_finished' 
> defined but not used [-Werror=unused-function]
> smartpqi/smartpqi_init.c:2013:13: error: 'pqi_wait_until_scan_finished' 
> defined but not used [-Werror=unused-function]
>
> We can avoid the warnings by removing the #ifdef around the
> handlers and instead marking them as __maybe_unused, which will
> let gcc drop the unused code silently.
>
> Fixes: f44d210312a6 ("scsi: smartpqi: add suspend and resume support")
> Signed-off-by: Arnd Bergmann 

I notice that today's linux-next no longer contains the patch that introduced
the warning.

 Arnd

[PATCH] scsi: smartpqi: mark PM functions as __maybe_unused

[Arnd Bergmann]

The newly added suspend/resume support causes harmless warnings
when CONFIG_PM is disabled:

smartpqi/smartpqi_init.c:5147:12: error: 'pqi_ctrl_wait_for_pending_io' defined 
but not used [-Werror=unused-function]
smartpqi/smartpqi_init.c:2019:13: error: 'pqi_wait_until_lun_reset_finished' 
defined but not used [-Werror=unused-function]
smartpqi/smartpqi_init.c:2013:13: error: 'pqi_wait_until_scan_finished' defined 
but not used [-Werror=unused-function]

We can avoid the warnings by removing the #ifdef around the
handlers and instead marking them as __maybe_unused, which will
let gcc drop the unused code silently.

Fixes: f44d210312a6 ("scsi: smartpqi: add suspend and resume support")
Signed-off-by: Arnd Bergmann 
---
 drivers/scsi/smartpqi/smartpqi_init.c | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/smartpqi/smartpqi_init.c 
b/drivers/scsi/smartpqi/smartpqi_init.c
index 0b11ae7e96dc..cb8f886e705c 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -6213,8 +6213,6 @@ static int pqi_ctrl_init(struct pqi_ctrl_info *ctrl_info)
return 0;
 }
 
-#if defined(CONFIG_PM)
-
 static void pqi_reinit_queues(struct pqi_ctrl_info *ctrl_info)
 {
unsigned int i;
@@ -6321,8 +6319,6 @@ static int pqi_ctrl_init_resume(struct pqi_ctrl_info 
*ctrl_info)
return 0;
 }
 
-#endif /* CONFIG_PM */
-
 static inline int pqi_set_pcie_completion_timeout(struct pci_dev *pci_dev,
u16 timeout)
 {
@@ -6696,9 +6692,7 @@ static void pqi_process_module_params(void)
pqi_process_lockup_action_param();
 }
 
-#if defined(CONFIG_PM)
-
-static int pqi_suspend(struct pci_dev *pci_dev, pm_message_t state)
+static __maybe_unused int pqi_suspend(struct pci_dev *pci_dev, pm_message_t 
state)
 {
struct pqi_ctrl_info *ctrl_info;
 
@@ -6728,7 +6722,7 @@ static int pqi_suspend(struct pci_dev *pci_dev, 
pm_message_t state)
return 0;
 }
 
-static int pqi_resume(struct pci_dev *pci_dev)
+static __maybe_unused int pqi_resume(struct pci_dev *pci_dev)
 {
int rc;
struct pqi_ctrl_info *ctrl_info;
@@ -6759,8 +6753,6 @@ static int pqi_resume(struct pci_dev *pci_dev)
return pqi_ctrl_init_resume(ctrl_info);
 }
 
-#endif /* CONFIG_PM */
-
 /* Define the PCI IDs for the controllers that we support. */
 static const struct pci_device_id pqi_pci_id_table[] = {
{
-- 
2.9.0

blktests and /dev/sg syzkaller reproducers

[Johannes Thumshirn]

Hi Omar,

I was thinking of adding some of the syszcaller reproducers I've
collected, which have been crashing the SCSI generic driver.

This would require blktests to have a build infrastructure (not a big
deal I know).

So my question is, are you comfortable with adding these tests (in a new
group probably) I'll look through my reproducer collection and work on a
patch set.

Byte,
Johannes
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH] scsi: csiostor: fix use after free in csio_hw_use_fwconfig()

[Johannes Thumshirn]

On 05/17/2017 05:00 PM, Varun Prakash wrote:
> mbp pointer is passed to csio_hw_validate_caps() so call
> mempool_free() after calling csio_hw_validate_caps().
> 
> Signed-off-by: Varun Prakash 
> ---

Fixes: 541c571fa2fd ("csiostor:Use firmware version from
cxgb4/t4fw_version.h")
Reviewed-by: Johannes Thumshirn 

Thanks,
Johannes
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH] scsi: zero per-cmd driver data for each MQ I/O

[Christoph Hellwig]

On Wed, May 17, 2017 at 11:05:18PM +, Bart Van Assche wrote:
> Thank you for the feedback. I'm working on a patch series that merges the 
> scsi-sq
> and scsi-mq code paths for command initialization and that should fix the bug 
> you
> encountered.

While that sounds great (I tried it a while ago but gave up due to
priorities) I think we should merge this patch as-is and backport it
to stable for now and rebase your series on top of it.

Re: [PATCH] scsi: csiostor: add support for Chelsio T6 adapters

[Johannes Thumshirn]

On 05/17/2017 05:00 PM, Varun Prakash wrote:
> Signed-off-by: Varun Prakash 
> ---

Please be a bit more verbose with the changelog.

Thanks,
Johannes
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH] scsi: zero per-cmd driver data for each MQ I/O

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig