[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v1] s390x/cpumodel: wire up cpu type + id for TC
From: |
Aurelien Jarno |
Subject: |
Re: [Qemu-devel] [PATCH v1] s390x/cpumodel: wire up cpu type + id for TCG |
Date: |
Thu, 1 Jun 2017 21:20:11 +0200 |
User-agent: |
NeoMutt/20170113 (1.7.2) |
On 2017-06-01 12:14, David Hildenbrand wrote:
> Let's properly expose the CPU type (machine-type number) via "STORE CPU
> ID" and "STORE SUBSYSTEM INFORMATION".
>
> As TCG emulates basic mode, the CPU identification number has the format
> "Annnnn", whereby A is the CPU address, and n are parts of the CPU serial
> number (0 for us for now).
>
> Signed-off-by: David Hildenbrand <address@hidden>
> ---
>
> Tested stidp with a kvm-unit-test that is still being worked on (waiting
> for Thomas' interception test to integrate). I think we are missing quite
> some "operand alignment checks" in other handlers, too.
>
> ---
> target/s390x/cpu.h | 1 -
> target/s390x/cpu_models.c | 2 --
> target/s390x/helper.h | 1 +
> target/s390x/insn-data.def | 2 +-
> target/s390x/misc_helper.c | 26 +++++++++++++++++++++++---
> target/s390x/translate.c | 11 ++++-------
> 6 files changed, 29 insertions(+), 14 deletions(-)
>
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index c74b419..02bd8bf 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -147,7 +147,6 @@ typedef struct CPUS390XState {
> CPU_COMMON
>
> uint32_t cpu_num;
> - uint32_t machine_type;
>
> uint64_t tod_offset;
> uint64_t tod_basetime;
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index b6220c8..99ec0c8 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -710,8 +710,6 @@ static inline void apply_cpu_model(const S390CPUModel
> *model, Error **errp)
>
> if (kvm_enabled()) {
> kvm_s390_apply_cpu_model(model, errp);
> - } else if (model) {
> - /* FIXME TCG - use data for stdip/stfl */
> }
>
> if (!*errp) {
> diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> index 0b70770..0c8f745 100644
> --- a/target/s390x/helper.h
> +++ b/target/s390x/helper.h
> @@ -121,6 +121,7 @@ DEF_HELPER_FLAGS_3(sturg, TCG_CALL_NO_WG, void, env, i64,
> i64)
> DEF_HELPER_1(per_check_exception, void, env)
> DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_RWG, void, env, i64, i64)
> DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64)
> +DEF_HELPER_2(stidp, void, env, i64)
>
> DEF_HELPER_2(xsch, void, env, i64)
> DEF_HELPER_2(csch, void, env, i64)
> diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
> index 55a7c52..83e7d01 100644
> --- a/target/s390x/insn-data.def
> +++ b/target/s390x/insn-data.def
> @@ -902,7 +902,7 @@
> /* STORE CPU ADDRESS */
> C(0xb212, STAP, S, Z, la2, 0, new, m1_16, stap, 0)
> /* STORE CPU ID */
> - C(0xb202, STIDP, S, Z, la2, 0, new, m1_64, stidp, 0)
> + C(0xb202, STIDP, S, Z, 0, a2, 0, 0, stidp, 0)
> /* STORE CPU TIMER */
> C(0xb209, STPT, S, Z, la2, 0, new, m1_64, stpt, 0)
> /* STORE FACILITY LIST */
> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
> index 1b9f448..f682511 100644
> --- a/target/s390x/misc_helper.c
> +++ b/target/s390x/misc_helper.c
> @@ -383,6 +383,7 @@ uint64_t HELPER(stpt)(CPUS390XState *env)
> uint32_t HELPER(stsi)(CPUS390XState *env, uint64_t a0,
> uint64_t r0, uint64_t r1)
> {
> + S390CPU *cpu = s390_env_get_cpu(env);
> int cc = 0;
> int sel1, sel2;
>
> @@ -402,12 +403,14 @@ uint32_t HELPER(stsi)(CPUS390XState *env, uint64_t a0,
> if ((sel1 == 1) && (sel2 == 1)) {
> /* Basic Machine Configuration */
> struct sysib_111 sysib;
> + char type[5] = {};
>
> memset(&sysib, 0, sizeof(sysib));
> ebcdic_put(sysib.manuf, "QEMU ", 16);
> - /* same as machine type number in STORE CPU ID */
> - ebcdic_put(sysib.type, "QEMU", 4);
> - /* same as model number in STORE CPU ID */
> + /* same as machine type number in STORE CPU ID, but in EBCDIC */
> + snprintf(type, ARRAY_SIZE(type), "%X", cpu->model->def->type);
> + ebcdic_put(sysib.type, type, 4);
> + /* model number (not stored in STORE CPU ID for z/Architecure) */
> ebcdic_put(sysib.model, "QEMU ", 16);
> ebcdic_put(sysib.sequence, "QEMU ", 16);
> ebcdic_put(sysib.plant, "QEMU", 4);
> @@ -736,3 +739,20 @@ uint32_t HELPER(stfle)(CPUS390XState *env, uint64_t addr)
> env->regs[0] = deposit64(env->regs[0], 0, 8, max_m1);
> return (count_m1 >= max_m1 ? 0 : 3);
> }
> +
> +#ifndef CONFIG_USER_ONLY
> +void HELPER(stidp)(CPUS390XState *env, uint64_t addr)
> +{
> + S390CPU *cpu = s390_env_get_cpu(env);
> + uint64_t cpuid = s390_cpuid_from_cpu_model(cpu->model);
> +
> + if (addr & 0x7) {
> + program_interrupt(env, PGM_SPECIFICATION, ILEN_LATER_INC);
> + return;
> + }
> +
> + /* basic mode, write the cpu address into the first 4 bit of the ID */
> + cpuid |= ((uint64_t)env->cpu_num & 0xf) << 54;
> + cpu_stq_data(env, addr, cpuid);
> +}
> +#endif
I don't really see the point of using an helper instead of just updating
the existing code. From what I understand the cpuid does not change at
runtime, so the s390_cpuid_from_cpu_model function can also be called
from translate.c.
Aurelien
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
address@hidden http://www.aurel32.net