Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/813451/?format=api
{ "id": 813451, "url": "http://patchwork.ozlabs.org/api/patches/813451/?format=api", "web_url": "http://patchwork.ozlabs.org/project/qemu-devel/patch/20170913132417.24384-14-david@redhat.com/", "project": { "id": 14, "url": "http://patchwork.ozlabs.org/api/projects/14/?format=api", "name": "QEMU Development", "link_name": "qemu-devel", "list_id": "qemu-devel.nongnu.org", "list_email": "qemu-devel@nongnu.org", "web_url": "", "scm_url": "", "webscm_url": "", "list_archive_url": "", "list_archive_url_format": "", "commit_url_format": "" }, "msgid": "<20170913132417.24384-14-david@redhat.com>", "list_archive_url": null, "date": "2017-09-13T13:24:08", "name": "[v5,13/22] target/s390x: use \"core-id\" for cpu number/address/id handling", "commit_ref": null, "pull_url": null, "state": "new", "archived": false, "hash": "29aadf1098713285c1f1c5fa2ba0008a9cfc1d3b", "submitter": { "id": 70402, "url": "http://patchwork.ozlabs.org/api/people/70402/?format=api", "name": "David Hildenbrand", "email": "david@redhat.com" }, "delegate": null, "mbox": "http://patchwork.ozlabs.org/project/qemu-devel/patch/20170913132417.24384-14-david@redhat.com/mbox/", "series": [ { "id": 2916, "url": "http://patchwork.ozlabs.org/api/series/2916/?format=api", "web_url": "http://patchwork.ozlabs.org/project/qemu-devel/list/?series=2916", "date": "2017-09-13T13:23:55", "name": "s390x cleanups and CPU hotplug via device_add", "version": 5, "mbox": "http://patchwork.ozlabs.org/series/2916/mbox/" } ], "comments": "http://patchwork.ozlabs.org/api/patches/813451/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/813451/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>", "X-Original-To": "incoming@patchwork.ozlabs.org", "Delivered-To": "patchwork-incoming@bilbo.ozlabs.org", "Authentication-Results": [ "ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)", "ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com", "ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=david@redhat.com" ], "Received": [ "from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xsjTP54pyz9sNV\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed, 13 Sep 2017 23:40:09 +1000 (AEST)", "from localhost ([::1]:42588 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 1ds7tf-0003of-Sw\n\tfor incoming@patchwork.ozlabs.org; Wed, 13 Sep 2017 09:40:07 -0400", "from eggs.gnu.org ([2001:4830:134:3::10]:39328)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <david@redhat.com>) id 1ds7fZ-0005gj-Pa\n\tfor qemu-devel@nongnu.org; Wed, 13 Sep 2017 09:25:40 -0400", "from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <david@redhat.com>) id 1ds7fX-00039z-O8\n\tfor qemu-devel@nongnu.org; Wed, 13 Sep 2017 09:25:33 -0400", "from mx1.redhat.com ([209.132.183.28]:43034)\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 1ds7fX-00039N-Eu\n\tfor qemu-devel@nongnu.org; Wed, 13 Sep 2017 09:25:31 -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 78AA3806B3;\n\tWed, 13 Sep 2017 13:25:30 +0000 (UTC)", "from t460s.redhat.com (ovpn-116-21.ams2.redhat.com [10.36.116.21])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id B92DD69FB6;\n\tWed, 13 Sep 2017 13:25:26 +0000 (UTC)" ], "DMARC-Filter": "OpenDMARC Filter v1.3.2 mx1.redhat.com 78AA3806B3", "From": "David Hildenbrand <david@redhat.com>", "To": "qemu-devel@nongnu.org", "Date": "Wed, 13 Sep 2017 15:24:08 +0200", "Message-Id": "<20170913132417.24384-14-david@redhat.com>", "In-Reply-To": "<20170913132417.24384-1-david@redhat.com>", "References": "<20170913132417.24384-1-david@redhat.com>", "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.26]);\n\tWed, 13 Sep 2017 13:25:30 +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": "[Qemu-devel] [PATCH v5 13/22] target/s390x: use \"core-id\" for cpu\n\tnumber/address/id handling", "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>, thuth@redhat.com,\n\tEduardo Habkost <ehabkost@redhat.com>, david@redhat.com,\n\tcohuck@redhat.com, Richard Henderson <richard.henderson@linaro.org>,\n\tAlexander Graf <agraf@suse.de>,\n\tMarkus Armbruster <armbru@redhat.com>, borntraeger@de.ibm.com,\n\tIgor Mammedov <imammedo@redhat.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>" }, "content": "Some time ago we discussed that using \"id\" as property name is not the\nright thing to do, as it is a reserved property for other devices and\nwill not work with device_add.\n\nSwitch 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,\nso rename env.cpu_num to env.core_id. We use \"core-id\" as this is the\ncommon term to use for device_add later on (x86 and ppc).\n\nWe can get rid of cpu->id now. Keep cpu_index and env->core_id in sync.\ncpu_index was already implicitly used by e.g. cpu_exists(), so keeping\nboth in sync seems to be the right thing to do.\n\ncpu_index will now no longer automatically get set via\ncpu_exec_realizefn(). For now, we were lucky that both implicitly stayed\nin sync.\n\nOur new cpu property \"core-id\" can be a static property. Range checks can\nbe avoided by using the correct type and the \"setting after realized\"\ncheck is done implicitly.\n\ndevice_add will later need the reserved \"id\" property. Hotplugging a CPU\non s390x will then be: \"device_add host-s390-cpu,id=cpu2,core-id=2\".\n\nReviewed-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>\nSigned-off-by: David Hildenbrand <david@redhat.com>\n---\n hw/s390x/s390-virtio-ccw.c | 2 +-\n target/s390x/cpu.c | 72 +++++++++++++---------------------------------\n target/s390x/cpu.h | 5 ++--\n target/s390x/cpu_models.c | 2 +-\n target/s390x/excp_helper.c | 2 +-\n target/s390x/helper.c | 4 +--\n target/s390x/misc_helper.c | 4 +--\n target/s390x/translate.c | 5 +---\n 8 files changed, 30 insertions(+), 66 deletions(-)", "diff": "diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c\nindex 417998ec28..9da9fd3994 100644\n--- a/hw/s390x/s390-virtio-ccw.c\n+++ b/hw/s390x/s390-virtio-ccw.c\n@@ -310,7 +310,7 @@ static void s390_cpu_plug(HotplugHandler *hotplug_dev,\n S390CPU *cpu = S390_CPU(dev);\n CPUState *cs = CPU(dev);\n \n- name = g_strdup_printf(\"cpu[%i]\", cpu->env.cpu_num);\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);\ndiff --git a/target/s390x/cpu.c b/target/s390x/cpu.c\nindex 5f9315fb16..87ebbe5b28 100644\n--- a/target/s390x/cpu.c\n+++ b/target/s390x/cpu.c\n@@ -36,6 +36,7 @@\n #include \"trace.h\"\n #include \"qapi/visitor.h\"\n #include \"exec/exec-all.h\"\n+#include \"hw/qdev-properties.h\"\n #ifndef CONFIG_USER_ONLY\n #include \"hw/hw.h\"\n #include \"sysemu/arch_init.h\"\n@@ -189,28 +190,31 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)\n }\n \n #if !defined(CONFIG_USER_ONLY)\n- if (cpu->id >= max_cpus) {\n- error_setg(&err, \"Unable to add CPU: %\" PRIi64\n- \", max allowed: %d\", cpu->id, max_cpus - 1);\n+ if (cpu->env.core_id >= max_cpus) {\n+ error_setg(&err, \"Unable to add CPU with core-id: %\" PRIu32\n+ \", maximum core-id: %d\", cpu->env.core_id,\n+ max_cpus - 1);\n goto out;\n }\n #else\n /* implicitly set for linux-user only */\n- cpu->id = scc->next_cpu_id;\n+ cpu->env.core_id = scc->next_cpu_id;\n #endif\n \n- if (cpu_exists(cpu->id)) {\n- error_setg(&err, \"Unable to add CPU: %\" PRIi64\n- \", it already exists\", cpu->id);\n+ if (cpu_exists(cpu->env.core_id)) {\n+ error_setg(&err, \"Unable to add CPU with core-id: %\" PRIu32\n+ \", it already exists\", cpu->env.core_id);\n goto out;\n }\n- if (cpu->id != scc->next_cpu_id) {\n- error_setg(&err, \"Unable to add CPU: %\" PRIi64\n- \", The next available id is %\" PRIi64, cpu->id,\n+ if (cpu->env.core_id != scc->next_cpu_id) {\n+ error_setg(&err, \"Unable to add CPU with core-id: %\" PRIu32\n+ \", the next available core-id is %\" PRIi64, cpu->env.core_id,\n scc->next_cpu_id);\n goto out;\n }\n \n+ /* sync cs->cpu_index and env->core_id. The latter is needed for TCG. */\n+ cs->cpu_index = env->core_id;\n cpu_exec_realizefn(cs, &err);\n if (err != NULL) {\n goto out;\n@@ -220,7 +224,6 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)\n #if !defined(CONFIG_USER_ONLY)\n qemu_register_reset(s390_cpu_machine_reset_cb, cpu);\n #endif\n- env->cpu_num = cpu->id;\n s390_cpu_gdb_init(cs);\n qemu_init_vcpu(cs);\n #if !defined(CONFIG_USER_ONLY)\n@@ -241,45 +244,6 @@ out:\n error_propagate(errp, err);\n }\n \n-static void s390x_cpu_get_id(Object *obj, Visitor *v, const char *name,\n- void *opaque, Error **errp)\n-{\n- S390CPU *cpu = S390_CPU(obj);\n- int64_t value = cpu->id;\n-\n- visit_type_int(v, name, &value, errp);\n-}\n-\n-static void s390x_cpu_set_id(Object *obj, Visitor *v, const char *name,\n- void *opaque, Error **errp)\n-{\n- S390CPU *cpu = S390_CPU(obj);\n- DeviceState *dev = DEVICE(obj);\n- const int64_t min = 0;\n- const int64_t max = UINT32_MAX;\n- Error *err = NULL;\n- int64_t value;\n-\n- if (dev->realized) {\n- error_setg(errp, \"Attempt to set property '%s' on '%s' after \"\n- \"it was realized\", name, object_get_typename(obj));\n- return;\n- }\n-\n- visit_type_int(v, name, &value, &err);\n- if (err) {\n- error_propagate(errp, err);\n- return;\n- }\n- if (value < min || value > max) {\n- error_setg(errp, \"Property %s.%s doesn't take value %\" PRId64\n- \" (minimum: %\" PRId64 \", maximum: %\" PRId64 \")\" ,\n- object_get_typename(obj), name, value, min, max);\n- return;\n- }\n- cpu->id = value;\n-}\n-\n static void s390_cpu_initfn(Object *obj)\n {\n CPUState *cs = CPU(obj);\n@@ -293,8 +257,6 @@ static void s390_cpu_initfn(Object *obj)\n cs->env_ptr = env;\n cs->halted = 1;\n cs->exception_index = EXCP_HLT;\n- object_property_add(OBJECT(cpu), \"id\", \"int64_t\", s390x_cpu_get_id,\n- s390x_cpu_set_id, NULL, NULL, NULL);\n s390_cpu_model_register_props(obj);\n #if !defined(CONFIG_USER_ONLY)\n qemu_get_timedate(&tm, 0);\n@@ -491,6 +453,11 @@ static gchar *s390_gdb_arch_name(CPUState *cs)\n return g_strdup(\"s390:64-bit\");\n }\n \n+static Property s390x_cpu_properties[] = {\n+ DEFINE_PROP_UINT32(\"core-id\", S390CPU, env.core_id, 0),\n+ DEFINE_PROP_END_OF_LIST()\n+};\n+\n static void s390_cpu_class_init(ObjectClass *oc, void *data)\n {\n S390CPUClass *scc = S390_CPU_CLASS(oc);\n@@ -500,6 +467,7 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)\n scc->next_cpu_id = 0;\n scc->parent_realize = dc->realize;\n dc->realize = s390_cpu_realizefn;\n+ dc->props = s390x_cpu_properties;\n \n scc->parent_reset = cc->reset;\n #if !defined(CONFIG_USER_ONLY)\ndiff --git a/target/s390x/cpu.h b/target/s390x/cpu.h\nindex 5295bd3c66..1c8456fa57 100644\n--- a/target/s390x/cpu.h\n+++ b/target/s390x/cpu.h\n@@ -150,7 +150,7 @@ struct CPUS390XState {\n \n CPU_COMMON\n \n- uint32_t cpu_num;\n+ uint32_t core_id; /* PoP \"CPU address\", same as cpu_index */\n uint64_t cpuid;\n \n uint64_t tod_offset;\n@@ -194,7 +194,6 @@ struct S390CPU {\n /*< public >*/\n \n CPUS390XState env;\n- int64_t id;\n S390CPUModel *model;\n /* needed for live migration */\n void *irqstate;\n@@ -690,7 +689,7 @@ const char *s390_default_cpu_model_name(void);\n \n /* helper.c */\n #define cpu_init(cpu_model) cpu_generic_init(TYPE_S390_CPU, cpu_model)\n-S390CPU *s390x_new_cpu(const char *cpu_model, int64_t id, Error **errp);\n+S390CPU *s390x_new_cpu(const char *cpu_model, uint32_t core_id, Error **errp);\n /* you can call this signal handler from your SIGBUS and SIGSEGV\n signal handlers to inform the virtual CPU of exceptions. non zero\n is returned if the signal was handled by the virtual CPU. */\ndiff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c\nindex 18cbf91d9c..8e20e7637b 100644\n--- a/target/s390x/cpu_models.c\n+++ b/target/s390x/cpu_models.c\n@@ -915,7 +915,7 @@ void s390_realize_cpu_model(CPUState *cs, Error **errp)\n cpu->env.cpuid = s390_cpuid_from_cpu_model(cpu->model);\n if (tcg_enabled()) {\n /* basic mode, write the cpu address into the first 4 bit of the ID */\n- cpu->env.cpuid = deposit64(cpu->env.cpuid, 54, 4, cpu->env.cpu_num);\n+ cpu->env.cpuid = deposit64(cpu->env.cpuid, 54, 4, cpu->env.core_id);\n }\n }\n \ndiff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c\nindex 14d3160e92..470cf8f5bc 100644\n--- a/target/s390x/excp_helper.c\n+++ b/target/s390x/excp_helper.c\n@@ -250,7 +250,7 @@ static void do_ext_interrupt(CPUS390XState *env)\n lowcore->ext_params2 = cpu_to_be64(q->param64);\n lowcore->external_old_psw.mask = cpu_to_be64(get_psw_mask(env));\n lowcore->external_old_psw.addr = cpu_to_be64(env->psw.addr);\n- lowcore->cpu_addr = cpu_to_be16(env->cpu_num | VIRTIO_SUBCODE_64);\n+ lowcore->cpu_addr = cpu_to_be16(env->core_id | VIRTIO_SUBCODE_64);\n mask = be64_to_cpu(lowcore->external_new_psw.mask);\n addr = be64_to_cpu(lowcore->external_new_psw.addr);\n \ndiff --git a/target/s390x/helper.c b/target/s390x/helper.c\nindex ba29504476..dfb24ef5b2 100644\n--- a/target/s390x/helper.c\n+++ b/target/s390x/helper.c\n@@ -104,7 +104,7 @@ S390CPU *cpu_s390x_create(const char *cpu_model, Error **errp)\n return S390_CPU(CPU(object_new(typename)));\n }\n \n-S390CPU *s390x_new_cpu(const char *cpu_model, int64_t id, Error **errp)\n+S390CPU *s390x_new_cpu(const char *cpu_model, uint32_t core_id, Error **errp)\n {\n S390CPU *cpu;\n Error *err = NULL;\n@@ -114,7 +114,7 @@ S390CPU *s390x_new_cpu(const char *cpu_model, int64_t id, Error **errp)\n goto out;\n }\n \n- object_property_set_int(OBJECT(cpu), id, \"id\", &err);\n+ object_property_set_int(OBJECT(cpu), core_id, \"core-id\", &err);\n if (err != NULL) {\n goto out;\n }\ndiff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c\nindex 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)) {\ndiff --git a/target/s390x/translate.c b/target/s390x/translate.c\nindex 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 \n", "prefixes": [ "v5", "13/22" ] }