[{"id":1767065,"web_url":"http://patchwork.ozlabs.org/comment/1767065/","msgid":"<20170912154313.3616c2bf@nial.brq.redhat.com>","list_archive_url":null,"date":"2017-09-12T13:43:13","subject":"Re: [Qemu-devel] [PATCH v4 18/21] s390x: implement\n\tquery-hotpluggable-cpus","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:47 +0200\nDavid Hildenbrand wrote:\n\n> CPU hotplug is only possible on a per core basis on s390x.\n> \n> As we now have ms->possible_cpus, we can get rid of the global variable\n> cpu_states.\n> \n> While rewriting s390_cpu_addr2state() completely to be based on\n> possible_cpus, move it to cpu.c, as it is independent of the virtio-ccw\n> machine.\nI'd split patch on\n 1) introduce possible cpus \n 2) rewrite s390_cpu_addr2state() using #2\n\n> Signed-off-by: David Hildenbrand \n> ---\n> hw/s390x/s390-virtio-ccw.c | 87 +++++++++++++++++++++++++++++-----------------\n> qapi-schema.json | 16 +++++++++\n> target/s390x/cpu.c | 17 +++++++++\n> target/s390x/cpu.h | 5 +--\n> 4 files changed, 89 insertions(+), 36 deletions(-)\n> \n> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c\n> index 120f82e339..0e10a4c73a 100644\n> --- a/hw/s390x/s390-virtio-ccw.c\n> +++ b/hw/s390x/s390-virtio-ccw.c\n> @@ -36,24 +36,12 @@\n> #include \"qapi/qmp/qerror.h\"\n> #include \"hw/nmi.h\"\n> \n> -static S390CPU **cpu_states;\n> -\n> -S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)\n> -{\n> - if (cpu_addr >= max_cpus) {\n> - return NULL;\n> - }\n> -\n> - /* Fast lookup via CPU ID */\n> - return cpu_states[cpu_addr];\n> -}\n> -\n> /* #define S390_TCG_SMP_SUPPORT */\n> \n> static void s390_init_cpus(MachineState *machine)\n> {\n> + MachineClass *mc = MACHINE_GET_CLASS(machine);\n> int i;\n> - gchar *name;\n> \n> if (machine->cpu_model == NULL) {\n> machine->cpu_model = s390_default_cpu_model_name();\n> @@ -66,17 +54,8 @@ static void s390_init_cpus(MachineState *machine)\n> }\n> #endif\n> \n> - cpu_states = g_new0(S390CPU *, max_cpus);\n> -\n> - for (i = 0; i < max_cpus; i++) {\n> - name = g_strdup_printf(\"cpu[%i]\", i);\n> - object_property_add_link(OBJECT(machine), name, TYPE_S390_CPU,\n> - (Object **) &cpu_states[i],\n> - object_property_allow_set_link,\n> - OBJ_PROP_LINK_UNREF_ON_RELEASE,\n> - &error_abort);\n> - g_free(name);\n> - }\n> + /* initialize possible_cpus */\n> + mc->possible_cpu_arch_ids(machine);\n> \n> for (i = 0; i < smp_cpus; i++) {\n> s390x_new_cpu(machine->cpu_model, i, &error_fatal);\n> @@ -307,17 +286,14 @@ static void ccw_init(MachineState *machine)\n> register_savevm_live(NULL, \"todclock\", 0, 1, &savevm_gtod, NULL);\n> }\n> \n> -static void s390_cpu_plug(HotplugHandler *hotplug_dev,\n> - DeviceState *dev, Error **errp)\n> +static void s390_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev,\n> + Error **errp)\n> {\n> - gchar *name;\n> + MachineState *ms = MACHINE(hotplug_dev);\n> S390CPU *cpu = S390_CPU(dev);\n> - CPUState *cs = CPU(dev);\n> \n> - name = g_strdup_printf(\"cpu[%i]\", cpu->env.core_id);\n> - object_property_set_link(OBJECT(hotplug_dev), OBJECT(cs), name,\n> - errp);\n> - g_free(name);\n> + g_assert(!ms->possible_cpus->cpus[cpu->env.core_id].cpu);\n> + ms->possible_cpus->cpus[cpu->env.core_id].cpu = OBJECT(dev);\n> }\n> \n> static void s390_machine_reset(void)\n> @@ -350,6 +326,50 @@ static void s390_machine_device_unplug_request(HotplugHandler *hotplug_dev,\n> }\n> }\n> \n> +static CPUArchId *s390_find_cpu_slot(MachineState *ms, uint32_t core_id,\n> + int *idx)\n> +{\n> + if (core_id >= ms->possible_cpus->len) {\n> + return NULL;\n> + }\n> + /* core_id corresponds to the index */\n> + if (idx) {\n> + *idx = core_id;\n> + }\n> + return &ms->possible_cpus->cpus[core_id];\n> +}\nit looks like cpu_index == core_id == idx in possible_cpus,\nis this helper really necessary?\n(we have it in x86 because of possible not 1:1 mapping)\n\nI'd drop it and just access array directly\n\n> +\n> +static CpuInstanceProperties s390_cpu_index_to_props(MachineState *machine,\n> + unsigned cpu_index)\n> +{\n> + CPUArchId *slot = s390_find_cpu_slot(machine, cpu_index, NULL);\n> +\n> + assert(slot);\n> + return slot->props;\n> +}\n> +\n> +static const CPUArchIdList *s390_possible_cpu_arch_ids(MachineState *ms)\n> +{\n> + int i;\n> +\n> + if (ms->possible_cpus) {\n> + g_assert(ms->possible_cpus && ms->possible_cpus->len == max_cpus);\n> + return ms->possible_cpus;\n> + }\n> +\n> + ms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +\n> + sizeof(CPUArchId) * max_cpus);\n> + ms->possible_cpus->len = max_cpus;\n> + for (i = 0; i < ms->possible_cpus->len; i++) {\n> + ms->possible_cpus->cpus[i].vcpus_count = 1;\n> + ms->possible_cpus->cpus[i].arch_id = i;\n> + ms->possible_cpus->cpus[i].props.has_core_id = true;\n> + ms->possible_cpus->cpus[i].props.core_id = i;\n> + }\n> +\n> + return ms->possible_cpus;\n> +}\n> +\n> static HotplugHandler *s390_get_hotplug_handler(MachineState *machine,\n> DeviceState *dev)\n> {\n> @@ -397,7 +417,10 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)\n> mc->no_sdcard = 1;\n> mc->use_sclp = 1;\n> mc->max_cpus = 248;\n> + mc->has_hotpluggable_cpus = true;\n> mc->get_hotplug_handler = s390_get_hotplug_handler;\n> + mc->cpu_index_to_instance_props = s390_cpu_index_to_props;\n> + mc->possible_cpu_arch_ids = s390_possible_cpu_arch_ids;\n> hc->plug = s390_machine_device_plug;\n> hc->unplug_request = s390_machine_device_unplug_request;\n> nc->nmi_monitor_handler = s390_nmi;\n> diff --git a/qapi-schema.json b/qapi-schema.json\n> index f3af2cb851..79e9f85404 100644\n> --- a/qapi-schema.json\n> +++ b/qapi-schema.json\n> @@ -3121,6 +3121,22 @@\n> # }\n> # ]}\n> #\n> +# For s390x-virtio-ccw machine type started with -smp 1,maxcpus=2 -cpu qemu\n> +# (Since: 2.11):\n> +#\n> +# -> { \"execute\": \"query-hotpluggable-cpus\" }\n> +# <- {\"return\": [\n> +# {\n> +# \"type\": \"qemu-s390-cpu\", \"vcpus-count\": 1,\n> +# \"props\": { \"core-id\": 1 }\n> +# },\n> +# {\n> +# \"qom-path\": \"/machine/unattached/device[0]\",\n> +# \"type\": \"qemu-s390-cpu\", \"vcpus-count\": 1,\n> +# \"props\": { \"core-id\": 0 }\n> +# }\n> +# ]}\n> +#\n> ##\n> { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }\n> \n> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c\n> index 5724dffb88..7f174f90a4 100644\n> --- a/target/s390x/cpu.c\n> +++ b/target/s390x/cpu.c\n> @@ -38,6 +38,7 @@\n> #include \"exec/exec-all.h\"\n> #include \"hw/qdev-properties.h\"\n> #ifndef CONFIG_USER_ONLY\n> +#include \"hw/boards.h\"\n> #include \"hw/hw.h\"\n> #include \"sysemu/arch_init.h\"\n> #include \"sysemu/sysemu.h\"\n> @@ -446,6 +447,22 @@ void s390_enable_css_support(S390CPU *cpu)\n> kvm_s390_enable_css_support(cpu);\n> }\n> }\n> +\n> +S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)\n> +{\ntarget/cpu.c and cpu itself preferably shouldn't pull in\nor depend on machine, so I'd keep s390_cpu_addr2state() where it's now\nor somewhere in board related files\n\n\n> + static MachineState *ms;\n> +\n> + if (!ms) {\n> + ms = MACHINE(qdev_get_machine());\n> + g_assert(ms->possible_cpus);\n> + }\n> +\n> + /* CPU address corresponds to the core_id and the index */\n> + if (cpu_addr >= ms->possible_cpus->len) {\n> + return NULL;\n> + }\n> + return S390_CPU(ms->possible_cpus->cpus[cpu_addr].cpu);\n> +}\n> #endif\n> \n> static gchar *s390_gdb_arch_name(CPUState *cs)\n> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h\n> index 1c8456fa57..5810079f48 100644\n> --- a/target/s390x/cpu.h\n> +++ b/target/s390x/cpu.h\n> @@ -671,6 +671,7 @@ int s390_cpu_restart(S390CPU *cpu);\n> void s390_enable_css_support(S390CPU *cpu);\n> int s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch_id,\n> int vq, bool assign);\n> +S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);\n> #ifndef CONFIG_USER_ONLY\n> unsigned int s390_cpu_set_state(uint8_t cpu_state, S390CPU *cpu);\n> #else\n> @@ -718,8 +719,4 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,\n> #define s390_cpu_virt_mem_check_write(cpu, laddr, ar, len) \\\n> s390_cpu_virt_mem_rw(cpu, laddr, ar, NULL, len, true)\n> \n> -\n> -/* outside of target/s390x/ */\n> -S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);\n> -\n> #endif","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=imammedo@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 3xs5c36vQdz9s5L\n\tfor ;\n\tTue, 12 Sep 2017 23:43:47 +1000 (AEST)","from localhost ([::1]:35974 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1drlTe-00052S-4q\n\tfor incoming@patchwork.ozlabs.org; Tue, 12 Sep 2017 09:43:46 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:49069)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1drlTH-000527-A7\n\tfor qemu-devel@nongnu.org; Tue, 12 Sep 2017 09:43:24 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1drlTE-0000t6-4G\n\tfor qemu-devel@nongnu.org; Tue, 12 Sep 2017 09:43:23 -0400","from mx1.redhat.com ([209.132.183.28]:58394)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1drlTD-0000sp-R8\n\tfor qemu-devel@nongnu.org; Tue, 12 Sep 2017 09:43:20 -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 85A434E047;\n\tTue, 12 Sep 2017 13:43:18 +0000 (UTC)","from nial.brq.redhat.com (unknown [10.43.2.209])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 61E7F77DDC;\n\tTue, 12 Sep 2017 13:43:14 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 85A434E047","Date":"Tue, 12 Sep 2017 15:43:13 +0200","From":"Igor Mammedov ","To":"David Hildenbrand ","Message-ID":"<20170912154313.3616c2bf@nial.brq.redhat.com>","In-Reply-To":"<20170911152150.12535-19-david@redhat.com>","References":"<20170911152150.12535-1-david@redhat.com>\n\t<20170911152150.12535-19-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.12","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:43:18 +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 18/21] s390x: implement\n\tquery-hotpluggable-cpus","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":1767090,"web_url":"http://patchwork.ozlabs.org/comment/1767090/","msgid":"<9e1ea879-6c89-23bc-b775-062343d782af@redhat.com>","list_archive_url":null,"date":"2017-09-12T14:03:04","subject":"Re: [Qemu-devel] [PATCH v4 18/21] s390x: implement\n\tquery-hotpluggable-cpus","submitter":{"id":70402,"url":"http://patchwork.ozlabs.org/api/people/70402/","name":"David Hildenbrand","email":"david@redhat.com"},"content":"On 12.09.2017 15:43, Igor Mammedov wrote:\n> On Mon, 11 Sep 2017 17:21:47 +0200\n> David Hildenbrand wrote:\n> \n>> CPU hotplug is only possible on a per core basis on s390x.\n>>\n>> As we now have ms->possible_cpus, we can get rid of the global variable\n>> cpu_states.\n>>\n>> While rewriting s390_cpu_addr2state() completely to be based on\n>> possible_cpus, move it to cpu.c, as it is independent of the virtio-ccw\n>> machine.\n> I'd split patch on\n> 1) introduce possible cpus \n> 2) rewrite s390_cpu_addr2state() using #2\n\nThan I have to keep the global variable + setting it for one patch.\nMight not be worth the trouble. Will have a look.\n\n[...]\n>> \n>> +static CPUArchId *s390_find_cpu_slot(MachineState *ms, uint32_t core_id,\n>> + int *idx)\n>> +{\n>> + if (core_id >= ms->possible_cpus->len) {\n>> + return NULL;\n>> + }\n>> + /* core_id corresponds to the index */\n>> + if (idx) {\n>> + *idx = core_id;\n>> + }\n>> + return &ms->possible_cpus->cpus[core_id];\n>> +}\n> it looks like cpu_index == core_id == idx in possible_cpus,\n> is this helper really necessary?\n> (we have it in x86 because of possible not 1:1 mapping)\n> \n> I'd drop it and just access array directly\n\nJust kept this because the other architectures also have this. I can of\ncourse drop it. The nice thing about this helper is the comment :)\n\n[...]\n>> }\n>> +\n>> +S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)\n>> +{\n> target/cpu.c and cpu itself preferably shouldn't pull in\n> or depend on machine, so I'd keep s390_cpu_addr2state() where it's now\n> or somewhere in board related files\n\nThomas requested this. I actually don't care., but it looks like a\ngeneric \"get_cpu_by_arch_id\" function. But I don't really want to go\nthat additional path now. And also I don't want to move this back and forth.\n\nThomas, what's your opinion?\n\nThanks!","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-mx01.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=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 3xs63b5fGqz9s7B\n\tfor ;\n\tWed, 13 Sep 2017 00:04:11 +1000 (AEST)","from localhost ([::1]:36061 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1drlnN-0007sW-QO\n\tfor incoming@patchwork.ozlabs.org; Tue, 12 Sep 2017 10:04:09 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:56679)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1drlmZ-0007nu-9j\n\tfor qemu-devel@nongnu.org; Tue, 12 Sep 2017 10:03:23 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1drlmT-0003ga-B1\n\tfor qemu-devel@nongnu.org; Tue, 12 Sep 2017 10:03:19 -0400","from mx1.redhat.com ([209.132.183.28]:42600)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1drlmT-0003gO-4V\n\tfor qemu-devel@nongnu.org; Tue, 12 Sep 2017 10:03:13 -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 1ACA281E0C;\n\tTue, 12 Sep 2017 14:03:12 +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 41D006685C;\n\tTue, 12 Sep 2017 14:03:04 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 1ACA281E0C","To":"Igor Mammedov ","References":"<20170911152150.12535-1-david@redhat.com>\n\t<20170911152150.12535-19-david@redhat.com>\n\t<20170912154313.3616c2bf@nial.brq.redhat.com>","From":"David Hildenbrand ","Organization":"Red Hat GmbH","Message-ID":"<9e1ea879-6c89-23bc-b775-062343d782af@redhat.com>","Date":"Tue, 12 Sep 2017 16:03:04 +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":"<20170912154313.3616c2bf@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.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.25]);\n\tTue, 12 Sep 2017 14:03: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 v4 18/21] s390x: implement\n\tquery-hotpluggable-cpus","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":1767121,"web_url":"http://patchwork.ozlabs.org/comment/1767121/","msgid":"","list_archive_url":null,"date":"2017-09-12T14:24:11","subject":"Re: [Qemu-devel] [PATCH v4 18/21] s390x: implement\n\tquery-hotpluggable-cpus","submitter":{"id":66152,"url":"http://patchwork.ozlabs.org/api/people/66152/","name":"Thomas Huth","email":"thuth@redhat.com"},"content":"On 12.09.2017 16:03, David Hildenbrand wrote:\n> On 12.09.2017 15:43, Igor Mammedov wrote:\n>> On Mon, 11 Sep 2017 17:21:47 +0200\n>> David Hildenbrand wrote:\n>>\n>>> CPU hotplug is only possible on a per core basis on s390x.\n>>>\n>>> As we now have ms->possible_cpus, we can get rid of the global variable\n>>> cpu_states.\n>>>\n>>> While rewriting s390_cpu_addr2state() completely to be based on\n>>> possible_cpus, move it to cpu.c, as it is independent of the virtio-ccw\n>>> machine.\n>> I'd split patch on\n>> 1) introduce possible cpus \n>> 2) rewrite s390_cpu_addr2state() using #2\n> \n> Than I have to keep the global variable + setting it for one patch.\n> Might not be worth the trouble. Will have a look.\n> \n> [...]\n>>> \n>>> +static CPUArchId *s390_find_cpu_slot(MachineState *ms, uint32_t core_id,\n>>> + int *idx)\n>>> +{\n>>> + if (core_id >= ms->possible_cpus->len) {\n>>> + return NULL;\n>>> + }\n>>> + /* core_id corresponds to the index */\n>>> + if (idx) {\n>>> + *idx = core_id;\n>>> + }\n>>> + return &ms->possible_cpus->cpus[core_id];\n>>> +}\n>> it looks like cpu_index == core_id == idx in possible_cpus,\n>> is this helper really necessary?\n>> (we have it in x86 because of possible not 1:1 mapping)\n>>\n>> I'd drop it and just access array directly\n> \n> Just kept this because the other architectures also have this. I can of\n> course drop it. The nice thing about this helper is the comment :)\n> \n> [...]\n>>> }\n>>> +\n>>> +S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)\n>>> +{\n>> target/cpu.c and cpu itself preferably shouldn't pull in\n>> or depend on machine, so I'd keep s390_cpu_addr2state() where it's now\n>> or somewhere in board related files\n> \n> Thomas requested this. I actually don't care., but it looks like a\n> generic \"get_cpu_by_arch_id\" function. But I don't really want to go\n> that additional path now. And also I don't want to move this back and forth.\n> \n> Thomas, what's your opinion?\n\nHmm, since the new version uses MachineState now, it's maybe really\nbetter to leave it in the hw/s390x/ directory, I guess. Sorry for the\ninconvenience ...\n\n Thomas","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-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=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 3xs6Wp1pNpz9s0Z\n\tfor ;\n\tWed, 13 Sep 2017 00:25:09 +1000 (AEST)","from localhost ([::1]:36188 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1drm7e-0007G4-G5\n\tfor incoming@patchwork.ozlabs.org; Tue, 12 Sep 2017 10:25:06 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:41650)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1drm79-0007Fw-Q9\n\tfor qemu-devel@nongnu.org; Tue, 12 Sep 2017 10:24:36 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1drm74-0002gt-9v\n\tfor qemu-devel@nongnu.org; Tue, 12 Sep 2017 10:24:35 -0400","from mx1.redhat.com ([209.132.183.28]:45424)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1drm74-0002gQ-0y\n\tfor qemu-devel@nongnu.org; Tue, 12 Sep 2017 10:24:30 -0400","from smtp.corp.redhat.com\n\t(int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id C23F57EA84;\n\tTue, 12 Sep 2017 14:24:28 +0000 (UTC)","from [10.40.204.137] (ovpn-204-137.brq.redhat.com [10.40.204.137])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 8ACA96CDA5;\n\tTue, 12 Sep 2017 14:24:13 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com C23F57EA84","To":"David Hildenbrand , Igor Mammedov ","References":"<20170911152150.12535-1-david@redhat.com>\n\t<20170911152150.12535-19-david@redhat.com>\n\t<20170912154313.3616c2bf@nial.brq.redhat.com>\n\t<9e1ea879-6c89-23bc-b775-062343d782af@redhat.com>","From":"Thomas Huth ","Message-ID":"","Date":"Tue, 12 Sep 2017 16:24:11 +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":"<9e1ea879-6c89-23bc-b775-062343d782af@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.14","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.28]);\n\tTue, 12 Sep 2017 14:24:28 +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 18/21] s390x: implement\n\tquery-hotpluggable-cpus","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 ,\n\tEduardo Habkost ,\n\tMarkus Armbruster , cohuck@redhat.com,\n\tRichard Henderson ,\n\tAlexander Graf , qemu-devel@nongnu.org,\n\tborntraeger@de.ibm.com, Paolo Bonzini ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]