lkml.org 
[lkml]   [2018]   [May]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH RFC 1/2] vfio-ccw: forward halt/clear to device if supported
From
Date
On 09/05/2018 17:49, Cornelia Huck wrote:
> The initial version of vfio-ccw did not support forwarding of the
> halt or clear functions to the device, and we had to emulate them
> instead.
>
> For versions of the vfio-ccw kernel implementation that indeed do
> support halt/clear (by indicating them in the fctl of the scsw in
> the io_region), we can simply start making use of it. If the kernel
> does not support handling halt/clear, fall back to emulation.
>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
> hw/s390x/css.c | 32 ++++++++++++++++++++++++++++----
> hw/vfio/ccw.c | 11 +++++++++--
> include/hw/s390x/css.h | 10 +++++++---
> 3 files changed, 44 insertions(+), 9 deletions(-)
>
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index 301bf1772f..b6727d0607 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -1180,6 +1180,16 @@ static void sch_handle_start_func_virtual(SubchDev *sch)
>
> }
>
> +static IOInstEnding sch_handle_clear_func_passthrough(SubchDev *sch)
> +{
> + return s390_ccw_cmd_request(sch);
> +}
> +
> +static IOInstEnding sch_handle_halt_func_passthrough(SubchDev *sch)
> +{
> + return s390_ccw_cmd_request(sch);
> +}
> +
> static IOInstEnding sch_handle_start_func_passthrough(SubchDev *sch)
> {
>
> @@ -1233,13 +1243,27 @@ IOInstEnding do_subchannel_work_virtual(SubchDev *sch)
> IOInstEnding do_subchannel_work_passthrough(SubchDev *sch)
> {
> SCSW *s = &sch->curr_status.scsw;
> + static bool no_halt_clear;
>
> + /* if the kernel does not support halt/clear, fall back to emulation */
> if (s->ctrl & SCSW_FCTL_CLEAR_FUNC) {
> - /* TODO: Clear handling */
> - sch_handle_clear_func(sch);
> + if (no_halt_clear) {
> + sch_handle_clear_func(sch);
> + } else {
> + if (sch_handle_clear_func_passthrough(sch) == IOINST_OPNOTSUPP) {
> + no_halt_clear = true;
> + sch_handle_halt_func(sch);
> + }
> + }
> } else if (s->ctrl & SCSW_FCTL_HALT_FUNC) {
> - /* TODO: Halt handling */
> - sch_handle_halt_func(sch);
> + if (no_halt_clear) {
> + sch_handle_halt_func(sch);
> + } else {
> + if (sch_handle_halt_func_passthrough(sch) == IOINST_OPNOTSUPP) {
> + no_halt_clear = true;
> + sch_handle_halt_func(sch);
> + }
> + }
> } else if (s->ctrl & SCSW_FCTL_START_FUNC) {
> return sch_handle_start_func_passthrough(sch);
> }
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> index e67392c5f9..247901ae41 100644
> --- a/hw/vfio/ccw.c
> +++ b/hw/vfio/ccw.c
> @@ -60,6 +60,7 @@ static IOInstEnding vfio_ccw_handle_request(SubchDev *sch)
>
> memset(region, 0, sizeof(*region));
>
> + /* orb is only valid for ssch, but does not hurt for other functions */
> memcpy(region->orb_area, &sch->orb, sizeof(ORB));
> memcpy(region->scsw_area, &sch->curr_status.scsw, sizeof(SCSW));
>
> @@ -70,8 +71,12 @@ again:
> if (errno == EAGAIN) {
> goto again;
> }
> - error_report("vfio-ccw: wirte I/O region failed with errno=%d", errno);
> - ret = -errno;
> + /* handle not supported operations like halt/clear on older kernels */
> + if (ret != -EOPNOTSUPP) {
> + error_report("vfio-ccw: write I/O region failed with errno=%d",
> + errno);
> + ret = -errno;
> + }
> } else {
> ret = region->ret_code;
> }
> @@ -83,6 +88,8 @@ again:
> case -ENODEV:
> case -EACCES:
> return IOINST_CC_NOT_OPERATIONAL;
> + case -EOPNOTSUPP:
> + return IOINST_OPNOTSUPP;
> case -EFAULT:
> default:
> sch_gen_unit_exception(sch);
> diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
> index 35facb47d2..e33f26882b 100644
> --- a/include/hw/s390x/css.h
> +++ b/include/hw/s390x/css.h
> @@ -100,9 +100,11 @@ typedef struct CcwDataStream {
> } CcwDataStream;
>
> /*
> - * IO instructions conclude according to this. Currently we have only
> - * cc codes. Valid values are 0, 1, 2, 3 and the generic semantic for
> + * IO instructions conclude according to this. One class of values are
> + * cc codes: Valid values are 0, 1, 2, 3 and the generic semantic for
> * IO instructions is described briefly. For more details consult the PoP.
> + * Additionally, other endings may occur due to internal processing errors
> + * like lack of support for an operation.
> */
> typedef enum IOInstEnding {
> /* produced expected result */
> @@ -112,7 +114,9 @@ typedef enum IOInstEnding {
> /* inst. ineffective because busy with previously initiated function */
> IOINST_CC_BUSY = 2,
> /* inst. ineffective because not operational */
> - IOINST_CC_NOT_OPERATIONAL = 3
> + IOINST_CC_NOT_OPERATIONAL = 3,
> + /* internal: operation not supported */
> + IOINST_OPNOTSUPP = 4
> } IOInstEnding;
>
> typedef struct SubchDev SubchDev;


Couldn't we introduce ABI versioning ?


--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany

\
 
 \ /
  Last update: 2018-05-11 11:54    [W:0.027 / U:2.960 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site