[{"id":1772600,"web_url":"http://patchwork.ozlabs.org/comment/1772600/","msgid":"<20170921111552.41167e80.cohuck@redhat.com>","list_archive_url":null,"date":"2017-09-21T09:15:52","subject":"Re: [Qemu-devel] [PATCH 1/2] s390x/3270: IDA support for 3270 via\n\tCcwDataStream","submitter":{"id":71914,"url":"http://patchwork.ozlabs.org/api/people/71914/","name":"Cornelia Huck","email":"cohuck@redhat.com"},"content":"On Wed, 20 Sep 2017 19:23:13 +0200\nHalil Pasic wrote:\n\n> Let us convert the 3270 code so it uses the recently introduced\n> CcwDataStream abstraction instead of blindly assuming direct data access.\n> \n> This patch does not change behavior beyond introducing IDA support: for\n> direct data access CCWs everything stays as-is. (If there are bugs, they\n> are also preserved).\n> \n> Signed-off-by: Halil Pasic \n> Acked-by: Christian Borntraeger \n> Reviewed-by: Dong Jia Shi \n> ---\n> hw/char/terminal3270.c | 18 +++++++++++-------\n> hw/s390x/3270-ccw.c | 4 ++--\n> include/hw/s390x/3270-ccw.h | 5 ++---\n> 3 files changed, 15 insertions(+), 12 deletions(-)\n> \n> diff --git a/hw/char/terminal3270.c b/hw/char/terminal3270.c\n> index 28f599111d..c976a63cc2 100644\n> --- a/hw/char/terminal3270.c\n> +++ b/hw/char/terminal3270.c\n> @@ -182,14 +182,18 @@ static void terminal_init(EmulatedCcw3270Device *dev, Error **errp)\n> terminal_read, chr_event, NULL, t, NULL, true);\n> }\n> \n> -static int read_payload_3270(EmulatedCcw3270Device *dev, uint32_t cda,\n> - uint16_t count)\n> +static inline CcwDataStream *get_cds(Terminal3270 *t)\n> +{\n> + return &(CCW_DEVICE(&t->cdev)->sch->cds);\n> +}\n> +\n> +static int read_payload_3270(EmulatedCcw3270Device *dev)\n> {\n> Terminal3270 *t = TERMINAL_3270(dev);\n> int len;\n> \n> - len = MIN(count, t->in_len);\n> - cpu_physical_memory_write(cda, t->inv, len);\n> + len = MIN(ccw_dstream_avail(get_cds(t)), t->in_len);\n> + ccw_dstream_write_buf(get_cds(t), t->inv, len);\n\nCCW_DEVICE() as called by get_cds() goes through qom, which implies a\nbit of overhead. Not sure if it makes sense to cache it in this\nfunction so you don't go through it multiple times. (Dito for the other\ncallback.)\n\n> t->in_len -= len;\n> \n> return len;\n\nLooks good.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=cohuck@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xyWFk4V1pz9t3w\n\tfor ;\n\tThu, 21 Sep 2017 19:16:41 +1000 (AEST)","from localhost ([::1]:52526 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1duxb4-0001uE-Rk\n\tfor incoming@patchwork.ozlabs.org; Thu, 21 Sep 2017 05:16:38 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:55435)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1duxaT-0001sI-5a\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 05:16:02 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1duxaP-0007pm-Ve\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 05:16:01 -0400","from mx1.redhat.com ([209.132.183.28]:46312)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1duxaP-0007pC-PS\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 05:15:57 -0400","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 89F99356D6;\n\tThu, 21 Sep 2017 09:15:56 +0000 (UTC)","from gondolin (dhcp-192-215.str.redhat.com [10.33.192.215])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 202025EDEB;\n\tThu, 21 Sep 2017 09:15:54 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 89F99356D6","Date":"Thu, 21 Sep 2017 11:15:52 +0200","From":"Cornelia Huck ","To":"Halil Pasic ","Message-ID":"<20170921111552.41167e80.cohuck@redhat.com>","In-Reply-To":"<20170920172314.102710-2-pasic@linux.vnet.ibm.com>","References":"<20170920172314.102710-1-pasic@linux.vnet.ibm.com>\n\t<20170920172314.102710-2-pasic@linux.vnet.ibm.com>","Organization":"Red Hat GmbH","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.30]);\n\tThu, 21 Sep 2017 09:15:56 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 1/2] s390x/3270: IDA support for 3270 via\n\tCcwDataStream","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"qemu-devel@nongnu.org, Alexander Graf ,\n\tChristian Borntraeger ,\n\t\"Jason J . Herne\" ,\n\tDong Jia Shi ,\n\tRichard Henderson ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1772713,"web_url":"http://patchwork.ozlabs.org/comment/1772713/","msgid":"","list_archive_url":null,"date":"2017-09-21T11:22:44","subject":"Re: [Qemu-devel] [PATCH 1/2] s390x/3270: IDA support for 3270 via\n\tCcwDataStream","submitter":{"id":68297,"url":"http://patchwork.ozlabs.org/api/people/68297/","name":"Halil Pasic","email":"pasic@linux.vnet.ibm.com"},"content":"On 09/21/2017 11:15 AM, Cornelia Huck wrote:\n>> +static inline CcwDataStream *get_cds(Terminal3270 *t)\n>> +{\n>> + return &(CCW_DEVICE(&t->cdev)->sch->cds);\n>> +}\n>> +\n>> +static int read_payload_3270(EmulatedCcw3270Device *dev)\n>> {\n>> Terminal3270 *t = TERMINAL_3270(dev);\n>> int len;\n>> \n>> - len = MIN(count, t->in_len);\n>> - cpu_physical_memory_write(cda, t->inv, len);\n>> + len = MIN(ccw_dstream_avail(get_cds(t)), t->in_len);\n>> + ccw_dstream_write_buf(get_cds(t), t->inv, len);\n> CCW_DEVICE() as called by get_cds() goes through qom, which implies a\n> bit of overhead. Not sure if it makes sense to cache it in this\n> function so you don't go through it multiple times. (Dito for the other\n> callback.)\n> \n\nI've cargo-culted this way of getting CCW_DEVICE(&t->cdev) to the CcwDevice\nform terminal_read (the pattern used at multiple places in the file). As\nfar as I can tell, the overhead basically depends on CONFIG_QOM_CAST_DEBUG.\n\nI have no idea what do we have in production, but my guess is, that\nONFIG_QOM_CAST_DEBUG makes only sense for development and testing\n(especially if proper test coverage is assumed).\nCan you enlighten me?\n\nCCW_DEVICE() may contain a run-time check (depending on CONFIG_QOM_CAST_DEBUG),\nwe however can make sure things are OK at compile time. This brings\nme to the next question. Does it even make sense to use OBJECT_CHECK based\nconstructs when going from specific to general (we don't actually need a\ncast here)? Obviously, for the other direction we really need a cast, so doing\na run-time check there does indeed provide added value.\n\nHalil\n\n\n>> t->in_len -= len;\n>> \n>> return len;","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xyZ3w0ZcWz9t3m\n\tfor ;\n\tThu, 21 Sep 2017 21:23:24 +1000 (AEST)","from localhost ([::1]:52994 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1duzZi-0006Sp-7P\n\tfor incoming@patchwork.ozlabs.org; Thu, 21 Sep 2017 07:23:22 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:56703)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1duzZI-0006P7-4s\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 07:23:00 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1duzZE-0007VC-4c\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 07:22:56 -0400","from mx0a-001b2d01.pphosted.com ([148.163.156.1]:37510)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1duzZD-0007RU-ST\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 07:22:52 -0400","from pps.filterd (m0098394.ppops.net [127.0.0.1])\n\tby mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv8LBKECj100171\n\tfor ; Thu, 21 Sep 2017 07:22:50 -0400","from e06smtp14.uk.ibm.com (e06smtp14.uk.ibm.com [195.75.94.110])\n\tby mx0a-001b2d01.pphosted.com with ESMTP id 2d4a5q0pne-1\n\t(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)\n\tfor ; Thu, 21 Sep 2017 07:22:50 -0400","from localhost\n\tby e06smtp14.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use\n\tOnly! Violators will be prosecuted\n\tfor from ;\n\tThu, 21 Sep 2017 12:22:48 +0100","from b06cxnps3074.portsmouth.uk.ibm.com (9.149.109.194)\n\tby e06smtp14.uk.ibm.com (192.168.101.144) with IBM ESMTP SMTP\n\tGateway: Authorized Use Only! Violators will be prosecuted; \n\tThu, 21 Sep 2017 12:22:45 +0100","from d06av21.portsmouth.uk.ibm.com (d06av21.portsmouth.uk.ibm.com\n\t[9.149.105.232])\n\tby b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with\n\tESMTP id v8LBMjrE17301588; Thu, 21 Sep 2017 11:22:45 GMT","from d06av21.portsmouth.uk.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id 6B58952041;\n\tThu, 21 Sep 2017 11:17:42 +0100 (BST)","from oc3836556865.ibm.com (unknown [9.152.224.207])\n\tby d06av21.portsmouth.uk.ibm.com (Postfix) with ESMTP id D20DB5203F; \n\tThu, 21 Sep 2017 11:17:41 +0100 (BST)"],"To":"Cornelia Huck ","References":"<20170920172314.102710-1-pasic@linux.vnet.ibm.com>\n\t<20170920172314.102710-2-pasic@linux.vnet.ibm.com>\n\t<20170921111552.41167e80.cohuck@redhat.com>","From":"Halil Pasic ","Date":"Thu, 21 Sep 2017 13:22:44 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170921111552.41167e80.cohuck@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"8bit","X-TM-AS-GCONF":"00","x-cbid":"17092111-0016-0000-0000-000004EEF71C","X-IBM-AV-DETECTION":"SAVI=unused REMOTE=unused XFE=unused","x-cbparentid":"17092111-0017-0000-0000-00002829341E","Message-Id":"","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-09-21_02:, , signatures=0","X-Proofpoint-Spam-Details":"rule=outbound_notspam policy=outbound score=0\n\tspamscore=0 suspectscore=0\n\tmalwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam\n\tadjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000\n\tdefinitions=main-1709210153","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 3.x [generic] [fuzzy]","X-Received-From":"148.163.156.1","Subject":"Re: [Qemu-devel] [PATCH 1/2] s390x/3270: IDA support for 3270 via\n\tCcwDataStream","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"qemu-devel@nongnu.org, Alexander Graf ,\n\tChristian Borntraeger ,\n\t\"Jason J . Herne\" ,\n\tDong Jia Shi ,\n\tRichard Henderson ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1772746,"web_url":"http://patchwork.ozlabs.org/comment/1772746/","msgid":"<20170921140506.42c5f7f6.cohuck@redhat.com>","list_archive_url":null,"date":"2017-09-21T12:05:06","subject":"Re: [Qemu-devel] [PATCH 1/2] s390x/3270: IDA support for 3270 via\n\tCcwDataStream","submitter":{"id":71914,"url":"http://patchwork.ozlabs.org/api/people/71914/","name":"Cornelia Huck","email":"cohuck@redhat.com"},"content":"On Thu, 21 Sep 2017 13:22:44 +0200\nHalil Pasic wrote:\n\n> On 09/21/2017 11:15 AM, Cornelia Huck wrote:\n> >> +static inline CcwDataStream *get_cds(Terminal3270 *t)\n> >> +{\n> >> + return &(CCW_DEVICE(&t->cdev)->sch->cds);\n> >> +}\n> >> +\n> >> +static int read_payload_3270(EmulatedCcw3270Device *dev)\n> >> {\n> >> Terminal3270 *t = TERMINAL_3270(dev);\n> >> int len;\n> >> \n> >> - len = MIN(count, t->in_len);\n> >> - cpu_physical_memory_write(cda, t->inv, len);\n> >> + len = MIN(ccw_dstream_avail(get_cds(t)), t->in_len);\n> >> + ccw_dstream_write_buf(get_cds(t), t->inv, len); \n> > CCW_DEVICE() as called by get_cds() goes through qom, which implies a\n> > bit of overhead. Not sure if it makes sense to cache it in this\n> > function so you don't go through it multiple times. (Dito for the other\n> > callback.)\n> > \n> \n> I've cargo-culted this way of getting CCW_DEVICE(&t->cdev) to the CcwDevice\n> form terminal_read (the pattern used at multiple places in the file). As\n> far as I can tell, the overhead basically depends on CONFIG_QOM_CAST_DEBUG.\n> \n> I have no idea what do we have in production, but my guess is, that\n> ONFIG_QOM_CAST_DEBUG makes only sense for development and testing\n> (especially if proper test coverage is assumed).\n> Can you enlighten me?\n> \n> CCW_DEVICE() may contain a run-time check (depending on CONFIG_QOM_CAST_DEBUG),\n> we however can make sure things are OK at compile time. This brings\n> me to the next question. Does it even make sense to use OBJECT_CHECK based\n> constructs when going from specific to general (we don't actually need a\n> cast here)? Obviously, for the other direction we really need a cast, so doing\n> a run-time check there does indeed provide added value.\n\nThe basic rule seems to be \"use a qom cast, unless you are on a fast\npath\" - even though qom debug makes the most sense while developing.\n\nBut let's not turn that into a big discussion: It's probably not really\nworth optimizing here.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx03.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx03.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=cohuck@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xyb0n2VqVz9t42\n\tfor ;\n\tThu, 21 Sep 2017 22:05:45 +1000 (AEST)","from localhost ([::1]:53367 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dv0Eh-0000hl-F6\n\tfor incoming@patchwork.ozlabs.org; Thu, 21 Sep 2017 08:05:43 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:38438)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dv0EH-0000gE-J2\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 08:05:18 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dv0EB-0007Uq-IO\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 08:05:17 -0400","from mx1.redhat.com ([209.132.183.28]:39916)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dv0EB-0007UF-BJ\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 08:05:11 -0400","from smtp.corp.redhat.com\n\t(int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 3E776C0AA5;\n\tThu, 21 Sep 2017 12:05:10 +0000 (UTC)","from gondolin (dhcp-192-215.str.redhat.com [10.33.192.215])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id D21F45D982;\n\tThu, 21 Sep 2017 12:05:08 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 3E776C0AA5","Date":"Thu, 21 Sep 2017 14:05:06 +0200","From":"Cornelia Huck ","To":"Halil Pasic ","Message-ID":"<20170921140506.42c5f7f6.cohuck@redhat.com>","In-Reply-To":"","References":"<20170920172314.102710-1-pasic@linux.vnet.ibm.com>\n\t<20170920172314.102710-2-pasic@linux.vnet.ibm.com>\n\t<20170921111552.41167e80.cohuck@redhat.com>\n\t","Organization":"Red Hat GmbH","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.14","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.27]);\n\tThu, 21 Sep 2017 12:05:10 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 1/2] s390x/3270: IDA support for 3270 via\n\tCcwDataStream","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"qemu-devel@nongnu.org, Alexander Graf ,\n\tChristian Borntraeger ,\n\t\"Jason J . Herne\" ,\n\tDong Jia Shi ,\n\tRichard Henderson ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1772959,"web_url":"http://patchwork.ozlabs.org/comment/1772959/","msgid":"","list_archive_url":null,"date":"2017-09-21T16:11:06","subject":"Re: [Qemu-devel] [PATCH 1/2] s390x/3270: IDA support for 3270 via\n\tCcwDataStream","submitter":{"id":68297,"url":"http://patchwork.ozlabs.org/api/people/68297/","name":"Halil Pasic","email":"pasic@linux.vnet.ibm.com"},"content":"On 09/21/2017 02:05 PM, Cornelia Huck wrote:\n> On Thu, 21 Sep 2017 13:22:44 +0200\n> Halil Pasic wrote:\n> \n>> On 09/21/2017 11:15 AM, Cornelia Huck wrote:\n>>>> +static inline CcwDataStream *get_cds(Terminal3270 *t)\n>>>> +{\n>>>> + return &(CCW_DEVICE(&t->cdev)->sch->cds);\n>>>> +}\n>>>> +\n>>>> +static int read_payload_3270(EmulatedCcw3270Device *dev)\n>>>> {\n>>>> Terminal3270 *t = TERMINAL_3270(dev);\n>>>> int len;\n>>>> \n>>>> - len = MIN(count, t->in_len);\n>>>> - cpu_physical_memory_write(cda, t->inv, len);\n>>>> + len = MIN(ccw_dstream_avail(get_cds(t)), t->in_len);\n>>>> + ccw_dstream_write_buf(get_cds(t), t->inv, len); \n>>> CCW_DEVICE() as called by get_cds() goes through qom, which implies a\n>>> bit of overhead. Not sure if it makes sense to cache it in this\n>>> function so you don't go through it multiple times. (Dito for the other\n>>> callback.)\n>>> \n>>\n>> I've cargo-culted this way of getting CCW_DEVICE(&t->cdev) to the CcwDevice\n>> form terminal_read (the pattern used at multiple places in the file). As\n>> far as I can tell, the overhead basically depends on CONFIG_QOM_CAST_DEBUG.\n>>\n>> I have no idea what do we have in production, but my guess is, that\n>> ONFIG_QOM_CAST_DEBUG makes only sense for development and testing\n>> (especially if proper test coverage is assumed).\n>> Can you enlighten me?\n>>\n>> CCW_DEVICE() may contain a run-time check (depending on CONFIG_QOM_CAST_DEBUG),\n>> we however can make sure things are OK at compile time. This brings\n>> me to the next question. Does it even make sense to use OBJECT_CHECK based\n>> constructs when going from specific to general (we don't actually need a\n>> cast here)? Obviously, for the other direction we really need a cast, so doing\n>> a run-time check there does indeed provide added value.\n> \n> The basic rule seems to be \"use a qom cast, unless you are on a fast\n> path\" - even though qom debug makes the most sense while developing.\n> \n\nDoes this imply \"don't use qom cast on fast path\"? I guess that would\nmean that we basically expect production builds being with CONFIG_QOM_CAST_DEBUG\ndefined.\n\n> But let's not turn that into a big discussion: It's probably not really\n> worth optimizing here.\n>\n\nI agree about the optimization. OTOH I do believe establishing best\npractices is important. I'm afraid, I'm seeing much 'more monkey see\nmonkey do' than healthy (in such an environment prior art is even\nmore important). I believe, understanding a best practice (candidate)\nshould always be a part of establishing a best practice. \n\nSorry, I may be inappropriate (you requested to not turn this into\na big discussion, and I'm afraid this goes in that direction). If\nI'm please ignore the stuff above this line.\n\nSo, there is nothing to be addressed about about this series so far.\nDoes this mean good for inclusion once prerequisites are met -- unless\nsomebody finds something?\n\nRegards,\nHalil","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xyhSg2yWpz9t42\n\tfor ;\n\tFri, 22 Sep 2017 02:11:47 +1000 (AEST)","from localhost ([::1]:54522 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dv44n-00079p-G1\n\tfor incoming@patchwork.ozlabs.org; Thu, 21 Sep 2017 12:11:45 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:52284)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dv44O-00079N-VU\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 12:11:22 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dv44M-0004j0-8a\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 12:11:20 -0400","from mx0b-001b2d01.pphosted.com ([148.163.158.5]:48290\n\thelo=mx0a-001b2d01.pphosted.com)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dv44M-0004iE-3J\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 12:11:18 -0400","from pps.filterd (m0098413.ppops.net [127.0.0.1])\n\tby mx0b-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv8LG4UQM094056\n\tfor ; Thu, 21 Sep 2017 12:11:13 -0400","from e06smtp12.uk.ibm.com (e06smtp12.uk.ibm.com [195.75.94.108])\n\tby mx0b-001b2d01.pphosted.com with ESMTP id 2d4e1h94c5-1\n\t(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)\n\tfor ; Thu, 21 Sep 2017 12:11:12 -0400","from localhost\n\tby e06smtp12.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use\n\tOnly! Violators will be prosecuted\n\tfor from ;\n\tThu, 21 Sep 2017 17:11:11 +0100","from b06cxnps3075.portsmouth.uk.ibm.com (9.149.109.195)\n\tby e06smtp12.uk.ibm.com (192.168.101.142) with IBM ESMTP SMTP\n\tGateway: Authorized Use Only! Violators will be prosecuted; \n\tThu, 21 Sep 2017 17:11:07 +0100","from d06av24.portsmouth.uk.ibm.com (mk.ibm.com [9.149.105.60])\n\tby b06cxnps3075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with\n\tESMTP id v8LGB7Mj31719590; Thu, 21 Sep 2017 16:11:07 GMT","from d06av24.portsmouth.uk.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id 8189842041;\n\tThu, 21 Sep 2017 17:07:16 +0100 (BST)","from d06av24.portsmouth.uk.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id 2B7F84203F;\n\tThu, 21 Sep 2017 17:07:16 +0100 (BST)","from oc3836556865.ibm.com (unknown [9.152.224.207])\n\tby d06av24.portsmouth.uk.ibm.com (Postfix) with ESMTP;\n\tThu, 21 Sep 2017 17:07:16 +0100 (BST)"],"From":"Halil Pasic ","To":"Cornelia Huck ","References":"<20170920172314.102710-1-pasic@linux.vnet.ibm.com>\n\t<20170920172314.102710-2-pasic@linux.vnet.ibm.com>\n\t<20170921111552.41167e80.cohuck@redhat.com>\n\t\n\t<20170921140506.42c5f7f6.cohuck@redhat.com>","Date":"Thu, 21 Sep 2017 18:11:06 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170921140506.42c5f7f6.cohuck@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-TM-AS-GCONF":"00","x-cbid":"17092116-0008-0000-0000-000004990D81","X-IBM-AV-DETECTION":"SAVI=unused REMOTE=unused XFE=unused","x-cbparentid":"17092116-0009-0000-0000-00001E2A4A4D","Message-Id":"","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-09-21_04:, , signatures=0","X-Proofpoint-Spam-Details":"rule=outbound_notspam policy=outbound score=0\n\tspamscore=0 suspectscore=0\n\tmalwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam\n\tadjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000\n\tdefinitions=main-1709210214","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 3.x [generic] [fuzzy]","X-Received-From":"148.163.158.5","Subject":"Re: [Qemu-devel] [PATCH 1/2] s390x/3270: IDA support for 3270 via\n\tCcwDataStream","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"qemu-devel@nongnu.org, Alexander Graf ,\n\tChristian Borntraeger ,\n\t\"Jason J . Herne\" ,\n\tDong Jia Shi ,\n\tRichard Henderson ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1773602,"web_url":"http://patchwork.ozlabs.org/comment/1773602/","msgid":"<20170922153801.709fd692.cohuck@redhat.com>","list_archive_url":null,"date":"2017-09-22T13:38:01","subject":"Re: [Qemu-devel] [PATCH 1/2] s390x/3270: IDA support for 3270 via\n\tCcwDataStream","submitter":{"id":71914,"url":"http://patchwork.ozlabs.org/api/people/71914/","name":"Cornelia Huck","email":"cohuck@redhat.com"},"content":"On Thu, 21 Sep 2017 18:11:06 +0200\nHalil Pasic wrote:\n\n> So, there is nothing to be addressed about about this series so far.\n> Does this mean good for inclusion once prerequisites are met -- unless\n> somebody finds something?\n\nIt is in my pipeline, and I currently don't see anything wrong with it.\n\nMore reviews are still welcome, though, as always :)","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=cohuck@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xzF1q34XWz9sPk\n\tfor ;\n\tFri, 22 Sep 2017 23:38:55 +1000 (AEST)","from localhost ([::1]:58979 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dvOAM-0007kA-RN\n\tfor incoming@patchwork.ozlabs.org; Fri, 22 Sep 2017 09:38:50 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:47924)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dvO9i-0007i7-ST\n\tfor qemu-devel@nongnu.org; Fri, 22 Sep 2017 09:38:14 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dvO9f-0005vF-25\n\tfor qemu-devel@nongnu.org; Fri, 22 Sep 2017 09:38:10 -0400","from mx1.redhat.com ([209.132.183.28]:54030)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dvO9e-0005uy-RL\n\tfor qemu-devel@nongnu.org; Fri, 22 Sep 2017 09:38:06 -0400","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id CB3B1267D4;\n\tFri, 22 Sep 2017 13:38:05 +0000 (UTC)","from gondolin (dhcp-192-215.str.redhat.com [10.33.192.215])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 9743860842;\n\tFri, 22 Sep 2017 13:38:04 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com CB3B1267D4","Date":"Fri, 22 Sep 2017 15:38:01 +0200","From":"Cornelia Huck ","To":"Halil Pasic ","Message-ID":"<20170922153801.709fd692.cohuck@redhat.com>","In-Reply-To":"","References":"<20170920172314.102710-1-pasic@linux.vnet.ibm.com>\n\t<20170920172314.102710-2-pasic@linux.vnet.ibm.com>\n\t<20170921111552.41167e80.cohuck@redhat.com>\n\t\n\t<20170921140506.42c5f7f6.cohuck@redhat.com>\n\t","Organization":"Red Hat GmbH","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.30]);\n\tFri, 22 Sep 2017 13:38:05 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 1/2] s390x/3270: IDA support for 3270 via\n\tCcwDataStream","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"qemu-devel@nongnu.org, Alexander Graf ,\n\tChristian Borntraeger ,\n\t\"Jason J . Herne\" ,\n\tDong Jia Shi ,\n\tRichard Henderson ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]