From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45884) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VQhp7-0005Mo-4R for qemu-devel@nongnu.org; Mon, 30 Sep 2013 14:04:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VQhp0-0001cn-Ut for qemu-devel@nongnu.org; Mon, 30 Sep 2013 14:03:57 -0400 Received: from cantor2.suse.de ([195.135.220.15]:50150 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VQhp0-0001ca-Ln for qemu-devel@nongnu.org; Mon, 30 Sep 2013 14:03:50 -0400 Message-ID: <5249BD03.60608@suse.de> Date: Mon, 30 Sep 2013 20:03:47 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1379945085-29086-1-git-send-email-rth@twiddle.net> <1379945085-29086-2-git-send-email-rth@twiddle.net> In-Reply-To: <1379945085-29086-2-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 01/10] target-s390: Move facilities bits to env List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson Cc: qemu-devel@nongnu.org On 09/23/2013 04:04 PM, Richard Henderson wrote: > Rather than simply hard-coding them in STFL instruction. > > Signed-off-by: Richard Henderson > --- > target-s390x/cpu.c | 3 +++ > target-s390x/cpu.h | 1 + > target-s390x/translate.c | 10 +++++----- > 3 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c > index 3c89f8a..ff691df 100644 > --- a/target-s390x/cpu.c > +++ b/target-s390x/cpu.c > @@ -181,6 +181,9 @@ static void s390_cpu_initfn(Object *obj) > env->cpu_num = cpu_num++; > env->ext_index = -1; > > + env->facilities[0] = 0xc000000000000000ull; > + env->facilities[1] = 0; Could we add CPU definitions along the way here? I'm fine with making z9 the default CPU type, but we should make this explicit :). > + > if (tcg_enabled()&& !inited) { > inited = true; > s390x_translate_init(); > diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h > index 8be5648..746aec8 100644 > --- a/target-s390x/cpu.h > +++ b/target-s390x/cpu.h > @@ -136,6 +136,7 @@ typedef struct CPUS390XState { > CPU_COMMON > > /* reset does memset(0) up to here */ > + uint64_t facilities[2]; > > int cpu_num; > uint8_t *storage_keys; > diff --git a/target-s390x/translate.c b/target-s390x/translate.c > index afe90eb..d4dc8ea 100644 > --- a/target-s390x/translate.c > +++ b/target-s390x/translate.c > @@ -3230,12 +3230,12 @@ static ExitStatus op_spt(DisasContext *s, DisasOps *o) > > static ExitStatus op_stfl(DisasContext *s, DisasOps *o) > { > - TCGv_i64 f, a; > - /* We really ought to have more complete indication of facilities > - that we implement. Address this when STFLE is implemented. */ > + TCGv_i64 f = tcg_temp_new_i64(); > + TCGv_i64 a = tcg_const_i64(200); > + > check_privileged(s); > - f = tcg_const_i64(0xc0000000); > - a = tcg_const_i64(200); > + tcg_gen_ld_i64(f, cpu_env, offsetof(CPUS390XState, facilities[0])); > + tcg_gen_shri_i64(f, f, 32); IMHO the facility list should be stored in DisasContext. That way we can check whether we're generating code against the correct target. Alex