[{"id":1767018,"web_url":"http://patchwork.ozlabs.org/comment/1767018/","msgid":"<20170912150737.395a5045@nial.brq.redhat.com>","list_archive_url":null,"date":"2017-09-12T13:07:37","subject":"Re: [Qemu-devel] [PATCH v4 13/21] target/s390x: use \"core-id\" for\n\tcpu number/address/id handling","submitter":{"id":11305,"url":"http://patchwork.ozlabs.org/api/people/11305/","name":"Igor Mammedov","email":"imammedo@redhat.com"},"content":"On Mon, 11 Sep 2017 17:21:42 +0200\nDavid Hildenbrand wrote:\n\n> Some time ago we discussed that using \"id\" as property name is not the\n> right thing to do, as it is a reserved property for other devices and\n> will not work with device_add.\n> \n> Switch to the term \"core-id\" instead, and use it as an equivalent to\n> \"CPU address\" mentioned in the PoP. There is no such thing as cpu number,\n> so rename env.cpu_num to env.core_id. We use \"core-id\" as this is the\n> common term to use for device_add later on (x86 and ppc).\nis there possibility that later The core (something that contains threads)\nwould appear/exist in real hw?\n(my concern here is that we would have to use some other name for it as\nrenaming of already shipped public property name would be not an option)\n\n\n> We can get rid of cpu->id now. Keep cpu_index and env->core_id in sync.\n> cpu_index was already implicitly used by e.g. cpu_exists(), so keeping\n> both in sync seems to be the right thing to do.\n> \n> cpu_index will now no longer automatically get set via\n> cpu_exec_realizefn(). For now, we were lucky that both implicitly stayed\n> in sync.\n> \n> Our new cpu property \"core-id\" can be a static property. Range checks can\n> be avoided by using the correct type and the \"setting after realized\"\n> check is done implicitly.\n> \n> device_add will later need the reserved \"id\" property. Hotplugging a CPU\n> on s390x will then be: \"device_add host-s390-cpu,id=cpu2,core-id=2\".\n> \n> Reviewed-by: Matthew Rosato \n> Signed-off-by: David Hildenbrand \n> ---\n[...]\n\n> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c\n> index f3624d75eb..293fc8428a 100644\n> --- a/target/s390x/misc_helper.c\n> +++ b/target/s390x/misc_helper.c\n> @@ -232,7 +232,7 @@ uint32_t HELPER(stsi)(CPUS390XState *env, uint64_t a0,\n> /* XXX make different for different CPUs? */\n> ebcdic_put(sysib.sequence, \"QEMUQEMUQEMUQEMU\", 16);\n> ebcdic_put(sysib.plant, \"QEMU\", 4);\n> - stw_p(&sysib.cpu_addr, env->cpu_num);\n> + stw_p(&sysib.cpu_addr, env->core_id);\n> cpu_physical_memory_write(a0, &sysib, sizeof(sysib));\n> } else if ((sel1 == 2) && (sel2 == 2)) {\n> /* Basic Machine CPUs */\n> @@ -260,7 +260,7 @@ uint32_t HELPER(stsi)(CPUS390XState *env, uint64_t a0,\n> /* XXX make different for different CPUs? */\n> ebcdic_put(sysib.sequence, \"QEMUQEMUQEMUQEMU\", 16);\n> ebcdic_put(sysib.plant, \"QEMU\", 4);\n> - stw_p(&sysib.cpu_addr, env->cpu_num);\n> + stw_p(&sysib.cpu_addr, env->core_id);\n> stw_p(&sysib.cpu_id, 0);\n> cpu_physical_memory_write(a0, &sysib, sizeof(sysib));\n> } else if ((sel1 == 2) && (sel2 == 2)) {\n> diff --git a/target/s390x/translate.c b/target/s390x/translate.c\n> index 909b12818d..5abd34fb34 100644\n> --- a/target/s390x/translate.c\n> +++ b/target/s390x/translate.c\n> @@ -3823,10 +3823,7 @@ static ExitStatus op_ssm(DisasContext *s, DisasOps *o)\n> static ExitStatus op_stap(DisasContext *s, DisasOps *o)\n> {\n> check_privileged(s);\n> - /* ??? Surely cpu address != cpu number. In any case the previous\n> - version of this stored more than the required half-word, so it\n> - is unlikely this has ever been tested. */\n> - tcg_gen_ld32u_i64(o->out, cpu_env, offsetof(CPUS390XState, cpu_num));\n> + tcg_gen_ld32u_i64(o->out, cpu_env, offsetof(CPUS390XState, core_id));\n> return NO_EXIT;\n> }\nI see core_id is used in several instructions,\ndoes it really have any influence on code executed by *-user target?","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=208.118.235.17; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","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=imammedo@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 3xs4q43tQvz9rxl\n\tfor ;\n\tTue, 12 Sep 2017 23:08:16 +1000 (AEST)","from localhost ([::1]:35760 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1drkvG-0000Pi-KV\n\tfor incoming@patchwork.ozlabs.org; Tue, 12 Sep 2017 09:08:14 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:34964)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1drkur-0000MJ-Kr\n\tfor qemu-devel@nongnu.org; Tue, 12 Sep 2017 09:07:55 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1drkun-0003Qz-Ct\n\tfor qemu-devel@nongnu.org; Tue, 12 Sep 2017 09:07:49 -0400","from mx1.redhat.com ([209.132.183.28]:26601)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1drkun-0003P7-3Z\n\tfor qemu-devel@nongnu.org; Tue, 12 Sep 2017 09:07:45 -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 331CEC04B92A;\n\tTue, 12 Sep 2017 13:07:44 +0000 (UTC)","from nial.brq.redhat.com (unknown [10.43.2.209])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 85F6060BEB;\n\tTue, 12 Sep 2017 13:07:38 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 331CEC04B92A","Date":"Tue, 12 Sep 2017 15:07:37 +0200","From":"Igor Mammedov ","To":"David Hildenbrand ","Message-ID":"<20170912150737.395a5045@nial.brq.redhat.com>","In-Reply-To":"<20170911152150.12535-14-david@redhat.com>","References":"<20170911152150.12535-1-david@redhat.com>\n\t<20170911152150.12535-14-david@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.31]);\n\tTue, 12 Sep 2017 13:07:44 +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 v4 13/21] target/s390x: use \"core-id\" for\n\tcpu number/address/id handling","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Matthew Rosato , thuth@redhat.com,\n\tEduardo Habkost ,\n\tMarkus Armbruster , cohuck@redhat.com,\n\tRichard Henderson ,\n\tqemu-devel@nongnu.org, Alexander Graf ,\n\tborntraeger@de.ibm.com, Paolo Bonzini ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1767026,"web_url":"http://patchwork.ozlabs.org/comment/1767026/","msgid":"<55850e26-53da-474a-328c-f56bad58774b@redhat.com>","list_archive_url":null,"date":"2017-09-12T13:15:34","subject":"Re: [Qemu-devel] [PATCH v4 13/21] target/s390x: use \"core-id\" for\n\tcpu number/address/id handling","submitter":{"id":70402,"url":"http://patchwork.ozlabs.org/api/people/70402/","name":"David Hildenbrand","email":"david@redhat.com"},"content":"On 12.09.2017 15:07, Igor Mammedov wrote:\n> On Mon, 11 Sep 2017 17:21:42 +0200\n> David Hildenbrand wrote:\n> \n>> Some time ago we discussed that using \"id\" as property name is not the\n>> right thing to do, as it is a reserved property for other devices and\n>> will not work with device_add.\n>>\n>> Switch to the term \"core-id\" instead, and use it as an equivalent to\n>> \"CPU address\" mentioned in the PoP. There is no such thing as cpu number,\n>> so rename env.cpu_num to env.core_id. We use \"core-id\" as this is the\n>> common term to use for device_add later on (x86 and ppc).\n> is there possibility that later The core (something that contains threads)\n> would appear/exist in real hw?\n> (my concern here is that we would have to use some other name for it as\n> renaming of already shipped public property name would be not an option)\n\nThere is the possibility (s390x has SMP support starting with z13), but\nit is really, really unlikely for KVM. There would be the chance of\nimplementing this somewhen in the future for TCG (although I also doubt\nthis will happen in the near future).\n\nIf so, I think there will only be one solution: a new machine type.\n\nFaking threads now would be just plain wrong. So I think this should\njust be fine for now.\n\n[...]\n>> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c\n>> index f3624d75eb..293fc8428a 100644\n>> --- a/target/s390x/misc_helper.c\n>> +++ b/target/s390x/misc_helper.c\n>> @@ -232,7 +232,7 @@ uint32_t HELPER(stsi)(CPUS390XState *env, uint64_t a0,\n>> /* XXX make different for different CPUs? */\n>> ebcdic_put(sysib.sequence, \"QEMUQEMUQEMUQEMU\", 16);\n>> ebcdic_put(sysib.plant, \"QEMU\", 4);\n>> - stw_p(&sysib.cpu_addr, env->cpu_num);\n>> + stw_p(&sysib.cpu_addr, env->core_id);\n>> cpu_physical_memory_write(a0, &sysib, sizeof(sysib));\n>> } else if ((sel1 == 2) && (sel2 == 2)) {\n>> /* Basic Machine CPUs */\n>> @@ -260,7 +260,7 @@ uint32_t HELPER(stsi)(CPUS390XState *env, uint64_t a0,\n>> /* XXX make different for different CPUs? */\n>> ebcdic_put(sysib.sequence, \"QEMUQEMUQEMUQEMU\", 16);\n>> ebcdic_put(sysib.plant, \"QEMU\", 4);\n>> - stw_p(&sysib.cpu_addr, env->cpu_num);\n>> + stw_p(&sysib.cpu_addr, env->core_id);\n>> stw_p(&sysib.cpu_id, 0);\n>> cpu_physical_memory_write(a0, &sysib, sizeof(sysib));\n>> } else if ((sel1 == 2) && (sel2 == 2)) {\n>> diff --git a/target/s390x/translate.c b/target/s390x/translate.c\n>> index 909b12818d..5abd34fb34 100644\n>> --- a/target/s390x/translate.c\n>> +++ b/target/s390x/translate.c\n>> @@ -3823,10 +3823,7 @@ static ExitStatus op_ssm(DisasContext *s, DisasOps *o)\n>> static ExitStatus op_stap(DisasContext *s, DisasOps *o)\n>> {\n>> check_privileged(s);\n>> - /* ??? Surely cpu address != cpu number. In any case the previous\n>> - version of this stored more than the required half-word, so it\n>> - is unlikely this has ever been tested. */\n>> - tcg_gen_ld32u_i64(o->out, cpu_env, offsetof(CPUS390XState, cpu_num));\n>> + tcg_gen_ld32u_i64(o->out, cpu_env, offsetof(CPUS390XState, core_id));\n>> return NO_EXIT;\n>> }\n> I see core_id is used in several instructions,\n> does it really have any influence on code executed by *-user target?\n> \nNope, these should all be privileged instructions and therefore not\nrelevant for -user targets.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx09.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx09.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=david@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xs5062lT0z9s0g\n\tfor ;\n\tTue, 12 Sep 2017 23:16:06 +1000 (AEST)","from localhost ([::1]:35802 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1drl2o-0003eG-Ee\n\tfor incoming@patchwork.ozlabs.org; Tue, 12 Sep 2017 09:16:03 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:38728)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1drl2V-0003e4-Hh\n\tfor qemu-devel@nongnu.org; Tue, 12 Sep 2017 09:15:44 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1drl2S-0007UL-9g\n\tfor qemu-devel@nongnu.org; Tue, 12 Sep 2017 09:15:43 -0400","from mx1.redhat.com ([209.132.183.28]:37424)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1drl2S-0007Te-1J\n\tfor qemu-devel@nongnu.org; Tue, 12 Sep 2017 09:15: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 F1FA64ACA4;\n\tTue, 12 Sep 2017 13:15:38 +0000 (UTC)","from [10.36.117.114] (ovpn-117-114.ams2.redhat.com [10.36.117.114])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 7E42277BE1;\n\tTue, 12 Sep 2017 13:15:34 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com F1FA64ACA4","To":"Igor Mammedov ","References":"<20170911152150.12535-1-david@redhat.com>\n\t<20170911152150.12535-14-david@redhat.com>\n\t<20170912150737.395a5045@nial.brq.redhat.com>","From":"David Hildenbrand ","Organization":"Red Hat GmbH","Message-ID":"<55850e26-53da-474a-328c-f56bad58774b@redhat.com>","Date":"Tue, 12 Sep 2017 15:15:34 +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":"<20170912150737.395a5045@nial.brq.redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.38]);\n\tTue, 12 Sep 2017 13:15: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 v4 13/21] target/s390x: use \"core-id\" for\n\tcpu number/address/id handling","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Matthew Rosato , thuth@redhat.com,\n\tEduardo Habkost ,\n\tMarkus Armbruster , cohuck@redhat.com,\n\tRichard Henderson ,\n\tqemu-devel@nongnu.org, Alexander Graf ,\n\tborntraeger@de.ibm.com, Paolo Bonzini ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]