qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 3/8] s390x: improve error handling for SSCH a


From: Dong Jia Shi
Subject: Re: [Qemu-devel] [PATCH v2 3/8] s390x: improve error handling for SSCH and RSCH
Date: Thu, 12 Oct 2017 13:44:50 +0800
User-agent: Mutt/1.5.21 (2010-09-15)

* Halil Pasic <address@hidden> [2017-10-11 12:54:51 +0200]:

> 
> 
> On 10/11/2017 05:47 AM, Dong Jia Shi wrote:
> > * Halil Pasic <address@hidden> [2017-10-04 17:41:39 +0200]:
> > 
> >> Simplify the error handling of the SSCH and RSCH handler avoiding
> >> arbitrary and cryptic error codes being used to tell how the instruction
> >> is supposed to end.  Let the code detecting the condition tell how it's
> >> to be handled in a less ambiguous way.  It's best to handle SSCH and RSCH
> >> in one go as the emulation of the two shares a lot of code.
> >>
> >> For passthrough this change isn't pure refactoring, but changes the
> >> way kernel reported EFAULT is handled. After clarifying the kernel
> >> interface we decided that EFAULT shall be mapped to unit exception.
> >> Same goes for unexpected error codes.
> >>
> >> Signed-off-by: Halil Pasic <address@hidden>
> >> ---
> >>
> >> AFAIR we decided in the previous round to rather do transformation
> >> and fixing in one patch than touch stuff twice. Hence this patch
> >> ain't pure transformation any more.
> >> ---
> >>  hw/s390x/css.c              | 83 
> >> +++++++++++++--------------------------------
> >>  hw/s390x/s390-ccw.c         | 11 +++---
> >>  hw/vfio/ccw.c               | 30 ++++++++++++----
> >>  include/hw/s390x/css.h      | 24 +++++++++----
> >>  include/hw/s390x/s390-ccw.h |  2 +-
> >>  target/s390x/ioinst.c       | 53 ++++-------------------------
> >>  6 files changed, 77 insertions(+), 126 deletions(-)
> >>
> >> 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);
> >> +        return (IOInstEnding){.cc = 0};
> > This behavior change is not mentioned in the commit message. Right?
> > 
> No this particular change is not. Should I mention it explicitly?
I think so.

> Maybe "Same goes for unexpected error codes and absence of required
> ORB flags."
Sounds good for me.

[...]

-- 
Dong Jia Shi




reply via email to

[Prev in Thread] Current Thread [Next in Thread]