* [PULL 1/5] vfio-ccw: Fix misleading comment when setting orb.cmd.c64
2019-07-16 10:09 [PULL 0/5] vfio-ccw fixes for 5.3 Cornelia Huck
@ 2019-07-16 10:09 ` Cornelia Huck
2019-07-16 10:09 ` [PULL 2/5] vfio-ccw: Fix memory leak and don't call cp_free in cp_init Cornelia Huck
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Cornelia Huck @ 2019-07-16 10:09 UTC (permalink / raw)
To: Heiko Carstens, Vasily Gorbik, Christian Borntraeger
Cc: Farhan Ali, Eric Farman, Halil Pasic, linux-s390, kvm, Cornelia Huck
From: Farhan Ali <alifm@linux.ibm.com>
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 <alifm@linux.ibm.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Message-Id: <f68636106aef0faeb6ce9712584d102d1b315ff8.1562854091.git.alifm@linux.ibm.com>
Reviewed-by: Eric Farman <farman@linux.ibm.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
drivers/s390/cio/vfio_ccw_cp.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
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;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PULL 2/5] vfio-ccw: Fix memory leak and don't call cp_free in cp_init
2019-07-16 10:09 [PULL 0/5] vfio-ccw fixes for 5.3 Cornelia Huck
2019-07-16 10:09 ` [PULL 1/5] vfio-ccw: Fix misleading comment when setting orb.cmd.c64 Cornelia Huck
@ 2019-07-16 10:09 ` Cornelia Huck
2019-07-16 10:09 ` [PULL 3/5] vfio-ccw: Set pa_nr to 0 if memory allocation fails for pa_iova_pfn Cornelia Huck
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Cornelia Huck @ 2019-07-16 10:09 UTC (permalink / raw)
To: Heiko Carstens, Vasily Gorbik, Christian Borntraeger
Cc: Farhan Ali, Eric Farman, Halil Pasic, linux-s390, kvm, Cornelia Huck
From: Farhan Ali <alifm@linux.ibm.com>
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 <alifm@linux.ibm.com>
Message-Id: <3173c4216f4555d9765eb6e4922534982bc820e4.1562854091.git.alifm@linux.ibm.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Eric Farman <farman@linux.ibm.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
drivers/s390/cio/vfio_ccw_cp.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
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;
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PULL 3/5] vfio-ccw: Set pa_nr to 0 if memory allocation fails for pa_iova_pfn
2019-07-16 10:09 [PULL 0/5] vfio-ccw fixes for 5.3 Cornelia Huck
2019-07-16 10:09 ` [PULL 1/5] vfio-ccw: Fix misleading comment when setting orb.cmd.c64 Cornelia Huck
2019-07-16 10:09 ` [PULL 2/5] vfio-ccw: Fix memory leak and don't call cp_free in cp_init Cornelia Huck
@ 2019-07-16 10:09 ` Cornelia Huck
2019-07-16 10:09 ` [PULL 4/5] vfio-ccw: Don't call cp_free if we are processing a channel program Cornelia Huck
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Cornelia Huck @ 2019-07-16 10:09 UTC (permalink / raw)
To: Heiko Carstens, Vasily Gorbik, Christian Borntraeger
Cc: Farhan Ali, Eric Farman, Halil Pasic, linux-s390, kvm, Cornelia Huck
From: Farhan Ali <alifm@linux.ibm.com>
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 <alifm@linux.ibm.com>
Reviewed-by: Eric Farman <farman@linux.ibm.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Message-Id: <33a89467ad6369196ae6edf820cbcb1e2d8d050c.1562854091.git.alifm@linux.ibm.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
drivers/s390/cio/vfio_ccw_cp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
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;
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PULL 4/5] vfio-ccw: Don't call cp_free if we are processing a channel program
2019-07-16 10:09 [PULL 0/5] vfio-ccw fixes for 5.3 Cornelia Huck
` (2 preceding siblings ...)
2019-07-16 10:09 ` [PULL 3/5] vfio-ccw: Set pa_nr to 0 if memory allocation fails for pa_iova_pfn Cornelia Huck
@ 2019-07-16 10:09 ` Cornelia Huck
2019-07-16 10:09 ` [PULL 5/5] vfio-ccw: Update documentation for csch/hsch Cornelia Huck
2019-07-17 9:43 ` [PULL 0/5] vfio-ccw fixes for 5.3 Cornelia Huck
5 siblings, 0 replies; 7+ messages in thread
From: Cornelia Huck @ 2019-07-16 10:09 UTC (permalink / raw)
To: Heiko Carstens, Vasily Gorbik, Christian Borntraeger
Cc: Farhan Ali, Eric Farman, Halil Pasic, linux-s390, kvm, Cornelia Huck
From: Farhan Ali <alifm@linux.ibm.com>
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 <alifm@linux.ibm.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Message-Id: <62e87bf67b38dc8d5760586e7c96d400db854ebe.1562854091.git.alifm@linux.ibm.com>
Reviewed-by: Eric Farman <farman@linux.ibm.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
drivers/s390/cio/vfio_ccw_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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);
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PULL 5/5] vfio-ccw: Update documentation for csch/hsch
2019-07-16 10:09 [PULL 0/5] vfio-ccw fixes for 5.3 Cornelia Huck
` (3 preceding siblings ...)
2019-07-16 10:09 ` [PULL 4/5] vfio-ccw: Don't call cp_free if we are processing a channel program Cornelia Huck
@ 2019-07-16 10:09 ` Cornelia Huck
2019-07-17 9:43 ` [PULL 0/5] vfio-ccw fixes for 5.3 Cornelia Huck
5 siblings, 0 replies; 7+ messages in thread
From: Cornelia Huck @ 2019-07-16 10:09 UTC (permalink / raw)
To: Heiko Carstens, Vasily Gorbik, Christian Borntraeger
Cc: Farhan Ali, Eric Farman, Halil Pasic, linux-s390, kvm, Cornelia Huck
From: Farhan Ali <alifm@linux.ibm.com>
We now support CLEAR SUBCHANNEL and HALT SUBCHANNEL
via ccw_cmd_region.
Fixes: d5afd5d135c8 ("vfio-ccw: add handling for async channel instructions")
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
Message-Id: <7d977612c3f3152ffb950d77ae11b4b25c1e20c4.1562854091.git.alifm@linux.ibm.com>
[CH: properly mark region as literal block]
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Eric Farman <farman@linux.ibm.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
Documentation/s390/vfio-ccw.rst | 31 ++++++++++++++++++++++++++++---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/Documentation/s390/vfio-ccw.rst b/Documentation/s390/vfio-ccw.rst
index 1f6d0b56d53e..be2af10e12b4 100644
--- a/Documentation/s390/vfio-ccw.rst
+++ b/Documentation/s390/vfio-ccw.rst
@@ -180,6 +180,13 @@ The process of how these work together.
add it to an iommu_group and a vfio_group. Then we could pass through
the mdev to a guest.
+
+VFIO-CCW Regions
+----------------
+
+The vfio-ccw driver exposes MMIO regions to accept requests from and return
+results to userspace.
+
vfio-ccw I/O region
-------------------
@@ -205,6 +212,25 @@ irb_area stores the I/O result.
ret_code stores a return code for each access of the region.
+This region is always available.
+
+vfio-ccw cmd region
+-------------------
+
+The vfio-ccw cmd region is used to accept asynchronous instructions
+from userspace.
+
+#define VFIO_CCW_ASYNC_CMD_HSCH (1 << 0)
+#define VFIO_CCW_ASYNC_CMD_CSCH (1 << 1)
+struct ccw_cmd_region {
+ __u32 command;
+ __u32 ret_code;
+} __packed;
+
+This region is exposed via region type VFIO_REGION_SUBTYPE_CCW_ASYNC_CMD.
+
+Currently, CLEAR SUBCHANNEL and HALT SUBCHANNEL use this region.
+
vfio-ccw operation details
--------------------------
@@ -306,9 +332,8 @@ Together with the corresponding work in QEMU, we can bring the passed
through DASD/ECKD device online in a guest now and use it as a block
device.
-While the current code allows the guest to start channel programs via
-START SUBCHANNEL, support for HALT SUBCHANNEL or CLEAR SUBCHANNEL is
-not yet implemented.
+The current code allows the guest to start channel programs via
+START SUBCHANNEL, and to issue HALT SUBCHANNEL and CLEAR SUBCHANNEL.
vfio-ccw supports classic (command mode) channel I/O only. Transport
mode (HPF) is not supported.
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PULL 0/5] vfio-ccw fixes for 5.3
2019-07-16 10:09 [PULL 0/5] vfio-ccw fixes for 5.3 Cornelia Huck
` (4 preceding siblings ...)
2019-07-16 10:09 ` [PULL 5/5] vfio-ccw: Update documentation for csch/hsch Cornelia Huck
@ 2019-07-17 9:43 ` Cornelia Huck
5 siblings, 0 replies; 7+ messages in thread
From: Cornelia Huck @ 2019-07-17 9:43 UTC (permalink / raw)
To: Heiko Carstens, Vasily Gorbik, Christian Borntraeger
Cc: Farhan Ali, Eric Farman, Halil Pasic, linux-s390, kvm
On Tue, 16 Jul 2019 12:09:03 +0200
Cornelia Huck <cohuck@redhat.com> wrote:
> The following changes since commit 9a159190414d461fdac7ae5bb749c2d532b35419:
>
> s390/unwind: avoid int overflow in outside_of_stack (2019-07-11 20:40:02 +0200)
>
> are available in the Git repository at:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/kvms390/vfio-ccw.git tags/vfio-ccw-20190716
>
> for you to fetch changes up to 127e62174041496b383f82d696e1592ce6838604:
>
> vfio-ccw: Update documentation for csch/hsch (2019-07-15 14:22:57 +0200)
>
> ----------------------------------------------------------------
> Fixes in vfio-ccw for older and newer issues.
>
> ----------------------------------------------------------------
>
> Farhan Ali (5):
> vfio-ccw: Fix misleading comment when setting orb.cmd.c64
> vfio-ccw: Fix memory leak and don't call cp_free in cp_init
> vfio-ccw: Set pa_nr to 0 if memory allocation fails for pa_iova_pfn
> vfio-ccw: Don't call cp_free if we are processing a channel program
> vfio-ccw: Update documentation for csch/hsch
>
> Documentation/s390/vfio-ccw.rst | 31 ++++++++++++++++++++++++++++---
> drivers/s390/cio/vfio_ccw_cp.c | 28 +++++++++++++++++-----------
> drivers/s390/cio/vfio_ccw_drv.c | 2 +-
> 3 files changed, 46 insertions(+), 15 deletions(-)
>
Argh, please disregard this one. New one incoming.
/me needs more sleep...
^ permalink raw reply [flat|nested] 7+ messages in thread