[{"id":1760486,"web_url":"http://patchwork.ozlabs.org/comment/1760486/","msgid":"<09179efb-0930-bd84-ff22-1f4b23fa6a8e@redhat.com>","list_archive_url":null,"date":"2017-08-30T20:58:16","subject":"Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside\n\tmachine state","submitter":{"id":66152,"url":"http://patchwork.ozlabs.org/api/people/66152/","name":"Thomas Huth","email":"thuth@redhat.com"},"content":"On 30.08.2017 19:05, David Hildenbrand wrote:\n> Let's avoid global variables. While at it, move both functions using it,\n> so we won't have to temporarily add includes (we'll be getting rid of\n> s390-virtio.c soon).\n> \n> Signed-off-by: David Hildenbrand <david@redhat.com>\n> ---\n>  hw/s390x/s390-virtio-ccw.c         | 39 ++++++++++++++++++++++++++++++++++++++\n>  hw/s390x/s390-virtio.c             | 38 -------------------------------------\n>  hw/s390x/s390-virtio.h             |  1 -\n>  include/hw/s390x/s390-virtio-ccw.h |  3 +++\n>  4 files changed, 42 insertions(+), 39 deletions(-)\n> \n> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c\n> index dd504dd5ae..ffd56af834 100644\n> --- a/hw/s390x/s390-virtio-ccw.c\n> +++ b/hw/s390x/s390-virtio-ccw.c\n> @@ -32,6 +32,45 @@\n>  #include \"migration/register.h\"\n>  #include \"cpu_models.h\"\n>  \n> +S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)\n> +{\n> +    S390CcwMachineState *ms = S390_CCW_MACHINE(qdev_get_machine());\n> +\n> +    if (cpu_addr >= max_cpus) {\n> +        return NULL;\n> +    }\n> +\n> +    /* Fast lookup via CPU ID */\n> +    return ms->cpus[cpu_addr];\n> +}\n\nI wonder whether that function should rather go into a file in\ntarget/s390x/ instead, since it is also used there and its prototype is\nin cpu.h ?\n\n[...]\n> diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h\n> index 41a9d2862b..4bef28ec39 100644\n> --- a/include/hw/s390x/s390-virtio-ccw.h\n> +++ b/include/hw/s390x/s390-virtio-ccw.h\n> @@ -21,11 +21,14 @@\n>  #define S390_MACHINE_CLASS(klass) \\\n>      OBJECT_CLASS_CHECK(S390CcwMachineClass, (klass), TYPE_S390_CCW_MACHINE)\n>  \n> +struct S390CPU;\n\nYou define a \"struct S390CPU\" here ...\n\n>  typedef struct S390CcwMachineState {\n>      /*< private >*/\n>      MachineState parent_obj;\n>  \n>      /*< public >*/\n> +    S390CPU **cpus;\n\n... but use the typedef'ed S390CPU here ... looks somewhat suspicious, I\nwonder whether the typedef is really in the right place?\n\n>      bool aes_key_wrap;\n>      bool dea_key_wrap;\n>      uint8_t loadparm[8];\n\nAnyway, that were just nits, I'm also fine with the patch as it is, so:\n\nReviewed-by: Thomas Huth <thuth@redhat.com>","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","ext-mx07.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx07.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=thuth@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 3xjHt43FkWz9s7h\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 31 Aug 2017 06:58:51 +1000 (AEST)","from localhost ([::1]:52631 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dnA4W-0005ev-Os\n\tfor incoming@patchwork.ozlabs.org; Wed, 30 Aug 2017 16:58:48 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:34406)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <thuth@redhat.com>) id 1dnA49-0005en-TK\n\tfor qemu-devel@nongnu.org; Wed, 30 Aug 2017 16:58:26 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <thuth@redhat.com>) id 1dnA46-00043q-Qn\n\tfor qemu-devel@nongnu.org; Wed, 30 Aug 2017 16:58:25 -0400","from mx1.redhat.com ([209.132.183.28]:23063)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <thuth@redhat.com>) id 1dnA46-00043a-Kp\n\tfor qemu-devel@nongnu.org; Wed, 30 Aug 2017 16:58:22 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\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 5060CC04B941;\n\tWed, 30 Aug 2017 20:58:21 +0000 (UTC)","from [10.36.116.28] (ovpn-116-28.ams2.redhat.com [10.36.116.28])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id E5FF79815A;\n\tWed, 30 Aug 2017 20:58:17 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 5060CC04B941","To":"David Hildenbrand <david@redhat.com>, qemu-devel@nongnu.org","References":"<20170830170601.15855-1-david@redhat.com>\n\t<20170830170601.15855-4-david@redhat.com>","From":"Thomas Huth <thuth@redhat.com>","Message-ID":"<09179efb-0930-bd84-ff22-1f4b23fa6a8e@redhat.com>","Date":"Wed, 30 Aug 2017 22:58:16 +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":"<20170830170601.15855-4-david@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.31]);\n\tWed, 30 Aug 2017 20:58:21 +0000 (UTC)","Content-Transfer-Encoding":"quoted-printable","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 v1 03/11] s390x: store cpu states inside\n\tmachine state","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"borntraeger@de.ibm.com, cohuck@redhat.com,\n\tRichard Henderson <richard.henderson@linaro.org>,\n\tAlexander Graf <agraf@suse.de>, Aurelien Jarno <aurelien@aurel32.net>","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1760974,"web_url":"http://patchwork.ozlabs.org/comment/1760974/","msgid":"<aa375deb-69ba-e0a8-d617-bbb304207a12@redhat.com>","list_archive_url":null,"date":"2017-08-31T13:11:28","subject":"Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside\n\tmachine state","submitter":{"id":70402,"url":"http://patchwork.ozlabs.org/api/people/70402/","name":"David Hildenbrand","email":"david@redhat.com"},"content":">> +S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)\n>> +{\n>> +    S390CcwMachineState *ms = S390_CCW_MACHINE(qdev_get_machine());\n>> +\n>> +    if (cpu_addr >= max_cpus) {\n>> +        return NULL;\n>> +    }\n>> +\n>> +    /* Fast lookup via CPU ID */\n>> +    return ms->cpus[cpu_addr];\n>> +}\n> \n> I wonder whether that function should rather go into a file in\n> target/s390x/ instead, since it is also used there and its prototype is\n> in cpu.h ?\n\nI thought about the same thing, but as it works directly on the machine,\nlike ri_allowed() and friends. So I decided to keep it here for now.\n\nI'll think about moving the definition also into\ninclude/hw/s390x/s390-virtio-ccw.h\n\n> \n> [...]\n>> diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h\n>> index 41a9d2862b..4bef28ec39 100644\n>> --- a/include/hw/s390x/s390-virtio-ccw.h\n>> +++ b/include/hw/s390x/s390-virtio-ccw.h\n>> @@ -21,11 +21,14 @@\n>>  #define S390_MACHINE_CLASS(klass) \\\n>>      OBJECT_CLASS_CHECK(S390CcwMachineClass, (klass), TYPE_S390_CCW_MACHINE)\n>>  \n>> +struct S390CPU;\n> \n> You define a \"struct S390CPU\" here ...\n> \n>>  typedef struct S390CcwMachineState {\n>>      /*< private >*/\n>>      MachineState parent_obj;\n>>  \n>>      /*< public >*/\n>> +    S390CPU **cpus;\n\nI'll simply convert this to struct S390CPU, so this header stays\nindependent from cpu headers.\n\n> \n> ... but use the typedef'ed S390CPU here ... looks somewhat suspicious, I\n> wonder whether the typedef is really in the right place?\n> \n>>      bool aes_key_wrap;\n>>      bool dea_key_wrap;\n>>      uint8_t loadparm[8];\n> \n> Anyway, that were just nits, I'm also fine with the patch as it is, so:\n> \n> Reviewed-by: Thomas Huth <thuth@redhat.com>\n> \n\nThanks!","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","ext-mx09.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx09.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=david@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 3xjjTC2rgFz9sPm\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 31 Aug 2017 23:12:15 +1000 (AEST)","from localhost ([::1]:55745 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dnPGX-0007KJ-Hg\n\tfor incoming@patchwork.ozlabs.org; Thu, 31 Aug 2017 09:12:13 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:53169)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <david@redhat.com>) id 1dnPG4-0007K2-TP\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 09:11:50 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <david@redhat.com>) id 1dnPFz-0007eG-02\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 09:11:44 -0400","from mx1.redhat.com ([209.132.183.28]:58276)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <david@redhat.com>) id 1dnPFy-0007dy-Pk\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 09:11:38 -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 C86F568B5;\n\tThu, 31 Aug 2017 13:11:37 +0000 (UTC)","from [10.36.118.11] (unknown [10.36.118.11])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id B4A4E9353B;\n\tThu, 31 Aug 2017 13:11:29 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com C86F568B5","To":"Thomas Huth <thuth@redhat.com>, qemu-devel@nongnu.org","References":"<20170830170601.15855-1-david@redhat.com>\n\t<20170830170601.15855-4-david@redhat.com>\n\t<09179efb-0930-bd84-ff22-1f4b23fa6a8e@redhat.com>","From":"David Hildenbrand <david@redhat.com>","Organization":"Red Hat GmbH","Message-ID":"<aa375deb-69ba-e0a8-d617-bbb304207a12@redhat.com>","Date":"Thu, 31 Aug 2017 15:11:28 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<09179efb-0930-bd84-ff22-1f4b23fa6a8e@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","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.38]);\n\tThu, 31 Aug 2017 13:11:37 +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 v1 03/11] s390x: store cpu states inside\n\tmachine state","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"borntraeger@de.ibm.com, cohuck@redhat.com,\n\tRichard Henderson <richard.henderson@linaro.org>,\n\tAlexander Graf <agraf@suse.de>, Aurelien Jarno <aurelien@aurel32.net>","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1761055,"web_url":"http://patchwork.ozlabs.org/comment/1761055/","msgid":"<55b99307-b817-1cb2-f5c8-901b877735cc@redhat.com>","list_archive_url":null,"date":"2017-08-31T14:23:44","subject":"Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside\n\tmachine state","submitter":{"id":70402,"url":"http://patchwork.ozlabs.org/api/people/70402/","name":"David Hildenbrand","email":"david@redhat.com"},"content":">> +struct S390CPU;\n> \n> You define a \"struct S390CPU\" here ...\n> \n>>  typedef struct S390CcwMachineState {\n>>      /*< private >*/\n>>      MachineState parent_obj;\n>>  \n>>      /*< public >*/\n>> +    S390CPU **cpus;\n> \n> ... but use the typedef'ed S390CPU here ... looks somewhat suspicious, I\n> wonder whether the typedef is really in the right place?\n\nGeneral question: how much do we care about headers that are not consistent?\n\nE.g. shall I forward declare or simply ignore if compilers don't bite me?\n\n\n> \n>>      bool aes_key_wrap;\n>>      bool dea_key_wrap;\n>>      uint8_t loadparm[8];\n> \n> Anyway, that were just nits, I'm also fine with the patch as it is, so:\n> \n> Reviewed-by: Thomas Huth <thuth@redhat.com>\n>","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","ext-mx07.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx07.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=david@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 3xjl4c5HmJz9s7F\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri,  1 Sep 2017 00:24:32 +1000 (AEST)","from localhost ([::1]:56039 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dnQOU-0007Nz-St\n\tfor incoming@patchwork.ozlabs.org; Thu, 31 Aug 2017 10:24:30 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:43105)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <david@redhat.com>) id 1dnQNr-0007Ia-Sh\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:23:53 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <david@redhat.com>) id 1dnQNo-0007JR-24\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:23:51 -0400","from mx1.redhat.com ([209.132.183.28]:47078)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <david@redhat.com>) id 1dnQNn-0007JF-RR\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:23:47 -0400","from smtp.corp.redhat.com\n\t(int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11])\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 C51D6C047B86;\n\tThu, 31 Aug 2017 14:23:46 +0000 (UTC)","from [10.36.118.11] (unknown [10.36.118.11])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 36A1B92D1A;\n\tThu, 31 Aug 2017 14:23:45 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com C51D6C047B86","To":"Thomas Huth <thuth@redhat.com>, qemu-devel@nongnu.org","References":"<20170830170601.15855-1-david@redhat.com>\n\t<20170830170601.15855-4-david@redhat.com>\n\t<09179efb-0930-bd84-ff22-1f4b23fa6a8e@redhat.com>","From":"David Hildenbrand <david@redhat.com>","Organization":"Red Hat GmbH","Message-ID":"<55b99307-b817-1cb2-f5c8-901b877735cc@redhat.com>","Date":"Thu, 31 Aug 2017 16:23:44 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<09179efb-0930-bd84-ff22-1f4b23fa6a8e@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.11","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.31]);\n\tThu, 31 Aug 2017 14:23:46 +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 v1 03/11] s390x: store cpu states inside\n\tmachine state","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"borntraeger@de.ibm.com, cohuck@redhat.com,\n\tRichard Henderson <richard.henderson@linaro.org>,\n\tAlexander Graf <agraf@suse.de>, Aurelien Jarno <aurelien@aurel32.net>","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1761059,"web_url":"http://patchwork.ozlabs.org/comment/1761059/","msgid":"<20170831162930.17657819.cohuck@redhat.com>","list_archive_url":null,"date":"2017-08-31T14:29:30","subject":"Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside\n\tmachine state","submitter":{"id":71914,"url":"http://patchwork.ozlabs.org/api/people/71914/","name":"Cornelia Huck","email":"cohuck@redhat.com"},"content":"On Thu, 31 Aug 2017 15:11:28 +0200\nDavid Hildenbrand <david@redhat.com> wrote:\n\n> >> +S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)\n> >> +{\n> >> +    S390CcwMachineState *ms = S390_CCW_MACHINE(qdev_get_machine());\n> >> +\n> >> +    if (cpu_addr >= max_cpus) {\n> >> +        return NULL;\n> >> +    }\n> >> +\n> >> +    /* Fast lookup via CPU ID */\n> >> +    return ms->cpus[cpu_addr];\n> >> +}  \n> > \n> > I wonder whether that function should rather go into a file in\n> > target/s390x/ instead, since it is also used there and its prototype is\n> > in cpu.h ?  \n> \n> I thought about the same thing, but as it works directly on the machine,\n> like ri_allowed() and friends. So I decided to keep it here for now.\n> \n> I'll think about moving the definition also into\n> include/hw/s390x/s390-virtio-ccw.h\n\nIt would be a bit nicer.","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","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 3xjlDF2R0Mz9s83\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri,  1 Sep 2017 00:31:09 +1000 (AEST)","from localhost ([::1]:56072 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dnQUt-0004p4-Cg\n\tfor incoming@patchwork.ozlabs.org; Thu, 31 Aug 2017 10:31:07 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:45367)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <cohuck@redhat.com>) id 1dnQTZ-00049N-8I\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:29:51 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <cohuck@redhat.com>) id 1dnQTV-0001B0-5S\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:29:45 -0400","from mx1.redhat.com ([209.132.183.28]:36210)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <cohuck@redhat.com>) id 1dnQTU-0001Ak-Ve\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:29:41 -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 C600925C2A;\n\tThu, 31 Aug 2017 14:29:39 +0000 (UTC)","from gondolin (ovpn-117-247.ams2.redhat.com [10.36.117.247])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id CB303776FC;\n\tThu, 31 Aug 2017 14:29:34 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com C600925C2A","Date":"Thu, 31 Aug 2017 16:29:30 +0200","From":"Cornelia Huck <cohuck@redhat.com>","To":"David Hildenbrand <david@redhat.com>","Message-ID":"<20170831162930.17657819.cohuck@redhat.com>","In-Reply-To":"<aa375deb-69ba-e0a8-d617-bbb304207a12@redhat.com>","References":"<20170830170601.15855-1-david@redhat.com>\n\t<20170830170601.15855-4-david@redhat.com>\n\t<09179efb-0930-bd84-ff22-1f4b23fa6a8e@redhat.com>\n\t<aa375deb-69ba-e0a8-d617-bbb304207a12@redhat.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.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.30]);\n\tThu, 31 Aug 2017 14:29:39 +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 v1 03/11] s390x: store cpu states inside\n\tmachine state","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"Thomas Huth <thuth@redhat.com>,\n\tRichard Henderson <richard.henderson@linaro.org>,\n\tqemu-devel@nongnu.org, Alexander Graf <agraf@suse.de>,\n\tborntraeger@de.ibm.com, Aurelien Jarno <aurelien@aurel32.net>","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1761060,"web_url":"http://patchwork.ozlabs.org/comment/1761060/","msgid":"<4694a0bd-6c4e-b762-158f-1f860990448f@redhat.com>","list_archive_url":null,"date":"2017-08-31T14:30:59","subject":"Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside\n\tmachine state","submitter":{"id":70402,"url":"http://patchwork.ozlabs.org/api/people/70402/","name":"David Hildenbrand","email":"david@redhat.com"},"content":"On 31.08.2017 16:29, Cornelia Huck wrote:\n> On Thu, 31 Aug 2017 15:11:28 +0200\n> David Hildenbrand <david@redhat.com> wrote:\n> \n>>>> +S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)\n>>>> +{\n>>>> +    S390CcwMachineState *ms = S390_CCW_MACHINE(qdev_get_machine());\n>>>> +\n>>>> +    if (cpu_addr >= max_cpus) {\n>>>> +        return NULL;\n>>>> +    }\n>>>> +\n>>>> +    /* Fast lookup via CPU ID */\n>>>> +    return ms->cpus[cpu_addr];\n>>>> +}  \n>>>\n>>> I wonder whether that function should rather go into a file in\n>>> target/s390x/ instead, since it is also used there and its prototype is\n>>> in cpu.h ?  \n>>\n>> I thought about the same thing, but as it works directly on the machine,\n>> like ri_allowed() and friends. So I decided to keep it here for now.\n>>\n>> I'll think about moving the definition also into\n>> include/hw/s390x/s390-virtio-ccw.h\n> \n> It would be a bit nicer.\n> \n\nAdding patches right now to move everything out of cpu.h that lies under\nthe \"/* outside of target/s390x/ */\" section. :)","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=david@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 3xjlFD1trpz9s7c\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri,  1 Sep 2017 00:32:00 +1000 (AEST)","from localhost ([::1]:56076 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dnQVi-0005eS-8N\n\tfor incoming@patchwork.ozlabs.org; Thu, 31 Aug 2017 10:31:58 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:45867)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <david@redhat.com>) id 1dnQV0-0005cj-Ci\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:31:15 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <david@redhat.com>) id 1dnQUv-0002IM-C9\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:31:14 -0400","from mx1.redhat.com ([209.132.183.28]:48834)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <david@redhat.com>) id 1dnQUv-0002Hq-6R\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:31:09 -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 2E89980C05;\n\tThu, 31 Aug 2017 14:31:08 +0000 (UTC)","from [10.36.118.11] (unknown [10.36.118.11])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 46ED7A3967;\n\tThu, 31 Aug 2017 14:31:00 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 2E89980C05","To":"Cornelia Huck <cohuck@redhat.com>","References":"<20170830170601.15855-1-david@redhat.com>\n\t<20170830170601.15855-4-david@redhat.com>\n\t<09179efb-0930-bd84-ff22-1f4b23fa6a8e@redhat.com>\n\t<aa375deb-69ba-e0a8-d617-bbb304207a12@redhat.com>\n\t<20170831162930.17657819.cohuck@redhat.com>","From":"David Hildenbrand <david@redhat.com>","Organization":"Red Hat GmbH","Message-ID":"<4694a0bd-6c4e-b762-158f-1f860990448f@redhat.com>","Date":"Thu, 31 Aug 2017 16:30:59 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170831162930.17657819.cohuck@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","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.26]);\n\tThu, 31 Aug 2017 14:31:08 +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 v1 03/11] s390x: store cpu states inside\n\tmachine state","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"Thomas Huth <thuth@redhat.com>,\n\tRichard Henderson <richard.henderson@linaro.org>,\n\tqemu-devel@nongnu.org, Alexander Graf <agraf@suse.de>,\n\tborntraeger@de.ibm.com, Aurelien Jarno <aurelien@aurel32.net>","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1761061,"web_url":"http://patchwork.ozlabs.org/comment/1761061/","msgid":"<63bc0433-dd9f-2b75-7885-5460949c821b@redhat.com>","list_archive_url":null,"date":"2017-08-31T14:31:51","subject":"Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside\n\tmachine state","submitter":{"id":66152,"url":"http://patchwork.ozlabs.org/api/people/66152/","name":"Thomas Huth","email":"thuth@redhat.com"},"content":"On 31.08.2017 16:23, David Hildenbrand wrote:\n> \n>>> +struct S390CPU;\n>>\n>> You define a \"struct S390CPU\" here ...\n>>\n>>>  typedef struct S390CcwMachineState {\n>>>      /*< private >*/\n>>>      MachineState parent_obj;\n>>>  \n>>>      /*< public >*/\n>>> +    S390CPU **cpus;\n>>\n>> ... but use the typedef'ed S390CPU here ... looks somewhat suspicious, I\n>> wonder whether the typedef is really in the right place?\n> \n> General question: how much do we care about headers that are not consistent?\n> \n> E.g. shall I forward declare or simply ignore if compilers don't bite me?\n\nMy remark was not so much about your patch, but about the original\ndefinition instead: \"struct S390CPU\" is declared in target/s390x/cpu.h,\nbut \"typedef struct S390CPU S390CPU\" is in target/s390x/cpu-qom.h. I\nthink they should rather be declared in the same header file instead. Or\nyour \"struct S390CPU;\" forward declaration should go into cpu-qom.h\ninstead, right in front of the typedef.\n\n Thomas","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=thuth@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 3xjlHS3H16z9s7c\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri,  1 Sep 2017 00:33:56 +1000 (AEST)","from localhost ([::1]:56087 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dnQXa-0007Py-J8\n\tfor incoming@patchwork.ozlabs.org; Thu, 31 Aug 2017 10:33:54 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:46316)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <thuth@redhat.com>) id 1dnQW1-0006gJ-VR\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:32:25 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <thuth@redhat.com>) id 1dnQVw-00030T-U9\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:32:17 -0400","from mx1.redhat.com ([209.132.183.28]:57694)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <thuth@redhat.com>) id 1dnQVw-0002zZ-ND\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:32:12 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\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 F191E883CF;\n\tThu, 31 Aug 2017 14:32:10 +0000 (UTC)","from [10.36.116.27] (ovpn-116-27.ams2.redhat.com [10.36.116.27])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id B3B7017194;\n\tThu, 31 Aug 2017 14:31:52 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com F191E883CF","To":"David Hildenbrand <david@redhat.com>, qemu-devel@nongnu.org","References":"<20170830170601.15855-1-david@redhat.com>\n\t<20170830170601.15855-4-david@redhat.com>\n\t<09179efb-0930-bd84-ff22-1f4b23fa6a8e@redhat.com>\n\t<55b99307-b817-1cb2-f5c8-901b877735cc@redhat.com>","From":"Thomas Huth <thuth@redhat.com>","Message-ID":"<63bc0433-dd9f-2b75-7885-5460949c821b@redhat.com>","Date":"Thu, 31 Aug 2017 16:31:51 +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":"<55b99307-b817-1cb2-f5c8-901b877735cc@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.26]);\n\tThu, 31 Aug 2017 14:32:11 +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 v1 03/11] s390x: store cpu states inside\n\tmachine state","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"borntraeger@de.ibm.com, cohuck@redhat.com,\n\tRichard Henderson <richard.henderson@linaro.org>,\n\tAlexander Graf <agraf@suse.de>, Aurelien Jarno <aurelien@aurel32.net>","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1761063,"web_url":"http://patchwork.ozlabs.org/comment/1761063/","msgid":"<8a4d349c-33c5-0e63-d11b-274d64e854fa@redhat.com>","list_archive_url":null,"date":"2017-08-31T14:36:09","subject":"Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside\n\tmachine state","submitter":{"id":70402,"url":"http://patchwork.ozlabs.org/api/people/70402/","name":"David Hildenbrand","email":"david@redhat.com"},"content":"On 31.08.2017 16:31, Thomas Huth wrote:\n> On 31.08.2017 16:23, David Hildenbrand wrote:\n>>\n>>>> +struct S390CPU;\n>>>\n>>> You define a \"struct S390CPU\" here ...\n>>>\n>>>>  typedef struct S390CcwMachineState {\n>>>>      /*< private >*/\n>>>>      MachineState parent_obj;\n>>>>  \n>>>>      /*< public >*/\n>>>> +    S390CPU **cpus;\n>>>\n>>> ... but use the typedef'ed S390CPU here ... looks somewhat suspicious, I\n>>> wonder whether the typedef is really in the right place?\n>>\n>> General question: how much do we care about headers that are not consistent?\n>>\n>> E.g. shall I forward declare or simply ignore if compilers don't bite me?\n> \n> My remark was not so much about your patch, but about the original\n> definition instead: \"struct S390CPU\" is declared in target/s390x/cpu.h,\n> but \"typedef struct S390CPU S390CPU\" is in target/s390x/cpu-qom.h. I\n> think they should rather be declared in the same header file instead. Or\n\nI agree, will have a look.\n\n> your \"struct S390CPU;\" forward declaration should go into cpu-qom.h\n> instead, right in front of the typedef.\n> \n\nLet me rephrase my question:\n\ninclude/hw/s390x/s390-virtio-ccw.h does not include cpu.h/cpu-qom.h\n\nIf compilers don't complain, do we have to forward declare at all? (I\nthink it is cleaner, but I would like to know what is suggested)\n\n>  Thomas\n>","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","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=david@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 3xjlM31z0Jz9s7G\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri,  1 Sep 2017 00:37:02 +1000 (AEST)","from localhost ([::1]:56107 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dnQaa-0000vX-KZ\n\tfor incoming@patchwork.ozlabs.org; Thu, 31 Aug 2017 10:37:00 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:47258)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <david@redhat.com>) id 1dnQZv-0000sS-Oy\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:36:24 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <david@redhat.com>) id 1dnQZq-0003v7-Pe\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:36:19 -0400","from mx1.redhat.com ([209.132.183.28]:19459)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <david@redhat.com>) id 1dnQZq-0003uw-Ji\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:36:14 -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 5CF71804E3;\n\tThu, 31 Aug 2017 14:36:13 +0000 (UTC)","from [10.36.118.11] (unknown [10.36.118.11])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id C1D41A2492;\n\tThu, 31 Aug 2017 14:36:10 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 5CF71804E3","To":"Thomas Huth <thuth@redhat.com>, qemu-devel@nongnu.org","References":"<20170830170601.15855-1-david@redhat.com>\n\t<20170830170601.15855-4-david@redhat.com>\n\t<09179efb-0930-bd84-ff22-1f4b23fa6a8e@redhat.com>\n\t<55b99307-b817-1cb2-f5c8-901b877735cc@redhat.com>\n\t<63bc0433-dd9f-2b75-7885-5460949c821b@redhat.com>","From":"David Hildenbrand <david@redhat.com>","Organization":"Red Hat GmbH","Message-ID":"<8a4d349c-33c5-0e63-d11b-274d64e854fa@redhat.com>","Date":"Thu, 31 Aug 2017 16:36:09 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<63bc0433-dd9f-2b75-7885-5460949c821b@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","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.27]);\n\tThu, 31 Aug 2017 14:36:13 +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 v1 03/11] s390x: store cpu states inside\n\tmachine state","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"borntraeger@de.ibm.com, cohuck@redhat.com,\n\tRichard Henderson <richard.henderson@linaro.org>,\n\tAlexander Graf <agraf@suse.de>, Aurelien Jarno <aurelien@aurel32.net>","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1761067,"web_url":"http://patchwork.ozlabs.org/comment/1761067/","msgid":"<20170831163836.58a92296.cohuck@redhat.com>","list_archive_url":null,"date":"2017-08-31T14:38:36","subject":"Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside\n\tmachine state","submitter":{"id":71914,"url":"http://patchwork.ozlabs.org/api/people/71914/","name":"Cornelia Huck","email":"cohuck@redhat.com"},"content":"On Thu, 31 Aug 2017 16:30:59 +0200\nDavid Hildenbrand <david@redhat.com> wrote:\n\n> On 31.08.2017 16:29, Cornelia Huck wrote:\n> > On Thu, 31 Aug 2017 15:11:28 +0200\n> > David Hildenbrand <david@redhat.com> wrote:\n> >   \n> >>>> +S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)\n> >>>> +{\n> >>>> +    S390CcwMachineState *ms = S390_CCW_MACHINE(qdev_get_machine());\n> >>>> +\n> >>>> +    if (cpu_addr >= max_cpus) {\n> >>>> +        return NULL;\n> >>>> +    }\n> >>>> +\n> >>>> +    /* Fast lookup via CPU ID */\n> >>>> +    return ms->cpus[cpu_addr];\n> >>>> +}    \n> >>>\n> >>> I wonder whether that function should rather go into a file in\n> >>> target/s390x/ instead, since it is also used there and its prototype is\n> >>> in cpu.h ?    \n> >>\n> >> I thought about the same thing, but as it works directly on the machine,\n> >> like ri_allowed() and friends. So I decided to keep it here for now.\n> >>\n> >> I'll think about moving the definition also into\n> >> include/hw/s390x/s390-virtio-ccw.h  \n> > \n> > It would be a bit nicer.\n> >   \n> \n> Adding patches right now to move everything out of cpu.h that lies under\n> the \"/* outside of target/s390x/ */\" section. :)\n> \n\nAh, you really care about your patch count, don't you? :)\n\n(I think it's a good idea.)","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","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 3xjlPm254Wz9s7G\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri,  1 Sep 2017 00:39:24 +1000 (AEST)","from localhost ([::1]:56119 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dnQcs-0002en-ED\n\tfor incoming@patchwork.ozlabs.org; Thu, 31 Aug 2017 10:39:22 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:48096)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <cohuck@redhat.com>) id 1dnQcP-0002dB-SZ\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:38:54 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <cohuck@redhat.com>) id 1dnQcL-0004ul-QH\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:38:53 -0400","from mx1.redhat.com ([209.132.183.28]:57586)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <cohuck@redhat.com>) id 1dnQcL-0004uX-Jv\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:38:49 -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 941E23D952;\n\tThu, 31 Aug 2017 14:38:48 +0000 (UTC)","from gondolin (ovpn-117-247.ams2.redhat.com [10.36.117.247])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 840CFA2928;\n\tThu, 31 Aug 2017 14:38:39 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 941E23D952","Date":"Thu, 31 Aug 2017 16:38:36 +0200","From":"Cornelia Huck <cohuck@redhat.com>","To":"David Hildenbrand <david@redhat.com>","Message-ID":"<20170831163836.58a92296.cohuck@redhat.com>","In-Reply-To":"<4694a0bd-6c4e-b762-158f-1f860990448f@redhat.com>","References":"<20170830170601.15855-1-david@redhat.com>\n\t<20170830170601.15855-4-david@redhat.com>\n\t<09179efb-0930-bd84-ff22-1f4b23fa6a8e@redhat.com>\n\t<aa375deb-69ba-e0a8-d617-bbb304207a12@redhat.com>\n\t<20170831162930.17657819.cohuck@redhat.com>\n\t<4694a0bd-6c4e-b762-158f-1f860990448f@redhat.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.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.30]);\n\tThu, 31 Aug 2017 14:38:48 +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 v1 03/11] s390x: store cpu states inside\n\tmachine state","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"Thomas Huth <thuth@redhat.com>,\n\tRichard Henderson <richard.henderson@linaro.org>,\n\tqemu-devel@nongnu.org, Alexander Graf <agraf@suse.de>,\n\tborntraeger@de.ibm.com, Aurelien Jarno <aurelien@aurel32.net>","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1761069,"web_url":"http://patchwork.ozlabs.org/comment/1761069/","msgid":"<06ff4883-d36d-8748-ee58-68f3e73cd192@redhat.com>","list_archive_url":null,"date":"2017-08-31T14:39:44","subject":"Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside\n\tmachine state","submitter":{"id":70402,"url":"http://patchwork.ozlabs.org/api/people/70402/","name":"David Hildenbrand","email":"david@redhat.com"},"content":"On 31.08.2017 16:38, Cornelia Huck wrote:\n> On Thu, 31 Aug 2017 16:30:59 +0200\n> David Hildenbrand <david@redhat.com> wrote:\n> \n>> On 31.08.2017 16:29, Cornelia Huck wrote:\n>>> On Thu, 31 Aug 2017 15:11:28 +0200\n>>> David Hildenbrand <david@redhat.com> wrote:\n>>>   \n>>>>>> +S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)\n>>>>>> +{\n>>>>>> +    S390CcwMachineState *ms = S390_CCW_MACHINE(qdev_get_machine());\n>>>>>> +\n>>>>>> +    if (cpu_addr >= max_cpus) {\n>>>>>> +        return NULL;\n>>>>>> +    }\n>>>>>> +\n>>>>>> +    /* Fast lookup via CPU ID */\n>>>>>> +    return ms->cpus[cpu_addr];\n>>>>>> +}    \n>>>>>\n>>>>> I wonder whether that function should rather go into a file in\n>>>>> target/s390x/ instead, since it is also used there and its prototype is\n>>>>> in cpu.h ?    \n>>>>\n>>>> I thought about the same thing, but as it works directly on the machine,\n>>>> like ri_allowed() and friends. So I decided to keep it here for now.\n>>>>\n>>>> I'll think about moving the definition also into\n>>>> include/hw/s390x/s390-virtio-ccw.h  \n>>>\n>>> It would be a bit nicer.\n>>>   \n>>\n>> Adding patches right now to move everything out of cpu.h that lies under\n>> the \"/* outside of target/s390x/ */\" section. :)\n>>\n> \n> Ah, you really care about your patch count, don't you? :)\n> \n> (I think it's a good idea.)\n> \n\n.... so you want me to squash everything into a single patch then?! ;)","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","ext-mx05.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx05.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=david@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 3xjlQz0nxFz9s7G\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri,  1 Sep 2017 00:40:27 +1000 (AEST)","from localhost ([::1]:56122 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dnQdt-0003H7-5S\n\tfor incoming@patchwork.ozlabs.org; Thu, 31 Aug 2017 10:40:25 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:48372)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <david@redhat.com>) id 1dnQdJ-0003FL-Fy\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:39:50 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <david@redhat.com>) id 1dnQdI-0005KU-JO\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:39:49 -0400","from mx1.redhat.com ([209.132.183.28]:59796)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <david@redhat.com>) id 1dnQdI-0005K9-Dp\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:39:48 -0400","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\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 363B813A60;\n\tThu, 31 Aug 2017 14:39:47 +0000 (UTC)","from [10.36.118.11] (unknown [10.36.118.11])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id B9458A2A6F;\n\tThu, 31 Aug 2017 14:39:45 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 363B813A60","To":"Cornelia Huck <cohuck@redhat.com>","References":"<20170830170601.15855-1-david@redhat.com>\n\t<20170830170601.15855-4-david@redhat.com>\n\t<09179efb-0930-bd84-ff22-1f4b23fa6a8e@redhat.com>\n\t<aa375deb-69ba-e0a8-d617-bbb304207a12@redhat.com>\n\t<20170831162930.17657819.cohuck@redhat.com>\n\t<4694a0bd-6c4e-b762-158f-1f860990448f@redhat.com>\n\t<20170831163836.58a92296.cohuck@redhat.com>","From":"David Hildenbrand <david@redhat.com>","Organization":"Red Hat GmbH","Message-ID":"<06ff4883-d36d-8748-ee58-68f3e73cd192@redhat.com>","Date":"Thu, 31 Aug 2017 16:39:44 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170831163836.58a92296.cohuck@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.29]);\n\tThu, 31 Aug 2017 14:39:47 +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 v1 03/11] s390x: store cpu states inside\n\tmachine state","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"Thomas Huth <thuth@redhat.com>,\n\tRichard Henderson <richard.henderson@linaro.org>,\n\tqemu-devel@nongnu.org, Alexander Graf <agraf@suse.de>,\n\tborntraeger@de.ibm.com, Aurelien Jarno <aurelien@aurel32.net>","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1761075,"web_url":"http://patchwork.ozlabs.org/comment/1761075/","msgid":"<5d498419-06b7-8de2-0b3e-014de0f3be71@redhat.com>","list_archive_url":null,"date":"2017-08-31T14:45:44","subject":"Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside\n\tmachine state","submitter":{"id":66152,"url":"http://patchwork.ozlabs.org/api/people/66152/","name":"Thomas Huth","email":"thuth@redhat.com"},"content":"On 31.08.2017 16:36, David Hildenbrand wrote:\n> On 31.08.2017 16:31, Thomas Huth wrote:\n>> On 31.08.2017 16:23, David Hildenbrand wrote:\n>>>\n>>>>> +struct S390CPU;\n>>>>\n>>>> You define a \"struct S390CPU\" here ...\n>>>>\n>>>>>  typedef struct S390CcwMachineState {\n>>>>>      /*< private >*/\n>>>>>      MachineState parent_obj;\n>>>>>  \n>>>>>      /*< public >*/\n>>>>> +    S390CPU **cpus;\n>>>>\n>>>> ... but use the typedef'ed S390CPU here ... looks somewhat suspicious, I\n>>>> wonder whether the typedef is really in the right place?\n>>>\n>>> General question: how much do we care about headers that are not consistent?\n>>>\n>>> E.g. shall I forward declare or simply ignore if compilers don't bite me?\n>>\n>> My remark was not so much about your patch, but about the original\n>> definition instead: \"struct S390CPU\" is declared in target/s390x/cpu.h,\n>> but \"typedef struct S390CPU S390CPU\" is in target/s390x/cpu-qom.h. I\n>> think they should rather be declared in the same header file instead. Or\n> \n> I agree, will have a look.\n> \n>> your \"struct S390CPU;\" forward declaration should go into cpu-qom.h\n>> instead, right in front of the typedef.\n>>\n> \n> Let me rephrase my question:\n> \n> include/hw/s390x/s390-virtio-ccw.h does not include cpu.h/cpu-qom.h\n> \n> If compilers don't complain, do we have to forward declare at all? (I\n> think it is cleaner, but I would like to know what is suggested)\n\nWell, doing a forward declaration with \"struct S390CPU;\" and then using\nthe typedef'd \"S390CPU\" without \"struct\" does not make much sense -\nthese are two \"different\" types. If you can use \"S390CPU\" here without\nthe \"struct S390CPU;\" declaration, that means the cpu-qom.h header got\nincluded indirectly already - so I'd suggest to simply remove the\n\"struct S390CPU;\" forward declaration from your patch.\n\n Thomas","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=thuth@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 3xjlZ16pjYz9s75\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri,  1 Sep 2017 00:46:33 +1000 (AEST)","from localhost ([::1]:56162 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dnQjo-0007m2-3o\n\tfor incoming@patchwork.ozlabs.org; Thu, 31 Aug 2017 10:46:32 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:50111)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <thuth@redhat.com>) id 1dnQjI-0007jW-Iz\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:46:03 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <thuth@redhat.com>) id 1dnQj9-0007dX-0O\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:46:00 -0400","from mx1.redhat.com ([209.132.183.28]:59534)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <thuth@redhat.com>) id 1dnQj8-0007dL-MR\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 10:45:50 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\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 99A7881DFF;\n\tThu, 31 Aug 2017 14:45:49 +0000 (UTC)","from [10.36.116.27] (ovpn-116-27.ams2.redhat.com [10.36.116.27])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id A72859ECCD;\n\tThu, 31 Aug 2017 14:45:45 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 99A7881DFF","To":"David Hildenbrand <david@redhat.com>, qemu-devel@nongnu.org","References":"<20170830170601.15855-1-david@redhat.com>\n\t<20170830170601.15855-4-david@redhat.com>\n\t<09179efb-0930-bd84-ff22-1f4b23fa6a8e@redhat.com>\n\t<55b99307-b817-1cb2-f5c8-901b877735cc@redhat.com>\n\t<63bc0433-dd9f-2b75-7885-5460949c821b@redhat.com>\n\t<8a4d349c-33c5-0e63-d11b-274d64e854fa@redhat.com>","From":"Thomas Huth <thuth@redhat.com>","Message-ID":"<5d498419-06b7-8de2-0b3e-014de0f3be71@redhat.com>","Date":"Thu, 31 Aug 2017 16:45: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":"<8a4d349c-33c5-0e63-d11b-274d64e854fa@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.25]);\n\tThu, 31 Aug 2017 14:45:49 +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 v1 03/11] s390x: store cpu states inside\n\tmachine state","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"borntraeger@de.ibm.com, cohuck@redhat.com,\n\tRichard Henderson <richard.henderson@linaro.org>,\n\tAlexander Graf <agraf@suse.de>, Aurelien Jarno <aurelien@aurel32.net>","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}}]