* [PATCH v13 0/2] zfcp: Use DMA streaming API to map I/O
@ 2026-10-21 8:33 M Nikhil
2026-10-21 8:33 ` [PATCH v13 1/2] zfcp: Replace sg-list with sg_table in zfcp_fsf_ct_els structure M Nikhil
2026-10-21 8:33 ` [PATCH v13 2/2] zfcp: Use DMA streaming API to map I/O M Nikhil
0 siblings, 2 replies; 7+ messages in thread
From: M Nikhil @ 2026-10-21 8:33 UTC (permalink / raw)
To: sashiko-reviews, linux-s390, linux-scsi; +Cc: niharp, pasic, bblock
In the existing zFCP driver, virtual addresses are translated directly
into bus (physical addresses) for DMA operations on all I/O buffers
passing from zFCP driver to the device.
Need for using DMA-API in zFCP driver:
In a Secure Execution environment, the host must ensure that device DMA
accesses are restricted to non-guest-private, page-sized memory blocks.
This prevents I/O failures that can occur when a device attempts to
access guest-private memory. Therefore, guest memory pages must be
prepared by the hypervisor before they become accessible to an I/O
device.
To support this requirement, the host Linux kernel must be reliably
informed whenever a memory region becomes eligible for device DMA access
and when that access must be revoked. This information must be
maintained correctly at least at page size granularity. To satisfy these
requirements, all I/O buffers used by the zFCP driver should be mapped
through the DMA API.
DMA API Integration in zFCP:
The zFCP driver is updated to use the DMA streaming API's to map all the
I/O buffers which will be passed to the device.
All zFCP I/O buffers (QTCB, SRB, CT/ELS and SCSI I/O) are represented as
scatter-gather lists and mapped through DMA API using dma_map_sgtable().
Once the DMA transfer is complete, the corresponding mappings are
released using dma_unmap_sgtable().
By routing all device-visible memory through the DMA mapping layer, the
driver ensures that:
1. DMA access is properly authorized before use.
2. DMA is revoked after use.
3. Secure Execution memory restrictions are honored.
M Nikhil (2):
zfcp: Replace sg-list with sg_table in zfcp_fsf_ct_els structure
zfcp: Use DMA streaming API to map I/O
drivers/s390/scsi/zfcp_aux.c | 16 ++
drivers/s390/scsi/zfcp_dbf.c | 16 +-
drivers/s390/scsi/zfcp_def.h | 4 +
drivers/s390/scsi/zfcp_ext.h | 11 +-
drivers/s390/scsi/zfcp_fc.c | 36 ++--
drivers/s390/scsi/zfcp_fsf.c | 299 +++++++++++++++++++++++++++++-----
drivers/s390/scsi/zfcp_fsf.h | 9 +-
drivers/s390/scsi/zfcp_qdio.c | 39 +++--
drivers/s390/scsi/zfcp_qdio.h | 27 +--
drivers/s390/scsi/zfcp_scsi.c | 26 +++
10 files changed, 386 insertions(+), 97 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v13 1/2] zfcp: Replace sg-list with sg_table in zfcp_fsf_ct_els structure
2026-10-21 8:33 [PATCH v13 0/2] zfcp: Use DMA streaming API to map I/O M Nikhil
@ 2026-10-21 8:33 ` M Nikhil
2026-10-21 8:40 ` sashiko-bot
2026-10-21 8:33 ` [PATCH v13 2/2] zfcp: Use DMA streaming API to map I/O M Nikhil
1 sibling, 1 reply; 7+ messages in thread
From: M Nikhil @ 2026-10-21 8:33 UTC (permalink / raw)
To: sashiko-reviews, linux-s390, linux-scsi; +Cc: niharp, pasic, bblock
The zfcp_fsf_ct_els data structure is updated to replace the sg_req and
sg_resp members, which are of type struct scatterlist, with request and
response sg_table instances.
These sg_table structure reference the corresponding scatterlists
(sg_req and sg_resp) maintained in struct zfcp_fc_req.
These changes allows to track the original and mapped scatterlist nents
once zfcp is converted to use DMA-API for mapping the sg lists.
Signed-off-by: M Nikhil <nikh1092@linux.ibm.com>
Reviewed-by: Benjamin Block <bblock@linux.ibm.com>
Reviewed-by: Nihar Panda <niharp@linux.ibm.com>
---
drivers/s390/scsi/zfcp_dbf.c | 16 ++++++++--------
drivers/s390/scsi/zfcp_fc.c | 36 ++++++++++++++++++++++++------------
drivers/s390/scsi/zfcp_fsf.c | 6 ++++--
drivers/s390/scsi/zfcp_fsf.h | 9 +++++----
4 files changed, 41 insertions(+), 26 deletions(-)
diff --git a/drivers/s390/scsi/zfcp_dbf.c b/drivers/s390/scsi/zfcp_dbf.c
index 81fb8af408e9..f7e737e076bf 100644
--- a/drivers/s390/scsi/zfcp_dbf.c
+++ b/drivers/s390/scsi/zfcp_dbf.c
@@ -617,9 +617,9 @@ void zfcp_dbf_san_req(char *tag, struct zfcp_fsf_req *fsf, u32 d_id)
if (unlikely(!debug_level_enabled(dbf->san, ZFCP_DBF_SAN_LEVEL)))
return;
- length = (u16)zfcp_qdio_real_bytes(ct_els->req);
- zfcp_dbf_san(tag, dbf, "san_req", ct_els->req, ZFCP_DBF_SAN_REQ,
- length, fsf->req_id, d_id, length);
+ length = (u16)zfcp_qdio_real_bytes(ct_els->sgt_req.sgl);
+ zfcp_dbf_san(tag, dbf, "san_req", ct_els->sgt_req.sgl,
+ ZFCP_DBF_SAN_REQ, length, fsf->req_id, d_id, length);
}
static u16 zfcp_dbf_san_res_cap_len_if_gpn_ft(char *tag,
@@ -627,9 +627,9 @@ static u16 zfcp_dbf_san_res_cap_len_if_gpn_ft(char *tag,
u16 len)
{
struct zfcp_fsf_ct_els *ct_els = fsf->data;
- struct fc_ct_hdr *reqh = sg_virt(ct_els->req);
+ struct fc_ct_hdr *reqh = sg_virt(ct_els->sgt_req.sgl);
struct fc_ns_gid_ft *reqn = (struct fc_ns_gid_ft *)(reqh + 1);
- struct scatterlist *resp_entry = ct_els->resp;
+ struct scatterlist *resp_entry = ct_els->sgt_resp.sgl;
struct fc_ct_hdr *resph;
struct fc_gpn_ft_resp *acc;
int max_entries, x, last = 0;
@@ -698,9 +698,9 @@ void zfcp_dbf_san_res(char *tag, struct zfcp_fsf_req *fsf)
if (unlikely(!debug_level_enabled(dbf->san, ZFCP_DBF_SAN_LEVEL)))
return;
- length = (u16)zfcp_qdio_real_bytes(ct_els->resp);
- zfcp_dbf_san(tag, dbf, "san_res", ct_els->resp, ZFCP_DBF_SAN_RES,
- length, fsf->req_id, ct_els->d_id,
+ length = (u16)zfcp_qdio_real_bytes(ct_els->sgt_resp.sgl);
+ zfcp_dbf_san(tag, dbf, "san_res", ct_els->sgt_resp.sgl,
+ ZFCP_DBF_SAN_RES, length, fsf->req_id, ct_els->d_id,
zfcp_dbf_san_res_cap_len_if_gpn_ft(tag, fsf, length));
}
diff --git a/drivers/s390/scsi/zfcp_fc.c b/drivers/s390/scsi/zfcp_fc.c
index 05d081297879..1fb6f2c3b607 100644
--- a/drivers/s390/scsi/zfcp_fc.c
+++ b/drivers/s390/scsi/zfcp_fc.c
@@ -393,10 +393,12 @@ static int zfcp_fc_ns_gid_pn_request(struct zfcp_port *port,
fc_req->ct_els.port = port;
fc_req->ct_els.handler = zfcp_fc_complete;
fc_req->ct_els.handler_data = &completion;
- fc_req->ct_els.req = &fc_req->sg_req;
- fc_req->ct_els.resp = &fc_req->sg_rsp;
+ fc_req->ct_els.sgt_req.sgl = &fc_req->sg_req;
+ fc_req->ct_els.sgt_resp.sgl = &fc_req->sg_rsp;
sg_init_one(&fc_req->sg_req, gid_pn_req, sizeof(*gid_pn_req));
sg_init_one(&fc_req->sg_rsp, gid_pn_rsp, sizeof(*gid_pn_rsp));
+ fc_req->ct_els.sgt_req.orig_nents = sg_nents(&fc_req->sg_req);
+ fc_req->ct_els.sgt_resp.orig_nents = sg_nents(&fc_req->sg_rsp);
zfcp_fc_ct_ns_init(&gid_pn_req->ct_hdr,
FC_NS_GID_PN, ZFCP_FC_CT_SIZE_PAGE);
@@ -558,12 +560,14 @@ static int zfcp_fc_adisc(struct zfcp_port *port)
return -ENOMEM;
fc_req->ct_els.port = port;
- fc_req->ct_els.req = &fc_req->sg_req;
- fc_req->ct_els.resp = &fc_req->sg_rsp;
+ fc_req->ct_els.sgt_req.sgl = &fc_req->sg_req;
+ fc_req->ct_els.sgt_resp.sgl = &fc_req->sg_rsp;
sg_init_one(&fc_req->sg_req, &fc_req->u.adisc.req,
sizeof(struct fc_els_adisc));
sg_init_one(&fc_req->sg_rsp, &fc_req->u.adisc.rsp,
sizeof(struct fc_els_adisc));
+ fc_req->ct_els.sgt_req.orig_nents = sg_nents(&fc_req->sg_req);
+ fc_req->ct_els.sgt_resp.orig_nents = sg_nents(&fc_req->sg_rsp);
fc_req->ct_els.handler = zfcp_fc_adisc_handler;
fc_req->ct_els.handler_data = fc_req;
@@ -706,8 +710,10 @@ static int zfcp_fc_send_gpn_ft(struct zfcp_fc_req *fc_req,
ct_els->handler = zfcp_fc_complete;
ct_els->handler_data = &completion;
- ct_els->req = &fc_req->sg_req;
- ct_els->resp = &fc_req->sg_rsp;
+ ct_els->sgt_req.sgl = &fc_req->sg_req;
+ ct_els->sgt_resp.sgl = &fc_req->sg_rsp;
+ ct_els->sgt_req.orig_nents = sg_nents(&fc_req->sg_req);
+ ct_els->sgt_resp.orig_nents = sg_nents(&fc_req->sg_rsp);
ret = zfcp_fsf_send_ct(&adapter->gs->ds, ct_els, NULL,
ZFCP_FC_CTELS_TMO);
@@ -865,8 +871,10 @@ static int zfcp_fc_gspn(struct zfcp_adapter *adapter,
ct_els->handler = zfcp_fc_complete;
ct_els->handler_data = &completion;
- ct_els->req = &fc_req->sg_req;
- ct_els->resp = &fc_req->sg_rsp;
+ ct_els->sgt_req.sgl = &fc_req->sg_req;
+ ct_els->sgt_resp.sgl = &fc_req->sg_rsp;
+ ct_els->sgt_req.orig_nents = sg_nents(&fc_req->sg_req);
+ ct_els->sgt_resp.orig_nents = sg_nents(&fc_req->sg_rsp);
ret = zfcp_fsf_send_ct(&adapter->gs->ds, ct_els, NULL,
ZFCP_FC_CTELS_TMO);
@@ -924,8 +932,10 @@ static void zfcp_fc_rspn(struct zfcp_adapter *adapter,
ct_els->handler = zfcp_fc_complete;
ct_els->handler_data = &completion;
- ct_els->req = &fc_req->sg_req;
- ct_els->resp = &fc_req->sg_rsp;
+ ct_els->sgt_req.sgl = &fc_req->sg_req;
+ ct_els->sgt_resp.sgl = &fc_req->sg_rsp;
+ ct_els->sgt_req.orig_nents = sg_nents(&fc_req->sg_req);
+ ct_els->sgt_resp.orig_nents = sg_nents(&fc_req->sg_rsp);
ret = zfcp_fsf_send_ct(&adapter->gs->ds, ct_els, NULL,
ZFCP_FC_CTELS_TMO);
@@ -1090,8 +1100,10 @@ int zfcp_fc_exec_bsg_job(struct bsg_job *job)
if (!(atomic_read(&adapter->status) & ZFCP_STATUS_COMMON_OPEN))
return -EINVAL;
- ct_els->req = job->request_payload.sg_list;
- ct_els->resp = job->reply_payload.sg_list;
+ ct_els->sgt_req.sgl = job->request_payload.sg_list;
+ ct_els->sgt_resp.sgl = job->reply_payload.sg_list;
+ ct_els->sgt_req.orig_nents = sg_nents(ct_els->sgt_req.sgl);
+ ct_els->sgt_resp.orig_nents = sg_nents(ct_els->sgt_resp.sgl);
ct_els->handler_data = job;
switch (bsg_request->msgcode) {
diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
index d407f16e77ef..a3b99839da9e 100644
--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -1242,7 +1242,8 @@ int zfcp_fsf_send_ct(struct zfcp_fc_wka_port *wka_port,
}
req->status |= ZFCP_STATUS_FSFREQ_CLEANUP;
- ret = zfcp_fsf_setup_ct_els(req, ct->req, ct->resp, timeout);
+ ret = zfcp_fsf_setup_ct_els(req, ct->sgt_req.sgl, ct->sgt_resp.sgl,
+ timeout);
if (ret)
goto failed_send;
@@ -1342,7 +1343,8 @@ int zfcp_fsf_send_els(struct zfcp_adapter *adapter, u32 d_id,
if (!zfcp_adapter_multi_buffer_active(adapter))
zfcp_qdio_sbal_limit(qdio, &req->qdio_req, 2);
- ret = zfcp_fsf_setup_ct_els(req, els->req, els->resp, timeout);
+ ret = zfcp_fsf_setup_ct_els(req, els->sgt_req.sgl, els->sgt_resp.sgl,
+ timeout);
if (ret)
goto failed_send;
diff --git a/drivers/s390/scsi/zfcp_fsf.h b/drivers/s390/scsi/zfcp_fsf.h
index 4b92e85ec71d..22e7fd933a78 100644
--- a/drivers/s390/scsi/zfcp_fsf.h
+++ b/drivers/s390/scsi/zfcp_fsf.h
@@ -487,8 +487,9 @@ struct zfcp_blk_drv_data {
/**
* struct zfcp_fsf_ct_els - zfcp data for ct or els request
- * @req: scatter-gather list for request, points to &zfcp_fc_req.sg_req or BSG
- * @resp: scatter-gather list for response, points to &zfcp_fc_req.sg_rsp or BSG
+ * @sgt_req: sg_table for request, its sgl points to &zfcp_fc_req.sg_req or BSG
+ * @sgt_resp: sg_table for response, its sgl points to &zfcp_fc_req.sg_resp
+ * or BSG
* @handler: handler function (called for response to the request)
* @handler_data: data passed to handler function
* @port: Optional pointer to port for zfcp internal ELS (only test link ADISC)
@@ -496,8 +497,8 @@ struct zfcp_blk_drv_data {
* @d_id: Destination ID of either open WKA port for CT or of D_ID for ELS
*/
struct zfcp_fsf_ct_els {
- struct scatterlist *req;
- struct scatterlist *resp;
+ struct sg_table sgt_req;
+ struct sg_table sgt_resp;
void (*handler)(void *);
void *handler_data;
struct zfcp_port *port;
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v13 2/2] zfcp: Use DMA streaming API to map I/O
2026-10-21 8:33 [PATCH v13 0/2] zfcp: Use DMA streaming API to map I/O M Nikhil
2026-10-21 8:33 ` [PATCH v13 1/2] zfcp: Replace sg-list with sg_table in zfcp_fsf_ct_els structure M Nikhil
@ 2026-10-21 8:33 ` M Nikhil
2026-10-21 9:04 ` sashiko-bot
1 sibling, 1 reply; 7+ messages in thread
From: M Nikhil @ 2026-10-21 8:33 UTC (permalink / raw)
To: sashiko-reviews, linux-s390, linux-scsi; +Cc: niharp, pasic, bblock
In the existing zFCP driver, virtual addresses are translated directly
into bus (physical) addresses for DMA operations on all I/O buffers
passed from the zFCP driver to the device.
Need for DMA API:
In secure execution environment, the host must ensure that device DMA
addresses are restricted to non-guest-private, page-sized memory blocks.
This prevents I/O failures that can occur when a device attempts to
access guest-private memory. Therefore, guest memory pages must be
prepared by the hypervisor before they become accessible to an I/O
device.
To support this requirement, the host linux kernel must be reliably
informed whenever a memory region becomes eligible for device DMA access
and when that access must be revoked. This information must be
maintained correctly at least at page level granularity.
To satisfy these requirements, all I/O buffers used by the zFCP driver
should be mapped through the DMA API.
DMA streaming API usage in zFCP:
Changes in this patch-series converts all I/O buffer mappings in the
zFCP driver to use the DMA streaming API. Specifically,
`dma_map_sgtable()` and `dma_unmap_sgtable()` are used to map and unmap
the scatter-gather lists.
All zFCP I/O buffers (QTCB, SRB, CT/ELS and SCSI I/O) are represented
using scatter-gather lists. The `sg_table` structure, which encapsulates
the scatterlist together with the original and mapped entry counts, is
used to manage DMA mappings efficiently.
Use struct sg_table to manage scatter-gather lists for QTCB, SRB,
CT/ELS, and SCSI I/O buffers. Initialize single-entry scatterlists for
buffers that are not already represented by scatter-gather tables. For
SCSI I/O, reuse the tables provided by the SCSI midlayer and temporarily
adjust orig_nents when fewer entries are populated than allocated.
Map buffers after updating their contents. For SCSI I/O, synchronize
req->qtcb for device access in zfcp_fsf_fcp_cmnd() before submitting the
request to QDIO. DMA unmapping handles synchronization for CPU access on
the receive path.
Signed-off-by: M Nikhil <nikh1092@linux.ibm.com>
Reviewed-by: Benjamin Block <bblock@linux.ibm.com>
Reviewed-by: Nihar Panda <niharp@linux.ibm.com>
---
drivers/s390/scsi/zfcp_aux.c | 16 ++
drivers/s390/scsi/zfcp_def.h | 4 +
drivers/s390/scsi/zfcp_ext.h | 11 +-
drivers/s390/scsi/zfcp_fsf.c | 299 ++++++++++++++++++++++++++++------
drivers/s390/scsi/zfcp_qdio.c | 39 +++--
drivers/s390/scsi/zfcp_qdio.h | 27 +--
drivers/s390/scsi/zfcp_scsi.c | 26 +++
7 files changed, 348 insertions(+), 74 deletions(-)
diff --git a/drivers/s390/scsi/zfcp_aux.c b/drivers/s390/scsi/zfcp_aux.c
index fea573e00f9d..20c905c68e1e 100644
--- a/drivers/s390/scsi/zfcp_aux.c
+++ b/drivers/s390/scsi/zfcp_aux.c
@@ -329,6 +329,19 @@ static void zfcp_destroy_adapter_work_queue(struct zfcp_adapter *adapter)
}
+static int zfcp_set_dma_mask_for_adapter(struct zfcp_adapter *adapter)
+{
+ int ret;
+
+ ret = dma_set_mask_and_coherent(&adapter->ccw_device->dev,
+ DMA_BIT_MASK(64));
+ if (ret)
+ dev_err(&adapter->ccw_device->dev,
+ "Setting the DMA mask failed for the FCP device.\n");
+
+ return ret;
+}
+
/**
* zfcp_adapter_enqueue - enqueue a new adapter to the list
* @ccw_device: pointer to the struct cc_device
@@ -404,6 +417,9 @@ struct zfcp_adapter *zfcp_adapter_enqueue(struct ccw_device *ccw_device)
rwlock_init(&adapter->erp_lock);
rwlock_init(&adapter->abort_lock);
+ if (zfcp_set_dma_mask_for_adapter(adapter))
+ goto failed;
+
if (zfcp_erp_thread_setup(adapter))
goto failed;
diff --git a/drivers/s390/scsi/zfcp_def.h b/drivers/s390/scsi/zfcp_def.h
index 6c761299a22f..5063921e8d93 100644
--- a/drivers/s390/scsi/zfcp_def.h
+++ b/drivers/s390/scsi/zfcp_def.h
@@ -322,6 +322,8 @@ static inline u64 zfcp_scsi_dev_lun(struct scsi_device *sdev)
* @pool: reference to memory pool if used for this request
* @issued: time when request was send (STCK)
* @handler: handler which should be called to process response
+ * @metadata_sgt: sg_table for metadata of a QTCB or SRB request
+ * @metadata_sg: scatterlist for metadata of a QTCB or SRB request
*/
struct zfcp_fsf_req {
struct list_head list;
@@ -337,6 +339,8 @@ struct zfcp_fsf_req {
mempool_t *pool;
unsigned long long issued;
void (*handler)(struct zfcp_fsf_req *);
+ struct sg_table metadata_sgt;
+ struct scatterlist metadata_sg;
};
static inline
diff --git a/drivers/s390/scsi/zfcp_ext.h b/drivers/s390/scsi/zfcp_ext.h
index 0eb9bc9d03e4..2439ef1a8e20 100644
--- a/drivers/s390/scsi/zfcp_ext.h
+++ b/drivers/s390/scsi/zfcp_ext.h
@@ -150,6 +150,11 @@ enum zfcp_fsf_print_fmt {
extern ssize_t zfcp_fsf_scnprint_fc_security(char *buf, size_t size,
u32 fc_security,
enum zfcp_fsf_print_fmt fmt);
+int zfcp_fsf_req_map_sg(struct zfcp_fsf_req *req, struct sg_table *sgt,
+ enum dma_data_direction dma_dir);
+void zfcp_fsf_req_unmap_sg(struct zfcp_fsf_req *req,
+ struct sg_table *sgt,
+ enum dma_data_direction dma_dir);
/* zfcp_qdio.c */
extern int zfcp_qdio_setup(struct zfcp_adapter *);
@@ -157,13 +162,15 @@ extern void zfcp_qdio_destroy(struct zfcp_qdio *);
extern int zfcp_qdio_sbal_get(struct zfcp_qdio *qdio)
__must_hold(qdio->req_q_lock);
extern int zfcp_qdio_send(struct zfcp_qdio *, struct zfcp_qdio_req *);
-extern int zfcp_qdio_sbals_from_sg(struct zfcp_qdio *, struct zfcp_qdio_req *,
- struct scatterlist *);
+int zfcp_qdio_sbals_from_sg(struct zfcp_qdio *qdio, struct sg_table *sgt,
+ struct zfcp_fsf_req *req,
+ enum dma_data_direction dma_dir);
extern void zfcp_qdio_shost_update(struct zfcp_adapter *const adapter,
const struct zfcp_qdio *const qdio);
extern int zfcp_qdio_open(struct zfcp_qdio *);
extern void zfcp_qdio_close(struct zfcp_qdio *);
extern void zfcp_qdio_siosl(struct zfcp_adapter *);
+void zfcp_qdio_zero_sbals(struct qdio_buffer *sbal[], int first, int cnt);
/* zfcp_scsi.c */
extern bool zfcp_experimental_dix;
diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
index a3b99839da9e..cb3530506ac2 100644
--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -14,6 +14,7 @@
#include <linux/types.h>
#include <linux/slab.h>
#include <scsi/fc/fc_els.h>
+#include <linux/dma-mapping.h>
#include "zfcp_ext.h"
#include "zfcp_fc.h"
#include "zfcp_dbf.h"
@@ -84,6 +85,86 @@ static void zfcp_fsf_class_not_supp(struct zfcp_fsf_req *req)
req->status |= ZFCP_STATUS_FSFREQ_ERROR;
}
+/**
+ * zfcp_fsf_req_map_sg - map the scattergather list for DMA transfer
+ * @req: pointer to zfcp_fsf_req
+ * @sgt: pointer to scattergather list table
+ * @dma_dir: DMA data direction
+ * Returns: 0 on success, -EIO otherwise
+ */
+int zfcp_fsf_req_map_sg(struct zfcp_fsf_req *req, struct sg_table *sgt,
+ enum dma_data_direction dma_dir)
+{
+ unsigned int saved_orig_nents;
+ int ret;
+
+ if (!valid_dma_direction(dma_dir))
+ return sgt->orig_nents ? -EIO : 0;
+
+ if (!sgt->orig_nents)
+ return 0;
+
+ /*
+ * The SCSI midlayer sets orig_nents to the allocated SG capacity and
+ * nents to the actually populated entry count (nents <= orig_nents).
+ * dma_map_sgtable() and dma_unmap_sgtable() both use orig_nents, so
+ * clamp it to the populated count to avoid mapping trailing zero-length
+ * entries. Only clamp when nents has been explicitly set (nents > 0);
+ * zFCP-internal sg_tables leave nents = 0 before mapping, relying
+ * solely on orig_nents.
+ */
+ saved_orig_nents = sgt->orig_nents;
+ if (sgt->nents > 0 && sgt->nents < sgt->orig_nents)
+ sgt->orig_nents = sgt->nents;
+
+ ret = dma_map_sgtable(&req->adapter->ccw_device->dev, sgt, dma_dir, 0);
+
+ sgt->orig_nents = saved_orig_nents;
+
+ return ret;
+}
+
+/**
+ * zfcp_fsf_req_unmap_sg - unmap the scattergather list after DMA transfer
+ * @req: pointer to zfcp_fsf_req
+ * @sgt: pointer to scattergather list table
+ * @dma_dir: DMA direction of the sg
+ */
+void zfcp_fsf_req_unmap_sg(struct zfcp_fsf_req *req, struct sg_table *sgt,
+ enum dma_data_direction dma_dir)
+{
+ unsigned int saved_orig_nents;
+
+ /*
+ * We still need the check for mapped sg count here to not perform
+ * unmap for SCSI commands which have zero payload.
+ */
+ if (!sgt->nents)
+ return;
+
+ saved_orig_nents = sgt->orig_nents;
+ if (sgt->nents < sgt->orig_nents)
+ sgt->orig_nents = sgt->nents;
+
+ dma_unmap_sgtable(&req->adapter->ccw_device->dev, sgt, dma_dir, 0);
+
+ sgt->orig_nents = saved_orig_nents;
+}
+
+/**
+ * zfcp_fsf_req_unmap_metadata - unmap the metadata associated with FSF req
+ * @req: pointer to zfcp_fsf_req
+ */
+static inline
+void zfcp_fsf_req_unmap_metadata(struct zfcp_fsf_req *const req)
+{
+ if (unlikely(zfcp_fsf_req_is_status_read_buffer(req)))
+ zfcp_fsf_req_unmap_sg(req, &req->metadata_sgt, DMA_FROM_DEVICE);
+ else
+ zfcp_fsf_req_unmap_sg(req, &req->metadata_sgt,
+ DMA_BIDIRECTIONAL);
+}
+
/**
* zfcp_fsf_req_free - free memory used by fsf request
* @req: pointer to struct zfcp_fsf_req
@@ -259,6 +340,8 @@ static void zfcp_fsf_status_read_handler(struct zfcp_fsf_req *req)
struct zfcp_adapter *adapter = req->adapter;
struct fsf_status_read_buffer *sr_buf = req->data;
+ zfcp_fsf_req_unmap_metadata(req);
+
if (req->status & ZFCP_STATUS_FSFREQ_DISMISSED) {
zfcp_dbf_hba_fsf_uss("fssrh_1", req);
mempool_free(virt_to_page(sr_buf), adapter->pool.sr_data);
@@ -457,6 +540,7 @@ static void zfcp_fsf_req_complete(struct zfcp_fsf_req *req)
return;
}
+ zfcp_fsf_req_unmap_metadata(req);
timer_delete_sync(&req->timer);
zfcp_fsf_protstatus_eval(req);
zfcp_fsf_fsfstatus_eval(req);
@@ -870,10 +954,27 @@ static struct zfcp_fsf_req *zfcp_fsf_req_create(struct zfcp_qdio *qdio,
req->qtcb->prefix.qtcb_version = FSF_QTCB_CURRENT_VERSION;
req->qtcb->header.req_handle = req->req_id;
req->qtcb->header.fsf_command = fsf_cmd;
- }
- zfcp_qdio_req_init(adapter->qdio, &req->qdio_req, req->req_id, sbtype,
- req->qtcb, sizeof(struct fsf_qtcb));
+ sg_init_one(&req->metadata_sg, req->qtcb,
+ sizeof(struct fsf_qtcb));
+ req->metadata_sgt.sgl = &req->metadata_sg;
+ req->metadata_sgt.orig_nents = 1;
+
+ if (zfcp_fsf_req_map_sg(req, &req->metadata_sgt,
+ DMA_BIDIRECTIONAL)) {
+ zfcp_fsf_req_free(req);
+ return ERR_PTR(-EIO);
+ }
+
+ zfcp_qdio_req_init(adapter->qdio, &req->qdio_req, req->req_id,
+ sbtype, true,
+ sg_dma_len(req->metadata_sgt.sgl),
+ sg_dma_address(req->metadata_sgt.sgl));
+ } else {
+ /* SRB */
+ zfcp_qdio_req_init(adapter->qdio, &req->qdio_req, req->req_id,
+ sbtype, false, 0, 0);
+ }
return req;
}
@@ -954,7 +1055,17 @@ int zfcp_fsf_status_read(struct zfcp_qdio *qdio)
memset(sr_buf, 0, sizeof(*sr_buf));
req->data = sr_buf;
- zfcp_qdio_fill_next(qdio, &req->qdio_req, sr_buf, sizeof(*sr_buf));
+ sg_init_one(&req->metadata_sg, sr_buf, sizeof(*sr_buf));
+ req->metadata_sgt.sgl = &req->metadata_sg;
+ req->metadata_sgt.orig_nents = 1;
+
+ retval = zfcp_fsf_req_map_sg(req, &req->metadata_sgt, DMA_FROM_DEVICE);
+ if (unlikely(retval))
+ goto failed_dma_map;
+
+ zfcp_qdio_fill_next(qdio, &req->qdio_req,
+ sg_dma_address(req->metadata_sgt.sgl),
+ sg_dma_len(req->metadata_sgt.sgl));
zfcp_qdio_set_sbale_last(qdio, &req->qdio_req);
retval = zfcp_fsf_req_send(req);
@@ -965,6 +1076,8 @@ int zfcp_fsf_status_read(struct zfcp_qdio *qdio)
goto out;
failed_req_send:
+ zfcp_fsf_req_unmap_metadata(req);
+failed_dma_map:
req->data = NULL;
mempool_free(virt_to_page(sr_buf), adapter->pool.sr_data);
failed_buf:
@@ -1076,6 +1189,7 @@ struct zfcp_fsf_req *zfcp_fsf_abort_fcp_cmnd(struct scsi_cmnd *scmnd)
}
out_error_free:
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
req = NULL;
out:
@@ -1091,6 +1205,9 @@ static void zfcp_fsf_send_ct_handler(struct zfcp_fsf_req *req)
ct->status = -EINVAL;
+ zfcp_fsf_req_unmap_sg(req, &ct->sgt_req, DMA_TO_DEVICE);
+ zfcp_fsf_req_unmap_sg(req, &ct->sgt_resp, DMA_FROM_DEVICE);
+
if (req->status & ZFCP_STATUS_FSFREQ_ERROR)
goto skip_fsfstatus;
@@ -1130,19 +1247,41 @@ static void zfcp_fsf_send_ct_handler(struct zfcp_fsf_req *req)
ct->handler(ct->handler_data);
}
-static void zfcp_fsf_setup_ct_els_unchained(struct zfcp_qdio *qdio,
- struct zfcp_qdio_req *q_req,
- struct scatterlist *sg_req,
- struct scatterlist *sg_resp)
+static int zfcp_fsf_setup_ct_els_unchained(struct zfcp_fsf_req *req,
+ struct zfcp_qdio *qdio,
+ struct sg_table *sg_req_table,
+ struct sg_table *sg_resp_table)
{
- zfcp_qdio_fill_next(qdio, q_req, sg_virt(sg_req), sg_req->length);
- zfcp_qdio_fill_next(qdio, q_req, sg_virt(sg_resp), sg_resp->length);
- zfcp_qdio_set_sbale_last(qdio, q_req);
+ int retval = 0;
+
+ retval = zfcp_fsf_req_map_sg(req, sg_req_table, DMA_TO_DEVICE);
+ if (unlikely(retval))
+ goto err;
+
+ retval = zfcp_fsf_req_map_sg(req, sg_resp_table,
+ DMA_FROM_DEVICE);
+ if (unlikely(retval))
+ goto err_unmap;
+
+ zfcp_qdio_fill_next(qdio, &req->qdio_req,
+ sg_dma_address(sg_req_table->sgl),
+ sg_dma_len(sg_req_table->sgl));
+ zfcp_qdio_fill_next(qdio, &req->qdio_req,
+ sg_dma_address(sg_resp_table->sgl),
+ sg_dma_len(sg_resp_table->sgl));
+ zfcp_qdio_set_sbale_last(qdio, &req->qdio_req);
+
+ return 0;
+
+err_unmap:
+ zfcp_fsf_req_unmap_sg(req, sg_req_table, DMA_TO_DEVICE);
+err:
+ return retval;
}
static int zfcp_fsf_setup_ct_els_sbals(struct zfcp_fsf_req *req,
- struct scatterlist *sg_req,
- struct scatterlist *sg_resp)
+ struct sg_table *sgt_req,
+ struct sg_table *sgt_resp)
{
struct zfcp_adapter *adapter = req->adapter;
struct zfcp_qdio *qdio = adapter->qdio;
@@ -1150,57 +1289,72 @@ static int zfcp_fsf_setup_ct_els_sbals(struct zfcp_fsf_req *req,
u32 feat = adapter->adapter_features;
if (zfcp_adapter_multi_buffer_active(adapter)) {
- if (zfcp_qdio_sbals_from_sg(qdio, &req->qdio_req, sg_req))
+ if (zfcp_qdio_sbals_from_sg(qdio, sgt_req, req, DMA_TO_DEVICE))
return -EIO;
+
qtcb->bottom.support.req_buf_length =
- zfcp_qdio_real_bytes(sg_req);
- if (zfcp_qdio_sbals_from_sg(qdio, &req->qdio_req, sg_resp))
- return -EIO;
+ zfcp_qdio_real_bytes(sgt_req->sgl);
+
+ if (zfcp_qdio_sbals_from_sg(qdio, sgt_resp, req, DMA_FROM_DEVICE))
+ goto err_unmap;
+
qtcb->bottom.support.resp_buf_length =
- zfcp_qdio_real_bytes(sg_resp);
+ zfcp_qdio_real_bytes(sgt_resp->sgl);
- zfcp_qdio_set_data_div(qdio, &req->qdio_req, sg_nents(sg_req));
+ zfcp_qdio_set_data_div(qdio, &req->qdio_req,
+ sgt_req->nents);
zfcp_qdio_set_sbale_last(qdio, &req->qdio_req);
zfcp_qdio_set_scount(qdio, &req->qdio_req);
return 0;
}
/* use single, unchained SBAL if it can hold the request */
- if (zfcp_qdio_sg_one_sbale(sg_req) && zfcp_qdio_sg_one_sbale(sg_resp)) {
- zfcp_fsf_setup_ct_els_unchained(qdio, &req->qdio_req,
- sg_req, sg_resp);
+ if (zfcp_qdio_sg_one_sbale(sgt_req->sgl) &&
+ zfcp_qdio_sg_one_sbale(sgt_resp->sgl)) {
+ if (zfcp_fsf_setup_ct_els_unchained(req, qdio, sgt_req,
+ sgt_resp))
+ return -EIO;
+
return 0;
}
if (!(feat & FSF_FEATURE_ELS_CT_CHAINED_SBALS))
return -EOPNOTSUPP;
- if (zfcp_qdio_sbals_from_sg(qdio, &req->qdio_req, sg_req))
+ if (zfcp_qdio_sbals_from_sg(qdio, sgt_req, req, DMA_TO_DEVICE))
return -EIO;
- qtcb->bottom.support.req_buf_length = zfcp_qdio_real_bytes(sg_req);
+ qtcb->bottom.support.req_buf_length =
+ zfcp_qdio_real_bytes(sgt_req->sgl);
zfcp_qdio_set_sbale_last(qdio, &req->qdio_req);
zfcp_qdio_skip_to_last_sbale(qdio, &req->qdio_req);
- if (zfcp_qdio_sbals_from_sg(qdio, &req->qdio_req, sg_resp))
- return -EIO;
+ if (zfcp_qdio_sbals_from_sg(qdio, sgt_resp, req, DMA_FROM_DEVICE))
+ goto err_unmap;
- qtcb->bottom.support.resp_buf_length = zfcp_qdio_real_bytes(sg_resp);
+ qtcb->bottom.support.resp_buf_length =
+ zfcp_qdio_real_bytes(sgt_resp->sgl);
zfcp_qdio_set_sbale_last(qdio, &req->qdio_req);
return 0;
+
+err_unmap:
+ zfcp_fsf_req_unmap_sg(req, sgt_req, DMA_TO_DEVICE);
+ zfcp_qdio_zero_sbals(qdio->req_q, req->qdio_req.sbal_first,
+ req->qdio_req.sbal_number);
+ return -EIO;
}
static int zfcp_fsf_setup_ct_els(struct zfcp_fsf_req *req,
- struct scatterlist *sg_req,
- struct scatterlist *sg_resp,
+ struct sg_table *sgt_req,
+ struct sg_table *sgt_resp,
unsigned int timeout)
{
int ret;
- ret = zfcp_fsf_setup_ct_els_sbals(req, sg_req, sg_resp);
+ ret = zfcp_fsf_setup_ct_els_sbals(req, sgt_req, sgt_resp);
if (ret)
return ret;
@@ -1242,8 +1396,7 @@ int zfcp_fsf_send_ct(struct zfcp_fc_wka_port *wka_port,
}
req->status |= ZFCP_STATUS_FSFREQ_CLEANUP;
- ret = zfcp_fsf_setup_ct_els(req, ct->sgt_req.sgl, ct->sgt_resp.sgl,
- timeout);
+ ret = zfcp_fsf_setup_ct_els(req, &ct->sgt_req, &ct->sgt_resp, timeout);
if (ret)
goto failed_send;
@@ -1256,12 +1409,17 @@ int zfcp_fsf_send_ct(struct zfcp_fc_wka_port *wka_port,
ret = zfcp_fsf_req_send(req);
if (ret)
- goto failed_send;
+ goto err_unmap;
+
/* NOTE: DO NOT TOUCH req PAST THIS POINT! */
goto out;
+err_unmap:
+ zfcp_fsf_req_unmap_sg(req, &ct->sgt_req, DMA_TO_DEVICE);
+ zfcp_fsf_req_unmap_sg(req, &ct->sgt_resp, DMA_FROM_DEVICE);
failed_send:
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
out:
spin_unlock_irq(&qdio->req_q_lock);
@@ -1275,6 +1433,9 @@ static void zfcp_fsf_send_els_handler(struct zfcp_fsf_req *req)
send_els->status = -EINVAL;
+ zfcp_fsf_req_unmap_sg(req, &send_els->sgt_req, DMA_TO_DEVICE);
+ zfcp_fsf_req_unmap_sg(req, &send_els->sgt_resp, DMA_FROM_DEVICE);
+
if (req->status & ZFCP_STATUS_FSFREQ_ERROR)
goto skip_fsfstatus;
@@ -1343,9 +1504,8 @@ int zfcp_fsf_send_els(struct zfcp_adapter *adapter, u32 d_id,
if (!zfcp_adapter_multi_buffer_active(adapter))
zfcp_qdio_sbal_limit(qdio, &req->qdio_req, 2);
- ret = zfcp_fsf_setup_ct_els(req, els->sgt_req.sgl, els->sgt_resp.sgl,
+ ret = zfcp_fsf_setup_ct_els(req, &els->sgt_req, &els->sgt_resp,
timeout);
-
if (ret)
goto failed_send;
@@ -1358,12 +1518,17 @@ int zfcp_fsf_send_els(struct zfcp_adapter *adapter, u32 d_id,
ret = zfcp_fsf_req_send(req);
if (ret)
- goto failed_send;
+ goto err_unmap;
+
/* NOTE: DO NOT TOUCH req PAST THIS POINT! */
goto out;
+err_unmap:
+ zfcp_fsf_req_unmap_sg(req, &els->sgt_req, DMA_TO_DEVICE);
+ zfcp_fsf_req_unmap_sg(req, &els->sgt_resp, DMA_FROM_DEVICE);
failed_send:
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
out:
spin_unlock_irq(&qdio->req_q_lock);
@@ -1404,6 +1569,7 @@ int zfcp_fsf_exchange_config_data(struct zfcp_erp_action *erp_action)
zfcp_fsf_start_erp_timer(req);
retval = zfcp_fsf_req_send(req);
if (retval) {
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
erp_action->fsf_req_id = 0;
}
@@ -1469,6 +1635,8 @@ int zfcp_fsf_exchange_config_data_sync(struct zfcp_qdio *qdio,
retval = -EIO;
else if (req->status & ZFCP_STATUS_FSFREQ_XDATAINCOMPLETE)
retval = -EAGAIN;
+ } else {
+ zfcp_fsf_req_unmap_metadata(req);
}
zfcp_fsf_req_free(req);
@@ -1516,6 +1684,7 @@ int zfcp_fsf_exchange_port_data(struct zfcp_erp_action *erp_action)
zfcp_fsf_start_erp_timer(req);
retval = zfcp_fsf_req_send(req);
if (retval) {
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
erp_action->fsf_req_id = 0;
}
@@ -1578,6 +1747,8 @@ int zfcp_fsf_exchange_port_data_sync(struct zfcp_qdio *qdio,
retval = -EIO;
else if (req->status & ZFCP_STATUS_FSFREQ_XDATAINCOMPLETE)
retval = -EAGAIN;
+ } else {
+ zfcp_fsf_req_unmap_metadata(req);
}
zfcp_fsf_req_free(req);
@@ -1810,6 +1981,7 @@ int zfcp_fsf_open_port(struct zfcp_erp_action *erp_action)
zfcp_fsf_start_erp_timer(req);
retval = zfcp_fsf_req_send(req);
if (retval) {
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
erp_action->fsf_req_id = 0;
put_device(&port->dev);
@@ -1876,6 +2048,7 @@ int zfcp_fsf_close_port(struct zfcp_erp_action *erp_action)
zfcp_fsf_start_erp_timer(req);
retval = zfcp_fsf_req_send(req);
if (retval) {
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
erp_action->fsf_req_id = 0;
}
@@ -1950,8 +2123,10 @@ int zfcp_fsf_open_wka_port(struct zfcp_fc_wka_port *wka_port)
zfcp_fsf_start_timer(req, ZFCP_FSF_REQUEST_TIMEOUT);
retval = zfcp_fsf_req_send(req);
- if (retval)
+ if (retval) {
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
+ }
/* NOTE: DO NOT TOUCH req PAST THIS POINT! */
out:
spin_unlock_irq(&qdio->req_q_lock);
@@ -2009,8 +2184,10 @@ int zfcp_fsf_close_wka_port(struct zfcp_fc_wka_port *wka_port)
zfcp_fsf_start_timer(req, ZFCP_FSF_REQUEST_TIMEOUT);
retval = zfcp_fsf_req_send(req);
- if (retval)
+ if (retval) {
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
+ }
/* NOTE: DO NOT TOUCH req PAST THIS POINT! */
out:
spin_unlock_irq(&qdio->req_q_lock);
@@ -2103,6 +2280,7 @@ int zfcp_fsf_close_physical_port(struct zfcp_erp_action *erp_action)
zfcp_fsf_start_erp_timer(req);
retval = zfcp_fsf_req_send(req);
if (retval) {
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
erp_action->fsf_req_id = 0;
}
@@ -2227,6 +2405,7 @@ int zfcp_fsf_open_lun(struct zfcp_erp_action *erp_action)
zfcp_fsf_start_erp_timer(req);
retval = zfcp_fsf_req_send(req);
if (retval) {
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
erp_action->fsf_req_id = 0;
}
@@ -2316,6 +2495,7 @@ int zfcp_fsf_close_lun(struct zfcp_erp_action *erp_action)
zfcp_fsf_start_erp_timer(req);
retval = zfcp_fsf_req_send(req);
if (retval) {
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
erp_action->fsf_req_id = 0;
}
@@ -2474,6 +2654,13 @@ static void zfcp_fsf_fcp_cmnd_handler(struct zfcp_fsf_req *req)
return;
}
+ if ((scsi_get_prot_op(scpnt) != SCSI_PROT_NORMAL) &&
+ scsi_prot_sg_count(scpnt))
+ zfcp_fsf_req_unmap_sg(req, &scpnt->prot_sdb->table,
+ scpnt->sc_data_direction);
+
+ zfcp_fsf_req_unmap_sg(req, &scpnt->sdb.table, scpnt->sc_data_direction);
+
zfcp_fsf_fcp_handler_common(req, scpnt->device);
if (unlikely(req->status & ZFCP_STATUS_FSFREQ_ERROR)) {
@@ -2603,6 +2790,10 @@ int zfcp_fsf_fcp_cmnd(struct scsi_cmnd *scsi_cmnd)
io->service_class = FSF_CLASS_3;
io->fcp_cmnd_length = FCP_CMND_LEN;
+ dma_sync_sgtable_for_device(&req->adapter->ccw_device->dev,
+ &req->metadata_sgt,
+ DMA_BIDIRECTIONAL);
+
if (scsi_get_prot_op(scsi_cmnd) != SCSI_PROT_NORMAL) {
io->data_block_length = scsi_prot_interval(scsi_cmnd);
io->ref_tag_value = scsi_prot_ref_tag(scsi_cmnd);
@@ -2617,20 +2808,22 @@ int zfcp_fsf_fcp_cmnd(struct scsi_cmnd *scsi_cmnd)
if ((scsi_get_prot_op(scsi_cmnd) != SCSI_PROT_NORMAL) &&
scsi_prot_sg_count(scsi_cmnd)) {
- zfcp_qdio_set_data_div(qdio, &req->qdio_req,
- scsi_prot_sg_count(scsi_cmnd));
- retval = zfcp_qdio_sbals_from_sg(qdio, &req->qdio_req,
- scsi_prot_sglist(scsi_cmnd));
+ retval = zfcp_qdio_sbals_from_sg(qdio,
+ &scsi_cmnd->prot_sdb->table,
+ req,
+ scsi_cmnd->sc_data_direction);
if (retval)
goto failed_scsi_cmnd;
+ zfcp_qdio_set_data_div(qdio, &req->qdio_req,
+ scsi_cmnd->prot_sdb->table.nents);
io->prot_data_length = zfcp_qdio_real_bytes(
scsi_prot_sglist(scsi_cmnd));
}
- retval = zfcp_qdio_sbals_from_sg(qdio, &req->qdio_req,
- scsi_sglist(scsi_cmnd));
+ retval = zfcp_qdio_sbals_from_sg(qdio, &scsi_cmnd->sdb.table, req,
+ scsi_cmnd->sc_data_direction);
if (unlikely(retval))
- goto failed_scsi_cmnd;
+ goto failed_normal_scsi_map;
zfcp_qdio_set_sbale_last(adapter->qdio, &req->qdio_req);
if (zfcp_adapter_multi_buffer_active(adapter))
@@ -2638,12 +2831,25 @@ int zfcp_fsf_fcp_cmnd(struct scsi_cmnd *scsi_cmnd)
retval = zfcp_fsf_req_send(req);
if (unlikely(retval))
- goto failed_scsi_cmnd;
+ goto failed_req_send;
+
/* NOTE: DO NOT TOUCH req PAST THIS POINT! */
goto out;
+failed_req_send:
+ zfcp_fsf_req_unmap_sg(req, &scsi_cmnd->sdb.table,
+ scsi_cmnd->sc_data_direction);
+failed_normal_scsi_map:
+ if ((scsi_get_prot_op(scsi_cmnd) != SCSI_PROT_NORMAL) &&
+ scsi_prot_sg_count(scsi_cmnd)) {
+ zfcp_fsf_req_unmap_sg(req, &scsi_cmnd->prot_sdb->table,
+ scsi_cmnd->sc_data_direction);
+ zfcp_qdio_zero_sbals(qdio->req_q, req->qdio_req.sbal_first,
+ req->qdio_req.sbal_number);
+ }
failed_scsi_cmnd:
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
scsi_cmnd->host_scribble = NULL;
out:
@@ -2719,6 +2925,7 @@ struct zfcp_fsf_req *zfcp_fsf_fcp_task_mgmt(struct scsi_device *sdev,
goto out;
}
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
req = NULL;
out:
diff --git a/drivers/s390/scsi/zfcp_qdio.c b/drivers/s390/scsi/zfcp_qdio.c
index bac8385e069f..50db08388514 100644
--- a/drivers/s390/scsi/zfcp_qdio.c
+++ b/drivers/s390/scsi/zfcp_qdio.c
@@ -39,7 +39,7 @@ static void zfcp_qdio_handler_error(struct zfcp_qdio *qdio, char *dbftag,
ZFCP_STATUS_COMMON_ERP_FAILED, dbftag);
}
-static void zfcp_qdio_zero_sbals(struct qdio_buffer *sbal[], int first, int cnt)
+void zfcp_qdio_zero_sbals(struct qdio_buffer *sbal[], int first, int cnt)
{
int i, sbal_idx;
@@ -233,31 +233,40 @@ zfcp_qdio_sbale_next(struct zfcp_qdio *qdio, struct zfcp_qdio_req *q_req)
}
/**
- * zfcp_qdio_sbals_from_sg - fill SBALs from scatter-gather list
+ * zfcp_qdio_sbals_from_sg - DMA map scatter-gather list and fill SBALs from it
* @qdio: pointer to struct zfcp_qdio
- * @q_req: pointer to struct zfcp_qdio_req
- * @sg: scatter-gather list
- * Returns: zero or -EINVAL on error
+ * @sgt: pointer to scattergather list table
+ * @req: pointer to zfcp_fsf_req
+ * @dma_dir: DMA direction of I/O
+ * Returns: zero on success, -EINVAL or -EIO otherwise
*/
-int zfcp_qdio_sbals_from_sg(struct zfcp_qdio *qdio, struct zfcp_qdio_req *q_req,
- struct scatterlist *sg)
+int zfcp_qdio_sbals_from_sg(struct zfcp_qdio *qdio, struct sg_table *sgt,
+ struct zfcp_fsf_req *req,
+ enum dma_data_direction dma_dir)
{
struct qdio_buffer_element *sbale;
+ int i;
+ struct scatterlist *sg_element;
+
+ if (zfcp_fsf_req_map_sg(req, sgt, dma_dir))
+ return -EIO;
/* set storage-block type for this request */
- sbale = zfcp_qdio_sbale_req(qdio, q_req);
- sbale->sflags |= q_req->sbtype;
+ sbale = zfcp_qdio_sbale_req(qdio, &req->qdio_req);
+ sbale->sflags |= req->qdio_req.sbtype;
- for (; sg; sg = sg_next(sg)) {
- sbale = zfcp_qdio_sbale_next(qdio, q_req);
+ for_each_sgtable_dma_sg(sgt, sg_element, i) {
+ sbale = zfcp_qdio_sbale_next(qdio, &req->qdio_req);
if (!sbale) {
atomic_inc(&qdio->req_q_full);
- zfcp_qdio_zero_sbals(qdio->req_q, q_req->sbal_first,
- q_req->sbal_number);
+ zfcp_qdio_zero_sbals(qdio->req_q,
+ req->qdio_req.sbal_first,
+ req->qdio_req.sbal_number);
+ zfcp_fsf_req_unmap_sg(req, sgt, dma_dir);
return -EINVAL;
}
- sbale->addr = u64_to_dma64(sg_phys(sg));
- sbale->length = sg->length;
+ sbale->addr = u64_to_dma64(sg_dma_address(sg_element));
+ sbale->length = sg_dma_len(sg_element);
}
return 0;
}
diff --git a/drivers/s390/scsi/zfcp_qdio.h b/drivers/s390/scsi/zfcp_qdio.h
index 8f7d2ae94441..9f89f2975f6f 100644
--- a/drivers/s390/scsi/zfcp_qdio.h
+++ b/drivers/s390/scsi/zfcp_qdio.h
@@ -11,6 +11,7 @@
#define ZFCP_QDIO_H
#include <linux/interrupt.h>
+#include <linux/types.h>
#include <asm/qdio.h>
#define ZFCP_QDIO_SBALE_LEN PAGE_SIZE
@@ -106,16 +107,19 @@ zfcp_qdio_sbale_curr(struct zfcp_qdio *qdio, struct zfcp_qdio_req *q_req)
* @q_req: the qdio request to start
* @req_id: The request id
* @sbtype: type flags to set for all sbals
- * @data: First data block
- * @len: Length of first data block
+ * @has_qtcb: FSF request has a QTCB
+ * @qtcb_len: Length of first data block QTCB, if any
+ * @qtcb_dma_handle: DMA handle of the QTCB, if any
*
* This is the start of putting the request into the queue, the last
* step is passing the request to zfcp_qdio_send. The request queue
* lock must be held during the whole process from init to send.
*/
static inline
-void zfcp_qdio_req_init(struct zfcp_qdio *qdio, struct zfcp_qdio_req *q_req,
- u64 req_id, u8 sbtype, void *data, u32 len)
+void zfcp_qdio_req_init(struct zfcp_qdio *qdio,
+ struct zfcp_qdio_req *q_req, u64 req_id,
+ u8 sbtype, bool has_qtcb, u32 qtcb_len,
+ dma_addr_t qtcb_dma_handle)
{
struct qdio_buffer_element *sbale;
int count = min(atomic_read(&qdio->req_q_free),
@@ -133,33 +137,34 @@ void zfcp_qdio_req_init(struct zfcp_qdio *qdio, struct zfcp_qdio_req *q_req,
sbale->eflags = 0;
sbale->sflags = SBAL_SFLAGS0_COMMAND | sbtype;
- if (unlikely(!data))
+ if (unlikely(!has_qtcb))
return;
sbale++;
- sbale->addr = virt_to_dma64(data);
- sbale->length = len;
+ sbale->addr = u64_to_dma64(qtcb_dma_handle);
+ sbale->length = qtcb_len;
}
/**
* zfcp_qdio_fill_next - Fill next sbale, only for single sbal requests
* @qdio: pointer to struct zfcp_qdio
* @q_req: pointer to struct zfcp_queue_req
- * @data: pointer to data
+ * @dma_handle: dma address
* @len: length of data
*
* This is only required for single sbal requests, calling it when
* wrapping around to the next sbal is a bug.
*/
static inline
-void zfcp_qdio_fill_next(struct zfcp_qdio *qdio, struct zfcp_qdio_req *q_req,
- void *data, u32 len)
+void zfcp_qdio_fill_next(struct zfcp_qdio *qdio,
+ struct zfcp_qdio_req *q_req,
+ dma_addr_t dma_handle, u32 len)
{
struct qdio_buffer_element *sbale;
BUG_ON(q_req->sbale_curr == qdio->max_sbale_per_sbal - 1);
q_req->sbale_curr++;
sbale = zfcp_qdio_sbale_curr(qdio, q_req);
- sbale->addr = virt_to_dma64(data);
+ sbale->addr = u64_to_dma64(dma_handle);
sbale->length = len;
}
diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c
index 3590113c2b74..e45c76d235d7 100644
--- a/drivers/s390/scsi/zfcp_scsi.c
+++ b/drivers/s390/scsi/zfcp_scsi.c
@@ -184,6 +184,20 @@ static int zfcp_scsi_eh_abort_handler(struct scsi_cmnd *scpnt)
zfcp_dbf_scsi_abort("abrt_or", scpnt, NULL);
return FAILED; /* completion could be in progress */
}
+
+ /*
+ * Unmap SCSI payload DMA mappings before clearing req->data.
+ * Once req->data is NULL the hardware completion handler cannot
+ * reach the sg_tables, causing a permanent IOVA leak.
+ */
+ if ((scsi_get_prot_op(scpnt) != SCSI_PROT_NORMAL) &&
+ scsi_prot_sg_count(scpnt))
+ zfcp_fsf_req_unmap_sg(old_req, &scpnt->prot_sdb->table,
+ scpnt->sc_data_direction);
+
+ zfcp_fsf_req_unmap_sg(old_req, &scpnt->sdb.table,
+ scpnt->sc_data_direction);
+
old_req->data = NULL;
/* don't access old fsf_req after releasing the abort_lock */
@@ -237,6 +251,7 @@ static void zfcp_scsi_forget_cmnd(struct zfcp_fsf_req *old_req, void *data)
{
struct zfcp_scsi_req_filter *filter =
(struct zfcp_scsi_req_filter *)data;
+ struct scsi_cmnd *scpnt;
/* already aborted - prevent side-effects - or not a SCSI command */
if (old_req->data == NULL ||
@@ -244,6 +259,8 @@ static void zfcp_scsi_forget_cmnd(struct zfcp_fsf_req *old_req, void *data)
old_req->qtcb->header.fsf_command != FSF_QTCB_FCP_CMND)
return;
+ scpnt = old_req->data;
+
/* (tmf_scope == FCP_TMF_TGT_RESET || tmf_scope == FCP_TMF_LUN_RESET) */
if (old_req->qtcb->header.port_handle != filter->port_handle)
return;
@@ -253,6 +270,15 @@ static void zfcp_scsi_forget_cmnd(struct zfcp_fsf_req *old_req, void *data)
return;
zfcp_dbf_scsi_nullcmnd((struct scsi_cmnd *)old_req->data, old_req);
+
+ if ((scsi_get_prot_op(scpnt) != SCSI_PROT_NORMAL) &&
+ scsi_prot_sg_count(scpnt))
+ zfcp_fsf_req_unmap_sg(old_req, &scpnt->prot_sdb->table,
+ scpnt->sc_data_direction);
+
+ zfcp_fsf_req_unmap_sg(old_req, &scpnt->sdb.table,
+ scpnt->sc_data_direction);
+
old_req->data = NULL;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v13 1/2] zfcp: Replace sg-list with sg_table in zfcp_fsf_ct_els structure
2026-10-21 8:33 ` [PATCH v13 1/2] zfcp: Replace sg-list with sg_table in zfcp_fsf_ct_els structure M Nikhil
@ 2026-10-21 8:40 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-21 8:40 UTC (permalink / raw)
To: M Nikhil, linux-s390
Cc: Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger
> The zfcp_fsf_ct_els data structure is updated to replace the sg_req and
> sg_resp members, which are of type struct scatterlist, with request and
> response sg_table instances.
>
> These sg_table structure reference the corresponding scatterlists
> (sg_req and sg_resp) maintained in struct zfcp_fc_req.
>
> These changes allows to track the original and mapped scatterlist nents
> once zfcp is converted to use DMA-API for mapping the sg lists.
>
> Signed-off-by: M Nikhil <nikh1092@linux.ibm.com>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261021083351.1626409-1-nikh1092@linux.ibm.com?part=1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v13 2/2] zfcp: Use DMA streaming API to map I/O
2026-10-21 8:33 ` [PATCH v13 2/2] zfcp: Use DMA streaming API to map I/O M Nikhil
@ 2026-10-21 9:04 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-21 9:04 UTC (permalink / raw)
To: M Nikhil, linux-s390, linux-scsi
Cc: Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] scsi: zfcp: premature DMA mapping and missing sync of QTCB in zfcp_fsf
- [Critical] scsi: zfcp: premature DMA unmap before abort in zfcp_scsi_eh_abort_handler()
- [High] scsi: zfcp: stale SBAL entries left in req_q on DMA map error paths
- [Medium] scsi: zfcp: stale sg_table nents in zfcp_fc_exec_bsg_job() truncates DMA map
--
commit 55b15d18ad86f261b72e93af06572f7a0dd07799
Author: M Nikhil <nikh1092@linux.ibm.com>
zfcp: Use DMA streaming API to map I/O
This commit converts zFCP I/O buffer handling (QTCB, SRB, CT/ELS, and SCSI
I/O buffers) from direct physical address translation to the DMA streaming
API using sg_table, dma_map_sgtable(), and dma_unmap_sgtable().
> diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
> index 12b3d2d0a2c87..2031055c48f8a 100644
> --- a/drivers/s390/scsi/zfcp_fsf.c
> +++ b/drivers/s390/scsi/zfcp_fsf.c
> @@ -14,6 +14,7 @@
> #include <linux/types.h>
> #include <linux/slab.h>
> #include <scsi/fc/fc_els.h>
> +#include <linux/dma-mapping.h>
> #include "zfcp_ext.h"
> #include "zfcp_fc.h"
> #include "zfcp_dbf.h"
> @@ -84,6 +85,86 @@ static void zfcp_fsf_class_not_supp(struct zfcp_fsf_req *req)
> req->status |= ZFCP_STATUS_FSFREQ_ERROR;
> }
>
> +/**
> + * zfcp_fsf_req_map_sg - map the scattergather list for DMA transfer
> + * @req: pointer to zfcp_fsf_req
> + * @sgt: pointer to scattergather list table
> + * @dma_dir: DMA data direction
> + * Returns: 0 on success, -EIO otherwise
> + */
> +int zfcp_fsf_req_map_sg(struct zfcp_fsf_req *req, struct sg_table *sgt,
> + enum dma_data_direction dma_dir)
> +{
> + unsigned int saved_orig_nents;
> + int ret;
> +
> + if (!valid_dma_direction(dma_dir))
> + return sgt->orig_nents ? -EIO : 0;
> +
> + if (!sgt->orig_nents)
> + return 0;
> +
> + /*
> + * The SCSI midlayer sets orig_nents to the allocated SG capacity and
> + * nents to the actually populated entry count (nents <= orig_nents).
> + * dma_map_sgtable() and dma_unmap_sgtable() both use orig_nents, so
> + * clamp it to the populated count to avoid mapping trailing zero-length
> + * entries. Only clamp when nents has been explicitly set (nents > 0);
> + * zFCP-internal sg_tables leave nents = 0 before mapping, relying
> + * solely on orig_nents.
> + */
> + saved_orig_nents = sgt->orig_nents;
> + if (sgt->nents > 0 && sgt->nents < sgt->orig_nents)
> + sgt->orig_nents = sgt->nents;
> +
> + ret = dma_map_sgtable(&req->adapter->ccw_device->dev, sgt, dma_dir, 0);
> +
> + sgt->orig_nents = saved_orig_nents;
> +
> + return ret;
> +}
> +
> +/**
> + * zfcp_fsf_req_unmap_sg - unmap the scattergather list after DMA transfer
> + * @req: pointer to zfcp_fsf_req
> + * @sgt: pointer to scattergather list table
> + * @dma_dir: DMA direction of the sg
> + */
> +void zfcp_fsf_req_unmap_sg(struct zfcp_fsf_req *req, struct sg_table *sgt,
> + enum dma_data_direction dma_dir)
> +{
> + unsigned int saved_orig_nents;
> +
> + /*
> + * We still need the check for mapped sg count here to not perform
> + * unmap for SCSI commands which have zero payload.
> + */
> + if (!sgt->nents)
> + return;
> +
> + saved_orig_nents = sgt->orig_nents;
> + if (sgt->nents < sgt->orig_nents)
> + sgt->orig_nents = sgt->nents;
> +
> + dma_unmap_sgtable(&req->adapter->ccw_device->dev, sgt, dma_dir, 0);
> +
> + sgt->orig_nents = saved_orig_nents;
> +}
[Severity: Medium]
Can a reused BSG request tag leave a stale sgt->nents value in
ct_els->sgt_req or ct_els->sgt_resp and cause zfcp_fsf_req_map_sg() to
truncate the DMA mapping?
In drivers/s390/scsi/zfcp_fsf.c, zfcp_fsf_req_map_sg() assumes that
zFCP-internal sg_table structures always have sgt->nents == 0 prior to
mapping, and clamps sgt->orig_nents to sgt->nents whenever
0 < sgt->nents < sgt->orig_nents. When the request completes,
zfcp_fsf_req_unmap_sg() leaves sgt->nents set to the mapped entry count.
However, in the FC BSG path:
bsg_transport_sg_io_fn()
bsg_queue_rq()
fc_bsg_dispatch()
zfcp_fc_exec_bsg_job()
zfcp_fsf_send_ct() / zfcp_fsf_send_els()
zfcp_fsf_setup_ct_els()
zfcp_fsf_setup_ct_els_sbals()
zfcp_qdio_sbals_from_sg()
zfcp_fsf_req_map_sg()
bsg_transport_sg_io_fn() only zeroes sizeof(struct bsg_job) and does not
clear job->dd_data (struct zfcp_fsf_ct_els) in the blk-mq request PDU. In
drivers/s390/scsi/zfcp_fc.c, zfcp_fc_exec_bsg_job() initializes sgl and
orig_nents without resetting nents:
ct_els->sgt_req.sgl = job->request_payload.sg_list;
ct_els->sgt_resp.sgl = job->reply_payload.sg_list;
ct_els->sgt_req.orig_nents = sg_nents(ct_els->sgt_req.sgl);
ct_els->sgt_resp.orig_nents = sg_nents(ct_els->sgt_resp.sgl);
If a BSG request tag is reused for a multi-segment payload after a command
that mapped fewer SG segments, won't zfcp_fsf_req_map_sg() clamp orig_nents
to the stale sgt->nents value and map only part of the scatterlist into
SBALEs while zfcp_qdio_real_bytes() sets req_buf_length and resp_buf_length
to the full byte count, causing the adapter to reject the request with
FSF_SBAL_MISMATCH?
[ ... ]
> @@ -870,10 +954,27 @@ static struct zfcp_fsf_req *zfcp_fsf_req_create(struct zfcp_qdio *qdio,
> req->qtcb->prefix.qtcb_version = FSF_QTCB_CURRENT_VERSION;
> req->qtcb->header.req_handle = req->req_id;
> req->qtcb->header.fsf_command = fsf_cmd;
> - }
>
> - zfcp_qdio_req_init(adapter->qdio, &req->qdio_req, req->req_id, sbtype,
> - req->qtcb, sizeof(struct fsf_qtcb));
> + sg_init_one(&req->metadata_sg, req->qtcb,
> + sizeof(struct fsf_qtcb));
> + req->metadata_sgt.sgl = &req->metadata_sg;
> + req->metadata_sgt.orig_nents = 1;
> +
> + if (zfcp_fsf_req_map_sg(req, &req->metadata_sgt,
> + DMA_BIDIRECTIONAL)) {
> + zfcp_fsf_req_free(req);
> + return ERR_PTR(-EIO);
> + }
> +
> + zfcp_qdio_req_init(adapter->qdio, &req->qdio_req, req->req_id,
> + sbtype, true,
> + sg_dma_len(req->metadata_sgt.sgl),
> + sg_dma_address(req->metadata_sgt.sgl));
> + } else {
> + /* SRB */
> + zfcp_qdio_req_init(adapter->qdio, &req->qdio_req, req->req_id,
> + sbtype, false, 0, 0);
> + }
>
> return req;
> }
[ ... ]
> @@ -1130,19 +1247,41 @@ static void zfcp_fsf_send_ct_handler(struct zfcp_fsf_req *req)
> ct->handler(ct->handler_data);
> }
>
> -static void zfcp_fsf_setup_ct_els_unchained(struct zfcp_qdio *qdio,
> - struct zfcp_qdio_req *q_req,
> - struct scatterlist *sg_req,
> - struct scatterlist *sg_resp)
> +static int zfcp_fsf_setup_ct_els_unchained(struct zfcp_fsf_req *req,
> + struct zfcp_qdio *qdio,
> + struct sg_table *sg_req_table,
> + struct sg_table *sg_resp_table)
> {
> - zfcp_qdio_fill_next(qdio, q_req, sg_virt(sg_req), sg_req->length);
> - zfcp_qdio_fill_next(qdio, q_req, sg_virt(sg_resp), sg_resp->length);
> - zfcp_qdio_set_sbale_last(qdio, q_req);
> + int retval = 0;
> +
> + retval = zfcp_fsf_req_map_sg(req, sg_req_table, DMA_TO_DEVICE);
> + if (unlikely(retval))
> + goto err;
> +
> + retval = zfcp_fsf_req_map_sg(req, sg_resp_table,
> + DMA_FROM_DEVICE);
> + if (unlikely(retval))
> + goto err_unmap;
> +
> + zfcp_qdio_fill_next(qdio, &req->qdio_req,
> + sg_dma_address(sg_req_table->sgl),
> + sg_dma_len(sg_req_table->sgl));
> + zfcp_qdio_fill_next(qdio, &req->qdio_req,
> + sg_dma_address(sg_resp_table->sgl),
> + sg_dma_len(sg_resp_table->sgl));
> + zfcp_qdio_set_sbale_last(qdio, &req->qdio_req);
> +
> + return 0;
> +
> +err_unmap:
> + zfcp_fsf_req_unmap_sg(req, sg_req_table, DMA_TO_DEVICE);
> +err:
> + return retval;
> }
>
> static int zfcp_fsf_setup_ct_els_sbals(struct zfcp_fsf_req *req,
> - struct scatterlist *sg_req,
> - struct scatterlist *sg_resp)
> + struct sg_table *sgt_req,
> + struct sg_table *sgt_resp)
> {
> struct zfcp_adapter *adapter = req->adapter;
> struct zfcp_qdio *qdio = adapter->qdio;
> @@ -1150,57 +1289,72 @@ static int zfcp_fsf_setup_ct_els_sbals(struct zfcp_fsf_req *req,
> u32 feat = adapter->adapter_features;
>
> if (zfcp_adapter_multi_buffer_active(adapter)) {
> - if (zfcp_qdio_sbals_from_sg(qdio, &req->qdio_req, sg_req))
> + if (zfcp_qdio_sbals_from_sg(qdio, sgt_req, req, DMA_TO_DEVICE))
> return -EIO;
> +
> qtcb->bottom.support.req_buf_length =
> - zfcp_qdio_real_bytes(sg_req);
> - if (zfcp_qdio_sbals_from_sg(qdio, &req->qdio_req, sg_resp))
> - return -EIO;
> + zfcp_qdio_real_bytes(sgt_req->sgl);
> +
> + if (zfcp_qdio_sbals_from_sg(qdio, sgt_resp, req, DMA_FROM_DEVICE))
> + goto err_unmap;
> +
> qtcb->bottom.support.resp_buf_length =
> - zfcp_qdio_real_bytes(sg_resp);
> + zfcp_qdio_real_bytes(sgt_resp->sgl);
>
> - zfcp_qdio_set_data_div(qdio, &req->qdio_req, sg_nents(sg_req));
> + zfcp_qdio_set_data_div(qdio, &req->qdio_req,
> + sgt_req->nents);
> zfcp_qdio_set_sbale_last(qdio, &req->qdio_req);
> zfcp_qdio_set_scount(qdio, &req->qdio_req);
> return 0;
> }
>
> /* use single, unchained SBAL if it can hold the request */
> - if (zfcp_qdio_sg_one_sbale(sg_req) && zfcp_qdio_sg_one_sbale(sg_resp)) {
> - zfcp_fsf_setup_ct_els_unchained(qdio, &req->qdio_req,
> - sg_req, sg_resp);
> + if (zfcp_qdio_sg_one_sbale(sgt_req->sgl) &&
> + zfcp_qdio_sg_one_sbale(sgt_resp->sgl)) {
> + if (zfcp_fsf_setup_ct_els_unchained(req, qdio, sgt_req,
> + sgt_resp))
> + return -EIO;
> +
> return 0;
> }
>
> if (!(feat & FSF_FEATURE_ELS_CT_CHAINED_SBALS))
> return -EOPNOTSUPP;
>
> - if (zfcp_qdio_sbals_from_sg(qdio, &req->qdio_req, sg_req))
> + if (zfcp_qdio_sbals_from_sg(qdio, sgt_req, req, DMA_TO_DEVICE))
> return -EIO;
>
> - qtcb->bottom.support.req_buf_length = zfcp_qdio_real_bytes(sg_req);
> + qtcb->bottom.support.req_buf_length =
> + zfcp_qdio_real_bytes(sgt_req->sgl);
>
> zfcp_qdio_set_sbale_last(qdio, &req->qdio_req);
> zfcp_qdio_skip_to_last_sbale(qdio, &req->qdio_req);
>
> - if (zfcp_qdio_sbals_from_sg(qdio, &req->qdio_req, sg_resp))
> - return -EIO;
> + if (zfcp_qdio_sbals_from_sg(qdio, sgt_resp, req, DMA_FROM_DEVICE))
> + goto err_unmap;
>
> - qtcb->bottom.support.resp_buf_length = zfcp_qdio_real_bytes(sg_resp);
> + qtcb->bottom.support.resp_buf_length =
> + zfcp_qdio_real_bytes(sgt_resp->sgl);
>
> zfcp_qdio_set_sbale_last(qdio, &req->qdio_req);
>
> return 0;
> +
> +err_unmap:
> + zfcp_fsf_req_unmap_sg(req, sgt_req, DMA_TO_DEVICE);
> + zfcp_qdio_zero_sbals(qdio->req_q, req->qdio_req.sbal_first,
> + req->qdio_req.sbal_number);
> + return -EIO;
> }
[ ... ]
> @@ -2606,6 +2793,10 @@ int zfcp_fsf_fcp_cmnd(struct scsi_cmnd *scsi_cmnd)
> io->service_class = FSF_CLASS_3;
> io->fcp_cmnd_length = FCP_CMND_LEN;
>
> + dma_sync_sgtable_for_device(&req->adapter->ccw_device->dev,
> + &req->metadata_sgt,
> + DMA_BIDIRECTIONAL);
> +
> if (scsi_get_prot_op(scsi_cmnd) != SCSI_PROT_NORMAL) {
> io->data_block_length = scsi_prot_interval(scsi_cmnd);
> io->ref_tag_value = scsi_prot_ref_tag(scsi_cmnd);
[Severity: Critical]
When SWIOTLB bounce buffering is active (such as under s390 Secure Execution),
won't the device see stale/zeroed QTCB contents for SCSI commands and all
other QTCB-based FSF requests?
In drivers/s390/scsi/zfcp_fsf.c, zfcp_fsf_req_create() maps req->qtcb with
zfcp_fsf_req_map_sg(req, &req->metadata_sgt, DMA_BIDIRECTIONAL) immediately
after setting only the initial prefix and header fields. Under SWIOTLB,
dma_map_sgtable() copies req->qtcb into the bounce buffer at that point.
In zfcp_fsf_fcp_cmnd(), dma_sync_sgtable_for_device() is called before the
remaining fields in req->qtcb->bottom.io are populated:
dma_sync_sgtable_for_device(&req->adapter->ccw_device->dev,
&req->metadata_sgt,
DMA_BIDIRECTIONAL);
if (scsi_get_prot_op(scsi_cmnd) != SCSI_PROT_NORMAL) {
io->data_block_length = scsi_prot_interval(scsi_cmnd);
io->ref_tag_value = scsi_prot_ref_tag(scsi_cmnd);
}
if (zfcp_fsf_set_data_dir(scsi_cmnd, &io->data_direction))
goto failed_scsi_cmnd;
BUILD_BUG_ON(sizeof(struct fcp_cmnd) > FSF_FCP_CMND_SIZE);
fcp_cmnd = &req->qtcb->bottom.io.fcp_cmnd.iu;
zfcp_fc_scsi_to_fcp(fcp_cmnd, scsi_cmnd);
As a result, io->data_block_length, io->ref_tag_value, io->data_direction,
fcp_cmnd (including the FCP LUN, CDB, and transfer length), and
io->prot_data_length are written after the sync and are never copied to the
bounce buffer.
In addition, none of the other QTCB-based FSF functions in
drivers/s390/scsi/zfcp_fsf.c (zfcp_fsf_abort_fcp_cmnd(), zfcp_fsf_send_ct(),
zfcp_fsf_send_els(), zfcp_fsf_exchange_config_data(),
zfcp_fsf_exchange_config_data_sync(), zfcp_fsf_exchange_port_data(),
zfcp_fsf_exchange_port_data_sync(), zfcp_fsf_open_port(),
zfcp_fsf_close_port(), zfcp_fsf_open_wka_port(), zfcp_fsf_close_wka_port(),
zfcp_fsf_close_physical_port(), zfcp_fsf_open_lun(), zfcp_fsf_close_lun(),
and zfcp_fsf_fcp_task_mgmt()) or zfcp_fsf_req_send() call
dma_sync_sgtable_for_device() after filling in req->qtcb.
Should req->qtcb be synced in zfcp_fsf_req_send() right before
zfcp_qdio_send(), or mapped only after req->qtcb is fully populated?
> @@ -2620,20 +2811,22 @@ int zfcp_fsf_fcp_cmnd(struct scsi_cmnd *scsi_cmnd)
>
> if ((scsi_get_prot_op(scsi_cmnd) != SCSI_PROT_NORMAL) &&
> scsi_prot_sg_count(scsi_cmnd)) {
> - zfcp_qdio_set_data_div(qdio, &req->qdio_req,
> - scsi_prot_sg_count(scsi_cmnd));
> - retval = zfcp_qdio_sbals_from_sg(qdio, &req->qdio_req,
> - scsi_prot_sglist(scsi_cmnd));
> + retval = zfcp_qdio_sbals_from_sg(qdio,
> + &scsi_cmnd->prot_sdb->table,
> + req,
> + scsi_cmnd->sc_data_direction);
> if (retval)
> goto failed_scsi_cmnd;
> + zfcp_qdio_set_data_div(qdio, &req->qdio_req,
> + scsi_cmnd->prot_sdb->table.nents);
> io->prot_data_length = zfcp_qdio_real_bytes(
> scsi_prot_sglist(scsi_cmnd));
> }
>
> - retval = zfcp_qdio_sbals_from_sg(qdio, &req->qdio_req,
> - scsi_sglist(scsi_cmnd));
> + retval = zfcp_qdio_sbals_from_sg(qdio, &scsi_cmnd->sdb.table, req,
> + scsi_cmnd->sc_data_direction);
> if (unlikely(retval))
> - goto failed_scsi_cmnd;
> + goto failed_normal_scsi_map;
>
> zfcp_qdio_set_sbale_last(adapter->qdio, &req->qdio_req);
> if (zfcp_adapter_multi_buffer_active(adapter))
> @@ -2641,12 +2834,25 @@ int zfcp_fsf_fcp_cmnd(struct scsi_cmnd *scsi_cmnd)
>
> retval = zfcp_fsf_req_send(req);
> if (unlikely(retval))
> - goto failed_scsi_cmnd;
> + goto failed_req_send;
> +
> /* NOTE: DO NOT TOUCH req PAST THIS POINT! */
>
> goto out;
>
> +failed_req_send:
> + zfcp_fsf_req_unmap_sg(req, &scsi_cmnd->sdb.table,
> + scsi_cmnd->sc_data_direction);
> +failed_normal_scsi_map:
> + if ((scsi_get_prot_op(scsi_cmnd) != SCSI_PROT_NORMAL) &&
> + scsi_prot_sg_count(scsi_cmnd)) {
> + zfcp_fsf_req_unmap_sg(req, &scsi_cmnd->prot_sdb->table,
> + scsi_cmnd->sc_data_direction);
> + zfcp_qdio_zero_sbals(qdio->req_q, req->qdio_req.sbal_first,
> + req->qdio_req.sbal_number);
> + }
> failed_scsi_cmnd:
> + zfcp_fsf_req_unmap_metadata(req);
> zfcp_fsf_req_free(req);
> scsi_cmnd->host_scribble = NULL;
> out:
[ ... ]
> diff --git a/drivers/s390/scsi/zfcp_qdio.c b/drivers/s390/scsi/zfcp_qdio.c
> index bac8385e069fa..50db08388514c 100644
> --- a/drivers/s390/scsi/zfcp_qdio.c
> +++ b/drivers/s390/scsi/zfcp_qdio.c
> @@ -233,31 +233,40 @@ zfcp_qdio_sbale_next(struct zfcp_qdio *qdio, struct zfcp_qdio_req *q_req)
> }
>
> /**
> - * zfcp_qdio_sbals_from_sg - fill SBALs from scatter-gather list
> + * zfcp_qdio_sbals_from_sg - DMA map scatter-gather list and fill SBALs from it
> * @qdio: pointer to struct zfcp_qdio
> - * @q_req: pointer to struct zfcp_qdio_req
> - * @sg: scatter-gather list
> - * Returns: zero or -EINVAL on error
> + * @sgt: pointer to scattergather list table
> + * @req: pointer to zfcp_fsf_req
> + * @dma_dir: DMA direction of I/O
> + * Returns: zero on success, -EINVAL or -EIO otherwise
> */
> -int zfcp_qdio_sbals_from_sg(struct zfcp_qdio *qdio, struct zfcp_qdio_req *q_req,
> - struct scatterlist *sg)
> +int zfcp_qdio_sbals_from_sg(struct zfcp_qdio *qdio, struct sg_table *sgt,
> + struct zfcp_fsf_req *req,
> + enum dma_data_direction dma_dir)
> {
> struct qdio_buffer_element *sbale;
> + int i;
> + struct scatterlist *sg_element;
> +
> + if (zfcp_fsf_req_map_sg(req, sgt, dma_dir))
> + return -EIO;
>
> /* set storage-block type for this request */
> - sbale = zfcp_qdio_sbale_req(qdio, q_req);
> - sbale->sflags |= q_req->sbtype;
> + sbale = zfcp_qdio_sbale_req(qdio, &req->qdio_req);
> + sbale->sflags |= req->qdio_req.sbtype;
>
> - for (; sg; sg = sg_next(sg)) {
> - sbale = zfcp_qdio_sbale_next(qdio, q_req);
> + for_each_sgtable_dma_sg(sgt, sg_element, i) {
> + sbale = zfcp_qdio_sbale_next(qdio, &req->qdio_req);
> if (!sbale) {
> atomic_inc(&qdio->req_q_full);
> - zfcp_qdio_zero_sbals(qdio->req_q, q_req->sbal_first,
> - q_req->sbal_number);
> + zfcp_qdio_zero_sbals(qdio->req_q,
> + req->qdio_req.sbal_first,
> + req->qdio_req.sbal_number);
> + zfcp_fsf_req_unmap_sg(req, sgt, dma_dir);
> return -EINVAL;
> }
> - sbale->addr = u64_to_dma64(sg_phys(sg));
> - sbale->length = sg->length;
> + sbale->addr = u64_to_dma64(sg_dma_address(sg_element));
> + sbale->length = sg_dma_len(sg_element);
> }
> return 0;
> }
> diff --git a/drivers/s390/scsi/zfcp_qdio.h b/drivers/s390/scsi/zfcp_qdio.h
> index 8f7d2ae94441a..9f89f2975f6ff 100644
> --- a/drivers/s390/scsi/zfcp_qdio.h
> +++ b/drivers/s390/scsi/zfcp_qdio.h
> @@ -106,16 +107,19 @@ zfcp_qdio_sbale_curr(struct zfcp_qdio *qdio, struct zfcp_qdio_req *q_req)
> * @q_req: the qdio request to start
> * @req_id: The request id
> * @sbtype: type flags to set for all sbals
> - * @data: First data block
> - * @len: Length of first data block
> + * @has_qtcb: FSF request has a QTCB
> + * @qtcb_len: Length of first data block QTCB, if any
> + * @qtcb_dma_handle: DMA handle of the QTCB, if any
> *
> * This is the start of putting the request into the queue, the last
> * step is passing the request to zfcp_qdio_send. The request queue
> * lock must be held during the whole process from init to send.
> */
> static inline
> -void zfcp_qdio_req_init(struct zfcp_qdio *qdio, struct zfcp_qdio_req *q_req,
> - u64 req_id, u8 sbtype, void *data, u32 len)
> +void zfcp_qdio_req_init(struct zfcp_qdio *qdio,
> + struct zfcp_qdio_req *q_req, u64 req_id,
> + u8 sbtype, bool has_qtcb, u32 qtcb_len,
> + dma_addr_t qtcb_dma_handle)
> {
> struct qdio_buffer_element *sbale;
> int count = min(atomic_read(&qdio->req_q_free),
> @@ -133,33 +137,34 @@ void zfcp_qdio_req_init(struct zfcp_qdio *qdio, struct zfcp_qdio_req *q_req,
> sbale->eflags = 0;
> sbale->sflags = SBAL_SFLAGS0_COMMAND | sbtype;
>
> - if (unlikely(!data))
> + if (unlikely(!has_qtcb))
> return;
> sbale++;
> - sbale->addr = virt_to_dma64(data);
> - sbale->length = len;
> + sbale->addr = u64_to_dma64(qtcb_dma_handle);
> + sbale->length = qtcb_len;
> }
[Severity: High]
If a payload DMA mapping fails after zfcp_fsf_req_create() has already
initialized the SBAL in qdio->req_q, doesn't this leave a stale, unmapped
QTCB DMA address in element[1] of qdio->req_q[qdio->req_q_idx]?
In zfcp_fsf_req_create(), zfcp_qdio_req_init() writes req_id and sflags to
element[0] and writes qtcb_dma_handle and qtcb_len to element[1] of the SBAL
at qdio->req_q_idx.
If zfcp_fsf_req_map_sg() then fails in zfcp_qdio_sbals_from_sg() or
zfcp_fsf_setup_ct_els_unchained(), it returns -EIO without zeroing the SBAL:
- In zfcp_fsf_fcp_cmnd() (drivers/s390/scsi/zfcp_fsf.c), if mapping
scsi_cmnd->sdb.table fails on a command without DIF/DIX protection data (or
if mapping scsi_cmnd->prot_sdb->table fails, or zfcp_fsf_set_data_dir()
fails), failed_normal_scsi_map and failed_scsi_cmnd unmap and free
req->qtcb without calling zfcp_qdio_zero_sbals().
- In zfcp_fsf_setup_ct_els_sbals() (drivers/s390/scsi/zfcp_fsf.c), if
mapping sgt_req fails or zfcp_fsf_setup_ct_els_unchained() fails, it
returns -EIO without calling zfcp_qdio_zero_sbals().
Because qdio->req_q_idx is only advanced in zfcp_qdio_send(), the next
request reuses the same SBAL slot. If that next request is
zfcp_fsf_status_read(), zfcp_qdio_req_init() is called with has_qtcb = false,
which returns early while leaving sbale_curr = 1 and leaving the freed QTCB's
DMA address and length in element[1], and zfcp_qdio_fill_next() then writes
sr_buf into element[2].
> diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c
> index ea4b968352f4d..091be339d309c 100644
> --- a/drivers/s390/scsi/zfcp_scsi.c
> +++ b/drivers/s390/scsi/zfcp_scsi.c
> @@ -184,6 +184,20 @@ static int zfcp_scsi_eh_abort_handler(struct scsi_cmnd *scpnt)
> zfcp_dbf_scsi_abort("abrt_or", scpnt, NULL);
> return FAILED; /* completion could be in progress */
> }
> +
> + /*
> + * Unmap SCSI payload DMA mappings before clearing req->data.
> + * Once req->data is NULL the hardware completion handler cannot
> + * reach the sg_tables, causing a permanent IOVA leak.
> + */
> + if ((scsi_get_prot_op(scpnt) != SCSI_PROT_NORMAL) &&
> + scsi_prot_sg_count(scpnt))
> + zfcp_fsf_req_unmap_sg(old_req, &scpnt->prot_sdb->table,
> + scpnt->sc_data_direction);
> +
> + zfcp_fsf_req_unmap_sg(old_req, &scpnt->sdb.table,
> + scpnt->sc_data_direction);
> +
> old_req->data = NULL;
>
> /* don't access old fsf_req after releasing the abort_lock */
[Severity: Critical]
Is it safe to unmap scpnt->prot_sdb->table and scpnt->sdb.table in
zfcp_scsi_eh_abort_handler() before the FCP adapter has actually aborted
old_req?
Here in drivers/s390/scsi/zfcp_scsi.c, zfcp_scsi_eh_abort_handler() unmaps
the SCSI command's DMA buffers and clears old_req->data before calling
zfcp_fsf_abort_fcp_cmnd(scpnt) and waiting on abrt_req->completion:
zfcp_fsf_req_unmap_sg(old_req, &scpnt->sdb.table,
scpnt->sc_data_direction);
old_req->data = NULL;
/* don't access old fsf_req after releasing the abort_lock */
write_unlock_irqrestore(&adapter->abort_lock, flags);
while (retry--) {
abrt_req = zfcp_fsf_abort_fcp_cmnd(scpnt);
Until the adapter processes the abort command (or if zfcp_fsf_abort_fcp_cmnd()
cannot be sent, fc_block_scsi_eh() returns early, or the abort fails and
returns FAILED), old_req is still active on the FCP channel and may continue
DMA transfers into or out of the unmapped buffers and released SWIOTLB bounce
pages.
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261021083351.1626409-1-nikh1092@linux.ibm.com?part=2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v13 2/2] zfcp: Use DMA streaming API to map I/O
2026-10-22 17:08 [PATCH v13 0/2] " M Nikhil
@ 2026-10-22 17:08 ` M Nikhil
2026-10-22 18:15 ` sashiko-bot
0 siblings, 1 reply; 7+ messages in thread
From: M Nikhil @ 2026-10-22 17:08 UTC (permalink / raw)
To: sashiko-reviews, linux-s390, linux-scsi; +Cc: niharp, pasic, bblock
In the existing zFCP driver, virtual addresses are translated directly
into bus (physical) addresses for DMA operations on all I/O buffers
passed from the zFCP driver to the device.
Need for DMA API:
In secure execution environment, the host must ensure that device DMA
addresses are restricted to non-guest-private, page-sized memory blocks.
This prevents I/O failures that can occur when a device attempts to
access guest-private memory. Therefore, guest memory pages must be
prepared by the hypervisor before they become accessible to an I/O
device.
To support this requirement, the host linux kernel must be reliably
informed whenever a memory region becomes eligible for device DMA access
and when that access must be revoked. This information must be
maintained correctly at least at page level granularity.
To satisfy these requirements, all I/O buffers used by the zFCP driver
should be mapped through the DMA API.
DMA streaming API usage in zFCP:
Changes in this patch-series converts all I/O buffer mappings in the
zFCP driver to use the DMA streaming API. Specifically,
`dma_map_sgtable()` and `dma_unmap_sgtable()` are used to map and unmap
the scatter-gather lists.
All zFCP I/O buffers (QTCB, SRB, CT/ELS and SCSI I/O) are represented
using scatter-gather lists. The `sg_table` structure, which encapsulates
the scatterlist together with the original and mapped entry counts, is
used to manage DMA mappings efficiently.
Use struct sg_table to manage scatter-gather lists for QTCB, SRB,
CT/ELS, and SCSI I/O buffers. Initialize single-entry scatterlists for
buffers that are not already represented by scatter-gather tables. For
SCSI I/O, reuse the tables provided by the SCSI midlayer and temporarily
adjust orig_nents when fewer entries are populated than allocated.
Map buffers after updating their contents. For SCSI I/O, synchronize
req->qtcb for device access in zfcp_fsf_fcp_cmnd() before submitting the
request to QDIO. DMA unmapping handles synchronization for CPU access on
the receive path.
Signed-off-by: M Nikhil <nikh1092@linux.ibm.com>
Reviewed-by: Benjamin Block <bblock@linux.ibm.com>
Reviewed-by: Nihar Panda <niharp@linux.ibm.com>
---
drivers/s390/scsi/zfcp_aux.c | 16 ++
drivers/s390/scsi/zfcp_def.h | 4 +
drivers/s390/scsi/zfcp_ext.h | 11 +-
drivers/s390/scsi/zfcp_fc.c | 12 ++
drivers/s390/scsi/zfcp_fsf.c | 323 +++++++++++++++++++++++++++++-----
drivers/s390/scsi/zfcp_qdio.c | 40 +++--
drivers/s390/scsi/zfcp_qdio.h | 27 +--
drivers/s390/scsi/zfcp_scsi.c | 67 ++++++-
8 files changed, 423 insertions(+), 77 deletions(-)
diff --git a/drivers/s390/scsi/zfcp_aux.c b/drivers/s390/scsi/zfcp_aux.c
index fea573e00f9d..20c905c68e1e 100644
--- a/drivers/s390/scsi/zfcp_aux.c
+++ b/drivers/s390/scsi/zfcp_aux.c
@@ -329,6 +329,19 @@ static void zfcp_destroy_adapter_work_queue(struct zfcp_adapter *adapter)
}
+static int zfcp_set_dma_mask_for_adapter(struct zfcp_adapter *adapter)
+{
+ int ret;
+
+ ret = dma_set_mask_and_coherent(&adapter->ccw_device->dev,
+ DMA_BIT_MASK(64));
+ if (ret)
+ dev_err(&adapter->ccw_device->dev,
+ "Setting the DMA mask failed for the FCP device.\n");
+
+ return ret;
+}
+
/**
* zfcp_adapter_enqueue - enqueue a new adapter to the list
* @ccw_device: pointer to the struct cc_device
@@ -404,6 +417,9 @@ struct zfcp_adapter *zfcp_adapter_enqueue(struct ccw_device *ccw_device)
rwlock_init(&adapter->erp_lock);
rwlock_init(&adapter->abort_lock);
+ if (zfcp_set_dma_mask_for_adapter(adapter))
+ goto failed;
+
if (zfcp_erp_thread_setup(adapter))
goto failed;
diff --git a/drivers/s390/scsi/zfcp_def.h b/drivers/s390/scsi/zfcp_def.h
index 6c761299a22f..5063921e8d93 100644
--- a/drivers/s390/scsi/zfcp_def.h
+++ b/drivers/s390/scsi/zfcp_def.h
@@ -322,6 +322,8 @@ static inline u64 zfcp_scsi_dev_lun(struct scsi_device *sdev)
* @pool: reference to memory pool if used for this request
* @issued: time when request was send (STCK)
* @handler: handler which should be called to process response
+ * @metadata_sgt: sg_table for metadata of a QTCB or SRB request
+ * @metadata_sg: scatterlist for metadata of a QTCB or SRB request
*/
struct zfcp_fsf_req {
struct list_head list;
@@ -337,6 +339,8 @@ struct zfcp_fsf_req {
mempool_t *pool;
unsigned long long issued;
void (*handler)(struct zfcp_fsf_req *);
+ struct sg_table metadata_sgt;
+ struct scatterlist metadata_sg;
};
static inline
diff --git a/drivers/s390/scsi/zfcp_ext.h b/drivers/s390/scsi/zfcp_ext.h
index 0eb9bc9d03e4..62d0a5686deb 100644
--- a/drivers/s390/scsi/zfcp_ext.h
+++ b/drivers/s390/scsi/zfcp_ext.h
@@ -150,6 +150,11 @@ enum zfcp_fsf_print_fmt {
extern ssize_t zfcp_fsf_scnprint_fc_security(char *buf, size_t size,
u32 fc_security,
enum zfcp_fsf_print_fmt fmt);
+int zfcp_fsf_req_map_sg(struct zfcp_fsf_req *req, struct sg_table *sgt,
+ enum dma_data_direction dma_dir);
+void zfcp_fsf_req_unmap_sg(struct device *dev,
+ struct sg_table *sgt,
+ enum dma_data_direction dma_dir);
/* zfcp_qdio.c */
extern int zfcp_qdio_setup(struct zfcp_adapter *);
@@ -157,13 +162,15 @@ extern void zfcp_qdio_destroy(struct zfcp_qdio *);
extern int zfcp_qdio_sbal_get(struct zfcp_qdio *qdio)
__must_hold(qdio->req_q_lock);
extern int zfcp_qdio_send(struct zfcp_qdio *, struct zfcp_qdio_req *);
-extern int zfcp_qdio_sbals_from_sg(struct zfcp_qdio *, struct zfcp_qdio_req *,
- struct scatterlist *);
+int zfcp_qdio_sbals_from_sg(struct zfcp_qdio *qdio, struct sg_table *sgt,
+ struct zfcp_fsf_req *req,
+ enum dma_data_direction dma_dir);
extern void zfcp_qdio_shost_update(struct zfcp_adapter *const adapter,
const struct zfcp_qdio *const qdio);
extern int zfcp_qdio_open(struct zfcp_qdio *);
extern void zfcp_qdio_close(struct zfcp_qdio *);
extern void zfcp_qdio_siosl(struct zfcp_adapter *);
+void zfcp_qdio_zero_sbals(struct qdio_buffer *sbal[], int first, int cnt);
/* zfcp_scsi.c */
extern bool zfcp_experimental_dix;
diff --git a/drivers/s390/scsi/zfcp_fc.c b/drivers/s390/scsi/zfcp_fc.c
index 1fb6f2c3b607..bc8dfdebe88f 100644
--- a/drivers/s390/scsi/zfcp_fc.c
+++ b/drivers/s390/scsi/zfcp_fc.c
@@ -1104,6 +1104,18 @@ int zfcp_fc_exec_bsg_job(struct bsg_job *job)
ct_els->sgt_resp.sgl = job->reply_payload.sg_list;
ct_els->sgt_req.orig_nents = sg_nents(ct_els->sgt_req.sgl);
ct_els->sgt_resp.orig_nents = sg_nents(ct_els->sgt_resp.sgl);
+ /*
+ * dd_data lives past the memset(job, 0, sizeof(*job)) boundary in
+ * bsg_transport_sg_io_fn() and is never zeroed by blk-mq on request
+ * reuse. A prior BSG command will have left sgt->nents set to the
+ * mapped entry count from dma_map_sgtable(). zfcp_fsf_req_map_sg()
+ * treats any non-zero nents as a SCSI-midlayer "populated < allocated"
+ * hint and clamps orig_nents to it, causing truncated DMA mappings for
+ * larger follow-on payloads. Reset nents so these sg_tables are
+ * handled like zFCP-internal ones (nents == 0 before mapping).
+ */
+ ct_els->sgt_req.nents = 0;
+ ct_els->sgt_resp.nents = 0;
ct_els->handler_data = job;
switch (bsg_request->msgcode) {
diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
index a3b99839da9e..25f15d67c37a 100644
--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -14,6 +14,7 @@
#include <linux/types.h>
#include <linux/slab.h>
#include <scsi/fc/fc_els.h>
+#include <linux/dma-mapping.h>
#include "zfcp_ext.h"
#include "zfcp_fc.h"
#include "zfcp_dbf.h"
@@ -84,6 +85,88 @@ static void zfcp_fsf_class_not_supp(struct zfcp_fsf_req *req)
req->status |= ZFCP_STATUS_FSFREQ_ERROR;
}
+/**
+ * zfcp_fsf_req_map_sg - map the scattergather list for DMA transfer
+ * @req: pointer to zfcp_fsf_req
+ * @sgt: pointer to scattergather list table
+ * @dma_dir: DMA data direction
+ * Returns: 0 on success, -EIO otherwise
+ */
+int zfcp_fsf_req_map_sg(struct zfcp_fsf_req *req, struct sg_table *sgt,
+ enum dma_data_direction dma_dir)
+{
+ unsigned int saved_orig_nents;
+ int ret;
+
+ if (!valid_dma_direction(dma_dir))
+ return sgt->orig_nents ? -EIO : 0;
+
+ if (!sgt->orig_nents)
+ return 0;
+
+ /*
+ * The SCSI midlayer sets orig_nents to the allocated SG capacity and
+ * nents to the actually populated entry count (nents <= orig_nents).
+ * dma_map_sgtable() and dma_unmap_sgtable() both use orig_nents, so
+ * clamp it to the populated count to avoid mapping trailing zero-length
+ * entries. Only clamp when nents has been explicitly set (nents > 0);
+ * zFCP-internal sg_tables leave nents = 0 before mapping, relying
+ * solely on orig_nents.
+ */
+ saved_orig_nents = sgt->orig_nents;
+ if (sgt->nents > 0 && sgt->nents < sgt->orig_nents)
+ sgt->orig_nents = sgt->nents;
+
+ ret = dma_map_sgtable(&req->adapter->ccw_device->dev, sgt, dma_dir, 0);
+
+ sgt->orig_nents = saved_orig_nents;
+
+ return ret;
+}
+
+/**
+ * zfcp_fsf_req_unmap_sg - unmap the scattergather list after DMA transfer
+ * @dev: pointer to the device used for DMA
+ * @sgt: pointer to scattergather list table
+ * @dma_dir: DMA direction of the sg
+ */
+void zfcp_fsf_req_unmap_sg(struct device *dev, struct sg_table *sgt,
+ enum dma_data_direction dma_dir)
+{
+ unsigned int saved_orig_nents;
+
+ /*
+ * We still need the check for mapped sg count here to not perform
+ * unmap for SCSI commands which have zero payload.
+ */
+ if (!sgt->nents)
+ return;
+
+ saved_orig_nents = sgt->orig_nents;
+ if (sgt->nents < sgt->orig_nents)
+ sgt->orig_nents = sgt->nents;
+
+ dma_unmap_sgtable(dev, sgt, dma_dir, 0);
+
+ sgt->orig_nents = saved_orig_nents;
+}
+
+/**
+ * zfcp_fsf_req_unmap_metadata - unmap the metadata associated with FSF req
+ * @req: pointer to zfcp_fsf_req
+ */
+static inline
+void zfcp_fsf_req_unmap_metadata(struct zfcp_fsf_req *const req)
+{
+ struct device *dev = &req->adapter->ccw_device->dev;
+
+ if (unlikely(zfcp_fsf_req_is_status_read_buffer(req)))
+ zfcp_fsf_req_unmap_sg(dev, &req->metadata_sgt, DMA_FROM_DEVICE);
+ else
+ zfcp_fsf_req_unmap_sg(dev, &req->metadata_sgt,
+ DMA_BIDIRECTIONAL);
+}
+
/**
* zfcp_fsf_req_free - free memory used by fsf request
* @req: pointer to struct zfcp_fsf_req
@@ -259,6 +342,8 @@ static void zfcp_fsf_status_read_handler(struct zfcp_fsf_req *req)
struct zfcp_adapter *adapter = req->adapter;
struct fsf_status_read_buffer *sr_buf = req->data;
+ zfcp_fsf_req_unmap_metadata(req);
+
if (req->status & ZFCP_STATUS_FSFREQ_DISMISSED) {
zfcp_dbf_hba_fsf_uss("fssrh_1", req);
mempool_free(virt_to_page(sr_buf), adapter->pool.sr_data);
@@ -457,6 +542,7 @@ static void zfcp_fsf_req_complete(struct zfcp_fsf_req *req)
return;
}
+ zfcp_fsf_req_unmap_metadata(req);
timer_delete_sync(&req->timer);
zfcp_fsf_protstatus_eval(req);
zfcp_fsf_fsfstatus_eval(req);
@@ -870,10 +956,27 @@ static struct zfcp_fsf_req *zfcp_fsf_req_create(struct zfcp_qdio *qdio,
req->qtcb->prefix.qtcb_version = FSF_QTCB_CURRENT_VERSION;
req->qtcb->header.req_handle = req->req_id;
req->qtcb->header.fsf_command = fsf_cmd;
- }
- zfcp_qdio_req_init(adapter->qdio, &req->qdio_req, req->req_id, sbtype,
- req->qtcb, sizeof(struct fsf_qtcb));
+ sg_init_one(&req->metadata_sg, req->qtcb,
+ sizeof(struct fsf_qtcb));
+ req->metadata_sgt.sgl = &req->metadata_sg;
+ req->metadata_sgt.orig_nents = 1;
+
+ if (zfcp_fsf_req_map_sg(req, &req->metadata_sgt,
+ DMA_BIDIRECTIONAL)) {
+ zfcp_fsf_req_free(req);
+ return ERR_PTR(-EIO);
+ }
+
+ zfcp_qdio_req_init(adapter->qdio, &req->qdio_req, req->req_id,
+ sbtype, true,
+ sg_dma_len(req->metadata_sgt.sgl),
+ sg_dma_address(req->metadata_sgt.sgl));
+ } else {
+ /* SRB */
+ zfcp_qdio_req_init(adapter->qdio, &req->qdio_req, req->req_id,
+ sbtype, false, 0, 0);
+ }
return req;
}
@@ -885,6 +988,15 @@ static int zfcp_fsf_req_send(struct zfcp_fsf_req *req)
struct zfcp_qdio *qdio = adapter->qdio;
u64 req_id = req->req_id;
+ /*
+ * Flush all CPU-side QTCB writes to the SWIOTLB bounce buffer now
+ * that the QTCB is fully populated. SRBs carry no outbound QTCB.
+ */
+ if (!is_srb)
+ dma_sync_sgtable_for_device(&adapter->ccw_device->dev,
+ &req->metadata_sgt,
+ DMA_BIDIRECTIONAL);
+
zfcp_reqlist_add(adapter->req_list, req);
req->qdio_req.qdio_outb_usage = atomic_read(&qdio->req_q_free);
@@ -954,7 +1066,17 @@ int zfcp_fsf_status_read(struct zfcp_qdio *qdio)
memset(sr_buf, 0, sizeof(*sr_buf));
req->data = sr_buf;
- zfcp_qdio_fill_next(qdio, &req->qdio_req, sr_buf, sizeof(*sr_buf));
+ sg_init_one(&req->metadata_sg, sr_buf, sizeof(*sr_buf));
+ req->metadata_sgt.sgl = &req->metadata_sg;
+ req->metadata_sgt.orig_nents = 1;
+
+ retval = zfcp_fsf_req_map_sg(req, &req->metadata_sgt, DMA_FROM_DEVICE);
+ if (unlikely(retval))
+ goto failed_dma_map;
+
+ zfcp_qdio_fill_next(qdio, &req->qdio_req,
+ sg_dma_address(req->metadata_sgt.sgl),
+ sg_dma_len(req->metadata_sgt.sgl));
zfcp_qdio_set_sbale_last(qdio, &req->qdio_req);
retval = zfcp_fsf_req_send(req);
@@ -965,6 +1087,8 @@ int zfcp_fsf_status_read(struct zfcp_qdio *qdio)
goto out;
failed_req_send:
+ zfcp_fsf_req_unmap_metadata(req);
+failed_dma_map:
req->data = NULL;
mempool_free(virt_to_page(sr_buf), adapter->pool.sr_data);
failed_buf:
@@ -1076,6 +1200,7 @@ struct zfcp_fsf_req *zfcp_fsf_abort_fcp_cmnd(struct scsi_cmnd *scmnd)
}
out_error_free:
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
req = NULL;
out:
@@ -1091,6 +1216,11 @@ static void zfcp_fsf_send_ct_handler(struct zfcp_fsf_req *req)
ct->status = -EINVAL;
+ zfcp_fsf_req_unmap_sg(&req->adapter->ccw_device->dev,
+ &ct->sgt_req, DMA_TO_DEVICE);
+ zfcp_fsf_req_unmap_sg(&req->adapter->ccw_device->dev,
+ &ct->sgt_resp, DMA_FROM_DEVICE);
+
if (req->status & ZFCP_STATUS_FSFREQ_ERROR)
goto skip_fsfstatus;
@@ -1130,19 +1260,42 @@ static void zfcp_fsf_send_ct_handler(struct zfcp_fsf_req *req)
ct->handler(ct->handler_data);
}
-static void zfcp_fsf_setup_ct_els_unchained(struct zfcp_qdio *qdio,
- struct zfcp_qdio_req *q_req,
- struct scatterlist *sg_req,
- struct scatterlist *sg_resp)
+static int zfcp_fsf_setup_ct_els_unchained(struct zfcp_fsf_req *req,
+ struct zfcp_qdio *qdio,
+ struct sg_table *sg_req_table,
+ struct sg_table *sg_resp_table)
{
- zfcp_qdio_fill_next(qdio, q_req, sg_virt(sg_req), sg_req->length);
- zfcp_qdio_fill_next(qdio, q_req, sg_virt(sg_resp), sg_resp->length);
- zfcp_qdio_set_sbale_last(qdio, q_req);
+ int retval = 0;
+
+ retval = zfcp_fsf_req_map_sg(req, sg_req_table, DMA_TO_DEVICE);
+ if (unlikely(retval))
+ goto err;
+
+ retval = zfcp_fsf_req_map_sg(req, sg_resp_table,
+ DMA_FROM_DEVICE);
+ if (unlikely(retval))
+ goto err_unmap;
+
+ zfcp_qdio_fill_next(qdio, &req->qdio_req,
+ sg_dma_address(sg_req_table->sgl),
+ sg_dma_len(sg_req_table->sgl));
+ zfcp_qdio_fill_next(qdio, &req->qdio_req,
+ sg_dma_address(sg_resp_table->sgl),
+ sg_dma_len(sg_resp_table->sgl));
+ zfcp_qdio_set_sbale_last(qdio, &req->qdio_req);
+
+ return 0;
+
+err_unmap:
+ zfcp_fsf_req_unmap_sg(&req->adapter->ccw_device->dev,
+ sg_req_table, DMA_TO_DEVICE);
+err:
+ return retval;
}
static int zfcp_fsf_setup_ct_els_sbals(struct zfcp_fsf_req *req,
- struct scatterlist *sg_req,
- struct scatterlist *sg_resp)
+ struct sg_table *sgt_req,
+ struct sg_table *sgt_resp)
{
struct zfcp_adapter *adapter = req->adapter;
struct zfcp_qdio *qdio = adapter->qdio;
@@ -1150,57 +1303,73 @@ static int zfcp_fsf_setup_ct_els_sbals(struct zfcp_fsf_req *req,
u32 feat = adapter->adapter_features;
if (zfcp_adapter_multi_buffer_active(adapter)) {
- if (zfcp_qdio_sbals_from_sg(qdio, &req->qdio_req, sg_req))
+ if (zfcp_qdio_sbals_from_sg(qdio, sgt_req, req, DMA_TO_DEVICE))
return -EIO;
+
qtcb->bottom.support.req_buf_length =
- zfcp_qdio_real_bytes(sg_req);
- if (zfcp_qdio_sbals_from_sg(qdio, &req->qdio_req, sg_resp))
- return -EIO;
+ zfcp_qdio_real_bytes(sgt_req->sgl);
+
+ if (zfcp_qdio_sbals_from_sg(qdio, sgt_resp, req, DMA_FROM_DEVICE))
+ goto err_unmap;
+
qtcb->bottom.support.resp_buf_length =
- zfcp_qdio_real_bytes(sg_resp);
+ zfcp_qdio_real_bytes(sgt_resp->sgl);
- zfcp_qdio_set_data_div(qdio, &req->qdio_req, sg_nents(sg_req));
+ zfcp_qdio_set_data_div(qdio, &req->qdio_req,
+ sgt_req->nents);
zfcp_qdio_set_sbale_last(qdio, &req->qdio_req);
zfcp_qdio_set_scount(qdio, &req->qdio_req);
return 0;
}
/* use single, unchained SBAL if it can hold the request */
- if (zfcp_qdio_sg_one_sbale(sg_req) && zfcp_qdio_sg_one_sbale(sg_resp)) {
- zfcp_fsf_setup_ct_els_unchained(qdio, &req->qdio_req,
- sg_req, sg_resp);
+ if (zfcp_qdio_sg_one_sbale(sgt_req->sgl) &&
+ zfcp_qdio_sg_one_sbale(sgt_resp->sgl)) {
+ if (zfcp_fsf_setup_ct_els_unchained(req, qdio, sgt_req,
+ sgt_resp))
+ return -EIO;
+
return 0;
}
if (!(feat & FSF_FEATURE_ELS_CT_CHAINED_SBALS))
return -EOPNOTSUPP;
- if (zfcp_qdio_sbals_from_sg(qdio, &req->qdio_req, sg_req))
+ if (zfcp_qdio_sbals_from_sg(qdio, sgt_req, req, DMA_TO_DEVICE))
return -EIO;
- qtcb->bottom.support.req_buf_length = zfcp_qdio_real_bytes(sg_req);
+ qtcb->bottom.support.req_buf_length =
+ zfcp_qdio_real_bytes(sgt_req->sgl);
zfcp_qdio_set_sbale_last(qdio, &req->qdio_req);
zfcp_qdio_skip_to_last_sbale(qdio, &req->qdio_req);
- if (zfcp_qdio_sbals_from_sg(qdio, &req->qdio_req, sg_resp))
- return -EIO;
+ if (zfcp_qdio_sbals_from_sg(qdio, sgt_resp, req, DMA_FROM_DEVICE))
+ goto err_unmap;
- qtcb->bottom.support.resp_buf_length = zfcp_qdio_real_bytes(sg_resp);
+ qtcb->bottom.support.resp_buf_length =
+ zfcp_qdio_real_bytes(sgt_resp->sgl);
zfcp_qdio_set_sbale_last(qdio, &req->qdio_req);
return 0;
+
+err_unmap:
+ zfcp_fsf_req_unmap_sg(&req->adapter->ccw_device->dev,
+ sgt_req, DMA_TO_DEVICE);
+ zfcp_qdio_zero_sbals(qdio->req_q, req->qdio_req.sbal_first,
+ req->qdio_req.sbal_number);
+ return -EIO;
}
static int zfcp_fsf_setup_ct_els(struct zfcp_fsf_req *req,
- struct scatterlist *sg_req,
- struct scatterlist *sg_resp,
+ struct sg_table *sgt_req,
+ struct sg_table *sgt_resp,
unsigned int timeout)
{
int ret;
- ret = zfcp_fsf_setup_ct_els_sbals(req, sg_req, sg_resp);
+ ret = zfcp_fsf_setup_ct_els_sbals(req, sgt_req, sgt_resp);
if (ret)
return ret;
@@ -1242,8 +1411,7 @@ int zfcp_fsf_send_ct(struct zfcp_fc_wka_port *wka_port,
}
req->status |= ZFCP_STATUS_FSFREQ_CLEANUP;
- ret = zfcp_fsf_setup_ct_els(req, ct->sgt_req.sgl, ct->sgt_resp.sgl,
- timeout);
+ ret = zfcp_fsf_setup_ct_els(req, &ct->sgt_req, &ct->sgt_resp, timeout);
if (ret)
goto failed_send;
@@ -1256,12 +1424,21 @@ int zfcp_fsf_send_ct(struct zfcp_fc_wka_port *wka_port,
ret = zfcp_fsf_req_send(req);
if (ret)
- goto failed_send;
+ goto err_unmap;
+
/* NOTE: DO NOT TOUCH req PAST THIS POINT! */
goto out;
+err_unmap:
+ zfcp_fsf_req_unmap_sg(&req->adapter->ccw_device->dev,
+ &ct->sgt_req, DMA_TO_DEVICE);
+ zfcp_fsf_req_unmap_sg(&req->adapter->ccw_device->dev,
+ &ct->sgt_resp, DMA_FROM_DEVICE);
failed_send:
+ zfcp_qdio_zero_sbals(qdio->req_q, req->qdio_req.sbal_first,
+ req->qdio_req.sbal_number);
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
out:
spin_unlock_irq(&qdio->req_q_lock);
@@ -1275,6 +1452,11 @@ static void zfcp_fsf_send_els_handler(struct zfcp_fsf_req *req)
send_els->status = -EINVAL;
+ zfcp_fsf_req_unmap_sg(&req->adapter->ccw_device->dev,
+ &send_els->sgt_req, DMA_TO_DEVICE);
+ zfcp_fsf_req_unmap_sg(&req->adapter->ccw_device->dev,
+ &send_els->sgt_resp, DMA_FROM_DEVICE);
+
if (req->status & ZFCP_STATUS_FSFREQ_ERROR)
goto skip_fsfstatus;
@@ -1343,9 +1525,8 @@ int zfcp_fsf_send_els(struct zfcp_adapter *adapter, u32 d_id,
if (!zfcp_adapter_multi_buffer_active(adapter))
zfcp_qdio_sbal_limit(qdio, &req->qdio_req, 2);
- ret = zfcp_fsf_setup_ct_els(req, els->sgt_req.sgl, els->sgt_resp.sgl,
+ ret = zfcp_fsf_setup_ct_els(req, &els->sgt_req, &els->sgt_resp,
timeout);
-
if (ret)
goto failed_send;
@@ -1358,12 +1539,21 @@ int zfcp_fsf_send_els(struct zfcp_adapter *adapter, u32 d_id,
ret = zfcp_fsf_req_send(req);
if (ret)
- goto failed_send;
+ goto err_unmap;
+
/* NOTE: DO NOT TOUCH req PAST THIS POINT! */
goto out;
+err_unmap:
+ zfcp_fsf_req_unmap_sg(&req->adapter->ccw_device->dev,
+ &els->sgt_req, DMA_TO_DEVICE);
+ zfcp_fsf_req_unmap_sg(&req->adapter->ccw_device->dev,
+ &els->sgt_resp, DMA_FROM_DEVICE);
failed_send:
+ zfcp_qdio_zero_sbals(qdio->req_q, req->qdio_req.sbal_first,
+ req->qdio_req.sbal_number);
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
out:
spin_unlock_irq(&qdio->req_q_lock);
@@ -1404,6 +1594,7 @@ int zfcp_fsf_exchange_config_data(struct zfcp_erp_action *erp_action)
zfcp_fsf_start_erp_timer(req);
retval = zfcp_fsf_req_send(req);
if (retval) {
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
erp_action->fsf_req_id = 0;
}
@@ -1469,6 +1660,8 @@ int zfcp_fsf_exchange_config_data_sync(struct zfcp_qdio *qdio,
retval = -EIO;
else if (req->status & ZFCP_STATUS_FSFREQ_XDATAINCOMPLETE)
retval = -EAGAIN;
+ } else {
+ zfcp_fsf_req_unmap_metadata(req);
}
zfcp_fsf_req_free(req);
@@ -1516,6 +1709,7 @@ int zfcp_fsf_exchange_port_data(struct zfcp_erp_action *erp_action)
zfcp_fsf_start_erp_timer(req);
retval = zfcp_fsf_req_send(req);
if (retval) {
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
erp_action->fsf_req_id = 0;
}
@@ -1578,6 +1772,8 @@ int zfcp_fsf_exchange_port_data_sync(struct zfcp_qdio *qdio,
retval = -EIO;
else if (req->status & ZFCP_STATUS_FSFREQ_XDATAINCOMPLETE)
retval = -EAGAIN;
+ } else {
+ zfcp_fsf_req_unmap_metadata(req);
}
zfcp_fsf_req_free(req);
@@ -1810,6 +2006,7 @@ int zfcp_fsf_open_port(struct zfcp_erp_action *erp_action)
zfcp_fsf_start_erp_timer(req);
retval = zfcp_fsf_req_send(req);
if (retval) {
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
erp_action->fsf_req_id = 0;
put_device(&port->dev);
@@ -1876,6 +2073,7 @@ int zfcp_fsf_close_port(struct zfcp_erp_action *erp_action)
zfcp_fsf_start_erp_timer(req);
retval = zfcp_fsf_req_send(req);
if (retval) {
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
erp_action->fsf_req_id = 0;
}
@@ -1950,8 +2148,10 @@ int zfcp_fsf_open_wka_port(struct zfcp_fc_wka_port *wka_port)
zfcp_fsf_start_timer(req, ZFCP_FSF_REQUEST_TIMEOUT);
retval = zfcp_fsf_req_send(req);
- if (retval)
+ if (retval) {
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
+ }
/* NOTE: DO NOT TOUCH req PAST THIS POINT! */
out:
spin_unlock_irq(&qdio->req_q_lock);
@@ -2009,8 +2209,10 @@ int zfcp_fsf_close_wka_port(struct zfcp_fc_wka_port *wka_port)
zfcp_fsf_start_timer(req, ZFCP_FSF_REQUEST_TIMEOUT);
retval = zfcp_fsf_req_send(req);
- if (retval)
+ if (retval) {
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
+ }
/* NOTE: DO NOT TOUCH req PAST THIS POINT! */
out:
spin_unlock_irq(&qdio->req_q_lock);
@@ -2103,6 +2305,7 @@ int zfcp_fsf_close_physical_port(struct zfcp_erp_action *erp_action)
zfcp_fsf_start_erp_timer(req);
retval = zfcp_fsf_req_send(req);
if (retval) {
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
erp_action->fsf_req_id = 0;
}
@@ -2227,6 +2430,7 @@ int zfcp_fsf_open_lun(struct zfcp_erp_action *erp_action)
zfcp_fsf_start_erp_timer(req);
retval = zfcp_fsf_req_send(req);
if (retval) {
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
erp_action->fsf_req_id = 0;
}
@@ -2316,6 +2520,7 @@ int zfcp_fsf_close_lun(struct zfcp_erp_action *erp_action)
zfcp_fsf_start_erp_timer(req);
retval = zfcp_fsf_req_send(req);
if (retval) {
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
erp_action->fsf_req_id = 0;
}
@@ -2474,6 +2679,15 @@ static void zfcp_fsf_fcp_cmnd_handler(struct zfcp_fsf_req *req)
return;
}
+ if ((scsi_get_prot_op(scpnt) != SCSI_PROT_NORMAL) &&
+ scsi_prot_sg_count(scpnt))
+ zfcp_fsf_req_unmap_sg(&req->adapter->ccw_device->dev,
+ &scpnt->prot_sdb->table,
+ scpnt->sc_data_direction);
+
+ zfcp_fsf_req_unmap_sg(&req->adapter->ccw_device->dev,
+ &scpnt->sdb.table, scpnt->sc_data_direction);
+
zfcp_fsf_fcp_handler_common(req, scpnt->device);
if (unlikely(req->status & ZFCP_STATUS_FSFREQ_ERROR)) {
@@ -2617,20 +2831,22 @@ int zfcp_fsf_fcp_cmnd(struct scsi_cmnd *scsi_cmnd)
if ((scsi_get_prot_op(scsi_cmnd) != SCSI_PROT_NORMAL) &&
scsi_prot_sg_count(scsi_cmnd)) {
- zfcp_qdio_set_data_div(qdio, &req->qdio_req,
- scsi_prot_sg_count(scsi_cmnd));
- retval = zfcp_qdio_sbals_from_sg(qdio, &req->qdio_req,
- scsi_prot_sglist(scsi_cmnd));
+ retval = zfcp_qdio_sbals_from_sg(qdio,
+ &scsi_cmnd->prot_sdb->table,
+ req,
+ scsi_cmnd->sc_data_direction);
if (retval)
goto failed_scsi_cmnd;
+ zfcp_qdio_set_data_div(qdio, &req->qdio_req,
+ scsi_cmnd->prot_sdb->table.nents);
io->prot_data_length = zfcp_qdio_real_bytes(
scsi_prot_sglist(scsi_cmnd));
}
- retval = zfcp_qdio_sbals_from_sg(qdio, &req->qdio_req,
- scsi_sglist(scsi_cmnd));
+ retval = zfcp_qdio_sbals_from_sg(qdio, &scsi_cmnd->sdb.table, req,
+ scsi_cmnd->sc_data_direction);
if (unlikely(retval))
- goto failed_scsi_cmnd;
+ goto failed_normal_scsi_map;
zfcp_qdio_set_sbale_last(adapter->qdio, &req->qdio_req);
if (zfcp_adapter_multi_buffer_active(adapter))
@@ -2638,12 +2854,26 @@ int zfcp_fsf_fcp_cmnd(struct scsi_cmnd *scsi_cmnd)
retval = zfcp_fsf_req_send(req);
if (unlikely(retval))
- goto failed_scsi_cmnd;
+ goto failed_req_send;
+
/* NOTE: DO NOT TOUCH req PAST THIS POINT! */
goto out;
+failed_req_send:
+ zfcp_fsf_req_unmap_sg(&req->adapter->ccw_device->dev,
+ &scsi_cmnd->sdb.table,
+ scsi_cmnd->sc_data_direction);
+failed_normal_scsi_map:
+ if ((scsi_get_prot_op(scsi_cmnd) != SCSI_PROT_NORMAL) &&
+ scsi_prot_sg_count(scsi_cmnd))
+ zfcp_fsf_req_unmap_sg(&req->adapter->ccw_device->dev,
+ &scsi_cmnd->prot_sdb->table,
+ scsi_cmnd->sc_data_direction);
failed_scsi_cmnd:
+ zfcp_qdio_zero_sbals(qdio->req_q, req->qdio_req.sbal_first,
+ req->qdio_req.sbal_number);
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
scsi_cmnd->host_scribble = NULL;
out:
@@ -2719,6 +2949,7 @@ struct zfcp_fsf_req *zfcp_fsf_fcp_task_mgmt(struct scsi_device *sdev,
goto out;
}
+ zfcp_fsf_req_unmap_metadata(req);
zfcp_fsf_req_free(req);
req = NULL;
out:
diff --git a/drivers/s390/scsi/zfcp_qdio.c b/drivers/s390/scsi/zfcp_qdio.c
index bac8385e069f..83855d2dd94a 100644
--- a/drivers/s390/scsi/zfcp_qdio.c
+++ b/drivers/s390/scsi/zfcp_qdio.c
@@ -39,7 +39,7 @@ static void zfcp_qdio_handler_error(struct zfcp_qdio *qdio, char *dbftag,
ZFCP_STATUS_COMMON_ERP_FAILED, dbftag);
}
-static void zfcp_qdio_zero_sbals(struct qdio_buffer *sbal[], int first, int cnt)
+void zfcp_qdio_zero_sbals(struct qdio_buffer *sbal[], int first, int cnt)
{
int i, sbal_idx;
@@ -233,31 +233,41 @@ zfcp_qdio_sbale_next(struct zfcp_qdio *qdio, struct zfcp_qdio_req *q_req)
}
/**
- * zfcp_qdio_sbals_from_sg - fill SBALs from scatter-gather list
+ * zfcp_qdio_sbals_from_sg - DMA map scatter-gather list and fill SBALs from it
* @qdio: pointer to struct zfcp_qdio
- * @q_req: pointer to struct zfcp_qdio_req
- * @sg: scatter-gather list
- * Returns: zero or -EINVAL on error
+ * @sgt: pointer to scattergather list table
+ * @req: pointer to zfcp_fsf_req
+ * @dma_dir: DMA direction of I/O
+ * Returns: zero on success, -EINVAL or -EIO otherwise
*/
-int zfcp_qdio_sbals_from_sg(struct zfcp_qdio *qdio, struct zfcp_qdio_req *q_req,
- struct scatterlist *sg)
+int zfcp_qdio_sbals_from_sg(struct zfcp_qdio *qdio, struct sg_table *sgt,
+ struct zfcp_fsf_req *req,
+ enum dma_data_direction dma_dir)
{
struct qdio_buffer_element *sbale;
+ int i;
+ struct scatterlist *sg_element;
+
+ if (zfcp_fsf_req_map_sg(req, sgt, dma_dir))
+ return -EIO;
/* set storage-block type for this request */
- sbale = zfcp_qdio_sbale_req(qdio, q_req);
- sbale->sflags |= q_req->sbtype;
+ sbale = zfcp_qdio_sbale_req(qdio, &req->qdio_req);
+ sbale->sflags |= req->qdio_req.sbtype;
- for (; sg; sg = sg_next(sg)) {
- sbale = zfcp_qdio_sbale_next(qdio, q_req);
+ for_each_sgtable_dma_sg(sgt, sg_element, i) {
+ sbale = zfcp_qdio_sbale_next(qdio, &req->qdio_req);
if (!sbale) {
atomic_inc(&qdio->req_q_full);
- zfcp_qdio_zero_sbals(qdio->req_q, q_req->sbal_first,
- q_req->sbal_number);
+ zfcp_qdio_zero_sbals(qdio->req_q,
+ req->qdio_req.sbal_first,
+ req->qdio_req.sbal_number);
+ zfcp_fsf_req_unmap_sg(&req->adapter->ccw_device->dev,
+ sgt, dma_dir);
return -EINVAL;
}
- sbale->addr = u64_to_dma64(sg_phys(sg));
- sbale->length = sg->length;
+ sbale->addr = u64_to_dma64(sg_dma_address(sg_element));
+ sbale->length = sg_dma_len(sg_element);
}
return 0;
}
diff --git a/drivers/s390/scsi/zfcp_qdio.h b/drivers/s390/scsi/zfcp_qdio.h
index 8f7d2ae94441..9f89f2975f6f 100644
--- a/drivers/s390/scsi/zfcp_qdio.h
+++ b/drivers/s390/scsi/zfcp_qdio.h
@@ -11,6 +11,7 @@
#define ZFCP_QDIO_H
#include <linux/interrupt.h>
+#include <linux/types.h>
#include <asm/qdio.h>
#define ZFCP_QDIO_SBALE_LEN PAGE_SIZE
@@ -106,16 +107,19 @@ zfcp_qdio_sbale_curr(struct zfcp_qdio *qdio, struct zfcp_qdio_req *q_req)
* @q_req: the qdio request to start
* @req_id: The request id
* @sbtype: type flags to set for all sbals
- * @data: First data block
- * @len: Length of first data block
+ * @has_qtcb: FSF request has a QTCB
+ * @qtcb_len: Length of first data block QTCB, if any
+ * @qtcb_dma_handle: DMA handle of the QTCB, if any
*
* This is the start of putting the request into the queue, the last
* step is passing the request to zfcp_qdio_send. The request queue
* lock must be held during the whole process from init to send.
*/
static inline
-void zfcp_qdio_req_init(struct zfcp_qdio *qdio, struct zfcp_qdio_req *q_req,
- u64 req_id, u8 sbtype, void *data, u32 len)
+void zfcp_qdio_req_init(struct zfcp_qdio *qdio,
+ struct zfcp_qdio_req *q_req, u64 req_id,
+ u8 sbtype, bool has_qtcb, u32 qtcb_len,
+ dma_addr_t qtcb_dma_handle)
{
struct qdio_buffer_element *sbale;
int count = min(atomic_read(&qdio->req_q_free),
@@ -133,33 +137,34 @@ void zfcp_qdio_req_init(struct zfcp_qdio *qdio, struct zfcp_qdio_req *q_req,
sbale->eflags = 0;
sbale->sflags = SBAL_SFLAGS0_COMMAND | sbtype;
- if (unlikely(!data))
+ if (unlikely(!has_qtcb))
return;
sbale++;
- sbale->addr = virt_to_dma64(data);
- sbale->length = len;
+ sbale->addr = u64_to_dma64(qtcb_dma_handle);
+ sbale->length = qtcb_len;
}
/**
* zfcp_qdio_fill_next - Fill next sbale, only for single sbal requests
* @qdio: pointer to struct zfcp_qdio
* @q_req: pointer to struct zfcp_queue_req
- * @data: pointer to data
+ * @dma_handle: dma address
* @len: length of data
*
* This is only required for single sbal requests, calling it when
* wrapping around to the next sbal is a bug.
*/
static inline
-void zfcp_qdio_fill_next(struct zfcp_qdio *qdio, struct zfcp_qdio_req *q_req,
- void *data, u32 len)
+void zfcp_qdio_fill_next(struct zfcp_qdio *qdio,
+ struct zfcp_qdio_req *q_req,
+ dma_addr_t dma_handle, u32 len)
{
struct qdio_buffer_element *sbale;
BUG_ON(q_req->sbale_curr == qdio->max_sbale_per_sbal - 1);
q_req->sbale_curr++;
sbale = zfcp_qdio_sbale_curr(qdio, q_req);
- sbale->addr = virt_to_dma64(data);
+ sbale->addr = u64_to_dma64(dma_handle);
sbale->length = len;
}
diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c
index 3590113c2b74..1ca3fc216932 100644
--- a/drivers/s390/scsi/zfcp_scsi.c
+++ b/drivers/s390/scsi/zfcp_scsi.c
@@ -184,6 +184,15 @@ static int zfcp_scsi_eh_abort_handler(struct scsi_cmnd *scpnt)
zfcp_dbf_scsi_abort("abrt_or", scpnt, NULL);
return FAILED; /* completion could be in progress */
}
+
+ /*
+ * Suppress the normal completion handler's unmap by clearing
+ * req->data now, under the abort_lock write-side. The actual
+ * DMA unmap is deferred until the hardware has stopped using
+ * the buffers (after wait_for_completion or after ERP has
+ * quiesced the adapter), so that we never call dma_unmap_sgtable
+ * while the FCP channel may still be writing into the pages.
+ */
old_req->data = NULL;
/* don't access old fsf_req after releasing the abort_lock */
@@ -198,18 +207,34 @@ static int zfcp_scsi_eh_abort_handler(struct scsi_cmnd *scpnt)
zfcp_erp_wait(adapter);
ret = fc_block_scsi_eh(scpnt);
if (ret) {
+ /*
+ * fc_block_scsi_eh returned FAST_IO_FAIL: the
+ * fast_io_fail_tmo fired. ERP has already called
+ * zfcp_fsf_req_dismiss_all() (which requires QDIO
+ * to be down first), so the hardware is no longer
+ * using the buffers. Pass FAST_IO_FAIL back to
+ * scsi_eh, but unmap first.
+ */
zfcp_dbf_scsi_abort("abrt_bl", scpnt, NULL);
- return ret;
+ retval = ret;
+ goto out_unmap;
}
if (!(atomic_read(&adapter->status) &
ZFCP_STATUS_COMMON_RUNNING)) {
+ /*
+ * Adapter is gone; ERP shut down QDIO and dismissed
+ * all requests before clearing RUNNING. Safe to
+ * unmap now.
+ */
zfcp_dbf_scsi_abort("abrt_ru", scpnt, NULL);
- return SUCCESS;
+ retval = SUCCESS;
+ goto out_unmap;
}
}
if (!abrt_req) {
zfcp_dbf_scsi_abort("abrt_ar", scpnt, NULL);
- return FAILED;
+ retval = FAILED;
+ goto out_unmap;
}
wait_for_completion(&abrt_req->completion);
@@ -224,6 +249,28 @@ static int zfcp_scsi_eh_abort_handler(struct scsi_cmnd *scpnt)
}
zfcp_dbf_scsi_abort(dbf_tag, scpnt, abrt_req);
zfcp_fsf_req_free(abrt_req);
+
+out_unmap:
+ /*
+ * The hardware has stopped using the SCSI command's DMA buffers:
+ * either the adapter confirmed the abort, ERP quiesced the adapter
+ * (zfcp_fsf_req_dismiss_all requires QDIO down), or fc_block_scsi_eh
+ * signalled link-level quiesce. Unmap here, directly from scpnt,
+ * because old_req must not be touched after the abort_lock was
+ * released. The normal completion handler (zfcp_fsf_fcp_cmnd_handler)
+ * skips its own unmap when it sees req->data == NULL, so there is no
+ * double-unmap.
+ */
+ if ((scsi_get_prot_op(scpnt) != SCSI_PROT_NORMAL) &&
+ scsi_prot_sg_count(scpnt))
+ zfcp_fsf_req_unmap_sg(&adapter->ccw_device->dev,
+ &scpnt->prot_sdb->table,
+ scpnt->sc_data_direction);
+
+ zfcp_fsf_req_unmap_sg(&adapter->ccw_device->dev,
+ &scpnt->sdb.table,
+ scpnt->sc_data_direction);
+
return retval;
}
@@ -237,6 +284,7 @@ static void zfcp_scsi_forget_cmnd(struct zfcp_fsf_req *old_req, void *data)
{
struct zfcp_scsi_req_filter *filter =
(struct zfcp_scsi_req_filter *)data;
+ struct scsi_cmnd *scpnt;
/* already aborted - prevent side-effects - or not a SCSI command */
if (old_req->data == NULL ||
@@ -244,6 +292,8 @@ static void zfcp_scsi_forget_cmnd(struct zfcp_fsf_req *old_req, void *data)
old_req->qtcb->header.fsf_command != FSF_QTCB_FCP_CMND)
return;
+ scpnt = old_req->data;
+
/* (tmf_scope == FCP_TMF_TGT_RESET || tmf_scope == FCP_TMF_LUN_RESET) */
if (old_req->qtcb->header.port_handle != filter->port_handle)
return;
@@ -253,6 +303,17 @@ static void zfcp_scsi_forget_cmnd(struct zfcp_fsf_req *old_req, void *data)
return;
zfcp_dbf_scsi_nullcmnd((struct scsi_cmnd *)old_req->data, old_req);
+
+ if ((scsi_get_prot_op(scpnt) != SCSI_PROT_NORMAL) &&
+ scsi_prot_sg_count(scpnt))
+ zfcp_fsf_req_unmap_sg(&old_req->adapter->ccw_device->dev,
+ &scpnt->prot_sdb->table,
+ scpnt->sc_data_direction);
+
+ zfcp_fsf_req_unmap_sg(&old_req->adapter->ccw_device->dev,
+ &scpnt->sdb.table,
+ scpnt->sc_data_direction);
+
old_req->data = NULL;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v13 2/2] zfcp: Use DMA streaming API to map I/O
2026-10-22 17:08 ` [PATCH v13 2/2] " M Nikhil
@ 2026-10-22 18:15 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-22 18:15 UTC (permalink / raw)
To: M Nikhil, linux-s390, linux-scsi
Cc: Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] scsi: zfcp: premature DMA unmap in zfcp_scsi_eh_abort_handler()
- [High] scsi: zfcp: double DMA unmap of scsi_cmnd in zfcp_scsi_eh_abort_handler()
- [High] scsi: zfcp: type confusion on TMF requests in zfcp_scsi_forget_cmnd()
--
commit 9c1fb6b4aad79a57f42f4a774fc0a9866fc4009c
Author: M Nikhil <nikh1092@linux.ibm.com>
zfcp: Use DMA streaming API to map I/O
This commit converts zFCP I/O buffer mappings for QTCB, SRB, CT/ELS, and
SCSI I/O requests to use the DMA streaming API via dma_map_sgtable() and
dma_unmap_sgtable(). It also updates the SCSI abort and reset paths to
unmap the scatter-gather lists associated with aborted or forgotten SCSI
commands.
> diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c
> index ea4b968352f4d..99baca26c9bf0 100644
> --- a/drivers/s390/scsi/zfcp_scsi.c
> +++ b/drivers/s390/scsi/zfcp_scsi.c
> @@ -184,6 +184,15 @@ static int zfcp_scsi_eh_abort_handler(struct scsi_cmnd *scpnt)
> zfcp_dbf_scsi_abort("abrt_or", scpnt, NULL);
> return FAILED; /* completion could be in progress */
> }
> +
> + /*
> + * Suppress the normal completion handler's unmap by clearing
> + * req->data now, under the abort_lock write-side. The actual
> + * DMA unmap is deferred until the hardware has stopped using
> + * the buffers (after wait_for_completion or after ERP has
> + * quiesced the adapter), so that we never call dma_unmap_sgtable
> + * while the FCP channel may still be writing into the pages.
> + */
> old_req->data = NULL;
[Severity: High]
Can this unmap scpnt->sdb.table and scpnt->prot_sdb->table a second time if
zfcp_scsi_forget_cmnd() has already run for old_req?
When a LUN or target reset TMF completes in zfcp_scsi_task_mgmt_function(),
for instance via an SG_SCSI_RESET ioctl in scsi_ioctl_reset(), it calls
zfcp_scsi_forget_cmnds() -> zfcp_scsi_forget_cmnd() under
adapter->abort_lock. That function unmaps scpnt->prot_sdb->table and
scpnt->sdb.table and sets old_req->data = NULL, while leaving old_req in
adapter->req_list, scpnt->host_scribble set to old_req->req_id, and
sgt->nents non-zero.
Because old_req->data is now NULL, zfcp_fsf_fcp_cmnd_handler() will not
call scsi_done(scpnt). If scpnt subsequently times out and enters
zfcp_scsi_eh_abort_handler(scpnt) while old_req is still in
adapter->req_list, zfcp_reqlist_find() still finds old_req:
drivers/s390/scsi/zfcp_scsi.c:zfcp_scsi_eh_abort_handler()
write_lock_irqsave(&adapter->abort_lock, flags);
old_req = zfcp_reqlist_find(adapter->req_list, old_reqid);
if (!old_req) {
write_unlock_irqrestore(&adapter->abort_lock, flags);
zfcp_dbf_scsi_abort("abrt_or", scpnt, NULL);
return FAILED; /* completion could be in progress */
}
...
old_req->data = NULL;
Unlike zfcp_fsf_fcp_cmnd_handler() and zfcp_scsi_forget_cmnd(),
zfcp_scsi_eh_abort_handler() does not check whether old_req->data is
already NULL before proceeding to out_unmap:, where zfcp_fsf_req_unmap_sg()
is called on scpnt->prot_sdb->table and scpnt->sdb.table again.
Should zfcp_scsi_eh_abort_handler() check if (!old_req || !old_req->data)
under adapter->abort_lock before taking ownership of the unmap?
>
> /* don't access old fsf_req after releasing the abort_lock */
> @@ -198,18 +207,34 @@ static int zfcp_scsi_eh_abort_handler(struct scsi_cmnd *scpnt)
> zfcp_erp_wait(adapter);
> ret = fc_block_scsi_eh(scpnt);
> if (ret) {
> + /*
> + * fc_block_scsi_eh returned FAST_IO_FAIL: the
> + * fast_io_fail_tmo fired. ERP has already called
> + * zfcp_fsf_req_dismiss_all() (which requires QDIO
> + * to be down first), so the hardware is no longer
> + * using the buffers. Pass FAST_IO_FAIL back to
> + * scsi_eh, but unmap first.
> + */
> zfcp_dbf_scsi_abort("abrt_bl", scpnt, NULL);
> - return ret;
> + retval = ret;
> + goto out_unmap;
> }
> if (!(atomic_read(&adapter->status) &
> ZFCP_STATUS_COMMON_RUNNING)) {
> + /*
> + * Adapter is gone; ERP shut down QDIO and dismissed
> + * all requests before clearing RUNNING. Safe to
> + * unmap now.
> + */
> zfcp_dbf_scsi_abort("abrt_ru", scpnt, NULL);
> - return SUCCESS;
> + retval = SUCCESS;
> + goto out_unmap;
> }
> }
> if (!abrt_req) {
> zfcp_dbf_scsi_abort("abrt_ar", scpnt, NULL);
> - return FAILED;
> + retval = FAILED;
> + goto out_unmap;
> }
>
> wait_for_completion(&abrt_req->completion);
> @@ -224,6 +249,28 @@ static int zfcp_scsi_eh_abort_handler(struct scsi_cmnd *scpnt)
> }
> zfcp_dbf_scsi_abort(dbf_tag, scpnt, abrt_req);
> zfcp_fsf_req_free(abrt_req);
> +
> +out_unmap:
> + /*
> + * The hardware has stopped using the SCSI command's DMA buffers:
> + * either the adapter confirmed the abort, ERP quiesced the adapter
> + * (zfcp_fsf_req_dismiss_all requires QDIO down), or fc_block_scsi_eh
> + * signalled link-level quiesce. Unmap here, directly from scpnt,
> + * because old_req must not be touched after the abort_lock was
> + * released. The normal completion handler (zfcp_fsf_fcp_cmnd_handler)
> + * skips its own unmap when it sees req->data == NULL, so there is no
> + * double-unmap.
> + */
> + if ((scsi_get_prot_op(scpnt) != SCSI_PROT_NORMAL) &&
> + scsi_prot_sg_count(scpnt))
> + zfcp_fsf_req_unmap_sg(&adapter->ccw_device->dev,
> + &scpnt->prot_sdb->table,
> + scpnt->sc_data_direction);
> +
> + zfcp_fsf_req_unmap_sg(&adapter->ccw_device->dev,
> + &scpnt->sdb.table,
> + scpnt->sc_data_direction);
> +
> return retval;
> }
[Severity: Critical]
Could jumping to out_unmap: on the failure and FAST_IO_FAIL paths in
zfcp_scsi_eh_abort_handler() unmap scpnt->sdb.table and
scpnt->prot_sdb->table while the FCP channel is still accessing those
buffers?
There appear to be three paths reaching out_unmap: where the hardware has
not yet stopped using scpnt's DMA buffers:
1. When all retries of zfcp_fsf_abort_fcp_cmnd(scpnt) return NULL (!abrt_req,
dbf_tag = "abrt_ar", retval = FAILED) while adapter->status still has
ZFCP_STATUS_COMMON_RUNNING and fc_block_scsi_eh(scpnt) returns 0 (for
example, if zfcp_qdio_sbal_get() or zfcp_fsf_req_create() fails, or if
zfcp_sdev->status lacks ZFCP_STATUS_COMMON_UNBLOCKED during LUN-level ERP
while the rport remains ONLINE). No abort command was sent, QDIO remains
active, and old_req is still in flight on the channel.
2. When abrt_req completes without ZFCP_STATUS_FSFREQ_ABORTSUCCEEDED or
ZFCP_STATUS_FSFREQ_ABORTNOTNEEDED (dbf_tag = "abrt_fa", retval = FAILED),
such as on FSF_ADAPTER_STATUS_AVAILABLE, FSF_LUN_BOXED, FSF_PORT_BOXED,
FSF_LUN_HANDLE_NOT_VALID, FSF_PORT_HANDLE_NOT_VALID, or when abrt_req times
out in zfcp_fsf_request_timeout_handler(). The adapter has not confirmed
that old_req was terminated, and any ERP recovery is only scheduled
asynchronously without waiting before out_unmap: runs.
3. When fc_block_scsi_eh(scpnt) returns FAST_IO_FAIL (dbf_tag = "abrt_bl").
fc_block_scsi_eh() waits on the rport's FC_PORTSTATE_BLOCKED state, which
occurs during port-level ERP or link-down handling without shutting down
QDIO or calling zfcp_fsf_req_dismiss_all(). In addition,
fc_timeout_fail_rport_io() sets FC_RPORT_FAST_FAIL_TIMEDOUT before calling
fc_terminate_rport_io():
drivers/scsi/scsi_transport_fc.c:fc_timeout_fail_rport_io()
if (rport->port_state != FC_PORTSTATE_BLOCKED)
return;
rport->flags |= FC_RPORT_FAST_FAIL_TIMEDOUT;
fc_terminate_rport_io(rport);
And zfcp_scsi_terminate_rport_io() only queues zfcp_erp_port_forced_reopen()
asynchronously without waiting for the port reopen to complete.
Furthermore, because old_req->data was already cleared to NULL at the start
of zfcp_scsi_eh_abort_handler(), neither zfcp_scsi_forget_cmnd() (during
subsequent SCSI EH LUN/target reset escalation) nor
zfcp_fsf_fcp_cmnd_handler() (during adapter reopen /
zfcp_fsf_req_dismiss_all()) will unmap the buffers once the hardware
actually quiesces.
Should old_req->data remain set and the DMA unmap be skipped when
zfcp_scsi_eh_abort_handler() does not confirm that old_req was aborted or
that the adapter was quiesced, leaving the unmap to the subsequent reset or
request dismissal path?
>
> @@ -237,6 +284,7 @@ static void zfcp_scsi_forget_cmnd(struct zfcp_fsf_req *old_req, void *data)
> {
> struct zfcp_scsi_req_filter *filter =
> (struct zfcp_scsi_req_filter *)data;
> + struct scsi_cmnd *scpnt;
>
> /* already aborted - prevent side-effects - or not a SCSI command */
> if (old_req->data == NULL ||
> @@ -244,6 +292,8 @@ static void zfcp_scsi_forget_cmnd(struct zfcp_fsf_req *old_req, void *data)
> old_req->qtcb->header.fsf_command != FSF_QTCB_FCP_CMND)
> return;
>
> + scpnt = old_req->data;
> +
> /* (tmf_scope == FCP_TMF_TGT_RESET || tmf_scope == FCP_TMF_LUN_RESET) */
> if (old_req->qtcb->header.port_handle != filter->port_handle)
> return;
> @@ -253,6 +303,17 @@ static void zfcp_scsi_forget_cmnd(struct zfcp_fsf_req *old_req, void *data)
> return;
>
> zfcp_dbf_scsi_nullcmnd((struct scsi_cmnd *)old_req->data, old_req);
> +
> + if ((scsi_get_prot_op(scpnt) != SCSI_PROT_NORMAL) &&
> + scsi_prot_sg_count(scpnt))
> + zfcp_fsf_req_unmap_sg(&old_req->adapter->ccw_device->dev,
> + &scpnt->prot_sdb->table,
> + scpnt->sc_data_direction);
> +
> + zfcp_fsf_req_unmap_sg(&old_req->adapter->ccw_device->dev,
> + &scpnt->sdb.table,
> + scpnt->sc_data_direction);
> +
> old_req->data = NULL;
> }
[Severity: High]
What happens in zfcp_scsi_forget_cmnd() if another FCP task management
request is in flight in adapter->req_list for the same port or LUN?
In zfcp_fsf_fcp_task_mgmt(), task management requests are also created with
fsf_command == FSF_QTCB_FCP_CMND, but req->data holds a struct scsi_device *
rather than a struct scsi_cmnd *:
drivers/s390/scsi/zfcp_fsf.c:zfcp_fsf_fcp_task_mgmt()
req = zfcp_fsf_req_create(qdio, FSF_QTCB_FCP_CMND,
SBAL_SFLAGS0_TYPE_WRITE,
qdio->adapter->pool.scsi_req);
...
req->data = sdev;
req->handler = zfcp_fsf_fcp_task_mgmt_handler;
req->qtcb->header.lun_handle = zfcp_sdev->lun_handle;
req->qtcb->header.port_handle = zfcp_sdev->port->handle;
req->qtcb->bottom.io.data_direction = FSF_DATADIR_CMND;
If two TMF requests are in flight concurrently (for example, an
SG_SCSI_RESET ioctl in scsi_ioctl_reset() racing with scsi_error_handler()
or another SG_SCSI_RESET ioctl), zfcp_scsi_forget_cmnds() will invoke
zfcp_scsi_forget_cmnd() on the other in-flight TMF request, cast its
struct scsi_device * in old_req->data to struct scsi_cmnd *, pass fields of
struct scsi_device to zfcp_fsf_req_unmap_sg(), and clear old_req->data to
NULL (which zfcp_fsf_fcp_task_mgmt_handler() later dereferences).
Should zfcp_scsi_forget_cmnd() also check that old_req is not a TMF request
(for example, by checking old_req->qtcb->bottom.io.data_direction !=
FSF_DATADIR_CMND or old_req->status & ZFCP_STATUS_FSFREQ_CLEANUP)?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261022170828.4189701-1-nikh1092@linux.ibm.com?part=2
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-10-22 18:15 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-21 8:33 [PATCH v13 0/2] zfcp: Use DMA streaming API to map I/O M Nikhil
2026-10-21 8:33 ` [PATCH v13 1/2] zfcp: Replace sg-list with sg_table in zfcp_fsf_ct_els structure M Nikhil
2026-10-21 8:40 ` sashiko-bot
2026-10-21 8:33 ` [PATCH v13 2/2] zfcp: Use DMA streaming API to map I/O M Nikhil
2026-10-21 9:04 ` sashiko-bot
-- strict thread matches above, loose matches on Subject: below --
2026-10-22 17:08 [PATCH v13 0/2] " M Nikhil
2026-10-22 17:08 ` [PATCH v13 2/2] " M Nikhil
2026-10-22 18:15 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox