From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49181) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bF1OM-0006QO-D4 for qemu-devel@nongnu.org; Mon, 20 Jun 2016 11:45:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bF1OG-0003XM-42 for qemu-devel@nongnu.org; Mon, 20 Jun 2016 11:45:33 -0400 Sender: Richard Henderson References: <1466431017-123868-1-git-send-email-afarallax@yandex.ru> From: Richard Henderson Message-ID: Date: Mon, 20 Jun 2016 08:45:21 -0700 MIME-Version: 1.0 In-Reply-To: <1466431017-123868-1-git-send-email-afarallax@yandex.ru> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] Improve the alignment check infrastructure List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Sorokin , qemu-devel@nongnu.org Cc: Paolo Bonzini , Peter Crosthwaite , Alexander Graf , qemu-arm@nongnu.org, Claudio Fontana , "Vassili Karpov (malc)" On 06/20/2016 06:56 AM, Sergey Sorokin wrote: > /* Flags stored in the low bits of the TLB virtual address. These are > - defined so that fast path ram access is all zeros. */ > + * defined so that fast path ram access is all zeros. > + * They start after address alignment bits. > + */ > +#define TLB_FLAGS_START_BIT 6 > /* Zero if TLB entry is valid. */ > -#define TLB_INVALID_MASK (1 << 3) > +#define TLB_INVALID_MASK (1 << (TLB_FLAGS_START_BIT + 0)) > /* Set if TLB entry references a clean RAM page. The iotlb entry will > contain the page physical address. */ > -#define TLB_NOTDIRTY (1 << 4) > +#define TLB_NOTDIRTY (1 << (TLB_FLAGS_START_BIT + 1)) > /* Set if TLB entry is an IO callback. */ > -#define TLB_MMIO (1 << 5) > +#define TLB_MMIO (1 << (TLB_FLAGS_START_BIT + 2)) I think we may need to assert that TLB_FLAGS_START_BIT + 3 < TARGET_PAGE_BITS. Or perhaps start from TARGET_PAGE_BITS-1 and subtract? I'm thinking of the AVR target currently under review which requires TARGET_PAGE_BITS == 8 in order to support the memory device layout. > @@ -1195,8 +1195,8 @@ static inline void tcg_out_tlb_load(TCGContext *s, TCGReg addrlo, TCGReg addrhi, > TCGType ttype = TCG_TYPE_I32; > TCGType tlbtype = TCG_TYPE_I32; > int trexw = 0, hrexw = 0, tlbrexw = 0; > - int s_mask = (1 << (opc & MO_SIZE)) - 1; > - bool aligned = (opc & MO_AMASK) == MO_ALIGN || s_mask == 0; > + int a_bits = get_alignment_bits(opc); > + uint64_t tlb_mask; tlb_mask should be target_ulong. > @@ -1099,9 +1109,15 @@ void tcg_dump_ops(TCGContext *s) > qemu_log(",$0x%x,%u", op, ix); > } else { > const char *s_al = "", *s_op; > + int a_bits; > if (op & MO_AMASK) { > - if ((op & MO_AMASK) == MO_ALIGN) { > - s_al = "al+"; > + a_bits = get_alignment_bits(op); > + if (a_bits >= 0) { > + if ((op & MO_SIZE) == a_bits) { > + s_al = "al+"; > + } else { > + s_al = alignment_name[a_bits]; > + } I think perhaps we should be more explicit about what's actually encoded here. Eg only print "al+" if (op & MO_AMASK) == MO_ALIGN. So if an explicit al4 is encoded for size 4, print that. Which does simplify all this code to +static const char * const alignment_name[MO_AMASK >> MO_ASHIFT] = { + [MO_UNALN >> MO_ASHIFT] = "un+", + [MO_ALIGN >> MO_ASHIFT] = "al+", + [MO_ALIGN_2 >> MO_ASHIFT] = "al2+", + [MO_ALIGN_4 >> MO_ASHIFT] = "al4+", + [MO_ALIGN_8 >> MO_ASHIFT] = "al8+", + [MO_ALIGN_16 >> MO_ASHIFT] = "al16+", + [MO_ALIGN_32 >> MO_ASHIFT] = "al32+", + [MO_ALIGN_64 >> MO_ASHIFT] = "al64+", +}; s_al = alignment_name[(op & MO_AMASK) >> MO_ASHIFT]; > + /* Specific alignment size. It must be equal or greater > + * than the access size. > + */ > + a >>= MO_ASHIFT; > + assert(a >= s); > + return a; tcg_debug_assert. r~