From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51539) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bpmcM-0007vM-4W for qemu-devel@nongnu.org; Thu, 29 Sep 2016 21:28:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bpmcJ-0001YB-T5 for qemu-devel@nongnu.org; Thu, 29 Sep 2016 21:28:01 -0400 Received: from mail-ua0-x22f.google.com ([2607:f8b0:400c:c08::22f]:33895) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bpmcJ-0001Y6-O8 for qemu-devel@nongnu.org; Thu, 29 Sep 2016 21:27:59 -0400 Received: by mail-ua0-x22f.google.com with SMTP id l16so63125950uaa.1 for ; Thu, 29 Sep 2016 18:27:59 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1474047287-145701-4-git-send-email-thomas.hanson@linaro.org> References: <1474047287-145701-1-git-send-email-thomas.hanson@linaro.org> <1474047287-145701-4-git-send-email-thomas.hanson@linaro.org> From: Peter Maydell Date: Thu, 29 Sep 2016 18:27:38 -0700 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH 3/3] target-arm: Comments to mark location of pending work for 56 bit addresses List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Hanson Cc: QEMU Developers , Grant Likely On 16 September 2016 at 10:34, Thomas Hanson wrote: > Certain instructions which can not directly load a tagged address value > may trigger a corner case when the address size is 56 bits. This is > because incrementing or offsetting from the current PC can cause an > arithetic roll-over into the tag bits. Per the ARM ARM spec, these cases > should also be addressed by cleaning up the tag field. > > This work was not done at this time since the changes could not be tested > with current CPU models. Comments have been added to flag the locations > where this will need to be fixed once a model is available. This is *not* why we haven't done this work. We haven't done it because the maximum virtual address size permitted by the architecture is less than 56 bits, and so this is a "can't happen" situation. > 3 comments added in same file to identify cases in a switch. This should be a separate patch, because it is unrelated to the tagged address stuff. > Signed-off-by: Thomas Hanson > --- > target-arm/translate-a64.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c > index 4d6f951..8810180 100644 > --- a/target-arm/translate-a64.c > +++ b/target-arm/translate-a64.c > @@ -1205,6 +1205,9 @@ static inline AArch64DecodeFn *lookup_disas_fn(const AArch64DecodeTable *table, > */ > static void disas_uncond_b_imm(DisasContext *s, uint32_t insn) > { > + /*If/when address size is 56 bits, this could overflow into address tag Missing space before "If". > + * byte, and that byte should be fixed per ARM ARM spec. > + */ > uint64_t addr = s->pc + sextract32(insn, 0, 26) * 4 - 4; > > if (insn & (1U << 31)) { > @@ -1232,6 +1235,9 @@ static void disas_comp_b_imm(DisasContext *s, uint32_t insn) > sf = extract32(insn, 31, 1); > op = extract32(insn, 24, 1); /* 0: CBZ; 1: CBNZ */ > rt = extract32(insn, 0, 5); > + /*If/when address size is 56 bits, this could overflow into address tag > + * byte, and that byte should be fixed per ARM ARM spec. > + */ > addr = s->pc + sextract32(insn, 5, 19) * 4 - 4; > > tcg_cmp = read_cpu_reg(s, rt, sf); > @@ -1260,6 +1266,9 @@ static void disas_test_b_imm(DisasContext *s, uint32_t insn) > > bit_pos = (extract32(insn, 31, 1) << 5) | extract32(insn, 19, 5); > op = extract32(insn, 24, 1); /* 0: TBZ; 1: TBNZ */ > + /*If/when address size is 56 bits, this could overflow into address tag > + * byte, and that byte should be fixed per ARM ARM spec. > + */ > addr = s->pc + sextract32(insn, 5, 14) * 4 - 4; > rt = extract32(insn, 0, 5); > > @@ -1289,6 +1298,9 @@ static void disas_cond_b_imm(DisasContext *s, uint32_t insn) > unallocated_encoding(s); > return; > } > + /*If/when address size is 56 bits, this could overflow into address tag > + * byte, and that byte should be fixed per ARM ARM spec. > + */ > addr = s->pc + sextract32(insn, 5, 19) * 4 - 4; > cond = extract32(insn, 0, 4); > > @@ -1636,12 +1648,12 @@ static void disas_exc(DisasContext *s, uint32_t insn) > * instruction works properly. > */ > switch (op2_ll) { > - case 1: > + case 1: /* SVC */ > gen_ss_advance(s); > gen_exception_insn(s, 0, EXCP_SWI, syn_aa64_svc(imm16), > default_exception_el(s)); > break; > - case 2: > + case 2: /* HVC */ > if (s->current_el == 0) { > unallocated_encoding(s); > break; > @@ -1654,7 +1666,7 @@ static void disas_exc(DisasContext *s, uint32_t insn) > gen_ss_advance(s); > gen_exception_insn(s, 0, EXCP_HVC, syn_aa64_hvc(imm16), 2); > break; > - case 3: > + case 3: /* SMC */ > if (s->current_el == 0) { > unallocated_encoding(s); > break; > -- > 1.9.1 thanks -- PMM