RE: [GIT PULL] SCSI fixes for 4.17-rc2

[Sathya Prakash Veerichetty]

James,
The mpt3sas should be changed to mptsas.  Just responding back to have
record on this problem is with old mptsas and not with mpt3sas managed
controllers.

Thanks
Sathya

-Original Message-
From: linux-scsi-ow...@vger.kernel.org
[mailto:linux-scsi-ow...@vger.kernel.org] On Behalf Of James Bottomley
Sent: Wednesday, April 25, 2018 2:44 PM
To: Andrew Morton; Linus Torvalds
Cc: linux-scsi; linux-kernel
Subject: [GIT PULL] SCSI fixes for 4.17-rc2

8 bug fixes, one spelling update and one tracepoint addition.  The most
serious is probably the mpt3sas write same fix because it means anyone using
these controllers sees errors when modern filesystems try to issue discards
(if the drive supports the WRITE SAME variant).

The patch is available here:

git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-fixes

The short changelog is:

Bart Van Assche (1):
  scsi: sd_zbc: Avoid that resetting a zone fails sporadically

Chris Leech (1):
  scsi: iscsi: respond to netlink with unicast when appropriate

Colin Ian King (1):
  scsi: fnic: fix spelling mistake in fnic stats "Abord" -> "Abort"

Douglas Gilbert (1):
  scsi: scsi_debug: IMMED related delay adjustments

John Pittman (1):
  scsi: core: remove reference to scsi_show_extd_sense()

Mahesh Rajashekhara (1):
  scsi: sd: Defer spinning up drive while SANITIZE is in progress

Martin K. Petersen (1):
  scsi: mptsas: Disable WRITE SAME

Ming Lei (1):
  scsi: target: fix crash with iscsi target and dvd

Ohad Sharabi (1):
  scsi: ufs: add trace event for ufs upiu

Vinson Lee (1):
  scsi: megaraid_sas: Do not log an error if FW successfully
initializes.

And the diffstat:

 drivers/message/fusion/mptsas.c |   1 +
 drivers/scsi/fnic/fnic_trace.c  |   2 +-
 drivers/scsi/megaraid/megaraid_sas_fusion.c |   6 +-
 drivers/scsi/scsi_debug.c   |  33 +--
 drivers/scsi/scsi_transport_iscsi.c |  29 +++---
 drivers/scsi/sd.c   |   2 +
 drivers/scsi/sd_zbc.c   | 140

 drivers/scsi/ufs/ufshcd.c   |  40 
 drivers/target/target_core_pscsi.c  |   2 +
 include/linux/blkdev.h  |   5 +
 include/scsi/scsi_dbg.h |   2 -
 include/trace/events/ufs.h  |  27 ++
 12 files changed, 205 insertions(+), 84 deletions(-)

With full diff below

James

---

diff --git a/drivers/message/fusion/mptsas.c
b/drivers/message/fusion/mptsas.c index 231f3a1e27bf..86503f60468f 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -1994,6 +1994,7 @@ static struct scsi_host_template
mptsas_driver_template = {
.cmd_per_lun= 7,
.use_clustering = ENABLE_CLUSTERING,
.shost_attrs= mptscsih_host_attrs,
+   .no_write_same  = 1,
 };

 static int mptsas_get_linkerrors(struct sas_phy *phy) diff --git
a/drivers/scsi/fnic/fnic_trace.c b/drivers/scsi/fnic/fnic_trace.c index
abddde11982b..98597b59c12a 100644
--- a/drivers/scsi/fnic/fnic_trace.c
+++ b/drivers/scsi/fnic/fnic_trace.c
@@ -296,7 +296,7 @@ int fnic_get_stats_data(struct stats_debug_info *debug,
  "Number of Abort FW Timeouts: %lld\n"
  "Number of Abort IO NOT Found: %lld\n"

- "Abord issued times: \n"
+ "Abort issued times: \n"
  "< 6 sec : %lld\n"
  " 6 sec - 20 sec : %lld\n"
  "20 sec - 30 sec : %lld\n"
diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c
b/drivers/scsi/megaraid/megaraid_sas_fusion.c
index ce97cde3b41c..f4d988dd1e9d 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
@@ -1124,12 +1124,12 @@ megasas_ioc_init_fusion(struct megasas_instance
*instance)
goto fail_fw_init;
}

-   ret = 0;
+   return 0;

 fail_fw_init:
dev_err(>pdev->dev,
-   "Init cmd return status %s for SCSI host %d\n",
-   ret ? "FAILED" : "SUCCESS", instance->host->host_no);
+   "Init cmd return status FAILED for SCSI host %d\n",
+   instance->host->host_no);

return ret;
 }
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index
9ef5e3b810f6..656c98e116a9 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -234,11 +234,13 @@ static const char *sdebug_version_date = "20180128";
 #define F_INV_OP   0x200
 #define F_FAKE_RW  0x400
 #define F_M_ACCESS 0x800   /* media access */
-#define F_LONG_DELAY   0x1000
+#define F_SSU_DELAY0x1000
+#define F_SYNC_DELAY   0x2000

 #define FF_RESPOND (F_RL_WLUN_OK | F_SKIP_UA | F_DELAY_OVERR)  #define
FF_MEDIA_IO (F_M_ACCESS | F_FAKE_RW)  #define FF_SA (F_SA_HIGH | 

RE: [PATCH v1] scsi: mpt3sas: Use lo_hi_writeq() helper

[Sathya Prakash Veerichetty]

The writeq is defined to ensure both the registers are written without any
interleaved access in between the two writels in 32 bit systems.  Also we
want to retain the order of writing the registers.  If needed I can dig out
the issue we have faced using writeq in 32 bit systems (happened five/six
years back).  So I would prefer leave the code as it is.

Thanks
Sathya
-Original Message-
From: mpt-fusionlinux@broadcom.com
[mailto:mpt-fusionlinux@broadcom.com] On Behalf Of James Bottomley
Sent: Wednesday, February 14, 2018 1:02 PM
To: Andy Shevchenko; Sathya Prakash; Chaitra P B; Suganath Prabu Subramani;
mpt-fusionlinux@broadcom.com; Martin K. Petersen;
linux-scsi@vger.kernel.org
Subject: Re: [PATCH v1] scsi: mpt3sas: Use lo_hi_writeq() helper

On Wed, 2018-02-14 at 21:48 +0200, Andy Shevchenko wrote:
> On Wed, 2018-02-14 at 11:40 -0800, James Bottomley wrote:
> >
> > On Wed, 2018-02-14 at 20:10 +0200, Andy Shevchenko wrote:
> > >
> > > Since we have a writeq() for 32-bit architectures as provided by
> > > IO non-atomic helpers, there is no need to open code it.
> > >
> > > Moreover sparse complains about this:
> > >
> > > drivers/scsi/mpt3sas/mpt3sas_base.c:2975:16: expected unsigned
> > > long long val
> > > drivers/scsi/mpt3sas/mpt3sas_base.c:2975:16: got restricted
> > > __le64
> > > 
> > >
> > > Fixing this by replacing custom writeq() with one provided by
> > > io-64-nonatomic-lo-hi.h header.
> > >
>
> >
> > >
> > > -#if defined(writeq) && defined(CONFIG_64BIT) -static inline void
> > > -_base_writeq(__u64 b, volatile void __iomem *addr, spinlock_t
> > > *writeq_lock)
> > > -{
> > > - writeq(cpu_to_le64(b), addr);
> > > -}
> > > -#else
> > >  static inline void
> > >  _base_writeq(__u64 b, volatile void __iomem *addr, spinlock_t
> > > *writeq_lock)
> > >  {
> > > @@ -2985,11 +2979,9 @@ _base_writeq(__u64 b, volatile void __iomem
> > > *addr, spinlock_t *writeq_lock)
> > >   __u64 data_out = cpu_to_le64(b);
> > >
> > >   spin_lock_irqsave(writeq_lock, flags);
> > > - writel((u32)(data_out), addr);
> > > - writel((u32)(data_out >> 32), (addr + 4));
> > > + writeq(data_out, addr);
> > >   spin_unlock_irqrestore(writeq_lock, flags);
> > >  }
> > > -#endif
> >
> > This would rather defeat the purpose of the original code, I think.
>
> James, TBH, I don't see any value of that lock. What it's protecting
> from? simultaneous thread doing writeq()? But this is pointless if at
> the same time you will have writel() to the device.

The lock prevents two threads doing an interleaved write to this specific
address which could be caused by two threads writing to the same address.
If I remember correctly the firmware hangs in that case.

> For my opinion perhaps complete removal of the custom writeq() and
> replacing it by just writeq() with enabled non-atomic helpers will do
> the job.
>
> The code is very old, and I have no idea why it's done this (strange)
> way.

The write seems to trigger starting the engine on the HBA if you look at the
code, which is why it must be written completely and in order.
 It's equivalent to a mailbox post.

James

RE: [PATCH 13/14] megaraid_sas: NVME passthru command support

[Sathya Prakash Veerichetty]

>>So even when used as a RAID member, there will be a device handle at
/dev/sdX for each NVMe device the megaraid controller manages?
In megaraid controller, you can expose bare NVMe drives and RAID volumes
created out of NVMe drives, when the RAID volume is created underlying
member drives will not have /dev/sdX entries associated with them, however
for bare NVMe drives there will be associated /dev/sdX entries.

RE: [PATCH 13/14] megaraid_sas: NVME passthru command support

[Sathya Prakash Veerichetty]

Bart et al,
Broadcom's Tri-mode HBAs and MegaRAID controllers are capable of
connecting with SAS, SATA, NVMe drives,  SAS expanders and PCIe switches
(with NVMe drives connected behind that) and are capable of creating RAID
volumes on top of similar family of drives.  In the case of RAID
controllers, all of those drives and RAID volumes are exposed to the OS as
generic SCSI devices and in the case of HBA only for SAS and SATA the
topology is exposed to OS through SAS transport layer and NVMe drives are
exposed as generic SCSI devices.  The SCSI CDB to specific packet (SATA
frames, SSP frames or NVMe) translation occurs in the hardware/firmware.
For the OS driver, the interface to interact is common across all the type
of devices and it is MPI SCSI IO Request.

The NVMe passthru support added in this patch is only for management
purpose and will let Broadcom specific management applications to send
some direct NVMe commands to the hardware/firmware solely for management
purpose.  For normal READ/WRITE I/O the preferred path is to issue SCSI
command to our hardware/firmware and let it translate to the NVMe.

We have many architectural constraints to directly expose NVMe drives to
NVMe subsystem for normal I/O usage and management usage and hence we
prefer not to go down the path.

This patch is just addition of new feature for our management applications
(which are common across many OSes) to access a specific type of MPI
command to manage NVMe drives connected behind our HBAs (which are
non-standard) in a vendor specific way, hence we think this patch is valid
to be accepted to megaraid driver.  Please let us know if more details are
required on the tri-mode controllers.

Thanks
Sathya


-Original Message-
From: Linux-nvme [mailto:linux-nvme-boun...@lists.infradead.org] On Behalf
Of Douglas Gilbert
Sent: Wednesday, January 10, 2018 1:06 PM
To: Bart Van Assche; h...@infradead.org; kashyap.de...@broadcom.com;
shivasharan.srikanteshw...@broadcom.com
Cc: sumit.sax...@broadcom.com; peter.riv...@broadcom.com;
linux-n...@lists.infradead.org; linux-scsi@vger.kernel.org
Subject: Re: [PATCH 13/14] megaraid_sas: NVME passthru command support

On 2018-01-10 11:22 AM, Bart Van Assche wrote:
> On Tue, 2018-01-09 at 22:07 +0530, Kashyap Desai wrote:
>> Overall NVME support behind MR controller is really a SCSI device. On
>> top of that, for MegaRaid, NVME device can be part of Virtual Disk
>> and those drive will not be exposed to the driver. User application
>> may like to talk to hidden NVME devices (part of VDs). This patch
>> will extend the existing interface for megaraid product in the same
>> way it is currently supported for other protocols like SMP, SATA
pass-through.
>
> It seems to me like there is a contradiction in the above paragraph:
> if some NVMe devices are not exposed to the driver, how can a user
> space application ever send NVMe commands to it?

I think that he meant that the NVMe physical devices (e.g. SSDs) are not
exposed to the upper layers (e.g. the SCSI mid-layer and above). The SCSI
subsystem has a no_uld_attach device flag that lets a LLD attach physical
devices but the sd driver and hence the block layer do not "see" them. The
idea is that maintenance programs like smartmontools can use them via the
bsg or sg drivers. The Megaraid driver code does not seem to use
no_uld_attach. Does the NVMe subsystem have similar "generic" (i.e.
non-block) devices accessible to the user space?

> Anyway, has it been considered to implement the NVMe support as an
> NVMe transport driver? The upstream kernel already supports NVMe
> communication with NVMe PCI devices, NVMe over RDMA and NVMe over FC.
> If communication to the NVMe devices behind the MegaRaid controller
> would be implemented as an NVMe transport driver then all
> functionality of the Linux NVMe driver could be reused, including its
sysfs entries.

Broadcom already sell "SAS" HBAs that have "tri-mode" phys. That is a phy
that can connect to a SAS device (e.g. a SAS expander), a SATA device or a
NVMe device. Now if I was Broadcom designing a 24 Gbps SAS-4 next
generation expander I would be thinking of using those tri-mode phys on
it. But then there is a problem, SAS currently supports 3 protocols: SSP
(for SCSI storage and enclosure management (SES)), STP (for SATA storage )
and SMP (for expander management). The problem is how those NVMe commands,
status and data cross the wire between the OS HBA (or MegaRaid type
controller) and an expander. Solving that might need some lateral
thinking.

On one hand the NVM Express folks seem to have shelved the idea of a SCSI
to NVMe Translation Layer (SNTL) and have not updated an old white paper
on the subject. Currently there is no SNTL on Linux (there was but it was
removed) or FreeBSD but there is one on Windows.

On the other hand I'm informed that recently the same body accepted the
SES-3 standard pretty much as-is. That is done with the addition of SES
Send and SES Receive commands to 

RE: [patch 2/2] scsi: mpt3sas: remove a stray KERN_INFO

[Sathya Prakash Veerichetty]

Thanks.
Acked-by: Sathya Prakash Veerichetty <sathya.prak...@broadcom.com>

-Original Message-
From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
Sent: Wednesday, November 8, 2017 1:38 AM
To: Sathya Prakash; Suganath Prabu Subramani
Cc: Chaitra P B; James E.J. Bottomley; Martin K. Petersen;
mpt-fusionlinux@broadcom.com; linux-scsi@vger.kernel.org;
kernel-janit...@vger.kernel.org
Subject: [patch 2/2] scsi: mpt3sas: remove a stray KERN_INFO

pr_info() has a KERN_INFO already so the second KERN_INFO isn't needed.

Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 3a9438a1704e..b258f210120a 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -8683,7 +8684,7 @@ _scsih_mark_responding_pcie_device(struct
MPT3SAS_ADAPTER *ioc,

if (pcie_device->handle ==
pcie_device_pg0->DevHandle)
goto out;
-   pr_info(KERN_INFO "\thandle changed
from(0x%04x)!!!\n",
+   pr_info("\thandle changed from(0x%04x)!!!\n",
pcie_device->handle);
pcie_device->handle = pcie_device_pg0->DevHandle;
if (sas_target_priv_data)

RE: [PATCH 1/2] scsi: mpt3sas: cleanup _scsih_pcie_enumeration_event()

[Sathya Prakash Veerichetty]

Thanks.

Acked-by: Sathya Prakash Veerichetty <sathya.prak...@broadcom.com>

-Original Message-
From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
Sent: Wednesday, November 8, 2017 1:38 AM
To: Sathya Prakash; Suganath Prabu Subramani
Cc: Chaitra P B; James E.J. Bottomley; Martin K. Petersen;
mpt-fusionlinux@broadcom.com; linux-scsi@vger.kernel.org;
kernel-janit...@vger.kernel.org
Subject: [PATCH 1/2] scsi: mpt3sas: cleanup
_scsih_pcie_enumeration_event()

The indenting wasn't right, because the last two prints weren't indented
far enough.  Also it used pr_info() where it was supposed to use
pr_cont().  I reversed the if statement and pulled the code in one tab and
did a couple other minor cleanups.

Fixes: 4318c7347847 ("scsi: mpt3sas: Handle NVMe PCIe device related
events generated from firmware.")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 3a9438a1704e..93b45e618edb 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -7700,17 +7700,18 @@ _scsih_pcie_enumeration_event(struct
MPT3SAS_ADAPTER *ioc,
Mpi26EventDataPCIeEnumeration_t *event_data =
(Mpi26EventDataPCIeEnumeration_t *)fw_event->event_data;

-   if (ioc->logging_level & MPT_DEBUG_EVENT_WORK_TASK) {
-   pr_info(MPT3SAS_FMT "pcie enumeration event: (%s) Flag
0x%02x",
-   ioc->name,
-   ((event_data->ReasonCode ==
-   MPI26_EVENT_PCIE_ENUM_RC_STARTED) ?
-   "started" : "completed"), event_data->Flags);
+   if (!(ioc->logging_level & MPT_DEBUG_EVENT_WORK_TASK))
+   return;
+
+   pr_info(MPT3SAS_FMT "pcie enumeration event: (%s) Flag 0x%02x",
+   ioc->name,
+   (event_data->ReasonCode ==
MPI26_EVENT_PCIE_ENUM_RC_STARTED) ?
+   "started" : "completed",
+   event_data->Flags);
if (event_data->EnumerationStatus)
-   pr_info("enumeration_status(0x%08x)",
-   le32_to_cpu(event_data->EnumerationStatus));
-   pr_info("\n");
-   }
+   pr_cont("enumeration_status(0x%08x)",
+   le32_to_cpu(event_data->EnumerationStatus));
+   pr_cont("\n");
 }

 /**

RE: [bug report] scsi: mpt3sas: Added support for nvme encapsulated request message.

[Sathya Prakash Veerichetty]

Dan,
The MPI structures are of variable length and can go up to a maximum of
128 bytes (a MPI frame size) and as MPI standard the variable length MPI
structures are left out with the last element as a single dword array.
Can we ignore the warning?  If not we need to modify the MPI structure to
have the NVMe_Command array to the maximum size of the frame (which is
typically 128 but can change across hardware generations)

Thanks
Sathya

-Original Message-
From: mpt-fusionlinux@broadcom.com
[mailto:mpt-fusionlinux@broadcom.com] On Behalf Of Dan Carpenter
Sent: Tuesday, November 7, 2017 4:34 AM
To: suganath-prabu.subram...@broadcom.com
Cc: mpt-fusionlinux@broadcom.com; linux-scsi@vger.kernel.org
Subject: [bug report] scsi: mpt3sas: Added support for nvme encapsulated
request message.

Hello Suganath Prabu Subramani,

The patch aff39e61218f: "scsi: mpt3sas: Added support for nvme
encapsulated request message." from Oct 31, 2017, leads to the following
static checker warning:

drivers/scsi/mpt3sas/mpt3sas_base.c:1459 _base_build_nvme_prp()
error: buffer overflow 'nvme_encap_request->NVMe_Command' 4 <= 24

drivers/scsi/mpt3sas/mpt3sas_base.c
  1453  /*
  1454   * Set pointers to PRP1 and PRP2, which are in the NVMe
command.
  1455   * PRP1 is located at a 24 byte offset from the start of
the NVMe
^^^ The ->NVMe_Command is
declared as a 4 byte array so this makes static checkers puzzled how there
are more than 24 bytes in it.

  1456   * command.  Then set the current PRP entry pointer to
PRP1.
  1457   */
  1458  prp1_entry = (__le64 *)(nvme_encap_request->NVMe_Command +
  1459  NVME_CMD_PRP1_OFFSET);
  1460  prp2_entry = (__le64 *)(nvme_encap_request->NVMe_Command +
  1461  NVME_CMD_PRP2_OFFSET);
  1462  prp_entry = prp1_entry;
  1463  /*
  1464   * For the PRP entries, use the specially allocated buffer
of
  1465   * contiguous memory.
  1466   */
  1467  prp_page = (__le64 *)mpt3sas_base_get_pcie_sgl(ioc, smid);
  1468  prp_page_phys = (__le64
*)mpt3sas_base_get_pcie_sgl_dma(ioc, smid);
  1469

regards,
dan carpenter

RE: [PATCH] mpt3sas: fix dma_addr_t casts

[Sathya Prakash Veerichetty]

Acked-by: Sathya Prakash Veerichetty <sathya.prak...@broadcom.com>

-Original Message-
From: Arnd Bergmann [mailto:a...@arndb.de]
Sent: Monday, November 6, 2017 6:35 AM
To: Sathya Prakash; Chaitra P B; Suganath Prabu Subramani; James E.J.
Bottomley; Martin K. Petersen
Cc: Arnd Bergmann; Tomas Henzl; Sreekanth Reddy; Hannes Reinecke; Romain
Perier; James Bottomley; Bart Van Assche;
mpt-fusionlinux@broadcom.com; linux-scsi@vger.kernel.org;
linux-ker...@vger.kernel.org
Subject: [PATCH] mpt3sas: fix dma_addr_t casts

The newly added base_make_prp_nvme function triggers a build warning on
some 32-bit configurations:

drivers/scsi/mpt3sas/mpt3sas_base.c: In function 'base_make_prp_nvme':
drivers/scsi/mpt3sas/mpt3sas_base.c:1664:13: error: cast from pointer to
integer of different size [-Werror=pointer-to-int-cast]
  msg_phys = (dma_addr_t)mpt3sas_base_get_pcie_sgl_dma(ioc, smid);

After taking a closer look, I found that the problem is that the new code
mixes up pointers and dma_addr_t values unnecessarily.

This changes it to use the correct types consistently, which lets us get
rid of a lot of type casts in the process. I'm also renaming some
variables to avoid confusion between physical and dma address spaces that
are often distinct.

Fixes: 016d5c35e278 ("scsi: mpt3sas: SGL to PRP Translation for I/Os to
NVMe devices")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 62
+
 drivers/scsi/mpt3sas/mpt3sas_base.h |  2 +-
 2 files changed, 30 insertions(+), 34 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 7a3f4d14f260..8027de465d47 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1437,11 +1437,11 @@ _base_build_nvme_prp(struct MPT3SAS_ADAPTER *ioc,
u16 smid,
size_t data_in_sz)
 {
int prp_size = NVME_PRP_SIZE;
-   __le64  *prp_entry, *prp1_entry, *prp2_entry,
*prp_entry_phys;
-   __le64  *prp_page, *prp_page_phys;
+   __le64  *prp_entry, *prp1_entry, *prp2_entry;
+   __le64  *prp_page;
+   dma_addr_t  prp_entry_dma, prp_page_dma, dma_addr;
u32 offset, entry_len;
u32 page_mask_result, page_mask;
-   dma_addr_t  paddr;
size_t  length;

/*
@@ -1465,7 +1465,7 @@ _base_build_nvme_prp(struct MPT3SAS_ADAPTER *ioc,
u16 smid,
 * contiguous memory.
 */
prp_page = (__le64 *)mpt3sas_base_get_pcie_sgl(ioc, smid);
-   prp_page_phys = (__le64 *)mpt3sas_base_get_pcie_sgl_dma(ioc,
smid);
+   prp_page_dma = mpt3sas_base_get_pcie_sgl_dma(ioc, smid);

/*
 * Check if we are within 1 entry of a page boundary we don't @@
-1476,21 +1476,21 @@ _base_build_nvme_prp(struct MPT3SAS_ADAPTER *ioc, u16
smid,
if (!page_mask_result) {
/* Bump up to next page boundary. */
prp_page = (__le64 *)((u8 *)prp_page + prp_size);
-   prp_page_phys = (__le64 *)((u8 *)prp_page_phys +
prp_size);
+   prp_page_dma = prp_page_dma + prp_size;
}

/*
 * Set PRP physical pointer, which initially points to the current
PRP
 * DMA memory page.
 */
-   prp_entry_phys = prp_page_phys;
+   prp_entry_dma = prp_page_dma;

/* Get physical address and length of the data buffer. */
if (data_in_sz) {
-   paddr = data_in_dma;
+   dma_addr = data_in_dma;
length = data_in_sz;
} else {
-   paddr = data_out_dma;
+   dma_addr = data_out_dma;
length = data_out_sz;
}

@@ -1500,8 +1500,7 @@ _base_build_nvme_prp(struct MPT3SAS_ADAPTER *ioc,
u16 smid,
 * Check if we need to put a list pointer here if we are
at
 * page boundary - prp_size (8 bytes).
 */
-   page_mask_result =
-   (uintptr_t)((u8 *)prp_entry_phys + prp_size) &
page_mask;
+   page_mask_result = (prp_entry_dma + prp_size) & page_mask;
if (!page_mask_result) {
/*
 * This is the last entry in a PRP List, so we
need to @@ -1515,13 +1514,13 @@ _base_build_nvme_prp(struct
MPT3SAS_ADAPTER *ioc, u16 smid,
 * contiguous, no need to get a new page -
it's
 * just the next address.
 */
-   prp_entry_phys++;
-   *prp_entry =
cpu_to_le64((uintptr_t)prp_entry_phys);
+   prp_entry_dma++;
+   *prp_entry = cpu_to_le64(prp_entry_dma);
prp_entry++;
}

/* Need to handle if entry will be part of a page. */
-   o

RE: [PATCH 3.10 141/319] scsi: mpt3sas: Fix secure erase premature termination

[Sathya Prakash Veerichetty]

Willy,
I think this patch had a problem and later modified to a different
blocking mechanism.  Could you please pull in the latest change for this?

Thanks
Sathya

-Original Message-
From: Willy Tarreau [mailto:w...@1wt.eu]
Sent: Sunday, February 05, 2017 12:19 PM
To: linux-ker...@vger.kernel.org; sta...@vger.kernel.org;
li...@roeck-us.net
Cc: Andrey Grodzovsky; linux-scsi@vger.kernel.org; Sathya Prakash; Chaitra
P B; Suganath Prabu Subramani; Sreekanth Reddy; Hannes Reinecke; Martin K
. Petersen; Willy Tarreau
Subject: [PATCH 3.10 141/319] scsi: mpt3sas: Fix secure erase premature
termination

From: Andrey Grodzovsky 

commit 18f6084a989ba1b38702f9af37a2e4049a924be6 upstream.

This is a work around for a bug with LSI Fusion MPT SAS2 when perfoming
secure erase. Due to the very long time the operation takes, commands
issued during the erase will time out and will trigger execution of the
abort hook. Even though the abort hook is called for the specific command
which timed out, this leads to entire device halt (scsi_state terminated)
and premature termination of the secure erase.

Set device state to busy while ATA passthrough commands are in progress.

[mkp: hand applied to 4.9/scsi-fixes, tweaked patch description]

Signed-off-by: Andrey Grodzovsky 
Acked-by: Sreekanth Reddy 
Cc: 
Cc: Sathya Prakash 
Cc: Chaitra P B 
Cc: Suganath Prabu Subramani 
Cc: Sreekanth Reddy 
Cc: Hannes Reinecke 
Signed-off-by: Martin K. Petersen 
Signed-off-by: Willy Tarreau 
---
 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index f8c4b85..e414b71 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -3515,6 +3515,10 @@ _scsih_eedp_error_handling(struct scsi_cmnd *scmd,
u16 ioc_status)
SAM_STAT_CHECK_CONDITION;
 }

+static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd) {
+   return (scmd->cmnd[0] == ATA_12 || scmd->cmnd[0] == ATA_16); }

 /**
  * _scsih_qcmd_lck - main scsi request entry point @@ -3543,6 +3547,13 @@
_scsih_qcmd_lck(struct scsi_cmnd *scmd, void (*done)(struct scsi_cmnd *))
scsi_print_command(scmd);
 #endif

+   /*
+* Lock the device for any subsequent command until command is
+* done.
+*/
+   if (ata_12_16_cmd(scmd))
+   scsi_internal_device_block(scmd->device);
+
scmd->scsi_done = done;
sas_device_priv_data = scmd->device->hostdata;
if (!sas_device_priv_data || !sas_device_priv_data->sas_target) {
@@ -4046,6 +4057,9 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16
smid, u8 msix_index, u32 reply)
if (scmd == NULL)
return 1;

+   if (ata_12_16_cmd(scmd))
+   scsi_internal_device_unblock(scmd->device, SDEV_RUNNING);
+
mpi_request = mpt3sas_base_get_msg_frame(ioc, smid);

if (mpi_reply == NULL) {
--
2.8.0.rc2.1.gbe9624a

RE: [PATCH] mpt3sas: Force request partial completion alignment

[Sathya Prakash Veerichetty]

Sreekanth,
Let us discuss internally more about this before ACKING this patch.

Thanks
Sathya

-Original Message-
From: Sreekanth Reddy [mailto:sreekanth.re...@broadcom.com]
Sent: Wednesday, January 04, 2017 4:33 AM
To: Guilherme G. Piccoli
Cc: linux-scsi@vger.kernel.org; PDL-MPT-FUSIONLINUX; Sathya Prakash; Chaitra
Basappa; Suganath Prabu Subramani; Brian King; mauri...@linux.vnet.ibm.com;
linux...@us.ibm.com
Subject: Re: [PATCH] mpt3sas: Force request partial completion alignment

Hi Guilherme,

Can please share us the driver logs (with driver logging_level set to
0x3f8) for the original issue (i.e. without this patch changes) and the HBA
firmware version you have used.

Also can you please share us the steps you have followed to reproduce this
issue, so that I can try on my setup.

Thanks,
Sreekanth

On Wed, Dec 28, 2016 at 6:21 PM, Guilherme G. Piccoli
 wrote:
> From: Ram Pai 
>
> The firmware or device, possibly under a heavy I/O load, can return on
> a partial unaligned boundary. Scsi-ml expects these requests to be
> completed on an alignment boundary. Scsi-ml blindly requeues the I/O
> without checking the alignment boundary of the I/O request for the
> remaining bytes. This leads to errors, since devices cannot perform
> non-aligned read/write operations.
>
> This patch fixes the issue in the driver. It aligns unaligned
> completions of FS requests, by truncating them to the nearest
> alignment boundary.
>
> Reported-by: Mauricio Faria De Oliveira 
> Signed-off-by: Guilherme G. Piccoli 
> Signed-off-by: Ram Pai 
> ---
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c | 16 
>  1 file changed, 16 insertions(+)
>
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index b5c966e..55332a3 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -4644,6 +4644,8 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16
> smid, u8 msix_index, u32 reply)
> struct MPT3SAS_DEVICE *sas_device_priv_data;
> u32 response_code = 0;
> unsigned long flags;
> +   unsigned int sector_sz;
> +   struct request *req;
>
> mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
> scmd = _scsih_scsi_lookup_get_clear(ioc, smid); @@ -4703,6
> +4705,20 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8
> msix_index, u32 reply)
> }
>
> xfer_cnt = le32_to_cpu(mpi_reply->TransferCount);
> +
> +   /* In case of bogus fw or device, we could end up having
> +* unaligned partial completion. We can force alignment here,
> +* then scsi-ml does not need to handle this misbehavior.
> +*/
> +   sector_sz = scmd->device->sector_size;
> +   req = scmd->request;
> +   if (unlikely(sector_sz && req && (req->cmd_type == REQ_TYPE_FS) &&
> +   (xfer_cnt % sector_sz))) {
> +   sdev_printk(KERN_INFO, scmd->device,
> +   "unaligned partial completion avoided\n");
> +   xfer_cnt = (xfer_cnt / sector_sz) * sector_sz;
> +   }
> +
> scsi_set_resid(scmd, scsi_bufflen(scmd) - xfer_cnt);
> if (ioc_status & MPI2_IOCSTATUS_FLAG_LOG_INFO_AVAILABLE)
> log_info =  le32_to_cpu(mpi_reply->IOCLogInfo);
> --
> 2.1.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v4 2/4] fusion: remove iopriority handling

[Sathya Prakash Veerichetty]

By removing the code below, we put all the commands for all the types of
devices (SAS/SATA) as simple-Q (requeue as the device require) and I am
not sure whether it is the intention of this change.

-Original Message-
From: Adam Manzanares [mailto:adam.manzana...@hgst.com]
Sent: Thursday, October 13, 2016 1:54 PM
To: ax...@kernel.dk; t...@kernel.org; dan.j.willi...@intel.com;
h...@suse.de; martin.peter...@oracle.com; mchri...@redhat.com;
toshi.k...@hpe.com; ming@canonical.com; sathya.prak...@broadcom.com;
chaitra.basa...@broadcom.com; suganath-prabu.subram...@broadcom.com
Cc: linux-bl...@vger.kernel.org; linux-...@vger.kernel.org;
linux-ker...@vger.kernel.org; mpt-fusionlinux@broadcom.com;
linux-scsi@vger.kernel.org; Adam Manzanares; Adam Manzanares
Subject: [PATCH v4 2/4] fusion: remove iopriority handling

The request priority is now by default coming from the ioc. It was not
clear what this code was trying to do based upon the iopriority class or
data. The driver should check that a device supports priorities and use
them according to the specificiations of ioprio.

Signed-off-by: Adam Manzanares 
---
 drivers/message/fusion/mptscsih.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/message/fusion/mptscsih.c
b/drivers/message/fusion/mptscsih.c
index 6c9fc11..4740bb6 100644
--- a/drivers/message/fusion/mptscsih.c
+++ b/drivers/message/fusion/mptscsih.c
@@ -1369,11 +1369,6 @@ mptscsih_qcmd(struct scsi_cmnd *SCpnt)
if ((vdevice->vtarget->tflags & MPT_TARGET_FLAGS_Q_YES)
&& (SCpnt->device->tagged_supported)) {
scsictl = scsidir | MPI_SCSIIO_CONTROL_SIMPLEQ;
-   if (SCpnt->request && SCpnt->request->ioprio) {
-   if (((SCpnt->request->ioprio & 0x7) == 1) ||
-   !(SCpnt->request->ioprio & 0x7))
-   scsictl |= MPI_SCSIIO_CONTROL_HEADOFQ;
-   }
} else
scsictl = scsidir | MPI_SCSIIO_CONTROL_UNTAGGED;

--
2.1.4

Western Digital Corporation (and its subsidiaries) E-mail Confidentiality
Notice & Disclaimer:

This e-mail and any files transmitted with it may contain confidential or
legally privileged information of WDC and/or its affiliates, and are
intended solely for the use of the individual or entity to which they are
addressed. If you are not the intended recipient, any disclosure, copying,
distribution or any action taken or omitted to be taken in reliance on it,
is prohibited. If you have received this e-mail in error, please notify
the sender immediately and delete the e-mail in its entirety from your
system.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] mpt3sas: Do scsi_remove_host() before deleting SAS PHY objects

[Sathya Prakash Veerichetty]

Our understanding is the relationship between the SCSI host and SAS end
devices is a parent-child and before ripping of SCSI host we need to rip of
all the children. Why the enclosure ends up trying to re-parent the SCSI
device from the enclosure to the SAS PHY even after we remove the SAS Phy?.
Doesn't this need to be taken care in enclosure_removal?

-Original Message-
From: Calvin Owens [mailto:calvinow...@fb.com]
Sent: Monday, May 16, 2016 2:25 PM
To: Elliott, Robert (Persistent Memory)
Cc: Sathya Prakash; Chaitra P B; Suganath Prabu Subramani; James E.J.
Bottomley; Martin K. Petersen; mpt-fusionlinux@broadcom.com;
linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org;
kernel-t...@fb.com; calvinow...@fb.com
Subject: Re: [PATCH] mpt3sas: Do scsi_remove_host() before deleting SAS PHY
objects

On Friday 05/13 at 21:17 +, Elliott, Robert (Persistent Memory) wrote:
>
>
> > -Original Message-
> > From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel-
> > ow...@vger.kernel.org] On Behalf Of Calvin Owens
> > Sent: Friday, May 13, 2016 3:28 PM
> ...
> > Subject: [PATCH] mpt3sas: Do scsi_remove_host() before deleting SAS
> > PHY objects
> >
> ...
> > The issue is that enclosure_remove_device() expects to be able to
> > re-add the device that was previously enclosured: so with SES
> > running, the order we unwind things matters in a way it otherwise
> > wouldn't.
> >
> > Since mpt3sas deletes the SAS objects before the SCSI hosts,
> > enclosure ends up trying to re-parent the SCSI device from the
> > enclosure to the SAS PHY which has already been deleted. This obviously
> > ends in sadness.
> >
> > The fix appears to be simple: just call scsi_remove_host() before we
> > call
> > sas_port_delete() and/or sas_remove_host().
> >
> ...
> > diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> > @@ -8149,6 +8149,8 @@ void scsih_remove(struct pci_dev *pdev)
> > _scsih_raid_device_remove(ioc, raid_device);
> > }
> >
> > +   scsi_remove_host(shost);
> > +
> > /* free ports attached to the sas_host */
> > list_for_each_entry_safe(mpt3sas_port, next_port,
> >>sas_hba.sas_port_list, port_list) { @@ -8172,7 +8174,6 @@
> > void scsih_remove(struct pci_dev *pdev)
> > }
> >
> > sas_remove_host(shost);
> > -   scsi_remove_host(shost);
> > mpt3sas_base_detach(ioc);
> > spin_lock(_lock);
> > list_del(>list);
>
> That change matches the pattern of all other drivers calling
> sas_remove_host, except for one:
>   drivers/message/fusion/mptsas.c
>
> That consensus means this comment in drivers/scsi/scsi_transport_sas.c
> is wrong:
>
> /**
>  * sas_remove_host  -  tear down a Scsi_Host's SAS data structures
>  * @shost:  Scsi Host that is torn down
>  *
>  * Removes all SAS PHYs and remote PHYs for a given Scsi_Host.
>  * Must be called just before scsi_remove_host for SAS HBAs.
>  */

Yes, exactly: that comment appears to be backwards, and as you say most
drivers appear to do the opposite (I looked at HPSA at least, which calls
sas_port_delete() before scsi_remove_host()).

I suppose I should send a patch to fix the comment as well? It'd be nice to
get some testing to be certain I'm not breaking somebody else's setup by
fixing mine though...

Thanks,
Calvin
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] mptsas: fix checks for dma mapping errors

[Sathya Prakash Veerichetty]

Please consider this patch as Ack-by: Sathya Prakash
Veerichetty<sathya.prak...@broadcom.com>

PS: We don't have test environment to test this patch as this is for an
old controller.  So ACKing based on code review and similar mpt3sas driver
code.

-Original Message-
From: Martin K. Petersen [mailto:martin.peter...@oracle.com]
Sent: Wednesday, April 27, 2016 7:18 PM
To: Alexey Khoroshilov
Cc: Sreekanth Reddy; Sathya Prakash; Chaitra P B; Suganath Prabu
Subramani; mpt-fusionlinux@broadcom.com; linux-scsi@vger.kernel.org;
linux-ker...@vger.kernel.org; ldv-proj...@linuxtesting.org
Subject: Re: [PATCH] mptsas: fix checks for dma mapping errors

>>>>> "Alexey" == Alexey Khoroshilov <khoroshi...@ispras.ru> writes:

Alexey> mptsas_smp_handler() checks for dma mapping errors by comparison
Alexey> returned address with zero, while pci_dma_mapping_error() should
Alexey> be used.

Broadcom folks, please review!

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] mpt3sas: Remove usage of 'struct timeval'

[Sathya Prakash Veerichetty]

Hi,
Please consider this patch as Ack-by: Sathya Prakash


Thanks,
Sathya

-Original Message-
From: mpt-fusionlinux@broadcom.com
[mailto:mpt-fusionlinux@broadcom.com] On Behalf Of Martin K. Petersen
Sent: Thursday, April 14, 2016 8:48 PM
To: Tina Ruchandani
Cc: mpt-fusionlinux@broadcom.com; Arnd Bergmann;
y2...@lists.linaro.org; linux-scsi@vger.kernel.org;
linux-ker...@vger.kernel.org; j...@linux.vnet.ibm.com;
suganath-prabu.subram...@avagotech.com; Chaitra P B; Sreekanth Reddy
Subject: Re: [PATCH] mpt3sas: Remove usage of 'struct timeval'

> "Tina" == Tina Ruchandani  writes:

Tina> 'struct timeval' will have its tv_sec value overflow on 32-bit
Tina> systems in year 2038 and beyond. This patch replaces the use of
Tina> struct timeval for computing mpi_request.TimeStamp, and instead
Tina> uses ktime_t which provides 64-bit seconds value. The timestamp
Tina> computed remains unaffected (milliseconds since Unix epoch).

Broadcom folks, please review.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] mpt3sas: fix possible NULL dereference

[Sathya Prakash Veerichetty]

We need to do some more changes in this.  The concept is first pool alloc
and then memory alloc in the pool, so the memory has to be freed if the
memory is allocated in the pool and irrespective of memory allocated or
not the pool has to be destroyed if it is created.  We will work
internally and provide a complete patch.

Thanks
Sathya

-Original Message-
From: Martin K. Petersen [mailto:martin.peter...@oracle.com]
Sent: Thursday, April 14, 2016 8:44 PM
To: Sudip Mukherjee
Cc: Sathya Prakash; Chaitra P B; Suganath Prabu Subramani; James E.J.
Bottomley; Martin K. Petersen; linux-ker...@vger.kernel.org;
mpt-fusionlinux@broadcom.com; linux-scsi@vger.kernel.org
Subject: Re: [PATCH] mpt3sas: fix possible NULL dereference

> "Sudip" == Sudip Mukherjee  writes:

Sudip> We are dereferencing ioc->sense_dma_pool in pci_pool_free() and
Sudip> after that we are checking if it is NULL, before calling
Sudip> pci_pool_destroy().  Lets check if it is NULL before calling both
Sudip> pci_pool_free() and pci_pool_destroy().

Broadcom folks, please review.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] mpt3sas - remove unused fw_event_work delayed_work

[Sathya Prakash Veerichetty]

Chaitra,
I think you are looking for the patch "mpt2sas: [Resend] Host Reset code
cleanup".
Joe,
The initial drivers for SAS2 controller's handle firmware reset using the
rescan barrier and later we redesigned it through the above patch.  I hope
it clarifies, please let us know if you are looking for additional details?

Thanks
Sathya

-Original Message-
From: Chaitra Basappa [mailto:chaitra.basa...@broadcom.com]
Sent: Friday, April 15, 2016 3:58 AM
To: Martin K. Petersen; Joe Lawrence
Cc: Sathya Prakash Veerichetty; emi...@redhat.com;
linux-scsi@vger.kernel.org; Suganath Prabu Subramani; Calvin Owens
Subject: RE: [PATCH] mpt3sas - remove unused fw_event_work delayed_work

Joe ,
 The below mentioned patch is an older patch, verified latest code also the
code before merging(mpt2sas & mpt3sas) didn't find changes of below patch.
So I am searching for the patch which has removed the functionality/changes
of the below patch.

I shall get back to you on this by Monday.

Thanks,
 Chaitra

-Original Message-
From: Martin K. Petersen [mailto:martin.peter...@oracle.com]
Sent: Friday, April 15, 2016 8:13 AM
To: Joe Lawrence
Cc: Chaitra Basappa; Sathya Prakash Veerichetty; emi...@redhat.com;
linux-scsi@vger.kernel.org; Suganath Prabu Subramani; Calvin Owens
Subject: Re: [PATCH] mpt3sas - remove unused fw_event_work delayed_work

>>>>> "Joe" == Joe Lawrence <joe.lawre...@stratus.com> writes:

Joe> Do we know why f1c35e6aea579 "mpt2sas: RESCAN Barrier work is added
Joe> in case of HBA reset" was unneeded for the mpt3 version?  If that
Joe> is interesting, that info could be added to v2 commit message as
Joe> well.

Chaitra?

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Off topic: Test suite for LLDs

[Sathya Prakash Veerichetty]

Hello folks,
I would like to know whether we have any standard test suite for
validating a low level driver by executing all the interfaces the low
level driver registers with SML?

Thanks
Sathya
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html