On 10/10/2017 04:36 PM, Halil Pasic wrote: > > > On 10/10/2017 03:07 PM, Cornelia Huck wrote: >> On Wed, 4 Oct 2017 17:41:39 +0200 >> Halil Pasic <pa...@linux.vnet.ibm.com> wrote: >> >>> 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 <pa...@linux.vnet.ibm.com> >>> --- >>> >>> 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(-) >> >> After browsing through this patch, I think the change will work just as >> well if you use e.g. #defines instead of the structure, won't it? >> > > Sure. We just loose type safety. For example if someone ever should try to > propagate a normal errno via return do_something() the compiler won't > catch it we effectively use int for both. > >
@Connie: Discussion seems to have died down quite a bit, and besides of the struct vs enum vs int and the point of Dong Jia I've seen no complaints. I would like to do a re-spin to accommodate the complaints, but before I abandon the IOInstEnding struct I would like to confirm, that this patch, and it's behavioral changes are OK. I would hate to do the dance backwards, because in the next iteration it turns out that we want to keep the "program_interrupt(env, PGM_ADDRESSING, 4);". Could you please pass your judgment on this patch (assuming I fix the issues found by Dong Jia)? Regards, Halil
