[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 13/21] target/s390x: use "core-id" for cpu nu
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH v4 13/21] target/s390x: use "core-id" for cpu number/address/id handling |
Date: |
Tue, 12 Sep 2017 15:07:37 +0200 |
On Mon, 11 Sep 2017 17:21:42 +0200
David Hildenbrand <address@hidden> wrote:
> Some time ago we discussed that using "id" as property name is not the
> right thing to do, as it is a reserved property for other devices and
> will not work with device_add.
>
> Switch to the term "core-id" instead, and use it as an equivalent to
> "CPU address" mentioned in the PoP. There is no such thing as cpu number,
> so rename env.cpu_num to env.core_id. We use "core-id" as this is the
> common term to use for device_add later on (x86 and ppc).
is there possibility that later The core (something that contains threads)
would appear/exist in real hw?
(my concern here is that we would have to use some other name for it as
renaming of already shipped public property name would be not an option)
> We can get rid of cpu->id now. Keep cpu_index and env->core_id in sync.
> cpu_index was already implicitly used by e.g. cpu_exists(), so keeping
> both in sync seems to be the right thing to do.
>
> cpu_index will now no longer automatically get set via
> cpu_exec_realizefn(). For now, we were lucky that both implicitly stayed
> in sync.
>
> Our new cpu property "core-id" can be a static property. Range checks can
> be avoided by using the correct type and the "setting after realized"
> check is done implicitly.
>
> device_add will later need the reserved "id" property. Hotplugging a CPU
> on s390x will then be: "device_add host-s390-cpu,id=cpu2,core-id=2".
>
> Reviewed-by: Matthew Rosato <address@hidden>
> Signed-off-by: David Hildenbrand <address@hidden>
> ---
[...]
> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
> index f3624d75eb..293fc8428a 100644
> --- a/target/s390x/misc_helper.c
> +++ b/target/s390x/misc_helper.c
> @@ -232,7 +232,7 @@ uint32_t HELPER(stsi)(CPUS390XState *env, uint64_t a0,
> /* XXX make different for different CPUs? */
> ebcdic_put(sysib.sequence, "QEMUQEMUQEMUQEMU", 16);
> ebcdic_put(sysib.plant, "QEMU", 4);
> - stw_p(&sysib.cpu_addr, env->cpu_num);
> + stw_p(&sysib.cpu_addr, env->core_id);
> cpu_physical_memory_write(a0, &sysib, sizeof(sysib));
> } else if ((sel1 == 2) && (sel2 == 2)) {
> /* Basic Machine CPUs */
> @@ -260,7 +260,7 @@ uint32_t HELPER(stsi)(CPUS390XState *env, uint64_t a0,
> /* XXX make different for different CPUs? */
> ebcdic_put(sysib.sequence, "QEMUQEMUQEMUQEMU", 16);
> ebcdic_put(sysib.plant, "QEMU", 4);
> - stw_p(&sysib.cpu_addr, env->cpu_num);
> + stw_p(&sysib.cpu_addr, env->core_id);
> stw_p(&sysib.cpu_id, 0);
> cpu_physical_memory_write(a0, &sysib, sizeof(sysib));
> } else if ((sel1 == 2) && (sel2 == 2)) {
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 909b12818d..5abd34fb34 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -3823,10 +3823,7 @@ static ExitStatus op_ssm(DisasContext *s, DisasOps *o)
> static ExitStatus op_stap(DisasContext *s, DisasOps *o)
> {
> check_privileged(s);
> - /* ??? Surely cpu address != cpu number. In any case the previous
> - version of this stored more than the required half-word, so it
> - is unlikely this has ever been tested. */
> - tcg_gen_ld32u_i64(o->out, cpu_env, offsetof(CPUS390XState, cpu_num));
> + tcg_gen_ld32u_i64(o->out, cpu_env, offsetof(CPUS390XState, core_id));
> return NO_EXIT;
> }
I see core_id is used in several instructions,
does it really have any influence on code executed by *-user target?
- [Qemu-devel] [PATCH v4 06/21] s390x: move subsystem_reset() to s390-virtio-ccw.h, (continued)
- [Qemu-devel] [PATCH v4 06/21] s390x: move subsystem_reset() to s390-virtio-ccw.h, David Hildenbrand, 2017/09/11
- [Qemu-devel] [PATCH v4 07/21] target/s390x: move some s390x typedefs to cpu-qom.h, David Hildenbrand, 2017/09/11
- [Qemu-devel] [PATCH v4 08/21] s390x: move sclp_service_call() to sclp.h, David Hildenbrand, 2017/09/11
- [Qemu-devel] [PATCH v4 10/21] target/s390x: use program_interrupt() in per_check_exception(), David Hildenbrand, 2017/09/11
- [Qemu-devel] [PATCH v4 09/21] target/s390x: use trigger_pgm_exception() in s390_cpu_handle_mmu_fault(), David Hildenbrand, 2017/09/11
- [Qemu-devel] [PATCH v4 11/21] s390x: allow only 1 CPU with TCG, David Hildenbrand, 2017/09/11
- [Qemu-devel] [PATCH v4 12/21] target/s390x: set cpu->id for linux user when realizing, David Hildenbrand, 2017/09/11
- [Qemu-devel] [PATCH v4 13/21] target/s390x: use "core-id" for cpu number/address/id handling, David Hildenbrand, 2017/09/11
- Re: [Qemu-devel] [PATCH v4 13/21] target/s390x: use "core-id" for cpu number/address/id handling,
Igor Mammedov <=
- [Qemu-devel] [PATCH v4 14/21] target/s390x: rename next_cpu_id to next_core_id, David Hildenbrand, 2017/09/11
- [Qemu-devel] [PATCH v4 16/21] s390x: allow cpu hotplug via device_add, David Hildenbrand, 2017/09/11
- [Qemu-devel] [PATCH v4 17/21] s390x: CPU hot unplug via device_del cannot work for now, David Hildenbrand, 2017/09/11
- [Qemu-devel] [PATCH v4 15/21] s390x: print CPU definitions in sorted order, David Hildenbrand, 2017/09/11
- [Qemu-devel] [PATCH v4 18/21] s390x: implement query-hotpluggable-cpus, David Hildenbrand, 2017/09/11