[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/8] s390x/css: be more consistent if broken
From: |
Dong Jia Shi |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/8] s390x/css: be more consistent if broken beyond repair |
Date: |
Mon, 9 Oct 2017 15:49:34 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
* Halil Pasic <address@hidden> [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 <address@hidden>
> ---
> 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 <address@hidden>
--
Dong Jia Shi
- Re: [Qemu-devel] [PATCH v2 2/8] s390x/css: IO instr handler ending control, (continued)
- Re: [Qemu-devel] [PATCH v2 2/8] s390x/css: IO instr handler ending control, Thomas Huth, 2017/10/09
- Re: [Qemu-devel] [PATCH v2 2/8] s390x/css: IO instr handler ending control, Halil Pasic, 2017/10/09
- Re: [Qemu-devel] [PATCH v2 2/8] s390x/css: IO instr handler ending control, Thomas Huth, 2017/10/10
- Re: [Qemu-devel] [PATCH v2 2/8] s390x/css: IO instr handler ending control, Cornelia Huck, 2017/10/10
- Re: [Qemu-devel] [PATCH v2 2/8] s390x/css: IO instr handler ending control, Halil Pasic, 2017/10/10
- Re: [Qemu-devel] [PATCH v2 2/8] s390x/css: IO instr handler ending control, Halil Pasic, 2017/10/10
- Re: [Qemu-devel] [PATCH v2 2/8] s390x/css: IO instr handler ending control, Cornelia Huck, 2017/10/09
- Re: [Qemu-devel] [PATCH v2 2/8] s390x/css: IO instr handler ending control, Halil Pasic, 2017/10/09
[Qemu-devel] [PATCH v2 4/8] s390x: refactor error handling for XSCH handler, Halil Pasic, 2017/10/04
[Qemu-devel] [PATCH v2 1/8] s390x/css: be more consistent if broken beyond repair, Halil Pasic, 2017/10/04
[Qemu-devel] [PATCH v2 8/8] s390x: factor out common ioinst handler logic, Halil Pasic, 2017/10/04
[Qemu-devel] [PATCH v2 6/8] s390x: refactor error handling for HSCH handler, Halil Pasic, 2017/10/04
[Qemu-devel] [PATCH v2 7/8] s390x: refactor error handling for MSCH handler, Halil Pasic, 2017/10/04