From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:41597)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1YztCZ-0008H6-AE
	for qemu-devel@nongnu.org; Tue, 02 Jun 2015 16:54:24 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1YztCW-00072q-3A
	for qemu-devel@nongnu.org; Tue, 02 Jun 2015 16:54:23 -0400
Received: from mail-qk0-x232.google.com ([2607:f8b0:400d:c09::232]:35516)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1YztCV-00072m-US
	for qemu-devel@nongnu.org; Tue, 02 Jun 2015 16:54:20 -0400
Received: by qkhq76 with SMTP id q76so79839340qkh.2
	for <qemu-devel@nongnu.org>; Tue, 02 Jun 2015 13:54:19 -0700 (PDT)
Sender: Richard Henderson <rth7680@gmail.com>
Message-ID: <556E17F6.2060306@twiddle.net>
Date: Tue, 02 Jun 2015 13:54:14 -0700
From: Richard Henderson <rth@twiddle.net>
MIME-Version: 1.0
References: <1433244411-9693-1-git-send-email-aurelien@aurel32.net>
	<1433244411-9693-4-git-send-email-aurelien@aurel32.net>
In-Reply-To: <1433244411-9693-4-git-send-email-aurelien@aurel32.net>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH RFC 3/5] softmmu: add a
	tlb_vaddr_to_host_fill function
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: Aurelien Jarno <aurelien@aurel32.net>, qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>, Alexander Graf <agraf@suse.de>, Yongbok Kim <yongbok.kim@imgtec.com>, Paolo Bonzini <pbonzini@redhat.com>, Leon Alrae <leon.alrae@imgtec.com>, =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= <afaerber@suse.de>

On 06/02/2015 04:26 AM, Aurelien Jarno wrote:
>      int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
> -    CPUTLBEntry *tlbentry = &env->tlb_table[mmu_idx][index];
> +    CPUTLBEntry *tlbentry;
>      target_ulong tlb_addr;
>      uintptr_t haddr;
>  
> +again:
> +    tlbentry = &env->tlb_table[mmu_idx][index];
> +
>      switch (access_type) {
> -    case 0:
> +    case MMU_DATA_LOAD:
>          tlb_addr = tlbentry->addr_read;
>          break;
> -    case 1:
> +    case MMU_DATA_STORE:
>          tlb_addr = tlbentry->addr_write;
>          break;
> -    case 2:
> +    case MMU_INST_FETCH:
>          tlb_addr = tlbentry->addr_code;
>          break;
>      default:
> @@ -347,10 +350,14 @@ static inline void *tlb_vaddr_to_host(CPUArchState *env, target_ulong addr,
>      if ((addr & TARGET_PAGE_MASK)
>          != (tlb_addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK))) {
>          /* TLB entry is for a different page */
> +        if (fill) {
> +            tlb_fill(ENV_GET_CPU(env), addr, access_type, mmu_idx, retaddr);
> +            goto again;
> +        }
>          return NULL;
>      }

To properly perform a fill, you also ought to check the victim cache.
There's a macro to do that in softmmu_template.h, which is why I
placed probe_write there.  It's not so convenient to use with a
variable type though.

In addition, the address of tlbentry cannot change, so there's no
point in recomputing that.  Indeed, you'd probably be better off
saving &addr_foo so that you only have to go through the switch once.

  switch (access_type) {
  case N:
    tlb_addr_ptr = &tlbentry->addr_foo;
    break;
  }
  tlb_addr = *tlb_addr_ptr;
  if (...) {
    if (!VICTIM_TLB_HIT(...)) {
      if (!fill) {
        return NULL;
      }
      tlb_fill(...);
    }
    tlb_addr = *tlb_addr_ptr;
  }

and thus there's no loop to be mis-predicted.


r~