* Halil Pasic <pa...@linux.vnet.ibm.com> [2017-10-04 17:41:39 +0200]:
[...]
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index 4f47dbc8b0..b2978c3bae 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -1003,12 +1003,11 @@ static void sch_handle_start_func_virtual(SubchDev
> *sch)
>
> }
>
> -static int sch_handle_start_func_passthrough(SubchDev *sch)
> +static IOInstEnding sch_handle_start_func_passthrough(SubchDev *sch)
> {
>
> PMCW *p = &sch->curr_status.pmcw;
> SCSW *s = &sch->curr_status.scsw;
> - int ret;
>
> ORB *orb = &sch->orb;
> if (!(s->ctrl & SCSW_ACTL_SUSP)) {
> @@ -1022,31 +1021,11 @@ static int sch_handle_start_func_passthrough(SubchDev
> *sch)
> */
> if (!(orb->ctrl0 & ORB_CTRL0_MASK_PFCH) ||
> !(orb->ctrl0 & ORB_CTRL0_MASK_C64)) {
> - return -EINVAL;
> + sch_gen_unit_exception(sch);
> + css_inject_io_interrupt(sch);
Last cycle, we agreed to add some log here. Sth. like:
warn_report("vfio-ccw requires PFCH and C64 flags set...");
I promised to do a fix for this piece of code. But since this patch
already fixed it, I guess what I have to do is to add the log only? Or
you would like to add it by yourself? ;)
> + return (IOInstEnding){.cc = 0};
> }
> -
> - ret = s390_ccw_cmd_request(orb, s, sch->driver_data);
> - switch (ret) {
> - /* Currently we don't update control block and just return the cc code.
> */
> - case 0:
> - break;
> - case -EBUSY:
> - break;
> - case -ENODEV:
> - break;
> - case -EACCES:
> - /* Let's reflect an inaccessible host device by cc 3. */
> - ret = -ENODEV;
> - break;
> - default:
> - /*
> - * All other return codes will trigger a program check,
> - * or set cc to 1.
> - */
> - break;
> - };
> -
> - return ret;
> + return s390_ccw_cmd_request(sch);
> }
>
> /*
[...]
> @@ -1084,16 +1063,15 @@ int do_subchannel_work_passthrough(SubchDev *sch)
> /* TODO: Halt handling */
> sch_handle_halt_func(sch);
> } else if (s->ctrl & SCSW_FCTL_START_FUNC) {
> - ret = sch_handle_start_func_passthrough(sch);
> + return sch_handle_start_func_passthrough(sch);
> }
> -
> - return ret;
> + return (IOInstEnding){.cc = 0};
> }
>
> -static int do_subchannel_work(SubchDev *sch)
> +static IOInstEnding do_subchannel_work(SubchDev *sch)
> {
> if (!sch->do_subchannel_work) {
> - return -EINVAL;
> + return (IOInstEnding){.cc = 1};
This keeps the logic here as-is, so it is right.
Anybody agrees that also adding an assert() here?
[...]
> diff --git a/hw/s390x/s390-ccw.c b/hw/s390x/s390-ccw.c
> index 8614dda6f8..5d2c305b71 100644
> --- a/hw/s390x/s390-ccw.c
> +++ b/hw/s390x/s390-ccw.c
> @@ -18,15 +18,14 @@
> #include "hw/s390x/css-bridge.h"
> #include "hw/s390x/s390-ccw.h"
>
> -int s390_ccw_cmd_request(ORB *orb, SCSW *scsw, void *data)
> +IOInstEnding s390_ccw_cmd_request(SubchDev *sch)
> {
> - S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(data);
> + S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(sch->driver_data);
>
> - if (cdc->handle_request) {
> - return cdc->handle_request(orb, scsw, data);
> - } else {
> - return -ENOSYS;
> + if (!cdc->handle_request) {
> + return (IOInstEnding){.cc = 1};
Same consideration as the last comment.
> }
> + return cdc->handle_request(sch);
> }
>
> static void s390_ccw_get_dev_info(S390CCWDevice *cdev,
[...]
--
Dong Jia Shi
