Message ID | 20171017140453.51099-1-pasic@linux.vnet.ibm.com |
---|---|
Headers | show |
Series | improve error handling for IO instr | expand |
On Tue, 17 Oct 2017 16:04:46 +0200 Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > Abstract > ======= > > The basic idea is: tell how to handle an unusual condition where it's > identified, instead of mapping it to an errno (more or less arbitrarily), > then possibly mapping these errnos around, to finally (mentally) map the > errno back to the condition and take appropriate action. > > According to Dong Jia the patch-set also has a functional value: for ccw > pass-through, he is planing to pass-through the instruction completion > information (cc or interruption condition) from the kernel, and this > patch set can pretty much be seen as a preparation for that. > > Changelog > ========= > > Patch 1 should be already applied to Conny's tree. I've included it > nevertheless so guys working on top of current master have everything in > place. > > v2 -> v3: > * somewhat uwillingly traded the type safe struct to a somewhat > type safe enum (because considered ugly) (Thomas, Conny) > * dropped 'template approach' patch which intended to further > consolidate IO inst. handlers having lot's of logic and code in > common (Conny) > * added warning if vfio-ccw ORB does not have the flags required > by the vfio-ccw implementation as suggested (Dong Jia) > * got rid of some unintentional changes (Dong Jia) > * reworded some stuff (comments, commit messages) (Dong Jia) > > v1 -> v2: > * use assert if do_subchannel_work without any functions being > accepted > * generate unit-exception if ccw-vfio can't handle an otherwise > good channel program (due to extra limitations) > * keep using return values opposed to recording into SubchDev > * split out 'add infrastructure' from 'refactor first handler' > * reworded some commit messanges and comments > * rebased on top of current master > * dropped r-b's and acks because of the magnitude of the > changes > > Testing > ======= > > Nothing happened since v2 except for a quick smoke test. Dong Jia gave v2 > a spin with a focus on vfio-ccw. @Dong Jia I would appreciate some proper > testing, especially regarding the changes in vfio-ccw (patch #3). Looks sane to me (if needed, I can fix up the minor things I found). In addition to some testing, I'd appreciate some review from others as well. > > Halil Pasic (7): > s390x/css: be more consistent if broken beyond repair > s390x/css: IO instr handler ending control > s390x: improve error handling for SSCH and RSCH > s390x: refactor error handling for XSCH handler > s390x: refactor error handling for CSCH handler > s390x: refactor error handling for HSCH handler > s390x: refactor error handling for MSCH handler > > hw/s390x/css.c | 163 ++++++++++++-------------------------------- > hw/s390x/s390-ccw.c | 11 ++- > hw/vfio/ccw.c | 28 ++++++-- > include/hw/s390x/css.h | 47 ++++++++++--- > include/hw/s390x/s390-ccw.h | 2 +- > target/s390x/ioinst.c | 136 +++++++----------------------------- > 6 files changed, 132 insertions(+), 255 deletions(-) >
On 10/17/2017 05:13 PM, Cornelia Huck wrote: > On Tue, 17 Oct 2017 16:04:46 +0200 > Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > >> Abstract >> ======= >> >> The basic idea is: tell how to handle an unusual condition where it's >> identified, instead of mapping it to an errno (more or less arbitrarily), >> then possibly mapping these errnos around, to finally (mentally) map the >> errno back to the condition and take appropriate action. >> >> According to Dong Jia the patch-set also has a functional value: for ccw >> pass-through, he is planing to pass-through the instruction completion >> information (cc or interruption condition) from the kernel, and this >> patch set can pretty much be seen as a preparation for that. >> >> Changelog >> ========= >> >> Patch 1 should be already applied to Conny's tree. I've included it >> nevertheless so guys working on top of current master have everything in >> place. >> >> v2 -> v3: >> * somewhat uwillingly traded the type safe struct to a somewhat >> type safe enum (because considered ugly) (Thomas, Conny) >> * dropped 'template approach' patch which intended to further >> consolidate IO inst. handlers having lot's of logic and code in >> common (Conny) >> * added warning if vfio-ccw ORB does not have the flags required >> by the vfio-ccw implementation as suggested (Dong Jia) >> * got rid of some unintentional changes (Dong Jia) >> * reworded some stuff (comments, commit messages) (Dong Jia) >> >> v1 -> v2: >> * use assert if do_subchannel_work without any functions being >> accepted >> * generate unit-exception if ccw-vfio can't handle an otherwise >> good channel program (due to extra limitations) >> * keep using return values opposed to recording into SubchDev >> * split out 'add infrastructure' from 'refactor first handler' >> * reworded some commit messanges and comments >> * rebased on top of current master >> * dropped r-b's and acks because of the magnitude of the >> changes >> >> Testing >> ======= >> >> Nothing happened since v2 except for a quick smoke test. Dong Jia gave v2 >> a spin with a focus on vfio-ccw. @Dong Jia I would appreciate some proper >> testing, especially regarding the changes in vfio-ccw (patch #3). > > Looks sane to me (if needed, I can fix up the minor things I found). > > In addition to some testing, I'd appreciate some review from others as > well. > Of course, I'm fine with the fixes (won't answer individually). I think both Dong Jia and Pierre have already put enough work in this to be credited with a tag, so I really hope they will get around to review this. I would be especially happy with an Tested-by: Dong Jia since this series is quite under-tested, and the changes in vfio-ccw aren't just minor. Of course I could come up with a test setup myself, but I hope Dong Jia already has one, and he is certainly more involved with vfio-ccw. Regards, Halil >> >> Halil Pasic (7): >> s390x/css: be more consistent if broken beyond repair >> s390x/css: IO instr handler ending control >> s390x: improve error handling for SSCH and RSCH >> s390x: refactor error handling for XSCH handler >> s390x: refactor error handling for CSCH handler >> s390x: refactor error handling for HSCH handler >> s390x: refactor error handling for MSCH handler >> >> hw/s390x/css.c | 163 ++++++++++++-------------------------------- >> hw/s390x/s390-ccw.c | 11 ++- >> hw/vfio/ccw.c | 28 ++++++-- >> include/hw/s390x/css.h | 47 ++++++++++--- >> include/hw/s390x/s390-ccw.h | 2 +- >> target/s390x/ioinst.c | 136 +++++++----------------------------- >> 6 files changed, 132 insertions(+), 255 deletions(-) >> > >
On Tue, 17 Oct 2017 18:19:20 +0200 Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > On 10/17/2017 05:13 PM, Cornelia Huck wrote: > > On Tue, 17 Oct 2017 16:04:46 +0200 > > Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > > > >> Abstract > >> ======= > >> > >> The basic idea is: tell how to handle an unusual condition where it's > >> identified, instead of mapping it to an errno (more or less arbitrarily), > >> then possibly mapping these errnos around, to finally (mentally) map the > >> errno back to the condition and take appropriate action. > >> > >> According to Dong Jia the patch-set also has a functional value: for ccw > >> pass-through, he is planing to pass-through the instruction completion > >> information (cc or interruption condition) from the kernel, and this > >> patch set can pretty much be seen as a preparation for that. > >> > >> Changelog > >> ========= > >> > >> Patch 1 should be already applied to Conny's tree. I've included it > >> nevertheless so guys working on top of current master have everything in > >> place. > >> > >> v2 -> v3: > >> * somewhat uwillingly traded the type safe struct to a somewhat > >> type safe enum (because considered ugly) (Thomas, Conny) > >> * dropped 'template approach' patch which intended to further > >> consolidate IO inst. handlers having lot's of logic and code in > >> common (Conny) > >> * added warning if vfio-ccw ORB does not have the flags required > >> by the vfio-ccw implementation as suggested (Dong Jia) > >> * got rid of some unintentional changes (Dong Jia) > >> * reworded some stuff (comments, commit messages) (Dong Jia) > >> > >> v1 -> v2: > >> * use assert if do_subchannel_work without any functions being > >> accepted > >> * generate unit-exception if ccw-vfio can't handle an otherwise > >> good channel program (due to extra limitations) > >> * keep using return values opposed to recording into SubchDev > >> * split out 'add infrastructure' from 'refactor first handler' > >> * reworded some commit messanges and comments > >> * rebased on top of current master > >> * dropped r-b's and acks because of the magnitude of the > >> changes > >> > >> Testing > >> ======= > >> > >> Nothing happened since v2 except for a quick smoke test. Dong Jia gave v2 > >> a spin with a focus on vfio-ccw. @Dong Jia I would appreciate some proper > >> testing, especially regarding the changes in vfio-ccw (patch #3). > > > > Looks sane to me (if needed, I can fix up the minor things I found). > > > > In addition to some testing, I'd appreciate some review from others as > > well. > > > > Of course, I'm fine with the fixes (won't answer individually). I think > both Dong Jia and Pierre have already put enough work in this to be credited > with a tag, so I really hope they will get around to review this. I would > be especially happy with an Tested-by: Dong Jia since this series is quite > under-tested, and the changes in vfio-ccw aren't just minor. > > Of course I could come up with a test setup myself, but I hope Dong Jia > already has one, and he is certainly more involved with vfio-ccw. FTR: I'll wait until tomorrow for more tags and then go ahead and apply (well, if no problem comes up in the meantime). I need to get a pull request out of the door this week.
* Halil Pasic <pasic@linux.vnet.ibm.com> [2017-10-17 18:19:20 +0200]: [...] > >> Testing > >> ======= > >> > >> Nothing happened since v2 except for a quick smoke test. Dong Jia gave v2 > >> a spin with a focus on vfio-ccw. @Dong Jia I would appreciate some proper > >> testing, especially regarding the changes in vfio-ccw (patch #3). > > > > Looks sane to me (if needed, I can fix up the minor things I found). > > > > In addition to some testing, I'd appreciate some review from others as > > well. > > > > Of course, I'm fine with the fixes (won't answer individually). I think > both Dong Jia and Pierre have already put enough work in this to be credited > with a tag, so I really hope they will get around to review this. I would > be especially happy with an Tested-by: Dong Jia since this series is quite > under-tested, and the changes in vfio-ccw aren't just minor. > > Of course I could come up with a test setup myself, but I hope Dong Jia > already has one, and he is certainly more involved with vfio-ccw. > Using Conny's s390-next branch + this series (#2-#7), I didn't notice any obvious problem during my fio test. So for the vfio-ccw related part: Tested-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> [...]
On Wed, 18 Oct 2017 16:23:47 +0800 Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> wrote: > * Halil Pasic <pasic@linux.vnet.ibm.com> [2017-10-17 18:19:20 +0200]: > > [...] > > > >> Testing > > >> ======= > > >> > > >> Nothing happened since v2 except for a quick smoke test. Dong Jia gave v2 > > >> a spin with a focus on vfio-ccw. @Dong Jia I would appreciate some proper > > >> testing, especially regarding the changes in vfio-ccw (patch #3). > > > > > > Looks sane to me (if needed, I can fix up the minor things I found). > > > > > > In addition to some testing, I'd appreciate some review from others as > > > well. > > > > > > > Of course, I'm fine with the fixes (won't answer individually). I think > > both Dong Jia and Pierre have already put enough work in this to be credited > > with a tag, so I really hope they will get around to review this. I would > > be especially happy with an Tested-by: Dong Jia since this series is quite > > under-tested, and the changes in vfio-ccw aren't just minor. > > > > Of course I could come up with a test setup myself, but I hope Dong Jia > > already has one, and he is certainly more involved with vfio-ccw. > > > Using Conny's s390-next branch + this series (#2-#7), I didn't notice > any obvious problem during my fio test. So for the vfio-ccw related > part: > Tested-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> Thanks! To which patches may I add the tag? :)
On Tue, 17 Oct 2017 16:04:46 +0200 Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > Abstract > ======= > > The basic idea is: tell how to handle an unusual condition where it's > identified, instead of mapping it to an errno (more or less arbitrarily), > then possibly mapping these errnos around, to finally (mentally) map the > errno back to the condition and take appropriate action. > > According to Dong Jia the patch-set also has a functional value: for ccw > pass-through, he is planing to pass-through the instruction completion > information (cc or interruption condition) from the kernel, and this > patch set can pretty much be seen as a preparation for that. > > Changelog > ========= > > Patch 1 should be already applied to Conny's tree. I've included it > nevertheless so guys working on top of current master have everything in > place. > > v2 -> v3: > * somewhat uwillingly traded the type safe struct to a somewhat > type safe enum (because considered ugly) (Thomas, Conny) > * dropped 'template approach' patch which intended to further > consolidate IO inst. handlers having lot's of logic and code in > common (Conny) > * added warning if vfio-ccw ORB does not have the flags required > by the vfio-ccw implementation as suggested (Dong Jia) > * got rid of some unintentional changes (Dong Jia) > * reworded some stuff (comments, commit messages) (Dong Jia) > > v1 -> v2: > * use assert if do_subchannel_work without any functions being > accepted > * generate unit-exception if ccw-vfio can't handle an otherwise > good channel program (due to extra limitations) > * keep using return values opposed to recording into SubchDev > * split out 'add infrastructure' from 'refactor first handler' > * reworded some commit messanges and comments > * rebased on top of current master > * dropped r-b's and acks because of the magnitude of the > changes > > Testing > ======= > > Nothing happened since v2 except for a quick smoke test. Dong Jia gave v2 > a spin with a focus on vfio-ccw. @Dong Jia I would appreciate some proper > testing, especially regarding the changes in vfio-ccw (patch #3). > > Halil Pasic (7): > s390x/css: be more consistent if broken beyond repair > s390x/css: IO instr handler ending control > s390x: improve error handling for SSCH and RSCH > s390x: refactor error handling for XSCH handler > s390x: refactor error handling for CSCH handler > s390x: refactor error handling for HSCH handler > s390x: refactor error handling for MSCH handler > > hw/s390x/css.c | 163 ++++++++++++-------------------------------- > hw/s390x/s390-ccw.c | 11 ++- > hw/vfio/ccw.c | 28 ++++++-- > include/hw/s390x/css.h | 47 ++++++++++--- > include/hw/s390x/s390-ccw.h | 2 +- > target/s390x/ioinst.c | 136 +++++++----------------------------- > 6 files changed, 132 insertions(+), 255 deletions(-) > FYI: I pushed out what I currently have to git://github.com/cohuck/qemu ioinst-retcode Feedback appreciated.
* Cornelia Huck <cohuck@redhat.com> [2017-10-18 11:53:10 +0200]: > On Wed, 18 Oct 2017 16:23:47 +0800 > Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> wrote: > > > * Halil Pasic <pasic@linux.vnet.ibm.com> [2017-10-17 18:19:20 +0200]: > > > > [...] > > > > > >> Testing > > > >> ======= > > > >> > > > >> Nothing happened since v2 except for a quick smoke test. Dong Jia gave v2 > > > >> a spin with a focus on vfio-ccw. @Dong Jia I would appreciate some proper > > > >> testing, especially regarding the changes in vfio-ccw (patch #3). > > > > > > > > Looks sane to me (if needed, I can fix up the minor things I found). > > > > > > > > In addition to some testing, I'd appreciate some review from others as > > > > well. > > > > > > > > > > Of course, I'm fine with the fixes (won't answer individually). I think > > > both Dong Jia and Pierre have already put enough work in this to be credited > > > with a tag, so I really hope they will get around to review this. I would > > > be especially happy with an Tested-by: Dong Jia since this series is quite > > > under-tested, and the changes in vfio-ccw aren't just minor. > > > > > > Of course I could come up with a test setup myself, but I hope Dong Jia > > > already has one, and he is certainly more involved with vfio-ccw. > > > > > Using Conny's s390-next branch + this series (#2-#7), I didn't notice > > any obvious problem during my fio test. So for the vfio-ccw related > > part: > > Tested-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> > > Thanks! > > To which patches may I add the tag? :) > The test should cover the vfio-ccw related part in patch #3, I assume. So add it there?
On Tue, 17 Oct 2017 16:04:46 +0200 Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > Abstract > ======= > > The basic idea is: tell how to handle an unusual condition where it's > identified, instead of mapping it to an errno (more or less arbitrarily), > then possibly mapping these errnos around, to finally (mentally) map the > errno back to the condition and take appropriate action. > > According to Dong Jia the patch-set also has a functional value: for ccw > pass-through, he is planing to pass-through the instruction completion > information (cc or interruption condition) from the kernel, and this > patch set can pretty much be seen as a preparation for that. > > Changelog > ========= > > Patch 1 should be already applied to Conny's tree. I've included it > nevertheless so guys working on top of current master have everything in > place. > > v2 -> v3: > * somewhat uwillingly traded the type safe struct to a somewhat > type safe enum (because considered ugly) (Thomas, Conny) > * dropped 'template approach' patch which intended to further > consolidate IO inst. handlers having lot's of logic and code in > common (Conny) > * added warning if vfio-ccw ORB does not have the flags required > by the vfio-ccw implementation as suggested (Dong Jia) > * got rid of some unintentional changes (Dong Jia) > * reworded some stuff (comments, commit messages) (Dong Jia) > > v1 -> v2: > * use assert if do_subchannel_work without any functions being > accepted > * generate unit-exception if ccw-vfio can't handle an otherwise > good channel program (due to extra limitations) > * keep using return values opposed to recording into SubchDev > * split out 'add infrastructure' from 'refactor first handler' > * reworded some commit messanges and comments > * rebased on top of current master > * dropped r-b's and acks because of the magnitude of the > changes > > Testing > ======= > > Nothing happened since v2 except for a quick smoke test. Dong Jia gave v2 > a spin with a focus on vfio-ccw. @Dong Jia I would appreciate some proper > testing, especially regarding the changes in vfio-ccw (patch #3). > > Halil Pasic (7): > s390x/css: be more consistent if broken beyond repair > s390x/css: IO instr handler ending control > s390x: improve error handling for SSCH and RSCH > s390x: refactor error handling for XSCH handler > s390x: refactor error handling for CSCH handler > s390x: refactor error handling for HSCH handler > s390x: refactor error handling for MSCH handler > > hw/s390x/css.c | 163 ++++++++++++-------------------------------- > hw/s390x/s390-ccw.c | 11 ++- > hw/vfio/ccw.c | 28 ++++++-- > include/hw/s390x/css.h | 47 ++++++++++--- > include/hw/s390x/s390-ccw.h | 2 +- > target/s390x/ioinst.c | 136 +++++++----------------------------- > 6 files changed, 132 insertions(+), 255 deletions(-) > Thanks, applied.