From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47002) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XTZ8u-0006xY-1O for qemu-devel@nongnu.org; Mon, 15 Sep 2014 12:28:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XTZ8l-0002cC-0X for qemu-devel@nongnu.org; Mon, 15 Sep 2014 12:28:43 -0400 Received: from mail-pa0-x22d.google.com ([2607:f8b0:400e:c03::22d]:38666) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XTZ8k-0002c6-PU for qemu-devel@nongnu.org; Mon, 15 Sep 2014 12:28:34 -0400 Received: by mail-pa0-f45.google.com with SMTP id rd3so6768359pab.18 for ; Mon, 15 Sep 2014 09:28:33 -0700 (PDT) Sender: Richard Henderson Message-ID: <541713AC.9010105@twiddle.net> Date: Mon, 15 Sep 2014 09:28:28 -0700 From: Richard Henderson MIME-Version: 1.0 References: <20140915105055.5548.45260.stgit@PASHA-ISP> In-Reply-To: <20140915105055.5548.45260.stgit@PASHA-ISP> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] softmmu: fixing usage of cpu_st/ld* from helpers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pavel Dovgalyuk , qemu-devel@nongnu.org Cc: pbonzini@redhat.com, zealot351@gmail.com, maria.klimushenkova@ispras.ru, batuzovk@ispras.ru On 09/15/2014 03:50 AM, Pavel Dovgalyuk wrote: > +/* inline helper ld function */ > + > +static inline DATA_TYPE > +glue(glue(helper_inline_ld, SUFFIX), MEMSUFFIX)(CPUArchState *env, > + target_ulong addr, > + int mmu_idx) > +{ > + return glue(glue(helper_call_ld, SUFFIX), MMUSUFFIX)(env, addr, mmu_idx, > + GETRA()); > +} You'd have to mark this always_inline to make absolutely sure that the caller's GETRA value is used. That said... > @@ -76,7 +87,8 @@ glue(glue(cpu_ld, USUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong ptr) > mmu_idx = CPU_MMU_INDEX; > if (unlikely(env->tlb_table[mmu_idx][page_index].ADDR_READ != > (addr & (TARGET_PAGE_MASK | (DATA_SIZE - 1))))) { > - res = glue(glue(helper_ld, SUFFIX), MMUSUFFIX)(env, addr, mmu_idx); > + res = glue(glue(helper_inline_ld, SUFFIX), MEMSUFFIX)(env, addr, > + mmu_idx); > } else { > uintptr_t hostaddr = addr + env->tlb_table[mmu_idx][page_index].addend; > res = glue(glue(ld, USUFFIX), _raw)(hostaddr); ... this is also the wrong context. The only GETRA value that helps you at all is the one from the *top level* helper -- the one that's directly called from TCG code. So, in the case of maskmov, helper_maskmov_xmm. Anything else and you aren't getting the call site address from the TCG code, and so can't be used to detect the PC of the MMU fault. I guess there are only two real possibilities: (1) Have the cpu_ldst_template helpers all be marked always_inline so that they could use GETRA. I'm not too fond of this because we'd still get the wrong results if these are not used from top-level helpers. (2) Add helpers that accept the GETRA value from the top-level helper. And not hidden within a macro or always_inline function. This helps us see what portions of the code have been audited for the new interface. This will involve quite a bit more code churn, but shouldn't been too difficult for any single function. r~