[{"id":1765072,"web_url":"http://patchwork.ozlabs.org/comment/1765072/","msgid":"<1603f6bf-f481-13c2-c7eb-ac155c5a14cc@redhat.com>","list_archive_url":null,"date":"2017-09-08T04:21:31","subject":"Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","submitter":{"id":66152,"url":"http://patchwork.ozlabs.org/api/people/66152/","name":"Thomas Huth","email":"thuth@redhat.com"},"content":"On 07.09.2017 22:13, David Hildenbrand wrote:\n> Implemented in sclp.c, so let's move it to the right include file.\n> Fix up one include. Do a forward declaration of CPUS390XState to fix the\n> two sclp consoles complaining.\n> \n> Signed-off-by: David Hildenbrand <david@redhat.com>\n> ---\n>  include/hw/s390x/sclp.h    | 2 ++\n>  target/s390x/cpu.h         | 1 -\n>  target/s390x/misc_helper.c | 1 +\n>  3 files changed, 3 insertions(+), 1 deletion(-)\n> \n> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h\n> index a72d096081..4b86a8a293 100644\n> --- a/include/hw/s390x/sclp.h\n> +++ b/include/hw/s390x/sclp.h\n> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev *init_sclp_memory_hotplug_dev(void);\n>  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);\n>  void sclp_service_interrupt(uint32_t sccb);\n>  void raise_irq_cpu_hotplug(void);\n> +typedef struct CPUS390XState CPUS390XState;\n> +int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);\n\nThat's dangerous and likely does not work with certain versions of GCC.\nYou can't do a \"forward declaration\" with typedef in C, I'm afraid. See\nfor example:\n\n https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg01454.html\n https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg03337.html\n https://stackoverflow.com/questions/8367646/redefinition-of-typedef\n\nAll this typedef'ing in QEMU is pretty bad ... we run into this problem\nagain and again. include/qemu/typedefs.h is just a work-around for this.\nI know people like typedefs for some reasons (I used to do that, too,\nbefore I realized the trouble with them), but IMHO we should rather\nadopt the typedef-related rules from the kernel coding conventions instead:\n\n https://www.kernel.org/doc/html/v4.13/process/coding-style.html#typedefs\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-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=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 3xpPL55LH0z9s75\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri,  8 Sep 2017 14:22:21 +1000 (AEST)","from localhost ([::1]:43236 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 1dqAo7-0006ur-CP\n\tfor incoming@patchwork.ozlabs.org; Fri, 08 Sep 2017 00:22:19 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:53595)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <thuth@redhat.com>) id 1dqAnX-0006rH-Ha\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 00:21:48 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <thuth@redhat.com>) id 1dqAnS-0005cy-LM\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 00:21:43 -0400","from mx1.redhat.com ([209.132.183.28]:56358)\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 1dqAnS-0005cQ-Et\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 00:21:38 -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 84E7C4E34A;\n\tFri,  8 Sep 2017 04:21:37 +0000 (UTC)","from [10.36.116.21] (ovpn-116-21.ams2.redhat.com [10.36.116.21])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 3555B71D7C;\n\tFri,  8 Sep 2017 04:21:33 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 84E7C4E34A","To":"David Hildenbrand <david@redhat.com>, qemu-devel@nongnu.org","References":"<20170907201335.13956-1-david@redhat.com>\n\t<20170907201335.13956-9-david@redhat.com>","From":"Thomas Huth <thuth@redhat.com>","Message-ID":"<1603f6bf-f481-13c2-c7eb-ac155c5a14cc@redhat.com>","Date":"Fri, 8 Sep 2017 06:21:31 +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":"<20170907201335.13956-9-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.38]);\n\tFri, 08 Sep 2017 04:21:37 +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 v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","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":"Matthew Rosato <mjrosato@linux.vnet.ibm.com>,\n\tEduardo Habkost <ehabkost@redhat.com>, cohuck@redhat.com,\n\tRichard Henderson <richard.henderson@linaro.org>,\n\tAlexander Graf <agraf@suse.de>,\n\tMarkus Armbruster <armbru@redhat.com>, borntraeger@de.ibm.com,\n\tPaolo Bonzini <pbonzini@redhat.com>","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":1765287,"web_url":"http://patchwork.ozlabs.org/comment/1765287/","msgid":"<87lglp9xwf.fsf@dusky.pond.sub.org>","list_archive_url":null,"date":"2017-09-08T12:29:20","subject":"Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","submitter":{"id":2645,"url":"http://patchwork.ozlabs.org/api/people/2645/","name":"Markus Armbruster","email":"armbru@redhat.com"},"content":"Thomas Huth <thuth@redhat.com> writes:\n\n> On 07.09.2017 22:13, David Hildenbrand wrote:\n>> Implemented in sclp.c, so let's move it to the right include file.\n>> Fix up one include. Do a forward declaration of CPUS390XState to fix the\n>> two sclp consoles complaining.\n>> \n>> Signed-off-by: David Hildenbrand <david@redhat.com>\n>> ---\n>>  include/hw/s390x/sclp.h    | 2 ++\n>>  target/s390x/cpu.h         | 1 -\n>>  target/s390x/misc_helper.c | 1 +\n>>  3 files changed, 3 insertions(+), 1 deletion(-)\n>> \n>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h\n>> index a72d096081..4b86a8a293 100644\n>> --- a/include/hw/s390x/sclp.h\n>> +++ b/include/hw/s390x/sclp.h\n>> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev *init_sclp_memory_hotplug_dev(void);\n>>  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);\n>>  void sclp_service_interrupt(uint32_t sccb);\n>>  void raise_irq_cpu_hotplug(void);\n>> +typedef struct CPUS390XState CPUS390XState;\n>> +int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);\n>\n> That's dangerous and likely does not work with certain versions of GCC.\n> You can't do a \"forward declaration\" with typedef in C, I'm afraid. See\n> for example:\n>\n>  https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg01454.html\n>  https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg03337.html\n>  https://stackoverflow.com/questions/8367646/redefinition-of-typedef\n>\n> All this typedef'ing in QEMU is pretty bad ... we run into this problem\n> again and again. include/qemu/typedefs.h is just a work-around for this.\n> I know people like typedefs for some reasons (I used to do that, too,\n> before I realized the trouble with them), but IMHO we should rather\n> adopt the typedef-related rules from the kernel coding conventions instead:\n>\n>  https://www.kernel.org/doc/html/v4.13/process/coding-style.html#typedefs\n\nI prefer the kernel style for typedefs myself.  But it's strictly a\nmatter of style.  Excessive typedeffing makes code harder to understand,\nit isn't wrong.  The part that's wrong is defining things more than\nonce, and that applies to everything, not just typedefs.\n\nSometimes you get away with defining something more than once.  It's\nstill wrong.\n\nYou're not supposed to define the same variable more than once, either\n(although many C compilers let you get away with it, see -fno-common).\nYou define it in *one* place.  If you need to declare it, declare it in\n*one* place, and make sure that place is in scope at the definition, so\nthe compiler can check the two match.\n\nLikewise, you're not supposed to define the same struct tag more than\nonce, and you should declare it in just one place.\n\nLikewise, you're not supposed to define (with typedef) the same type\nmore than once.  There is no such thing as a typedef declaration.\n\nqemu/typedefs.h is not a work-around for a typedef-happy style.  Its\npurpose is breaking inclusion cycles.  Secondary purpose is reducing the\nneed for non-cyclic includes.  If we didn't typedef, we'd still put our\nstruct declarations there.","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=208.118.235.17; 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=armbru@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [208.118.235.17])\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 3xpc8b6lfwz9s81\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri,  8 Sep 2017 22:29:51 +1000 (AEST)","from localhost ([::1]:45226 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 1dqIPt-0002j1-Ma\n\tfor incoming@patchwork.ozlabs.org; Fri, 08 Sep 2017 08:29:49 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:44654)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <armbru@redhat.com>) id 1dqIPZ-0002ig-1A\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 08:29:30 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <armbru@redhat.com>) id 1dqIPV-0001Vd-Tg\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 08:29:29 -0400","from mx1.redhat.com ([209.132.183.28]:39782)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <armbru@redhat.com>) id 1dqIPV-0001Uu-Kb\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 08:29:25 -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 996394A6EA;\n\tFri,  8 Sep 2017 12:29:24 +0000 (UTC)","from blackfin.pond.sub.org (ovpn-116-75.ams2.redhat.com\n\t[10.36.116.75])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 563BB6AD28;\n\tFri,  8 Sep 2017 12:29:22 +0000 (UTC)","by blackfin.pond.sub.org (Postfix, from userid 1000)\n\tid CD2101138645; Fri,  8 Sep 2017 14:29:20 +0200 (CEST)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 996394A6EA","From":"Markus Armbruster <armbru@redhat.com>","To":"Thomas Huth <thuth@redhat.com>","References":"<20170907201335.13956-1-david@redhat.com>\n\t<20170907201335.13956-9-david@redhat.com>\n\t<1603f6bf-f481-13c2-c7eb-ac155c5a14cc@redhat.com>","Date":"Fri, 08 Sep 2017 14:29:20 +0200","In-Reply-To":"<1603f6bf-f481-13c2-c7eb-ac155c5a14cc@redhat.com> (Thomas Huth's\n\tmessage of \"Fri, 8 Sep 2017 06:21:31 +0200\")","Message-ID":"<87lglp9xwf.fsf@dusky.pond.sub.org>","User-Agent":"Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux)","MIME-Version":"1.0","Content-Type":"text/plain","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\tFri, 08 Sep 2017 12:29:24 +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 v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","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":"Matthew Rosato <mjrosato@linux.vnet.ibm.com>,\n\tEduardo Habkost <ehabkost@redhat.com>,\n\tDavid Hildenbrand <david@redhat.com>, cohuck@redhat.com,\n\tRichard Henderson <richard.henderson@linaro.org>,\n\tqemu-devel@nongnu.org, Alexander Graf <agraf@suse.de>,\n\tborntraeger@de.ibm.com, Paolo Bonzini <pbonzini@redhat.com>","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":1765304,"web_url":"http://patchwork.ozlabs.org/comment/1765304/","msgid":"<b8f78724-b6be-3e95-60fa-8c7dcafe2eec@redhat.com>","list_archive_url":null,"date":"2017-09-08T12:46:36","subject":"Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","submitter":{"id":70402,"url":"http://patchwork.ozlabs.org/api/people/70402/","name":"David Hildenbrand","email":"david@redhat.com"},"content":"On 08.09.2017 06:21, Thomas Huth wrote:\n> On 07.09.2017 22:13, David Hildenbrand wrote:\n>> Implemented in sclp.c, so let's move it to the right include file.\n>> Fix up one include. Do a forward declaration of CPUS390XState to fix the\n>> two sclp consoles complaining.\n>>\n>> Signed-off-by: David Hildenbrand <david@redhat.com>\n>> ---\n>>  include/hw/s390x/sclp.h    | 2 ++\n>>  target/s390x/cpu.h         | 1 -\n>>  target/s390x/misc_helper.c | 1 +\n>>  3 files changed, 3 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h\n>> index a72d096081..4b86a8a293 100644\n>> --- a/include/hw/s390x/sclp.h\n>> +++ b/include/hw/s390x/sclp.h\n>> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev *init_sclp_memory_hotplug_dev(void);\n>>  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);\n>>  void sclp_service_interrupt(uint32_t sccb);\n>>  void raise_irq_cpu_hotplug(void);\n>> +typedef struct CPUS390XState CPUS390XState;\n>> +int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);\n> \n> That's dangerous and likely does not work with certain versions of GCC.\n> You can't do a \"forward declaration\" with typedef in C, I'm afraid. See\n> for example:\n> \n>  https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg01454.html\n>  https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg03337.html\n>  https://stackoverflow.com/questions/8367646/redefinition-of-typedef\n> \n> All this typedef'ing in QEMU is pretty bad ... we run into this problem\n> again and again. include/qemu/typedefs.h is just a work-around for this.\n> I know people like typedefs for some reasons (I used to do that, too,\n> before I realized the trouble with them), but IMHO we should rather\n> adopt the typedef-related rules from the kernel coding conventions instead:\n> \n>  https://www.kernel.org/doc/html/v4.13/process/coding-style.html#typedefs\n> \n>   Thomas\n> \n\nYes, this is really nasty. And I wasn't aware of the involved issues.\n\nThis seems to be the only feasible solution (including cpu.h sounds\nwrong and will require a bunch of other includes):\n\n\ndiff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h\nindex a72d096081..ce80915a02 100644\n--- a/include/hw/s390x/sclp.h\n+++ b/include/hw/s390x/sclp.h\n@@ -242,5 +242,7 @@ sclpMemoryHotplugDev\n*init_sclp_memory_hotplug_dev(void);\n sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);\n void sclp_service_interrupt(uint32_t sccb);\n void raise_irq_cpu_hotplug(void);\n+struct CPUS390XState;\n+int sclp_service_call(struct CPUS390XState *env, uint64_t sccb,\nuint32_t code);\n\n #endif\ndiff --git a/target/s390x/cpu.h b/target/s390x/cpu.h\nindex 3aa2e46aac..032d1de2e8 100644\n--- a/target/s390x/cpu.h\n+++ b/target/s390x/cpu.h\n@@ -721,6 +721,5 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr,\nuint8_t ar, void *hostbuf,\n\n /* outside of target/s390x/ */\n S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);\n-int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);\n\n #endif\ndiff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c\nindex b142db71c6..8b07535b02 100644\n--- a/target/s390x/misc_helper.c\n+++ b/target/s390x/misc_helper.c\n@@ -35,6 +35,7 @@\n #include \"sysemu/sysemu.h\"\n #include \"hw/s390x/ebcdic.h\"\n #include \"hw/s390x/s390-virtio-hcall.h\"\n+#include \"hw/s390x/sclp.h\"\n #endif\n\n /* #define DEBUG_HELPER */\n\n\nOpinions?","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-mx04.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx04.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 3xpcXj5hzTz9s2G\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri,  8 Sep 2017 22:47:16 +1000 (AEST)","from localhost ([::1]:45293 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 1dqIgg-0001mK-5I\n\tfor incoming@patchwork.ozlabs.org; Fri, 08 Sep 2017 08:47:10 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:49868)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <david@redhat.com>) id 1dqIgH-0001hG-4M\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 08:46:46 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <david@redhat.com>) id 1dqIgC-0007bP-VQ\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 08:46:45 -0400","from mx1.redhat.com ([209.132.183.28]:58666)\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 1dqIgC-0007a2-Mn\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 08:46:40 -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 92F8280460;\n\tFri,  8 Sep 2017 12:46:39 +0000 (UTC)","from [10.36.117.121] (ovpn-117-121.ams2.redhat.com [10.36.117.121])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 199A7610B0;\n\tFri,  8 Sep 2017 12:46:36 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 92F8280460","To":"Thomas Huth <thuth@redhat.com>, qemu-devel@nongnu.org","References":"<20170907201335.13956-1-david@redhat.com>\n\t<20170907201335.13956-9-david@redhat.com>\n\t<1603f6bf-f481-13c2-c7eb-ac155c5a14cc@redhat.com>","From":"David Hildenbrand <david@redhat.com>","Organization":"Red Hat GmbH","Message-ID":"<b8f78724-b6be-3e95-60fa-8c7dcafe2eec@redhat.com>","Date":"Fri, 8 Sep 2017 14:46:36 +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":"<1603f6bf-f481-13c2-c7eb-ac155c5a14cc@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.28]);\n\tFri, 08 Sep 2017 12:46: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 v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","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":"Matthew Rosato <mjrosato@linux.vnet.ibm.com>,\n\tEduardo Habkost <ehabkost@redhat.com>, cohuck@redhat.com,\n\tRichard Henderson <richard.henderson@linaro.org>,\n\tAlexander Graf <agraf@suse.de>,\n\tMarkus Armbruster <armbru@redhat.com>, borntraeger@de.ibm.com,\n\tPaolo Bonzini <pbonzini@redhat.com>","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":1765877,"web_url":"http://patchwork.ozlabs.org/comment/1765877/","msgid":"<20170909220737.GL7570@localhost.localdomain>","list_archive_url":null,"date":"2017-09-09T22:07:37","subject":"Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","submitter":{"id":195,"url":"http://patchwork.ozlabs.org/api/people/195/","name":"Eduardo Habkost","email":"ehabkost@redhat.com"},"content":"On Fri, Sep 08, 2017 at 02:46:36PM +0200, David Hildenbrand wrote:\n> On 08.09.2017 06:21, Thomas Huth wrote:\n> > On 07.09.2017 22:13, David Hildenbrand wrote:\n> >> Implemented in sclp.c, so let's move it to the right include file.\n> >> Fix up one include. Do a forward declaration of CPUS390XState to fix the\n> >> two sclp consoles complaining.\n> >>\n> >> Signed-off-by: David Hildenbrand <david@redhat.com>\n> >> ---\n> >>  include/hw/s390x/sclp.h    | 2 ++\n> >>  target/s390x/cpu.h         | 1 -\n> >>  target/s390x/misc_helper.c | 1 +\n> >>  3 files changed, 3 insertions(+), 1 deletion(-)\n> >>\n> >> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h\n> >> index a72d096081..4b86a8a293 100644\n> >> --- a/include/hw/s390x/sclp.h\n> >> +++ b/include/hw/s390x/sclp.h\n> >> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev *init_sclp_memory_hotplug_dev(void);\n> >>  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);\n> >>  void sclp_service_interrupt(uint32_t sccb);\n> >>  void raise_irq_cpu_hotplug(void);\n> >> +typedef struct CPUS390XState CPUS390XState;\n> >> +int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);\n> > \n> > That's dangerous and likely does not work with certain versions of GCC.\n> > You can't do a \"forward declaration\" with typedef in C, I'm afraid. See\n> > for example:\n> > \n> >  https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg01454.html\n> >  https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg03337.html\n> >  https://stackoverflow.com/questions/8367646/redefinition-of-typedef\n> > \n> > All this typedef'ing in QEMU is pretty bad ... we run into this problem\n> > again and again. include/qemu/typedefs.h is just a work-around for this.\n> > I know people like typedefs for some reasons (I used to do that, too,\n> > before I realized the trouble with them), but IMHO we should rather\n> > adopt the typedef-related rules from the kernel coding conventions instead:\n> > \n> >  https://www.kernel.org/doc/html/v4.13/process/coding-style.html#typedefs\n> > \n> >   Thomas\n> > \n> \n> Yes, this is really nasty. And I wasn't aware of the involved issues.\n> \n> This seems to be the only feasible solution (including cpu.h sounds\n> wrong and will require a bunch of other includes):\n> \n> \n> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h\n> index a72d096081..ce80915a02 100644\n> --- a/include/hw/s390x/sclp.h\n> +++ b/include/hw/s390x/sclp.h\n> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev\n> *init_sclp_memory_hotplug_dev(void);\n>  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);\n>  void sclp_service_interrupt(uint32_t sccb);\n>  void raise_irq_cpu_hotplug(void);\n> +struct CPUS390XState;\n> +int sclp_service_call(struct CPUS390XState *env, uint64_t sccb,\n> uint32_t code);\n> \n>  #endif\n\nWhy not use typedefs.h?\n\nSigned-off-by: Eduardo Habkost <ehabkost@redhat.com>\n---\ndiff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h\nindex 4b86a8a293..3512bf8283 100644\n--- a/include/hw/s390x/sclp.h\n+++ b/include/hw/s390x/sclp.h\n@@ -242,7 +242,6 @@ sclpMemoryHotplugDev *init_sclp_memory_hotplug_dev(void);\n sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);\n void sclp_service_interrupt(uint32_t sccb);\n void raise_irq_cpu_hotplug(void);\n-typedef struct CPUS390XState CPUS390XState;\n int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);\n \n #endif\ndiff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h\nindex 39bc8351a3..9c97bffa92 100644\n--- a/include/qemu/typedefs.h\n+++ b/include/qemu/typedefs.h\n@@ -21,6 +21,7 @@ typedef struct Chardev Chardev;\n typedef struct CompatProperty CompatProperty;\n typedef struct CPUAddressSpace CPUAddressSpace;\n typedef struct CPUState CPUState;\n+typedef struct CPUS390XState CPUS390XState;\n typedef struct DeviceListener DeviceListener;\n typedef struct DeviceState DeviceState;\n typedef struct DirtyBitmapSnapshot DirtyBitmapSnapshot;\ndiff --git a/target/s390x/cpu.h b/target/s390x/cpu.h\nindex 032d1de2e8..f99a82cd5e 100644\n--- a/target/s390x/cpu.h\n+++ b/target/s390x/cpu.h\n@@ -80,7 +80,7 @@ typedef struct MchkQueue {\n     uint16_t type;\n } MchkQueue;\n \n-typedef struct CPUS390XState {\n+struct CPUS390XState {\n     uint64_t regs[16];     /* GP registers */\n     /*\n      * The floating point registers are part of the vector registers.\n@@ -174,7 +174,7 @@ typedef struct CPUS390XState {\n     /* currently processed sigp order */\n     uint8_t sigp_order;\n \n-} CPUS390XState;\n+};\n \n static inline CPU_DoubleU *get_freg(CPUS390XState *cs, int nr)\n {\n\n\n\n> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h\n> index 3aa2e46aac..032d1de2e8 100644\n> --- a/target/s390x/cpu.h\n> +++ b/target/s390x/cpu.h\n> @@ -721,6 +721,5 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr,\n> uint8_t ar, void *hostbuf,\n> \n>  /* outside of target/s390x/ */\n>  S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);\n> -int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);\n> \n>  #endif\n> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c\n> index b142db71c6..8b07535b02 100644\n> --- a/target/s390x/misc_helper.c\n> +++ b/target/s390x/misc_helper.c\n> @@ -35,6 +35,7 @@\n>  #include \"sysemu/sysemu.h\"\n>  #include \"hw/s390x/ebcdic.h\"\n>  #include \"hw/s390x/s390-virtio-hcall.h\"\n> +#include \"hw/s390x/sclp.h\"\n>  #endif\n> \n>  /* #define DEBUG_HELPER */\n> \n> \n> Opinions?\n> \n> \n> \n> -- \n> \n> Thanks,\n> \n> David","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-mx08.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx08.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=ehabkost@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 3xqSxT2Xx9z9sPk\n\tfor <incoming@patchwork.ozlabs.org>;\n\tSun, 10 Sep 2017 08:08:10 +1000 (AEST)","from localhost ([::1]:51011 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 1dqnv3-0002x6-E6\n\tfor incoming@patchwork.ozlabs.org; Sat, 09 Sep 2017 18:08:05 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:60552)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <ehabkost@redhat.com>) id 1dqnul-0002wB-5D\n\tfor qemu-devel@nongnu.org; Sat, 09 Sep 2017 18:07:48 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <ehabkost@redhat.com>) id 1dqnuh-0006Jr-Ux\n\tfor qemu-devel@nongnu.org; Sat, 09 Sep 2017 18:07:47 -0400","from mx1.redhat.com ([209.132.183.28]:40594)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <ehabkost@redhat.com>) id 1dqnuh-0006JP-L9\n\tfor qemu-devel@nongnu.org; Sat, 09 Sep 2017 18:07:43 -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 2A28CC0587C1;\n\tSat,  9 Sep 2017 22:07:42 +0000 (UTC)","from localhost (ovpn-116-45.gru2.redhat.com [10.97.116.45])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 990F75C896;\n\tSat,  9 Sep 2017 22:07:39 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 2A28CC0587C1","Date":"Sat, 9 Sep 2017 19:07:37 -0300","From":"Eduardo Habkost <ehabkost@redhat.com>","To":"David Hildenbrand <david@redhat.com>","Message-ID":"<20170909220737.GL7570@localhost.localdomain>","References":"<20170907201335.13956-1-david@redhat.com>\n\t<20170907201335.13956-9-david@redhat.com>\n\t<1603f6bf-f481-13c2-c7eb-ac155c5a14cc@redhat.com>\n\t<b8f78724-b6be-3e95-60fa-8c7dcafe2eec@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<b8f78724-b6be-3e95-60fa-8c7dcafe2eec@redhat.com>","X-Fnord":"you can see the fnord","User-Agent":"Mutt/1.8.3 (2017-05-23)","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.32]);\n\tSat, 09 Sep 2017 22:07:42 +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 v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","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":"Matthew Rosato <mjrosato@linux.vnet.ibm.com>,\n\tThomas Huth <thuth@redhat.com>,\n\tMarkus Armbruster <armbru@redhat.com>, cohuck@redhat.com,\n\tRichard Henderson <richard.henderson@linaro.org>,\n\tqemu-devel@nongnu.org, Alexander Graf <agraf@suse.de>,\n\tborntraeger@de.ibm.com, Paolo Bonzini <pbonzini@redhat.com>","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":1766032,"web_url":"http://patchwork.ozlabs.org/comment/1766032/","msgid":"<18360db6-0a06-375e-5ff7-a601c34b7123@redhat.com>","list_archive_url":null,"date":"2017-09-11T02:19:03","subject":"Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","submitter":{"id":66152,"url":"http://patchwork.ozlabs.org/api/people/66152/","name":"Thomas Huth","email":"thuth@redhat.com"},"content":"On 08.09.2017 14:29, Markus Armbruster wrote:\n> Thomas Huth <thuth@redhat.com> writes:\n> \n>> On 07.09.2017 22:13, David Hildenbrand wrote:\n>>> Implemented in sclp.c, so let's move it to the right include file.\n>>> Fix up one include. Do a forward declaration of CPUS390XState to fix the\n>>> two sclp consoles complaining.\n>>>\n>>> Signed-off-by: David Hildenbrand <david@redhat.com>\n>>> ---\n>>>  include/hw/s390x/sclp.h    | 2 ++\n>>>  target/s390x/cpu.h         | 1 -\n>>>  target/s390x/misc_helper.c | 1 +\n>>>  3 files changed, 3 insertions(+), 1 deletion(-)\n>>>\n>>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h\n>>> index a72d096081..4b86a8a293 100644\n>>> --- a/include/hw/s390x/sclp.h\n>>> +++ b/include/hw/s390x/sclp.h\n>>> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev *init_sclp_memory_hotplug_dev(void);\n>>>  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);\n>>>  void sclp_service_interrupt(uint32_t sccb);\n>>>  void raise_irq_cpu_hotplug(void);\n>>> +typedef struct CPUS390XState CPUS390XState;\n>>> +int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);\n>>\n>> That's dangerous and likely does not work with certain versions of GCC.\n>> You can't do a \"forward declaration\" with typedef in C, I'm afraid. See\n>> for example:\n>>\n>>  https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg01454.html\n>>  https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg03337.html\n>>  https://stackoverflow.com/questions/8367646/redefinition-of-typedef\n>>\n>> All this typedef'ing in QEMU is pretty bad ... we run into this problem\n>> again and again. include/qemu/typedefs.h is just a work-around for this.\n>> I know people like typedefs for some reasons (I used to do that, too,\n>> before I realized the trouble with them), but IMHO we should rather\n>> adopt the typedef-related rules from the kernel coding conventions instead:\n>>\n>>  https://www.kernel.org/doc/html/v4.13/process/coding-style.html#typedefs\n> \n> I prefer the kernel style for typedefs myself.  But it's strictly a\n> matter of style.  Excessive typedeffing makes code harder to understand,\n> it isn't wrong.  The part that's wrong is defining things more than\n> once, and that applies to everything, not just typedefs.\n> \n> Sometimes you get away with defining something more than once.  It's\n> still wrong.\n> \n> You're not supposed to define the same variable more than once, either\n> (although many C compilers let you get away with it, see -fno-common).\n> You define it in *one* place.  If you need to declare it, declare it in\n> *one* place, and make sure that place is in scope at the definition, so\n> the compiler can check the two match.\n> \n> Likewise, you're not supposed to define the same struct tag more than\n> once, and you should declare it in just one place.\n\nAFAIK it's perfect valid C to do a forward declaration of a struct\nmultiple times by just writing e.g. \"struct CPUS390XState;\" somewhere in\nyour code. This is also what is done in various Linux kernel headers all\nover the place.\n\n> Likewise, you're not supposed to define (with typedef) the same type\n> more than once.  There is no such thing as a typedef declaration.\n> \n> qemu/typedefs.h is not a work-around for a typedef-happy style.  Its\n> purpose is breaking inclusion cycles.  Secondary purpose is reducing the\n> need for non-cyclic includes.  If we didn't typedef, we'd still put our\n> struct declarations there.\n\nNo, since it's not required for struct forward declarations, only to\navoid multiple typedef definitions.\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-mx08.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx08.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 3xrBT85tsmz9s7g\n\tfor <incoming@patchwork.ozlabs.org>;\n\tMon, 11 Sep 2017 12:19:38 +1000 (AEST)","from localhost ([::1]:55046 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 1drEK0-00009W-12\n\tfor incoming@patchwork.ozlabs.org; Sun, 10 Sep 2017 22:19:36 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:54007)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <thuth@redhat.com>) id 1drEJh-00008a-EO\n\tfor qemu-devel@nongnu.org; Sun, 10 Sep 2017 22:19:18 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <thuth@redhat.com>) id 1drEJe-0005z1-9a\n\tfor qemu-devel@nongnu.org; Sun, 10 Sep 2017 22:19:17 -0400","from mx1.redhat.com ([209.132.183.28]:35368)\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 1drEJe-0005y9-0j\n\tfor qemu-devel@nongnu.org; Sun, 10 Sep 2017 22:19: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 1D8F4C1CE521;\n\tMon, 11 Sep 2017 02:19:12 +0000 (UTC)","from [10.36.116.54] (ovpn-116-54.ams2.redhat.com [10.36.116.54])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 5329860621;\n\tMon, 11 Sep 2017 02:19:04 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 1D8F4C1CE521","To":"Markus Armbruster <armbru@redhat.com>","References":"<20170907201335.13956-1-david@redhat.com>\n\t<20170907201335.13956-9-david@redhat.com>\n\t<1603f6bf-f481-13c2-c7eb-ac155c5a14cc@redhat.com>\n\t<87lglp9xwf.fsf@dusky.pond.sub.org>","From":"Thomas Huth <thuth@redhat.com>","Message-ID":"<18360db6-0a06-375e-5ff7-a601c34b7123@redhat.com>","Date":"Mon, 11 Sep 2017 04:19:03 +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":"<87lglp9xwf.fsf@dusky.pond.sub.org>","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.32]);\n\tMon, 11 Sep 2017 02:19:12 +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 v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","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":"Matthew Rosato <mjrosato@linux.vnet.ibm.com>,\n\tEduardo Habkost <ehabkost@redhat.com>,\n\tDavid Hildenbrand <david@redhat.com>, cohuck@redhat.com,\n\tRichard Henderson <richard.henderson@linaro.org>,\n\tqemu-devel@nongnu.org, Alexander Graf <agraf@suse.de>,\n\tborntraeger@de.ibm.com, Paolo Bonzini <pbonzini@redhat.com>","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":1766033,"web_url":"http://patchwork.ozlabs.org/comment/1766033/","msgid":"<5d20eb5c-85cd-e500-0b98-c044c5176726@redhat.com>","list_archive_url":null,"date":"2017-09-11T02:23:09","subject":"Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","submitter":{"id":66152,"url":"http://patchwork.ozlabs.org/api/people/66152/","name":"Thomas Huth","email":"thuth@redhat.com"},"content":"On 10.09.2017 00:07, Eduardo Habkost wrote:\n> On Fri, Sep 08, 2017 at 02:46:36PM +0200, David Hildenbrand wrote:\n>> On 08.09.2017 06:21, Thomas Huth wrote:\n>>> On 07.09.2017 22:13, David Hildenbrand wrote:\n>>>> Implemented in sclp.c, so let's move it to the right include file.\n>>>> Fix up one include. Do a forward declaration of CPUS390XState to fix the\n>>>> two sclp consoles complaining.\n>>>>\n>>>> Signed-off-by: David Hildenbrand <david@redhat.com>\n>>>> ---\n>>>>  include/hw/s390x/sclp.h    | 2 ++\n>>>>  target/s390x/cpu.h         | 1 -\n>>>>  target/s390x/misc_helper.c | 1 +\n>>>>  3 files changed, 3 insertions(+), 1 deletion(-)\n>>>>\n>>>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h\n>>>> index a72d096081..4b86a8a293 100644\n>>>> --- a/include/hw/s390x/sclp.h\n>>>> +++ b/include/hw/s390x/sclp.h\n>>>> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev *init_sclp_memory_hotplug_dev(void);\n>>>>  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);\n>>>>  void sclp_service_interrupt(uint32_t sccb);\n>>>>  void raise_irq_cpu_hotplug(void);\n>>>> +typedef struct CPUS390XState CPUS390XState;\n>>>> +int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);\n>>>\n>>> That's dangerous and likely does not work with certain versions of GCC.\n>>> You can't do a \"forward declaration\" with typedef in C, I'm afraid. See\n>>> for example:\n>>>\n>>>  https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg01454.html\n>>>  https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg03337.html\n>>>  https://stackoverflow.com/questions/8367646/redefinition-of-typedef\n>>>\n>>> All this typedef'ing in QEMU is pretty bad ... we run into this problem\n>>> again and again. include/qemu/typedefs.h is just a work-around for this.\n>>> I know people like typedefs for some reasons (I used to do that, too,\n>>> before I realized the trouble with them), but IMHO we should rather\n>>> adopt the typedef-related rules from the kernel coding conventions instead:\n>>>\n>>>  https://www.kernel.org/doc/html/v4.13/process/coding-style.html#typedefs\n>>>\n>>>   Thomas\n>>>\n>>\n>> Yes, this is really nasty. And I wasn't aware of the involved issues.\n>>\n>> This seems to be the only feasible solution (including cpu.h sounds\n>> wrong and will require a bunch of other includes):\n>>\n>>\n>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h\n>> index a72d096081..ce80915a02 100644\n>> --- a/include/hw/s390x/sclp.h\n>> +++ b/include/hw/s390x/sclp.h\n>> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev\n>> *init_sclp_memory_hotplug_dev(void);\n>>  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);\n>>  void sclp_service_interrupt(uint32_t sccb);\n>>  void raise_irq_cpu_hotplug(void);\n>> +struct CPUS390XState;\n>> +int sclp_service_call(struct CPUS390XState *env, uint64_t sccb,\n>> uint32_t code);\n>>\n>>  #endif\n> \n> Why not use typedefs.h?\n> \n> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>\n> ---\n> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h\n> index 4b86a8a293..3512bf8283 100644\n> --- a/include/hw/s390x/sclp.h\n> +++ b/include/hw/s390x/sclp.h\n> @@ -242,7 +242,6 @@ sclpMemoryHotplugDev *init_sclp_memory_hotplug_dev(void);\n>  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);\n>  void sclp_service_interrupt(uint32_t sccb);\n>  void raise_irq_cpu_hotplug(void);\n> -typedef struct CPUS390XState CPUS390XState;\n>  int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);\n>  \n>  #endif\n> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h\n> index 39bc8351a3..9c97bffa92 100644\n> --- a/include/qemu/typedefs.h\n> +++ b/include/qemu/typedefs.h\n\nUsing include/qemu/typedefs.h here is IMHO really ugly. Do we really\nwant to pollute a common include file with target specific code? My\npreferences are first to avoid typdefs, but if we really need/want them\n(do we? There is no comment about this in our coding styles), I think we\nshould rather introduce target-specific typedefs.h headers, too, for\neverything that is not part of the common code.\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 3xrBYq5XRXz9sRg\n\tfor <incoming@patchwork.ozlabs.org>;\n\tMon, 11 Sep 2017 12:23:43 +1000 (AEST)","from localhost ([::1]:55055 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 1drENx-0001Bz-Ub\n\tfor incoming@patchwork.ozlabs.org; Sun, 10 Sep 2017 22:23:41 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:54789)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <thuth@redhat.com>) id 1drENc-0001Bq-OE\n\tfor qemu-devel@nongnu.org; Sun, 10 Sep 2017 22:23:22 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <thuth@redhat.com>) id 1drENY-0000DB-Ga\n\tfor qemu-devel@nongnu.org; Sun, 10 Sep 2017 22:23:20 -0400","from mx1.redhat.com ([209.132.183.28]:58328)\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 1drENY-0000CQ-7I\n\tfor qemu-devel@nongnu.org; Sun, 10 Sep 2017 22:23:16 -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 3525D198932;\n\tMon, 11 Sep 2017 02:23:15 +0000 (UTC)","from [10.36.116.54] (ovpn-116-54.ams2.redhat.com [10.36.116.54])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id DF4335C549;\n\tMon, 11 Sep 2017 02:23:10 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 3525D198932","To":"Eduardo Habkost <ehabkost@redhat.com>,\n\tDavid Hildenbrand <david@redhat.com>","References":"<20170907201335.13956-1-david@redhat.com>\n\t<20170907201335.13956-9-david@redhat.com>\n\t<1603f6bf-f481-13c2-c7eb-ac155c5a14cc@redhat.com>\n\t<b8f78724-b6be-3e95-60fa-8c7dcafe2eec@redhat.com>\n\t<20170909220737.GL7570@localhost.localdomain>","From":"Thomas Huth <thuth@redhat.com>","Message-ID":"<5d20eb5c-85cd-e500-0b98-c044c5176726@redhat.com>","Date":"Mon, 11 Sep 2017 04:23:09 +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":"<20170909220737.GL7570@localhost.localdomain>","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\tMon, 11 Sep 2017 02:23:15 +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 v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","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":"Matthew Rosato <mjrosato@linux.vnet.ibm.com>,\n\tMarkus Armbruster <armbru@redhat.com>, cohuck@redhat.com,\n\tRichard Henderson <richard.henderson@linaro.org>,\n\tqemu-devel@nongnu.org, Alexander Graf <agraf@suse.de>,\n\tborntraeger@de.ibm.com, Paolo Bonzini <pbonzini@redhat.com>","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":1766204,"web_url":"http://patchwork.ozlabs.org/comment/1766204/","msgid":"<c758ae70-a664-d13a-dd24-768f71e3f2e4@redhat.com>","list_archive_url":null,"date":"2017-09-11T10:23:10","subject":"Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","submitter":{"id":2701,"url":"http://patchwork.ozlabs.org/api/people/2701/","name":"Paolo Bonzini","email":"pbonzini@redhat.com"},"content":"On 10/09/2017 00:07, Eduardo Habkost wrote:\n> On Fri, Sep 08, 2017 at 02:46:36PM +0200, David Hildenbrand wrote:\n>> On 08.09.2017 06:21, Thomas Huth wrote:\n>>> On 07.09.2017 22:13, David Hildenbrand wrote:\n>>>> Implemented in sclp.c, so let's move it to the right include file.\n>>>> Fix up one include. Do a forward declaration of CPUS390XState to fix the\n>>>> two sclp consoles complaining.\n>>>>\n>>>> Signed-off-by: David Hildenbrand <david@redhat.com>\n>>>> ---\n>>>>  include/hw/s390x/sclp.h    | 2 ++\n>>>>  target/s390x/cpu.h         | 1 -\n>>>>  target/s390x/misc_helper.c | 1 +\n>>>>  3 files changed, 3 insertions(+), 1 deletion(-)\n>>>>\n>>>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h\n>>>> index a72d096081..4b86a8a293 100644\n>>>> --- a/include/hw/s390x/sclp.h\n>>>> +++ b/include/hw/s390x/sclp.h\n>>>> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev *init_sclp_memory_hotplug_dev(void);\n>>>>  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);\n>>>>  void sclp_service_interrupt(uint32_t sccb);\n>>>>  void raise_irq_cpu_hotplug(void);\n>>>> +typedef struct CPUS390XState CPUS390XState;\n>>>> +int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);\n>>>\n>>> That's dangerous and likely does not work with certain versions of GCC.\n>>> You can't do a \"forward declaration\" with typedef in C, I'm afraid. See\n>>> for example:\n>>>\n>>>  https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg01454.html\n>>>  https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg03337.html\n>>>  https://stackoverflow.com/questions/8367646/redefinition-of-typedef\n>>>\n>>> All this typedef'ing in QEMU is pretty bad ... we run into this problem\n>>> again and again. include/qemu/typedefs.h is just a work-around for this.\n>>> I know people like typedefs for some reasons (I used to do that, too,\n>>> before I realized the trouble with them), but IMHO we should rather\n>>> adopt the typedef-related rules from the kernel coding conventions instead:\n>>>\n>>>  https://www.kernel.org/doc/html/v4.13/process/coding-style.html#typedefs\n>>>\n>>>   Thomas\n>>>\n>>\n>> Yes, this is really nasty. And I wasn't aware of the involved issues.\n>>\n>> This seems to be the only feasible solution (including cpu.h sounds\n>> wrong and will require a bunch of other includes):\n>>\n>>\n>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h\n>> index a72d096081..ce80915a02 100644\n>> --- a/include/hw/s390x/sclp.h\n>> +++ b/include/hw/s390x/sclp.h\n>> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev\n>> *init_sclp_memory_hotplug_dev(void);\n>>  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);\n>>  void sclp_service_interrupt(uint32_t sccb);\n>>  void raise_irq_cpu_hotplug(void);\n>> +struct CPUS390XState;\n>> +int sclp_service_call(struct CPUS390XState *env, uint64_t sccb,\n>> uint32_t code);\n>>\n>>  #endif\n> \n> Why not use typedefs.h?\n\nSee Markus's reply.  But, maybe it's even better to use S390CPU* and\ninclude target/s390x/cpu-qom.h, which by design provides as little\ndefinitions as needed.\n\n> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>\n> ---\n> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h\n> index 4b86a8a293..3512bf8283 100644\n> --- a/include/hw/s390x/sclp.h\n> +++ b/include/hw/s390x/sclp.h\n> @@ -242,7 +242,6 @@ sclpMemoryHotplugDev *init_sclp_memory_hotplug_dev(void);\n>  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);\n>  void sclp_service_interrupt(uint32_t sccb);\n>  void raise_irq_cpu_hotplug(void);\n> -typedef struct CPUS390XState CPUS390XState;\n>  int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);\n>  \n>  #endif\n> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h\n> index 39bc8351a3..9c97bffa92 100644\n> --- a/include/qemu/typedefs.h\n> +++ b/include/qemu/typedefs.h\n> @@ -21,6 +21,7 @@ typedef struct Chardev Chardev;\n>  typedef struct CompatProperty CompatProperty;\n>  typedef struct CPUAddressSpace CPUAddressSpace;\n>  typedef struct CPUState CPUState;\n> +typedef struct CPUS390XState CPUS390XState;\n>  typedef struct DeviceListener DeviceListener;\n>  typedef struct DeviceState DeviceState;\n>  typedef struct DirtyBitmapSnapshot DirtyBitmapSnapshot;\n> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h\n> index 032d1de2e8..f99a82cd5e 100644\n> --- a/target/s390x/cpu.h\n> +++ b/target/s390x/cpu.h\n> @@ -80,7 +80,7 @@ typedef struct MchkQueue {\n>      uint16_t type;\n>  } MchkQueue;\n>  \n> -typedef struct CPUS390XState {\n> +struct CPUS390XState {\n>      uint64_t regs[16];     /* GP registers */\n>      /*\n>       * The floating point registers are part of the vector registers.\n> @@ -174,7 +174,7 @@ typedef struct CPUS390XState {\n>      /* currently processed sigp order */\n>      uint8_t sigp_order;\n>  \n> -} CPUS390XState;\n> +};\n>  \n>  static inline CPU_DoubleU *get_freg(CPUS390XState *cs, int nr)\n>  {\n> \n> \n> \n>> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h\n>> index 3aa2e46aac..032d1de2e8 100644\n>> --- a/target/s390x/cpu.h\n>> +++ b/target/s390x/cpu.h\n>> @@ -721,6 +721,5 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr,\n>> uint8_t ar, void *hostbuf,\n>>\n>>  /* outside of target/s390x/ */\n>>  S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);\n>> -int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);\n>>\n>>  #endif\n>> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c\n>> index b142db71c6..8b07535b02 100644\n>> --- a/target/s390x/misc_helper.c\n>> +++ b/target/s390x/misc_helper.c\n>> @@ -35,6 +35,7 @@\n>>  #include \"sysemu/sysemu.h\"\n>>  #include \"hw/s390x/ebcdic.h\"\n>>  #include \"hw/s390x/s390-virtio-hcall.h\"\n>> +#include \"hw/s390x/sclp.h\"\n>>  #endif\n>>\n>>  /* #define DEBUG_HELPER */\n>>\n>>\n>> Opinions?\n>>\n>>\n>>\n>> -- \n>>\n>> Thanks,\n>>\n>> David\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-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=pbonzini@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 3xrPD00nhBz9s7f\n\tfor <incoming@patchwork.ozlabs.org>;\n\tMon, 11 Sep 2017 20:23:59 +1000 (AEST)","from localhost ([::1]:56541 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 1drLsj-00040l-Pn\n\tfor incoming@patchwork.ozlabs.org; Mon, 11 Sep 2017 06:23:57 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:59015)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <pbonzini@redhat.com>) id 1drLs8-0003xR-Pe\n\tfor qemu-devel@nongnu.org; Mon, 11 Sep 2017 06:23:22 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <pbonzini@redhat.com>) id 1drLs5-0007U8-K9\n\tfor qemu-devel@nongnu.org; Mon, 11 Sep 2017 06:23:20 -0400","from mx1.redhat.com ([209.132.183.28]:39824)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <pbonzini@redhat.com>) id 1drLs5-0007Tb-BL\n\tfor qemu-devel@nongnu.org; Mon, 11 Sep 2017 06:23:17 -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 28AA71A2263;\n\tMon, 11 Sep 2017 10:23:16 +0000 (UTC)","from [10.32.181.85] (unknown [10.32.181.85])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id B4F0B6060D;\n\tMon, 11 Sep 2017 10:23:11 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 28AA71A2263","To":"Eduardo Habkost <ehabkost@redhat.com>,\n\tDavid Hildenbrand <david@redhat.com>","References":"<20170907201335.13956-1-david@redhat.com>\n\t<20170907201335.13956-9-david@redhat.com>\n\t<1603f6bf-f481-13c2-c7eb-ac155c5a14cc@redhat.com>\n\t<b8f78724-b6be-3e95-60fa-8c7dcafe2eec@redhat.com>\n\t<20170909220737.GL7570@localhost.localdomain>","From":"Paolo Bonzini <pbonzini@redhat.com>","Message-ID":"<c758ae70-a664-d13a-dd24-768f71e3f2e4@redhat.com>","Date":"Mon, 11 Sep 2017 12:23:10 +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":"<20170909220737.GL7570@localhost.localdomain>","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\tMon, 11 Sep 2017 10:23:16 +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 v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","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":"Matthew Rosato <mjrosato@linux.vnet.ibm.com>,\n\tThomas Huth <thuth@redhat.com>,\n\tMarkus Armbruster <armbru@redhat.com>, cohuck@redhat.com,\n\tRichard Henderson <richard.henderson@linaro.org>,\n\tqemu-devel@nongnu.org, Alexander Graf <agraf@suse.de>,\n\tborntraeger@de.ibm.com","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":1766311,"web_url":"http://patchwork.ozlabs.org/comment/1766311/","msgid":"<ae8dbb55-4b34-cfd7-3305-47c0ae3e1ca6@redhat.com>","list_archive_url":null,"date":"2017-09-11T13:45:44","subject":"Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","submitter":{"id":70402,"url":"http://patchwork.ozlabs.org/api/people/70402/","name":"David Hildenbrand","email":"david@redhat.com"},"content":"On 11.09.2017 12:23, Paolo Bonzini wrote:\n> On 10/09/2017 00:07, Eduardo Habkost wrote:\n>> On Fri, Sep 08, 2017 at 02:46:36PM +0200, David Hildenbrand wrote:\n>>> On 08.09.2017 06:21, Thomas Huth wrote:\n>>>> On 07.09.2017 22:13, David Hildenbrand wrote:\n>>>>> Implemented in sclp.c, so let's move it to the right include file.\n>>>>> Fix up one include. Do a forward declaration of CPUS390XState to fix the\n>>>>> two sclp consoles complaining.\n>>>>>\n>>>>> Signed-off-by: David Hildenbrand <david@redhat.com>\n>>>>> ---\n>>>>>  include/hw/s390x/sclp.h    | 2 ++\n>>>>>  target/s390x/cpu.h         | 1 -\n>>>>>  target/s390x/misc_helper.c | 1 +\n>>>>>  3 files changed, 3 insertions(+), 1 deletion(-)\n>>>>>\n>>>>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h\n>>>>> index a72d096081..4b86a8a293 100644\n>>>>> --- a/include/hw/s390x/sclp.h\n>>>>> +++ b/include/hw/s390x/sclp.h\n>>>>> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev *init_sclp_memory_hotplug_dev(void);\n>>>>>  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);\n>>>>>  void sclp_service_interrupt(uint32_t sccb);\n>>>>>  void raise_irq_cpu_hotplug(void);\n>>>>> +typedef struct CPUS390XState CPUS390XState;\n>>>>> +int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);\n>>>>\n>>>> That's dangerous and likely does not work with certain versions of GCC.\n>>>> You can't do a \"forward declaration\" with typedef in C, I'm afraid. See\n>>>> for example:\n>>>>\n>>>>  https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg01454.html\n>>>>  https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg03337.html\n>>>>  https://stackoverflow.com/questions/8367646/redefinition-of-typedef\n>>>>\n>>>> All this typedef'ing in QEMU is pretty bad ... we run into this problem\n>>>> again and again. include/qemu/typedefs.h is just a work-around for this.\n>>>> I know people like typedefs for some reasons (I used to do that, too,\n>>>> before I realized the trouble with them), but IMHO we should rather\n>>>> adopt the typedef-related rules from the kernel coding conventions instead:\n>>>>\n>>>>  https://www.kernel.org/doc/html/v4.13/process/coding-style.html#typedefs\n>>>>\n>>>>   Thomas\n>>>>\n>>>\n>>> Yes, this is really nasty. And I wasn't aware of the involved issues.\n>>>\n>>> This seems to be the only feasible solution (including cpu.h sounds\n>>> wrong and will require a bunch of other includes):\n>>>\n>>>\n>>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h\n>>> index a72d096081..ce80915a02 100644\n>>> --- a/include/hw/s390x/sclp.h\n>>> +++ b/include/hw/s390x/sclp.h\n>>> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev\n>>> *init_sclp_memory_hotplug_dev(void);\n>>>  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);\n>>>  void sclp_service_interrupt(uint32_t sccb);\n>>>  void raise_irq_cpu_hotplug(void);\n>>> +struct CPUS390XState;\n>>> +int sclp_service_call(struct CPUS390XState *env, uint64_t sccb,\n>>> uint32_t code);\n>>>\n>>>  #endif\n>>\n>> Why not use typedefs.h?\n> \n> See Markus's reply.  But, maybe it's even better to use S390CPU* and\n> include target/s390x/cpu-qom.h, which by design provides as little\n> definitions as needed.\n\nI'll go with that approach. I'll drop the dependency from cpu-qom.h to\ncpu_models.h (by moving typedefs to cpu-qom.h). This makes it compile at\nhopefully should then be good enough for now.\n\n(this approach implies dropping patch \"[PATCH v3 05/21] target/s390x:\nmove typedef of S390CPU to its definition\").\n\nThanks!\n\n> \n>> Signed-off-by: Eduardo Habkost <ehabkost@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-mx08.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx08.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 3xrTkK5dD4z9s0Z\n\tfor <incoming@patchwork.ozlabs.org>;\n\tMon, 11 Sep 2017 23:47:05 +1000 (AEST)","from localhost ([::1]:57882 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 1drP3H-0007sk-TR\n\tfor incoming@patchwork.ozlabs.org; Mon, 11 Sep 2017 09:47:03 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:39869)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <david@redhat.com>) id 1drP2E-0007Ty-D5\n\tfor qemu-devel@nongnu.org; Mon, 11 Sep 2017 09:46:02 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <david@redhat.com>) id 1drP28-0007ES-HL\n\tfor qemu-devel@nongnu.org; Mon, 11 Sep 2017 09:45:58 -0400","from mx1.redhat.com ([209.132.183.28]:55132)\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 1drP27-0007DO-Vl\n\tfor qemu-devel@nongnu.org; Mon, 11 Sep 2017 09:45:52 -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 67515C0B8D57;\n\tMon, 11 Sep 2017 13:45:49 +0000 (UTC)","from [10.36.116.48] (ovpn-116-48.ams2.redhat.com [10.36.116.48])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id B3F566B26F;\n\tMon, 11 Sep 2017 13:45:45 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 67515C0B8D57","To":"Paolo Bonzini <pbonzini@redhat.com>,\n\tEduardo Habkost <ehabkost@redhat.com>","References":"<20170907201335.13956-1-david@redhat.com>\n\t<20170907201335.13956-9-david@redhat.com>\n\t<1603f6bf-f481-13c2-c7eb-ac155c5a14cc@redhat.com>\n\t<b8f78724-b6be-3e95-60fa-8c7dcafe2eec@redhat.com>\n\t<20170909220737.GL7570@localhost.localdomain>\n\t<c758ae70-a664-d13a-dd24-768f71e3f2e4@redhat.com>","From":"David Hildenbrand <david@redhat.com>","Organization":"Red Hat GmbH","Message-ID":"<ae8dbb55-4b34-cfd7-3305-47c0ae3e1ca6@redhat.com>","Date":"Mon, 11 Sep 2017 15: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":"<c758ae70-a664-d13a-dd24-768f71e3f2e4@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.32]);\n\tMon, 11 Sep 2017 13: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 v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","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":"Matthew Rosato <mjrosato@linux.vnet.ibm.com>,\n\tThomas Huth <thuth@redhat.com>,\n\tMarkus Armbruster <armbru@redhat.com>, cohuck@redhat.com,\n\tRichard Henderson <richard.henderson@linaro.org>,\n\tqemu-devel@nongnu.org, Alexander Graf <agraf@suse.de>,\n\tborntraeger@de.ibm.com","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":1766438,"web_url":"http://patchwork.ozlabs.org/comment/1766438/","msgid":"<8cf0bf17-c1cb-ee13-768c-dcf1836687c2@redhat.com>","list_archive_url":null,"date":"2017-09-11T17:56:19","subject":"Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","submitter":{"id":70402,"url":"http://patchwork.ozlabs.org/api/people/70402/","name":"David Hildenbrand","email":"david@redhat.com"},"content":">>>>\n>>>>  #endif\n>>>\n>>> Why not use typedefs.h?\n>>\n>> See Markus's reply.  But, maybe it's even better to use S390CPU* and\n>> include target/s390x/cpu-qom.h, which by design provides as little\n>> definitions as needed.\n> \n> I don't see an argument against moving typedef CPUS390XState to\n> typedefs.h in Markus' reply.  I see one argument for it (reducing\n> the need for non-cyclic includes).\n> \n> cpu-qom.h includes cpu.h, so I don't know why using S390CPU*\n> would solve any problem.  I don't disagree about changing the\n> function to use S390CPU* eventually, but it would still require\n> us make a choice between: a) including the header where the\n> typedef name is declared (cpu.h or cpu-qom.h); or b) moving the\n> typedef name declaration to typedefs.h.\n\nIt includes qom/cpu.h, not cpu.h. That's why using cpu-qom.h for such\ntypedefs works (see v4).\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-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 3xrbfB4RV3z9s7F\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 12 Sep 2017 04:13:48 +1000 (AEST)","from localhost ([::1]:59664 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 1drTDO-0005hW-Qf\n\tfor incoming@patchwork.ozlabs.org; Mon, 11 Sep 2017 14:13:46 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:51680)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <david@redhat.com>) id 1drSwe-00076G-Ah\n\tfor qemu-devel@nongnu.org; Mon, 11 Sep 2017 13:56:29 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <david@redhat.com>) id 1drSwa-0004wg-EO\n\tfor qemu-devel@nongnu.org; Mon, 11 Sep 2017 13:56:28 -0400","from mx1.redhat.com ([209.132.183.28]:44640)\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 1drSwa-0004vn-88\n\tfor qemu-devel@nongnu.org; Mon, 11 Sep 2017 13:56:24 -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 12762B9F3C;\n\tMon, 11 Sep 2017 17:56:23 +0000 (UTC)","from [10.36.116.74] (ovpn-116-74.ams2.redhat.com [10.36.116.74])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id BC1685C2E4;\n\tMon, 11 Sep 2017 17:56:20 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 12762B9F3C","To":"Eduardo Habkost <ehabkost@redhat.com>,\n\tPaolo Bonzini <pbonzini@redhat.com>","References":"<20170907201335.13956-1-david@redhat.com>\n\t<20170907201335.13956-9-david@redhat.com>\n\t<1603f6bf-f481-13c2-c7eb-ac155c5a14cc@redhat.com>\n\t<b8f78724-b6be-3e95-60fa-8c7dcafe2eec@redhat.com>\n\t<20170909220737.GL7570@localhost.localdomain>\n\t<c758ae70-a664-d13a-dd24-768f71e3f2e4@redhat.com>\n\t<20170911175224.GP7570@localhost.localdomain>","From":"David Hildenbrand <david@redhat.com>","Organization":"Red Hat GmbH","Message-ID":"<8cf0bf17-c1cb-ee13-768c-dcf1836687c2@redhat.com>","Date":"Mon, 11 Sep 2017 19:56:19 +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":"<20170911175224.GP7570@localhost.localdomain>","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.27]);\n\tMon, 11 Sep 2017 17:56:23 +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 v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","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":"Matthew Rosato <mjrosato@linux.vnet.ibm.com>,\n\tThomas Huth <thuth@redhat.com>,\n\tMarkus Armbruster <armbru@redhat.com>, cohuck@redhat.com,\n\tRichard Henderson <richard.henderson@linaro.org>,\n\tqemu-devel@nongnu.org, Alexander Graf <agraf@suse.de>,\n\tborntraeger@de.ibm.com","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":1766448,"web_url":"http://patchwork.ozlabs.org/comment/1766448/","msgid":"<20170911182207.GR7570@localhost.localdomain>","list_archive_url":null,"date":"2017-09-11T18:22:07","subject":"Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","submitter":{"id":195,"url":"http://patchwork.ozlabs.org/api/people/195/","name":"Eduardo Habkost","email":"ehabkost@redhat.com"},"content":"On Mon, Sep 11, 2017 at 04:23:09AM +0200, Thomas Huth wrote:\n> On 10.09.2017 00:07, Eduardo Habkost wrote:\n> > On Fri, Sep 08, 2017 at 02:46:36PM +0200, David Hildenbrand wrote:\n> >> On 08.09.2017 06:21, Thomas Huth wrote:\n> >>> On 07.09.2017 22:13, David Hildenbrand wrote:\n> >>>> Implemented in sclp.c, so let's move it to the right include file.\n> >>>> Fix up one include. Do a forward declaration of CPUS390XState to fix the\n> >>>> two sclp consoles complaining.\n> >>>>\n> >>>> Signed-off-by: David Hildenbrand <david@redhat.com>\n> >>>> ---\n> >>>>  include/hw/s390x/sclp.h    | 2 ++\n> >>>>  target/s390x/cpu.h         | 1 -\n> >>>>  target/s390x/misc_helper.c | 1 +\n> >>>>  3 files changed, 3 insertions(+), 1 deletion(-)\n> >>>>\n> >>>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h\n> >>>> index a72d096081..4b86a8a293 100644\n> >>>> --- a/include/hw/s390x/sclp.h\n> >>>> +++ b/include/hw/s390x/sclp.h\n> >>>> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev *init_sclp_memory_hotplug_dev(void);\n> >>>>  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);\n> >>>>  void sclp_service_interrupt(uint32_t sccb);\n> >>>>  void raise_irq_cpu_hotplug(void);\n> >>>> +typedef struct CPUS390XState CPUS390XState;\n> >>>> +int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);\n> >>>\n> >>> That's dangerous and likely does not work with certain versions of GCC.\n> >>> You can't do a \"forward declaration\" with typedef in C, I'm afraid. See\n> >>> for example:\n> >>>\n> >>>  https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg01454.html\n> >>>  https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg03337.html\n> >>>  https://stackoverflow.com/questions/8367646/redefinition-of-typedef\n> >>>\n> >>> All this typedef'ing in QEMU is pretty bad ... we run into this problem\n> >>> again and again. include/qemu/typedefs.h is just a work-around for this.\n> >>> I know people like typedefs for some reasons (I used to do that, too,\n> >>> before I realized the trouble with them), but IMHO we should rather\n> >>> adopt the typedef-related rules from the kernel coding conventions instead:\n> >>>\n> >>>  https://www.kernel.org/doc/html/v4.13/process/coding-style.html#typedefs\n> >>>\n> >>>   Thomas\n> >>>\n> >>\n> >> Yes, this is really nasty. And I wasn't aware of the involved issues.\n> >>\n> >> This seems to be the only feasible solution (including cpu.h sounds\n> >> wrong and will require a bunch of other includes):\n> >>\n> >>\n> >> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h\n> >> index a72d096081..ce80915a02 100644\n> >> --- a/include/hw/s390x/sclp.h\n> >> +++ b/include/hw/s390x/sclp.h\n> >> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev\n> >> *init_sclp_memory_hotplug_dev(void);\n> >>  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);\n> >>  void sclp_service_interrupt(uint32_t sccb);\n> >>  void raise_irq_cpu_hotplug(void);\n> >> +struct CPUS390XState;\n> >> +int sclp_service_call(struct CPUS390XState *env, uint64_t sccb,\n> >> uint32_t code);\n> >>\n> >>  #endif\n> > \n> > Why not use typedefs.h?\n> > \n> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>\n> > ---\n> > diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h\n> > index 4b86a8a293..3512bf8283 100644\n> > --- a/include/hw/s390x/sclp.h\n> > +++ b/include/hw/s390x/sclp.h\n> > @@ -242,7 +242,6 @@ sclpMemoryHotplugDev *init_sclp_memory_hotplug_dev(void);\n> >  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);\n> >  void sclp_service_interrupt(uint32_t sccb);\n> >  void raise_irq_cpu_hotplug(void);\n> > -typedef struct CPUS390XState CPUS390XState;\n> >  int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);\n> >  \n> >  #endif\n> > diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h\n> > index 39bc8351a3..9c97bffa92 100644\n> > --- a/include/qemu/typedefs.h\n> > +++ b/include/qemu/typedefs.h\n> \n> Using include/qeemu/typedefs.h here is IMHO really ugly. Do we really\n> want to pollute a common include file with target specific code? My\n> preferences are first to avoid typdefs, but if we really need/want them\n> (do we? There is no comment about this in our coding styles), I think we\n> should rather introduce target-specific typedefs.h headers, too, for\n> everything that is not part of the common code.\n\nI don't see any advantage in splitting typedefs.h into\narch-specific files.  We don't split typedefs.h into\nsubsystem-specific or device-specific headers, so I don't see we\nwould need a per-architecture split either.  typedefs.h is just a\nglobal collection of type identifiers that helps us reduce header\ndependency hell.\n\n(Anyway, the current problem is now going solved by using\nS390CPU* instead of CPUS390XState*, so there's no need to touch\ntypedefs.h this time.)\n\nAbout keeping using typedefs: I don't have an strong opinion\nfor/against them[1], but I would prefer to keep style consistent\neven if it's not explicitly documented.\n\n[1] The fact that it would make typedefs.h completely unnecessary\n    makes me inclined towards the suggestion to stop using them.","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=ehabkost@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 3xrbws1htbz9sBd\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 12 Sep 2017 04:26:37 +1000 (AEST)","from localhost ([::1]:59799 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 1drTPn-0001Wn-9r\n\tfor incoming@patchwork.ozlabs.org; Mon, 11 Sep 2017 14:26:35 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:35546)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <ehabkost@redhat.com>) id 1drTLc-0006kE-IA\n\tfor qemu-devel@nongnu.org; Mon, 11 Sep 2017 14:22:18 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <ehabkost@redhat.com>) id 1drTLY-0002MS-8T\n\tfor qemu-devel@nongnu.org; Mon, 11 Sep 2017 14:22:16 -0400","from mx1.redhat.com ([209.132.183.28]:38274)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <ehabkost@redhat.com>) id 1drTLX-0002L7-VL\n\tfor qemu-devel@nongnu.org; Mon, 11 Sep 2017 14:22:12 -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 DE009C07F67F;\n\tMon, 11 Sep 2017 18:22:10 +0000 (UTC)","from localhost (ovpn-116-15.gru2.redhat.com [10.97.116.15])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 991826A820;\n\tMon, 11 Sep 2017 18:22:08 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com DE009C07F67F","Date":"Mon, 11 Sep 2017 15:22:07 -0300","From":"Eduardo Habkost <ehabkost@redhat.com>","To":"Thomas Huth <thuth@redhat.com>","Message-ID":"<20170911182207.GR7570@localhost.localdomain>","References":"<20170907201335.13956-1-david@redhat.com>\n\t<20170907201335.13956-9-david@redhat.com>\n\t<1603f6bf-f481-13c2-c7eb-ac155c5a14cc@redhat.com>\n\t<b8f78724-b6be-3e95-60fa-8c7dcafe2eec@redhat.com>\n\t<20170909220737.GL7570@localhost.localdomain>\n\t<5d20eb5c-85cd-e500-0b98-c044c5176726@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<5d20eb5c-85cd-e500-0b98-c044c5176726@redhat.com>","X-Fnord":"you can see the fnord","User-Agent":"Mutt/1.8.3 (2017-05-23)","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\tMon, 11 Sep 2017 18:22: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 v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","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":"Matthew Rosato <mjrosato@linux.vnet.ibm.com>,\n\tDavid Hildenbrand <david@redhat.com>,\n\tMarkus Armbruster <armbru@redhat.com>, cohuck@redhat.com,\n\tRichard Henderson <richard.henderson@linaro.org>,\n\tAlexander Graf <agraf@suse.de>, qemu-devel@nongnu.org,\n\tborntraeger@de.ibm.com, Paolo Bonzini <pbonzini@redhat.com>","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":1766450,"web_url":"http://patchwork.ozlabs.org/comment/1766450/","msgid":"<20170911175224.GP7570@localhost.localdomain>","list_archive_url":null,"date":"2017-09-11T17:52:24","subject":"Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","submitter":{"id":195,"url":"http://patchwork.ozlabs.org/api/people/195/","name":"Eduardo Habkost","email":"ehabkost@redhat.com"},"content":"On Mon, Sep 11, 2017 at 12:23:10PM +0200, Paolo Bonzini wrote:\n> On 10/09/2017 00:07, Eduardo Habkost wrote:\n> > On Fri, Sep 08, 2017 at 02:46:36PM +0200, David Hildenbrand wrote:\n> >> On 08.09.2017 06:21, Thomas Huth wrote:\n> >>> On 07.09.2017 22:13, David Hildenbrand wrote:\n> >>>> Implemented in sclp.c, so let's move it to the right include file.\n> >>>> Fix up one include. Do a forward declaration of CPUS390XState to fix the\n> >>>> two sclp consoles complaining.\n> >>>>\n> >>>> Signed-off-by: David Hildenbrand <david@redhat.com>\n> >>>> ---\n> >>>>  include/hw/s390x/sclp.h    | 2 ++\n> >>>>  target/s390x/cpu.h         | 1 -\n> >>>>  target/s390x/misc_helper.c | 1 +\n> >>>>  3 files changed, 3 insertions(+), 1 deletion(-)\n> >>>>\n> >>>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h\n> >>>> index a72d096081..4b86a8a293 100644\n> >>>> --- a/include/hw/s390x/sclp.h\n> >>>> +++ b/include/hw/s390x/sclp.h\n> >>>> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev *init_sclp_memory_hotplug_dev(void);\n> >>>>  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);\n> >>>>  void sclp_service_interrupt(uint32_t sccb);\n> >>>>  void raise_irq_cpu_hotplug(void);\n> >>>> +typedef struct CPUS390XState CPUS390XState;\n> >>>> +int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);\n> >>>\n> >>> That's dangerous and likely does not work with certain versions of GCC.\n> >>> You can't do a \"forward declaration\" with typedef in C, I'm afraid. See\n> >>> for example:\n> >>>\n> >>>  https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg01454.html\n> >>>  https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg03337.html\n> >>>  https://stackoverflow.com/questions/8367646/redefinition-of-typedef\n> >>>\n> >>> All this typedef'ing in QEMU is pretty bad ... we run into this problem\n> >>> again and again. include/qemu/typedefs.h is just a work-around for this.\n> >>> I know people like typedefs for some reasons (I used to do that, too,\n> >>> before I realized the trouble with them), but IMHO we should rather\n> >>> adopt the typedef-related rules from the kernel coding conventions instead:\n> >>>\n> >>>  https://www.kernel.org/doc/html/v4.13/process/coding-style.html#typedefs\n> >>>\n> >>>   Thomas\n> >>>\n> >>\n> >> Yes, this is really nasty. And I wasn't aware of the involved issues.\n> >>\n> >> This seems to be the only feasible solution (including cpu.h sounds\n> >> wrong and will require a bunch of other includes):\n> >>\n> >>\n> >> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h\n> >> index a72d096081..ce80915a02 100644\n> >> --- a/include/hw/s390x/sclp.h\n> >> +++ b/include/hw/s390x/sclp.h\n> >> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev\n> >> *init_sclp_memory_hotplug_dev(void);\n> >>  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);\n> >>  void sclp_service_interrupt(uint32_t sccb);\n> >>  void raise_irq_cpu_hotplug(void);\n> >> +struct CPUS390XState;\n> >> +int sclp_service_call(struct CPUS390XState *env, uint64_t sccb,\n> >> uint32_t code);\n> >>\n> >>  #endif\n> > \n> > Why not use typedefs.h?\n> \n> See Markus's reply.  But, maybe it's even better to use S390CPU* and\n> include target/s390x/cpu-qom.h, which by design provides as little\n> definitions as needed.\n\nI don't see an argument against moving typedef CPUS390XState to\ntypedefs.h in Markus' reply.  I see one argument for it (reducing\nthe need for non-cyclic includes).\n\ncpu-qom.h includes cpu.h, so I don't know why using S390CPU*\nwould solve any problem.  I don't disagree about changing the\nfunction to use S390CPU* eventually, but it would still require\nus make a choice between: a) including the header where the\ntypedef name is declared (cpu.h or cpu-qom.h); or b) moving the\ntypedef name declaration to typedefs.h.\n\n> \n> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>\n> > ---\n> > diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h\n> > index 4b86a8a293..3512bf8283 100644\n> > --- a/include/hw/s390x/sclp.h\n> > +++ b/include/hw/s390x/sclp.h\n> > @@ -242,7 +242,6 @@ sclpMemoryHotplugDev *init_sclp_memory_hotplug_dev(void);\n> >  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);\n> >  void sclp_service_interrupt(uint32_t sccb);\n> >  void raise_irq_cpu_hotplug(void);\n> > -typedef struct CPUS390XState CPUS390XState;\n> >  int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);\n> >  \n> >  #endif\n> > diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h\n> > index 39bc8351a3..9c97bffa92 100644\n> > --- a/include/qemu/typedefs.h\n> > +++ b/include/qemu/typedefs.h\n> > @@ -21,6 +21,7 @@ typedef struct Chardev Chardev;\n> >  typedef struct CompatProperty CompatProperty;\n> >  typedef struct CPUAddressSpace CPUAddressSpace;\n> >  typedef struct CPUState CPUState;\n> > +typedef struct CPUS390XState CPUS390XState;\n> >  typedef struct DeviceListener DeviceListener;\n> >  typedef struct DeviceState DeviceState;\n> >  typedef struct DirtyBitmapSnapshot DirtyBitmapSnapshot;\n> > diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h\n> > index 032d1de2e8..f99a82cd5e 100644\n> > --- a/target/s390x/cpu.h\n> > +++ b/target/s390x/cpu.h\n> > @@ -80,7 +80,7 @@ typedef struct MchkQueue {\n> >      uint16_t type;\n> >  } MchkQueue;\n> >  \n> > -typedef struct CPUS390XState {\n> > +struct CPUS390XState {\n> >      uint64_t regs[16];     /* GP registers */\n> >      /*\n> >       * The floating point registers are part of the vector registers.\n> > @@ -174,7 +174,7 @@ typedef struct CPUS390XState {\n> >      /* currently processed sigp order */\n> >      uint8_t sigp_order;\n> >  \n> > -} CPUS390XState;\n> > +};\n> >  \n> >  static inline CPU_DoubleU *get_freg(CPUS390XState *cs, int nr)\n> >  {\n> > \n> > \n> > \n> >> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h\n> >> index 3aa2e46aac..032d1de2e8 100644\n> >> --- a/target/s390x/cpu.h\n> >> +++ b/target/s390x/cpu.h\n> >> @@ -721,6 +721,5 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr,\n> >> uint8_t ar, void *hostbuf,\n> >>\n> >>  /* outside of target/s390x/ */\n> >>  S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);\n> >> -int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);\n> >>\n> >>  #endif\n> >> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c\n> >> index b142db71c6..8b07535b02 100644\n> >> --- a/target/s390x/misc_helper.c\n> >> +++ b/target/s390x/misc_helper.c\n> >> @@ -35,6 +35,7 @@\n> >>  #include \"sysemu/sysemu.h\"\n> >>  #include \"hw/s390x/ebcdic.h\"\n> >>  #include \"hw/s390x/s390-virtio-hcall.h\"\n> >> +#include \"hw/s390x/sclp.h\"\n> >>  #endif\n> >>\n> >>  /* #define DEBUG_HELPER */\n> >>\n> >>\n> >> Opinions?\n> >>\n> >>\n> >>\n> >> -- \n> >>\n> >> Thanks,\n> >>\n> >> David\n> > \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=ehabkost@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 3xrbzN22TKz9sNr\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 12 Sep 2017 04:28:48 +1000 (AEST)","from localhost ([::1]:59826 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 1drTRu-0003ed-BH\n\tfor incoming@patchwork.ozlabs.org; Mon, 11 Sep 2017 14:28:46 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:50084)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <ehabkost@redhat.com>) id 1drSt2-0003Tc-Ee\n\tfor qemu-devel@nongnu.org; Mon, 11 Sep 2017 13:52:46 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <ehabkost@redhat.com>) id 1drSsy-0001mp-B6\n\tfor qemu-devel@nongnu.org; Mon, 11 Sep 2017 13:52:44 -0400","from mx1.redhat.com ([209.132.183.28]:46164)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <ehabkost@redhat.com>) id 1drSsy-0001jq-2L\n\tfor qemu-devel@nongnu.org; Mon, 11 Sep 2017 13:52:40 -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 4777DC0AA55D;\n\tMon, 11 Sep 2017 17:52:38 +0000 (UTC)","from localhost (ovpn-116-15.gru2.redhat.com [10.97.116.15])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id A0A9B7053A;\n\tMon, 11 Sep 2017 17:52:25 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 4777DC0AA55D","Date":"Mon, 11 Sep 2017 14:52:24 -0300","From":"Eduardo Habkost <ehabkost@redhat.com>","To":"Paolo Bonzini <pbonzini@redhat.com>","Message-ID":"<20170911175224.GP7570@localhost.localdomain>","References":"<20170907201335.13956-1-david@redhat.com>\n\t<20170907201335.13956-9-david@redhat.com>\n\t<1603f6bf-f481-13c2-c7eb-ac155c5a14cc@redhat.com>\n\t<b8f78724-b6be-3e95-60fa-8c7dcafe2eec@redhat.com>\n\t<20170909220737.GL7570@localhost.localdomain>\n\t<c758ae70-a664-d13a-dd24-768f71e3f2e4@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<c758ae70-a664-d13a-dd24-768f71e3f2e4@redhat.com>","X-Fnord":"you can see the fnord","User-Agent":"Mutt/1.8.3 (2017-05-23)","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.31]);\n\tMon, 11 Sep 2017 17:52:38 +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 v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","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":"Matthew Rosato <mjrosato@linux.vnet.ibm.com>,\n\tThomas Huth <thuth@redhat.com>, David Hildenbrand <david@redhat.com>, \n\tMarkus Armbruster <armbru@redhat.com>, cohuck@redhat.com,\n\tRichard Henderson <richard.henderson@linaro.org>,\n\tqemu-devel@nongnu.org, Alexander Graf <agraf@suse.de>,\n\tborntraeger@de.ibm.com","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":1766452,"web_url":"http://patchwork.ozlabs.org/comment/1766452/","msgid":"<20170911180632.GQ7570@localhost.localdomain>","list_archive_url":null,"date":"2017-09-11T18:06:32","subject":"Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","submitter":{"id":195,"url":"http://patchwork.ozlabs.org/api/people/195/","name":"Eduardo Habkost","email":"ehabkost@redhat.com"},"content":"On Mon, Sep 11, 2017 at 07:56:19PM +0200, David Hildenbrand wrote:\n> \n> >>>>\n> >>>>  #endif\n> >>>\n> >>> Why not use typedefs.h?\n> >>\n> >> See Markus's reply.  But, maybe it's even better to use S390CPU* and\n> >> include target/s390x/cpu-qom.h, which by design provides as little\n> >> definitions as needed.\n> > \n> > I don't see an argument against moving typedef CPUS390XState to\n> > typedefs.h in Markus' reply.  I see one argument for it (reducing\n> > the need for non-cyclic includes).\n> > \n> > cpu-qom.h includes cpu.h, so I don't know why using S390CPU*\n> > would solve any problem.  I don't disagree about changing the\n> > function to use S390CPU* eventually, but it would still require\n> > us make a choice between: a) including the header where the\n> > typedef name is declared (cpu.h or cpu-qom.h); or b) moving the\n> > typedef name declaration to typedefs.h.\n> \n> It includes qom/cpu.h, not cpu.h. That's why using cpu-qom.h for such\n> typedefs works (see v4).\n> \n\nOops, I was looking at an older tree (before commit 347b1a5c).\nYou're right, sorry for the noise.","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-mx10.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=ehabkost@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 3xrc2H0bh4z9sNr\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 12 Sep 2017 04:31:19 +1000 (AEST)","from localhost ([::1]:59868 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 1drTUK-00062K-UH\n\tfor incoming@patchwork.ozlabs.org; Mon, 11 Sep 2017 14:31:16 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:56898)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <ehabkost@redhat.com>) id 1drT6e-00081T-8T\n\tfor qemu-devel@nongnu.org; Mon, 11 Sep 2017 14:06:49 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <ehabkost@redhat.com>) id 1drT6Y-0005aR-Bq\n\tfor qemu-devel@nongnu.org; Mon, 11 Sep 2017 14:06:48 -0400","from mx1.redhat.com ([209.132.183.28]:39814)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <ehabkost@redhat.com>) id 1drT6Y-0005Za-6N\n\tfor qemu-devel@nongnu.org; Mon, 11 Sep 2017 14:06:42 -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 14BAE9944E;\n\tMon, 11 Sep 2017 18:06:41 +0000 (UTC)","from localhost (ovpn-116-15.gru2.redhat.com [10.97.116.15])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 9894C6442B;\n\tMon, 11 Sep 2017 18:06:35 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 14BAE9944E","Date":"Mon, 11 Sep 2017 15:06:32 -0300","From":"Eduardo Habkost <ehabkost@redhat.com>","To":"David Hildenbrand <david@redhat.com>","Message-ID":"<20170911180632.GQ7570@localhost.localdomain>","References":"<20170907201335.13956-1-david@redhat.com>\n\t<20170907201335.13956-9-david@redhat.com>\n\t<1603f6bf-f481-13c2-c7eb-ac155c5a14cc@redhat.com>\n\t<b8f78724-b6be-3e95-60fa-8c7dcafe2eec@redhat.com>\n\t<20170909220737.GL7570@localhost.localdomain>\n\t<c758ae70-a664-d13a-dd24-768f71e3f2e4@redhat.com>\n\t<20170911175224.GP7570@localhost.localdomain>\n\t<8cf0bf17-c1cb-ee13-768c-dcf1836687c2@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<8cf0bf17-c1cb-ee13-768c-dcf1836687c2@redhat.com>","X-Fnord":"you can see the fnord","User-Agent":"Mutt/1.8.3 (2017-05-23)","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.39]);\n\tMon, 11 Sep 2017 18:06:41 +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 v3 08/21] s390x: move sclp_service_call()\n\tto sclp.h","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":"Matthew Rosato <mjrosato@linux.vnet.ibm.com>,\n\tThomas Huth <thuth@redhat.com>,\n\tMarkus Armbruster <armbru@redhat.com>, cohuck@redhat.com,\n\tRichard Henderson <richard.henderson@linaro.org>,\n\tqemu-devel@nongnu.org, Alexander Graf <agraf@suse.de>,\n\tborntraeger@de.ibm.com, Paolo Bonzini <pbonzini@redhat.com>","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>"}}]