From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57505) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1e2cRH-0002B5-1Y for qemu-devel@nongnu.org; Thu, 12 Oct 2017 08:18:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1e2cRC-0000Yv-8l for qemu-devel@nongnu.org; Thu, 12 Oct 2017 08:18:11 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:41846) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1e2cRB-0000YB-VQ for qemu-devel@nongnu.org; Thu, 12 Oct 2017 08:18:06 -0400 Received: from pps.filterd (m0098410.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id v9CCEYiH000743 for ; Thu, 12 Oct 2017 08:18:01 -0400 Received: from e06smtp12.uk.ibm.com (e06smtp12.uk.ibm.com [195.75.94.108]) by mx0a-001b2d01.pphosted.com with ESMTP id 2dj5e5g43b-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Thu, 12 Oct 2017 08:18:01 -0400 Received: from localhost by e06smtp12.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 12 Oct 2017 13:17:58 +0100 References: <20171004154144.88995-1-pasic@linux.vnet.ibm.com> <20171004154144.88995-4-pasic@linux.vnet.ibm.com> <20171010150733.444aa2fe.cohuck@redhat.com> <3e560962-d748-ccfa-ccab-c54f1a30bab4@linux.vnet.ibm.com> <20171012141137.07fb0391.cohuck@redhat.com> From: Halil Pasic Date: Thu, 12 Oct 2017 14:17:55 +0200 MIME-Version: 1.0 In-Reply-To: <20171012141137.07fb0391.cohuck@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Message-Id: Subject: Re: [Qemu-devel] [PATCH v2 3/8] s390x: improve error handling for SSCH and RSCH List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cornelia Huck Cc: Dong Jia Shi , Pierre Morel , Thomas Huth , qemu-devel@nongnu.org On 10/12/2017 02:11 PM, Cornelia Huck wrote: > On Thu, 12 Oct 2017 14:06:42 +0200 > Halil Pasic wrote: > >> 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 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 >>>>> --- >>>>> >>>>> 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)? > > I don't think I really have complaints. Currently quite busy with other > things, though. > Thanks a lot! Unless told otherwise I intend to do a v3 beginning next week with the things we agreed on: use enum, fix the things by Dong Jia, and drop the last 'template approach' patch from the series. If somebody finds something else I'm gonna fix that too of course. Regards, Halil