From c9f597a4d6d7a01590571291f659a2f146111e34 Mon Sep 17 00:00:00 2001 From: Farhan Ali Date: Thu, 11 Jul 2019 10:28:51 -0400 Subject: vfio-ccw: Fix misleading comment when setting orb.cmd.c64 The comment is misleading because it tells us that we should set orb.cmd.c64 before calling ccwchain_calc_length, otherwise the function ccwchain_calc_length would return an error. This is not completely accurate. We want to allow an orb without cmd.c64, and this is fine as long as the channel program does not use IDALs. But we do want to reject any channel program that uses IDALs and does not set the flag, which is what we do in ccwchain_calc_length. After we have done the ccw processing, we need to set cmd.c64, as we use IDALs for all translated channel programs. Also for better code readability let's move the setting of cmd.c64 within the non error path. Fixes: fb9e7880af35 ("vfio: ccw: push down unsupported IDA check") Signed-off-by: Farhan Ali Reviewed-by: Cornelia Huck Message-Id: Reviewed-by: Eric Farman Signed-off-by: Cornelia Huck --- drivers/s390/cio/vfio_ccw_cp.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'drivers') diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c index 1d4c893ead23..46967c664c0f 100644 --- a/drivers/s390/cio/vfio_ccw_cp.c +++ b/drivers/s390/cio/vfio_ccw_cp.c @@ -645,14 +645,15 @@ int cp_init(struct channel_program *cp, struct device *mdev, union orb *orb) if (ret) cp_free(cp); - /* It is safe to force: if not set but idals used - * ccwchain_calc_length returns an error. - */ - cp->orb.cmd.c64 = 1; - - if (!ret) + if (!ret) { cp->initialized = true; + /* It is safe to force: if it was not set but idals used + * ccwchain_calc_length would have returned an error. + */ + cp->orb.cmd.c64 = 1; + } + return ret; } -- cgit v1.2.3 From 8b515be512a2435bb8aedc6390cbe140167f9eb9 Mon Sep 17 00:00:00 2001 From: Farhan Ali Date: Thu, 11 Jul 2019 10:28:52 -0400 Subject: vfio-ccw: Fix memory leak and don't call cp_free in cp_init We don't set cp->initialized to true so calling cp_free will just return and not do anything. Also fix a memory leak where we fail to free a ccwchain on an error. Fixes: 812271b910 ("s390/cio: Squash cp_free() and cp_unpin_free()") Signed-off-by: Farhan Ali Message-Id: <3173c4216f4555d9765eb6e4922534982bc820e4.1562854091.git.alifm@linux.ibm.com> Reviewed-by: Cornelia Huck Reviewed-by: Eric Farman Signed-off-by: Cornelia Huck --- drivers/s390/cio/vfio_ccw_cp.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c index 46967c664c0f..e4e8724eddaa 100644 --- a/drivers/s390/cio/vfio_ccw_cp.c +++ b/drivers/s390/cio/vfio_ccw_cp.c @@ -421,7 +421,7 @@ static int ccwchain_loop_tic(struct ccwchain *chain, static int ccwchain_handle_ccw(u32 cda, struct channel_program *cp) { struct ccwchain *chain; - int len; + int len, ret; /* Copy 2K (the most we support today) of possible CCWs */ len = copy_from_iova(cp->mdev, cp->guest_cp, cda, @@ -448,7 +448,12 @@ static int ccwchain_handle_ccw(u32 cda, struct channel_program *cp) memcpy(chain->ch_ccw, cp->guest_cp, len * sizeof(struct ccw1)); /* Loop for tics on this new chain. */ - return ccwchain_loop_tic(chain, cp); + ret = ccwchain_loop_tic(chain, cp); + + if (ret) + ccwchain_free(chain); + + return ret; } /* Loop for TICs. */ @@ -642,8 +647,6 @@ int cp_init(struct channel_program *cp, struct device *mdev, union orb *orb) /* Build a ccwchain for the first CCW segment */ ret = ccwchain_handle_ccw(orb->cmd.cpa, cp); - if (ret) - cp_free(cp); if (!ret) { cp->initialized = true; -- cgit v1.2.3 From c1ab69268d124ebdbb3864580808188ccd3ea355 Mon Sep 17 00:00:00 2001 From: Farhan Ali Date: Thu, 11 Jul 2019 10:28:53 -0400 Subject: vfio-ccw: Set pa_nr to 0 if memory allocation fails for pa_iova_pfn So we don't call try to call vfio_unpin_pages() incorrectly. Fixes: 0a19e61e6d4c ("vfio: ccw: introduce channel program interfaces") Signed-off-by: Farhan Ali Reviewed-by: Eric Farman Reviewed-by: Cornelia Huck Message-Id: <33a89467ad6369196ae6edf820cbcb1e2d8d050c.1562854091.git.alifm@linux.ibm.com> Signed-off-by: Cornelia Huck --- drivers/s390/cio/vfio_ccw_cp.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c index e4e8724eddaa..3645d1720c4b 100644 --- a/drivers/s390/cio/vfio_ccw_cp.c +++ b/drivers/s390/cio/vfio_ccw_cp.c @@ -72,8 +72,10 @@ static int pfn_array_alloc(struct pfn_array *pa, u64 iova, unsigned int len) sizeof(*pa->pa_iova_pfn) + sizeof(*pa->pa_pfn), GFP_KERNEL); - if (unlikely(!pa->pa_iova_pfn)) + if (unlikely(!pa->pa_iova_pfn)) { + pa->pa_nr = 0; return -ENOMEM; + } pa->pa_pfn = pa->pa_iova_pfn + pa->pa_nr; pa->pa_iova_pfn[0] = pa->pa_iova >> PAGE_SHIFT; -- cgit v1.2.3 From f4c9939433bd396d0b08e803b2b880a9d02682b9 Mon Sep 17 00:00:00 2001 From: Farhan Ali Date: Thu, 11 Jul 2019 10:28:54 -0400 Subject: vfio-ccw: Don't call cp_free if we are processing a channel program There is a small window where it's possible that we could be working on an interrupt (queued in the workqueue) and setting up a channel program (i.e allocating memory, pinning pages, translating address). This can lead to allocating and freeing the channel program at the same time and can cause memory corruption. Let's not call cp_free if we are currently processing a channel program. The only way we know for sure that we don't have a thread setting up a channel program is when the state is set to VFIO_CCW_STATE_CP_PENDING. Fixes: d5afd5d135c8 ("vfio-ccw: add handling for async channel instructions") Signed-off-by: Farhan Ali Reviewed-by: Cornelia Huck Message-Id: <62e87bf67b38dc8d5760586e7c96d400db854ebe.1562854091.git.alifm@linux.ibm.com> Reviewed-by: Eric Farman Signed-off-by: Cornelia Huck --- drivers/s390/cio/vfio_ccw_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c index 2b90a5ecaeb9..9208c0e56c33 100644 --- a/drivers/s390/cio/vfio_ccw_drv.c +++ b/drivers/s390/cio/vfio_ccw_drv.c @@ -88,7 +88,7 @@ static void vfio_ccw_sch_io_todo(struct work_struct *work) (SCSW_ACTL_DEVACT | SCSW_ACTL_SCHACT)); if (scsw_is_solicited(&irb->scsw)) { cp_update_scsw(&private->cp, &irb->scsw); - if (is_final) + if (is_final && private->state == VFIO_CCW_STATE_CP_PENDING) cp_free(&private->cp); } mutex_lock(&private->io_mutex); -- cgit v1.2.3 From a6ec414a4dd529eeac5c3ea51c661daba3397108 Mon Sep 17 00:00:00 2001 From: Julian Wiedmann Date: Thu, 11 Jul 2019 18:17:36 +0200 Subject: s390/qdio: add sanity checks to the fast-requeue path If the device driver were to send out a full queue's worth of SBALs, current code would end up discovering the last of those SBALs as PRIMED and erroneously skip the SIGA-w. This immediately stalls the queue. Add a check to not attempt fast-requeue in this case. While at it also make sure that the state of the previous SBAL was successfully extracted before inspecting it. Signed-off-by: Julian Wiedmann Reviewed-by: Jens Remus Signed-off-by: Heiko Carstens --- drivers/s390/cio/qdio_main.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'drivers') diff --git a/drivers/s390/cio/qdio_main.c b/drivers/s390/cio/qdio_main.c index 730c4e68094b..7f5adf02f095 100644 --- a/drivers/s390/cio/qdio_main.c +++ b/drivers/s390/cio/qdio_main.c @@ -1558,13 +1558,13 @@ static int handle_outbound(struct qdio_q *q, unsigned int callflags, rc = qdio_kick_outbound_q(q, phys_aob); } else if (need_siga_sync(q)) { rc = qdio_siga_sync_q(q); + } else if (count < QDIO_MAX_BUFFERS_PER_Q && + get_buf_state(q, prev_buf(bufnr), &state, 0) > 0 && + state == SLSB_CU_OUTPUT_PRIMED) { + /* The previous buffer is not processed yet, tack on. */ + qperf_inc(q, fast_requeue); } else { - /* try to fast requeue buffers */ - get_buf_state(q, prev_buf(bufnr), &state, 0); - if (state != SLSB_CU_OUTPUT_PRIMED) - rc = qdio_kick_outbound_q(q, 0); - else - qperf_inc(q, fast_requeue); + rc = qdio_kick_outbound_q(q, 0); } /* in case of SIGA errors we must process the error immediately */ -- cgit v1.2.3 From 69e96207ebf90ff8d5bac457134b0d4569f6634e Mon Sep 17 00:00:00 2001 From: Julian Wiedmann Date: Mon, 1 Jul 2019 14:19:29 +0200 Subject: s390/qdio: restrict QAOB usage to IQD unicast queues The IQD mcast queue doesn't support QAOB mode, so skip the qdio_enable_async_operation() setup call for this queue. This avoids the allocation of an unneeded QAOB pointer array, and sets up q->use_cq properly so that drivers are prohibited from using QAOBs for mcast traffic. Take this opportunity to streamline the q->use_cq and aob != 0 checks. The path to qdio_siga_output() is straight-forward, we don't need to worry about being called with bad operands. Signed-off-by: Julian Wiedmann Signed-off-by: Heiko Carstens --- drivers/s390/cio/qdio_main.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'drivers') diff --git a/drivers/s390/cio/qdio_main.c b/drivers/s390/cio/qdio_main.c index 7f5adf02f095..4142c85e77d8 100644 --- a/drivers/s390/cio/qdio_main.c +++ b/drivers/s390/cio/qdio_main.c @@ -319,9 +319,7 @@ static int qdio_siga_output(struct qdio_q *q, unsigned int *busy_bit, int retries = 0, cc; unsigned long laob = 0; - WARN_ON_ONCE(aob && ((queue_type(q) != QDIO_IQDIO_QFMT) || - !q->u.out.use_cq)); - if (q->u.out.use_cq && aob != 0) { + if (aob) { fc = QDIO_SIGA_WRITEQ; laob = aob; } @@ -621,9 +619,6 @@ static inline unsigned long qdio_aob_for_buffer(struct qdio_output_q *q, { unsigned long phys_aob = 0; - if (!q->use_cq) - return 0; - if (!q->aobs[bufnr]) { struct qaob *aob = qdio_allocate_aob(); q->aobs[bufnr] = aob; @@ -1308,6 +1303,8 @@ static void qdio_detect_hsicq(struct qdio_irq *irq_ptr) for_each_output_queue(irq_ptr, q, i) { if (use_cq) { + if (multicast_outbound(q)) + continue; if (qdio_enable_async_operation(&q->u.out) < 0) { use_cq = 0; continue; @@ -1553,7 +1550,8 @@ static int handle_outbound(struct qdio_q *q, unsigned int callflags, /* One SIGA-W per buffer required for unicast HSI */ WARN_ON_ONCE(count > 1 && !multicast_outbound(q)); - phys_aob = qdio_aob_for_buffer(&q->u.out, bufnr); + if (q->u.out.use_cq) + phys_aob = qdio_aob_for_buffer(&q->u.out, bufnr); rc = qdio_kick_outbound_q(q, phys_aob); } else if (need_siga_sync(q)) { -- cgit v1.2.3 From 4f419eb14272e0698e8c55bb5f3f266cc2a21c81 Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Tue, 23 Jul 2019 17:11:01 +0200 Subject: virtio/s390: fix race on airq_areas[] The access to airq_areas was racy ever since the adapter interrupts got introduced to virtio-ccw, but since commit 39c7dcb15892 ("virtio/s390: make airq summary indicators DMA") this became an issue in practice as well. Namely before that commit the airq_info that got overwritten was still functional. After that commit however the two infos share a summary_indicator, which aggravates the situation. Which means auto-online mechanism occasionally hangs the boot with virtio_blk. Signed-off-by: Halil Pasic Reported-by: Marc Hartmayer Reviewed-by: Cornelia Huck Cc: stable@vger.kernel.org Fixes: 96b14536d935 ("virtio-ccw: virtio-ccw adapter interrupt support.") Signed-off-by: Heiko Carstens --- drivers/s390/virtio/virtio_ccw.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers') diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c index 1a55e5942d36..957889a42d2e 100644 --- a/drivers/s390/virtio/virtio_ccw.c +++ b/drivers/s390/virtio/virtio_ccw.c @@ -145,6 +145,8 @@ struct airq_info { struct airq_iv *aiv; }; static struct airq_info *airq_areas[MAX_AIRQ_AREAS]; +static DEFINE_MUTEX(airq_areas_lock); + static u8 *summary_indicators; static inline u8 *get_summary_indicator(struct airq_info *info) @@ -265,9 +267,11 @@ static unsigned long get_airq_indicator(struct virtqueue *vqs[], int nvqs, unsigned long bit, flags; for (i = 0; i < MAX_AIRQ_AREAS && !indicator_addr; i++) { + mutex_lock(&airq_areas_lock); if (!airq_areas[i]) airq_areas[i] = new_airq_info(i); info = airq_areas[i]; + mutex_unlock(&airq_areas_lock); if (!info) return 0; write_lock_irqsave(&info->lock, flags); -- cgit v1.2.3