From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:48223)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <agraf@suse.de>) id 1VQhyE-0007yD-92
	for qemu-devel@nongnu.org; Mon, 30 Sep 2013 14:13:28 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <agraf@suse.de>) id 1VQhy8-0004ph-3t
	for qemu-devel@nongnu.org; Mon, 30 Sep 2013 14:13:22 -0400
Received: from cantor2.suse.de ([195.135.220.15]:50476 helo=mx2.suse.de)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <agraf@suse.de>) id 1VQhy7-0004od-Fx
	for qemu-devel@nongnu.org; Mon, 30 Sep 2013 14:13:15 -0400
Message-ID: <5249BF37.50000@suse.de>
Date: Mon, 30 Sep 2013 20:13:11 +0200
From: Alexander Graf <agraf@suse.de>
MIME-Version: 1.0
References: <1379945085-29086-1-git-send-email-rth@twiddle.net>
	<1379945085-29086-8-git-send-email-rth@twiddle.net>
In-Reply-To: <1379945085-29086-8-git-send-email-rth@twiddle.net>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 07/10] target-s390: Fix STIDP
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<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>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Richard Henderson <rth@twiddle.net>
Cc: qemu-devel@nongnu.org

On 09/23/2013 04:04 PM, Richard Henderson wrote:
> The implementation had been incomplete, as we did not store the
> machine type.
>
> Signed-off-by: Richard Henderson<rth@twiddle.net>
> ---
>   target-s390x/cpu.c       |  2 ++
>   target-s390x/cpu.h       | 14 +++++++++++++-
>   target-s390x/translate.c |  2 +-
>   3 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index 01ff49b..d003dcf 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -257,6 +257,8 @@ static void s390_cpu_initfn(Object *obj)
>       env->facilities[0] |= FAC0_Z9_109;
>   #endif
>
> +    env->machine_type = 0x20940000;   /* ??? Also Z9-109.  */

This really wants to be selected by -cpu.

> +
>       if (tcg_enabled()&&  !inited) {
>           inited = true;
>           s390x_translate_init();
> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> index a0bafef..95f9cab 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -197,7 +197,19 @@ typedef struct CPUS390XState {
>       /* reset does memset(0) up to here */
>       uint64_t facilities[2];
>
> -    int cpu_num;
> +    union {
> +        uint64_t cpuid;
> +        struct {
> +#ifdef HOST_WORDS_BIGENDIAN
> +            uint32_t cpu_num;
> +            uint32_t machine_type;
> +#else
> +            uint32_t machine_type;
> +            uint32_t cpu_num;
> +#endif

Are we guaranteed that we don't need to pack? Also anonymous 
unions/structs are a gcc extension IIRC. And why do you swap endianness 
here, but not above when defining the machine_type value?


Alex

> +        };
> +    };
> +
>       uint8_t *storage_keys;
>
>       uint64_t tod_offset;
> diff --git a/target-s390x/translate.c b/target-s390x/translate.c
> index 192d54e..25a6537 100644
> --- a/target-s390x/translate.c
> +++ b/target-s390x/translate.c
> @@ -3242,7 +3242,7 @@ static ExitStatus op_stctl(DisasContext *s, DisasOps *o)
>   static ExitStatus op_stidp(DisasContext *s, DisasOps *o)
>   {
>       check_privileged(s);
> -    tcg_gen_ld32u_i64(o->out, cpu_env, offsetof(CPUS390XState, cpu_num));
> +    tcg_gen_ld_i64(o->out, cpu_env, offsetof(CPUS390XState, cpuid));
>       return NO_EXIT;
>   }
>