* Halil Pasic <pa...@linux.vnet.ibm.com> [2017-10-04 17:41:37 +0200]:
> Calling do_subchannel_work with no function control flags set in SCSW is
> a programming error. Currently the handle this differently in
?:
s/the/we/
> do_subchannel_work_virtual and do_subchannel_work_passthrough. Let's be
> consistent and guard with a common assert against this programming error.
>
> Signed-off-by: Halil Pasic <pa...@linux.vnet.ibm.com>
> ---
> hw/s390x/css.c | 16 ++++------------
> 1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index c59be1aad1..4f47dbc8b0 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -1067,9 +1067,6 @@ int do_subchannel_work_virtual(SubchDev *sch)
> } else if (s->ctrl & SCSW_FCTL_START_FUNC) {
> /* Triggered by both ssch and rsch. */
> sch_handle_start_func_virtual(sch);
> - } else {
> - /* Cannot happen. */
> - return 0;
> }
> css_inject_io_interrupt(sch);
> return 0;
> @@ -1077,22 +1074,17 @@ int do_subchannel_work_virtual(SubchDev *sch)
>
> int do_subchannel_work_passthrough(SubchDev *sch)
> {
> - int ret;
> + int ret = 0;
> SCSW *s = &sch->curr_status.scsw;
>
> if (s->ctrl & SCSW_FCTL_CLEAR_FUNC) {
> /* TODO: Clear handling */
> sch_handle_clear_func(sch);
> - ret = 0;
> } else if (s->ctrl & SCSW_FCTL_HALT_FUNC) {
> /* TODO: Halt handling */
> sch_handle_halt_func(sch);
> - ret = 0;
A bit surprise to see these. I'm fine with these changes though.
> } else if (s->ctrl & SCSW_FCTL_START_FUNC) {
> ret = sch_handle_start_func_passthrough(sch);
> - } else {
> - /* Cannot happen. */
> - return -ENODEV;
> }
>
> return ret;
> @@ -1100,11 +1092,11 @@ int do_subchannel_work_passthrough(SubchDev *sch)
>
> static int do_subchannel_work(SubchDev *sch)
> {
> - if (sch->do_subchannel_work) {
> - return sch->do_subchannel_work(sch);
> - } else {
> + if (!sch->do_subchannel_work) {
> return -EINVAL;
> }
> + g_assert(sch->curr_status.scsw.ctrl & SCSW_CTRL_MASK_FCTL);
> + return sch->do_subchannel_work(sch);
> }
>
> static void copy_pmcw_to_guest(PMCW *dest, const PMCW *src)
With the fix of the message:
Reviewed-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
--
Dong Jia Shi
