[PATCH v4 1/1] vfio-ccw: Enable transparent CCW IPL from DASD
Remove the explicit prefetch check when using vfio-ccw devices.
This check does not trigger in practice as all Linux channel programs
are intended to use prefetch.
It is expected that all ORBs issued by Linux will request prefetch.
Although non-prefetching ORBs are not rejected, they will prefetch
nonetheless. A warning is issued up to once per 5 seconds when a
forced prefetch occurs.
A non-prefetch ORB does not necessarily result in an error, however
frequent encounters with non-prefetch ORBs indicate that channel
programs are being executed in a way that is inconsistent with what
the guest is requesting. While there is currently no known case of an
error caused by forced prefetch, it is possible in theory that forced
prefetch could result in an error if applied to a channel program that
is dependent on non-prefetch.
Signed-off-by: Jared Rossi
---
Documentation/s390/vfio-ccw.rst | 6 ++
drivers/s390/cio/vfio_ccw_cp.c | 19 ---
2 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/Documentation/s390/vfio-ccw.rst b/Documentation/s390/vfio-ccw.rst
index fca9c4f5bd9c..23e7d136f8b4 100644
--- a/Documentation/s390/vfio-ccw.rst
+++ b/Documentation/s390/vfio-ccw.rst
@@ -335,6 +335,12 @@ device.
The current code allows the guest to start channel programs via
START SUBCHANNEL, and to issue HALT SUBCHANNEL and CLEAR SUBCHANNEL.
+Currently all channel programs are prefetched, regardless of the
+p-bit setting in the ORB. As a result, self modifying channel
+programs are not supported. For this reason, IPL has to be handled as
+a special case by a userspace/guest program; this has been implemented
+in QEMU's s390-ccw bios as of QEMU 4.1.
+
vfio-ccw supports classic (command mode) channel I/O only. Transport
mode (HPF) is not supported.
diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c
index 3645d1720c4b..f237480c3d43 100644
--- a/drivers/s390/cio/vfio_ccw_cp.c
+++ b/drivers/s390/cio/vfio_ccw_cp.c
@@ -8,6 +8,7 @@
*Xiao Feng Ren
*/
+#include
#include
#include
#include
@@ -625,23 +626,27 @@ static int ccwchain_fetch_one(struct ccwchain *chain,
* the target channel program from @orb->cmd.iova to the new ccwchain(s).
*
* Limitations:
- * 1. Supports only prefetch enabled mode.
- * 2. Supports idal(c64) ccw chaining.
- * 3. Supports 4k idaw.
+ * 1. Supports idal(c64) ccw chaining.
+ * 2. Supports 4k idaw.
*
* Returns:
* %0 on success and a negative error value on failure.
*/
int cp_init(struct channel_program *cp, struct device *mdev, union orb *orb)
{
+ /* custom ratelimit used to avoid flood during guest IPL */
+ static DEFINE_RATELIMIT_STATE(ratelimit_state, 5 * HZ, 1);
int ret;
/*
-* XXX:
-* Only support prefetch enable mode now.
+* We only support prefetching the channel program. We assume all
channel
+* programs executed by supported guests likewise support prefetching.
+* Executing a channel program that does not specify prefetching will
+* typically not cause an error, but a warning is issued to help
identify
+* the problem if something does break.
*/
- if (!orb->cmd.pfch)
- return -EOPNOTSUPP;
+ if (!orb->cmd.pfch && __ratelimit(_state))
+ dev_warn(mdev, "Prefetching channel program even though
prefetch not specified in ORB");
INIT_LIST_HEAD(>ccwchain_list);
memcpy(>orb, orb, sizeof(*orb));
--
2.17.0
[PATCH v4 0/1] vfio-ccw: Enable transparent CCW IPL from DASD
Remove the explicit prefetch check when using vfio-ccw devices. This check does not trigger in practice as all Linux channel programs are intended to use prefetch. Version 4 makes only stylistic and word choice changes to comments and/or documentation. Jared Rossi (1): vfio-ccw: Enable transparent CCW IPL from DASD Documentation/s390/vfio-ccw.rst | 6 ++ drivers/s390/cio/vfio_ccw_cp.c | 19 --- 2 files changed, 18 insertions(+), 7 deletions(-) -- 2.17.0
[PATCH v3 1/1] vfio-ccw: Enable transparent CCW IPL from DASD
Remove the explicit prefetch check when using vfio-ccw devices.
This check does not trigger in practice as all Linux channel programs
are intended to use prefetch.
It is expected that all ORBs issued by Linux will request prefetch.
Although non-prefetching ORBs are not rejected, they will prefetch
nonetheless. A warning is issued up to once per 5 seconds when a
forced prefetch occurs.
A non-prefetch ORB does not necessarily result in an error, however
frequent encounters with non-prefetch ORBs indicate that channel
programs are being executed in a way that is inconsistent with what
the guest is requesting. While there is currently no known case of an
error caused by forced prefetch, it is possible in theory that forced
prefetch could result in an error if applied to a channel program that
is dependent on non-prefetch.
Signed-off-by: Jared Rossi
---
Documentation/s390/vfio-ccw.rst | 6 ++
drivers/s390/cio/vfio_ccw_cp.c | 19 ---
2 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/Documentation/s390/vfio-ccw.rst b/Documentation/s390/vfio-ccw.rst
index fca9c4f5bd9c..23e7d136f8b4 100644
--- a/Documentation/s390/vfio-ccw.rst
+++ b/Documentation/s390/vfio-ccw.rst
@@ -335,6 +335,12 @@ device.
The current code allows the guest to start channel programs via
START SUBCHANNEL, and to issue HALT SUBCHANNEL and CLEAR SUBCHANNEL.
+Currently all channel programs are prefetched, regardless of the
+p-bit setting in the ORB. As a result, self modifying channel
+programs are not supported. For this reason, IPL has to be handled as
+a special case by a userspace/guest program; this has been implemented
+in QEMU's s390-ccw bios as of QEMU 4.1.
+
vfio-ccw supports classic (command mode) channel I/O only. Transport
mode (HPF) is not supported.
diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c
index 3645d1720c4b..d423ca934779 100644
--- a/drivers/s390/cio/vfio_ccw_cp.c
+++ b/drivers/s390/cio/vfio_ccw_cp.c
@@ -8,6 +8,7 @@
*Xiao Feng Ren
*/
+#include
#include
#include
#include
@@ -625,23 +626,27 @@ static int ccwchain_fetch_one(struct ccwchain *chain,
* the target channel program from @orb->cmd.iova to the new ccwchain(s).
*
* Limitations:
- * 1. Supports only prefetch enabled mode.
- * 2. Supports idal(c64) ccw chaining.
- * 3. Supports 4k idaw.
+ * 1. Supports idal(c64) ccw chaining.
+ * 2. Supports 4k idaw.
*
* Returns:
* %0 on success and a negative error value on failure.
*/
int cp_init(struct channel_program *cp, struct device *mdev, union orb *orb)
{
+ static DEFINE_RATELIMIT_STATE(ratelimit_state, 5 * HZ, 1);
int ret;
/*
-* XXX:
-* Only support prefetch enable mode now.
+* We only support prefetching the channel program. We assume all
channel
+* programs executed by supported guests (i.e. Linux) likewise support
+* prefetching. Even if prefetching is not specified the channel program
+* is still executed using prefetch. Executing a channel program that
+* does not specify prefetching will typically not cause an error, but a
+* warning is issued to help identify the problem if something does
break.
*/
- if (!orb->cmd.pfch)
- return -EOPNOTSUPP;
+ if (!orb->cmd.pfch && __ratelimit(_state))
+ dev_warn(mdev, "executing channel program with prefetch, but
prefetch isn't specified");
INIT_LIST_HEAD(>ccwchain_list);
memcpy(>orb, orb, sizeof(*orb));
--
2.17.0
[PATCH v3 0/1] vfio-ccw: Enable transparent CCW IPL from DASD
Remove the explicit prefetch check when using vfio-ccw devices. This check does not trigger in practice as all Linux channel programs are intended to use prefetch. Version 3 improves logging by including the UUID of the vfio device that triggers the warning. A custom rate limit is used because the generic rate limit of 10 per 5 seconds will still result in multiple warnings during IPL. The warning message has been clarfied to reflect that a channel program will be executed using prefetch even though prefetch was not specified. The text of warning itself does not explicitly refer to non-prefetching channel programs as unsupported because it will trigger during IPL, which is a normal and expected sequence. Likewise, because we expect the message to appear during IPL, the warning also does not explicitly alert to the potential of an error, rather it simply notes that a channel program is being executed in a way other than specified. Verson 3 also makes some word choice changes to the documentation. Jared Rossi (1): vfio-ccw: Enable transparent CCW IPL from DASD Documentation/s390/vfio-ccw.rst | 6 ++ drivers/s390/cio/vfio_ccw_cp.c | 19 --- 2 files changed, 18 insertions(+), 7 deletions(-) -- 2.17.0
[PATCH v2 0/1] vfio-ccw: Enable transparent CCW IPL from DASD
Remove the explicit prefetch check when using vfio-ccw devices. This check is not needed in practice as all Linux channel programs are intended to use prefetch. Version 2 improves logging by issuing a warning if a non-prefetch ORB is encountered. A kernel warning is printed when an ORB without the p-bit set is initialized. The warning is limited to once per five seconds because, if encountered, non-prefetch ORBs tend to appear in bursts of hundreds per second. The commit message is expanded to highlight the potential for future risk if non-prefetch dependent channel programs become prevalent. Furthermore, a note of prefetch only support is added to the limitations section of the vfio-ccw.rst. Jared Rossi (1): vfio-ccw: Enable transparent CCW IPL from DASD Documentation/s390/vfio-ccw.rst | 4 drivers/s390/cio/vfio_ccw_cp.c | 16 +++- 2 files changed, 11 insertions(+), 9 deletions(-) -- 2.17.0
[PATCH v2 1/1] vfio-ccw: Enable transparent CCW IPL from DASD
Remove the explicit prefetch check when using vfio-ccw devices.
This check is not needed in practice as all Linux channel programs
are intended to use prefetch.
It is expected that all ORBs issued by Linux will request prefetch.
Although non-prefetching ORBs are not rejected, they will prefetch
nonetheless. A warning is issued up to once per 5 seconds when a
forced prefetch occurs.
A non-prefetch ORB does not necessarily result in an error, however
frequent encounters with non-prefetch ORBs indicates that channel
programs are being executed in a way that is inconsistent with what
the guest is requesting. While there are currently no known errors
caused by forced prefetch, it is possible in theory that forced
prefetch could result in an error if applied to a channel program
that is dependent on non-prefetch.
Signed-off-by: Jared Rossi
---
Documentation/s390/vfio-ccw.rst | 4
drivers/s390/cio/vfio_ccw_cp.c | 16 +++-
2 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/Documentation/s390/vfio-ccw.rst b/Documentation/s390/vfio-ccw.rst
index fca9c4f5bd9c..8f71071f4403 100644
--- a/Documentation/s390/vfio-ccw.rst
+++ b/Documentation/s390/vfio-ccw.rst
@@ -335,6 +335,10 @@ device.
The current code allows the guest to start channel programs via
START SUBCHANNEL, and to issue HALT SUBCHANNEL and CLEAR SUBCHANNEL.
+Currently all channel programs are prefetched, regardless of the
+p-bit setting in the ORB. As a result, self modifying channel
+programs are not supported (IPL is handled as a special case).
+
vfio-ccw supports classic (command mode) channel I/O only. Transport
mode (HPF) is not supported.
diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c
index 3645d1720c4b..48802e9827b6 100644
--- a/drivers/s390/cio/vfio_ccw_cp.c
+++ b/drivers/s390/cio/vfio_ccw_cp.c
@@ -8,6 +8,7 @@
*Xiao Feng Ren
*/
+#include
#include
#include
#include
@@ -625,23 +626,20 @@ static int ccwchain_fetch_one(struct ccwchain *chain,
* the target channel program from @orb->cmd.iova to the new ccwchain(s).
*
* Limitations:
- * 1. Supports only prefetch enabled mode.
- * 2. Supports idal(c64) ccw chaining.
- * 3. Supports 4k idaw.
+ * 1. Supports idal(c64) ccw chaining.
+ * 2. Supports 4k idaw.
*
* Returns:
* %0 on success and a negative error value on failure.
*/
int cp_init(struct channel_program *cp, struct device *mdev, union orb *orb)
{
+ static DEFINE_RATELIMIT_STATE(ratelimit_state, 5 * HZ, 1);
int ret;
- /*
-* XXX:
-* Only support prefetch enable mode now.
-*/
- if (!orb->cmd.pfch)
- return -EOPNOTSUPP;
+ /* All Linux channel programs are expected to support prefetching */
+ if (!orb->cmd.pfch && __ratelimit(_state))
+ printk(KERN_WARNING "vfio_ccw_cp: prefetch will be forced\n");
INIT_LIST_HEAD(>ccwchain_list);
memcpy(>orb, orb, sizeof(*orb));
--
2.17.0
Re: [PATCH 1/1] vfio-ccw: Enable transparent CCW IPL from DASD
On 2020-04-24 08:50, Halil Pasic wrote: On Thu, 23 Apr 2020 16:25:39 -0400 Eric Farman wrote: On 4/23/20 11:11 AM, Cornelia Huck wrote: > On Thu, 23 Apr 2020 15:56:20 +0200 > Halil Pasic wrote: > >> On Fri, 17 Apr 2020 14:29:39 -0400 >> Jared Rossi wrote: >> >>> Remove the explicit prefetch check when using vfio-ccw devices. >>> This check is not needed as all Linux channel programs are intended >>> to use prefetch and will be executed in the same way regardless. >> >> Hm. This is a guest thing or? So you basically say, it is OK to do >> this, because you know that the guest is gonna be Linux and that it >> the channel program is intended to use prefetch -- but the ORB supplied >> by the guest that designates the channel program happens to state the >> opposite. >> >> Or am I missing something? > > I see this as a kind of architecture compliance/ease of administration > tradeoff, as we none of the guests we currently support uses something > that breaks with prefetching outside of IPL (which has a different > workaround).> And that workaround AFAIR makes sure that we don't issue a CP that is self-modifying or otherwise reliant on non-prefetch. So any time we see a self-modifying program we know, we have an incompatible setup. In any case I believe the commit message is inadequate, as it does not reflect about the risks. > One thing that still concerns me a bit is debuggability if a future > guest indeed does want to dynamically rewrite a channel program: the +1 for some debuggability, just in general > guest thinks it instructed the device to not prefetch, and then > suddenly things do not work as expected. We can log when a guest > submits an orb without prefetch set, but we can't find out if the guest > actually does something that relies on non-prefetch. Without going too far down a non-prefetch rabbit-hole, can we use the cpa_within_range logic to see if the address of the CCW being fetched exists as the CDA of an earlier (non-TIC) CCW in the chain we're processing, and tracing/logging/messaging something about a possible conflict? (Jared, you did some level of this tracing with our real/synthetic tests some time ago. Any chance something of it could be polished and made useful, without being overly heavy on the mainline path?) Back then I believe I made a proposal on how this logic could look like. I think all we need is checking for self rewrites (ccw reads to the addresses that comprise the complete original channel program), and for status-modifier 'skips'. The latter could be easily done by putting some sort of poison at the end of the detected channel program segments. From what I previously did with the tracing, I don't think that there is a practical way to determine if a cp is actually doing something that relies on non-prefetch. It seems we would need to examine the CCWs to find reads and also validate the addresses those CCWs access to check if there is a conflict. Probably this is too much overhead considering that we expect it to be a rare occurrence? Is it too simplistic to print a kernel warning stating that an ORB did not have the p-bit set, but it is being prefetched anyway? Regards, Jared Rossi
