Re: [qemu-s390x] [PATCH v5] vfio-ccw: support async command subregion

[Farhan Ali]

On 06/07/2019 10:53 AM, Cornelia Huck wrote:
> A vfio-ccw device may provide an async command subregion for
> issuing halt/clear subchannel requests. If it is present, use
> it for sending halt/clear request to the device; if not, fall
> back to emulation (as done today).
>
> Signed-off-by: Cornelia Huck <address@hidden>
> ---
>
> v4->v5:
> - It seems we need to take the indirection via the class for the
>    callbacks after all :(
> - Dropped Eric's R-b: for that reason
>
> ---
>   hw/s390x/css.c              |  27 +++++++--
>   hw/s390x/s390-ccw.c         |  20 +++++++
>   hw/vfio/ccw.c               | 112 +++++++++++++++++++++++++++++++++++-
>   include/hw/s390x/css.h      |   3 +
>   include/hw/s390x/s390-ccw.h |   2 +
>   5 files changed, 158 insertions(+), 6 deletions(-)
>
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index ad310b9f94bc..b92395f165e6 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -22,6 +22,7 @@
>   #include "trace.h"
>   #include "hw/s390x/s390_flic.h"
>   #include "hw/s390x/s390-virtio-ccw.h"
> +#include "hw/s390x/s390-ccw.h"
>   
>   typedef struct CrwContainer {
>       CRW crw;
> @@ -1205,6 +1206,26 @@ static void sch_handle_start_func_virtual(SubchDev *sch)
>   
>   }
>   
> +static void sch_handle_halt_func_passthrough(SubchDev *sch)
> +{
> +    int ret;
> +
> +    ret = s390_ccw_halt(sch);
> +    if (ret == -ENOSYS) {
> +        sch_handle_halt_func(sch);
> +    }
> +}
> +
> +static void sch_handle_clear_func_passthrough(SubchDev *sch)
> +{
> +    int ret;
> +
> +    ret = s390_ccw_clear(sch);
> +    if (ret == -ENOSYS) {
> +        sch_handle_clear_func(sch);
> +    }
> +}
> +

do we need an extra s390_ccw_clear/halt functions? can't we just call 
cdc->clear/halt in the passthrough functions?

>   static IOInstEnding sch_handle_start_func_passthrough(SubchDev *sch)
>   {
>       SCHIB *schib = &sch->curr_status;
> @@ -1244,11 +1265,9 @@ IOInstEnding do_subchannel_work_passthrough(SubchDev 
> *sch)
>       SCHIB *schib = &sch->curr_status;
>   
>       if (schib->scsw.ctrl & SCSW_FCTL_CLEAR_FUNC) {
> -        /* TODO: Clear handling */
> -        sch_handle_clear_func(sch);
> +        sch_handle_clear_func_passthrough(sch);
>       } else if (schib->scsw.ctrl & SCSW_FCTL_HALT_FUNC) {
> -        /* TODO: Halt handling */
> -        sch_handle_halt_func(sch);
> +        sch_handle_halt_func_passthrough(sch);
>       } else if (schib->scsw.ctrl & SCSW_FCTL_START_FUNC) {
>           return sch_handle_start_func_passthrough(sch);
>       }
> diff --git a/hw/s390x/s390-ccw.c b/hw/s390x/s390-ccw.c
> index f5f025d1b6ca..951be5ab0245 100644
> --- a/hw/s390x/s390-ccw.c
> +++ b/hw/s390x/s390-ccw.c
> @@ -29,6 +29,26 @@ IOInstEnding s390_ccw_cmd_request(SubchDev *sch)
>       return cdc->handle_request(sch);
>   }
>   
> +int s390_ccw_halt(SubchDev *sch)
> +{
> +    S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(sch->driver_data);
> +
> +    if (!cdc->handle_halt) {
> +        return -ENOSYS;
> +    }
> +    return cdc->handle_halt(sch);
> +}
> +
> +int s390_ccw_clear(SubchDev *sch)
> +{
> +    S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(sch->driver_data);
> +
> +    if (!cdc->handle_clear) {
> +        return -ENOSYS;
> +    }
> +    return cdc->handle_clear(sch);
> +}
> +