From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:44436)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <tommusta@gmail.com>) id 1W7sFu-00080Y-P6
	for qemu-devel@nongnu.org; Mon, 27 Jan 2014 14:54:11 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <tommusta@gmail.com>) id 1W7sFm-0007Jt-Ba
	for qemu-devel@nongnu.org; Mon, 27 Jan 2014 14:54:02 -0500
Message-ID: <52E6B946.1080006@gmail.com>
Date: Mon, 27 Jan 2014 13:53:42 -0600
From: Tom Musta <tommusta@gmail.com>
MIME-Version: 1.0
References: <1390845264-2532-1-git-send-email-tommusta@gmail.com>
	<1390845264-2532-6-git-send-email-tommusta@gmail.com>
	<F601BB58-7E1E-4AD5-8567-616C3349B166@suse.de>
In-Reply-To: <F601BB58-7E1E-4AD5-8567-616C3349B166@suse.de>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH 5/8] target-ppc: Load Quadword
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: Alexander Graf <agraf@suse.de>
Cc: PowerPC <qemu-ppc@nongnu.org>, QEMU Developers <qemu-devel@nongnu.org>

On 1/27/2014 12:55 PM, Alexander Graf wrote:
> You're mixing two semantically separate things here. legal_in_user_mode doesn't really indicate that le_mode isn't usable. I'm sure if you just make this two if()'s with two separate bools that get assigned the same value gcc will be smart enough to optimize it just as well as this combined branch.
> 

Hmmm ... I'm not sure that I see the problem.  Perhaps the comment should be clearer.
And I guess there is really no need to compute the legal_in_user_mode flag since it
is only used once.

Prior to ISA 2.07, lq was not legal in user mode; attempting to execute lq when MSR[PR]=1
resulted in a privileged instruction exception.  Also, when MSR[PR]=0 and MSR[LE]=1, an
alignment exception was generated irrespective of the computed address.

Starting with ISA 2.07, both of these restrictions are lifted.  So the proposed code is
as follows:

static void gen_lq(DisasContext *ctx)
{
    /* lq is a legal user mode instruction starting in ISA 2.07 */
    int legal_in_user_mode = (ctx->insns_flags2 & PPC2_LSQ_ISA207) != 0;

    if (!legal_in_user_mode) {
#if defined(CONFIG_USER_ONLY)
        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
        return;
#else
        if (unlikely(ctx->mem_idx == 0)) {
            gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
            return;
        }

        if (unlikely(ctx->le_mode)) {
            /* Little-endian mode is not handled */
            gen_exception_err(ctx, POWERPC_EXCP_ALIGN, POWERPC_EXCP_ALIGN_LE);
            return;
        }
#endif
    }

    int ra, rd;
    TCGv EA;
    ... // rest of implementation


P.S.  I think there should be an alignment check after the EA is computed.