Subject: Re: [PATCH v11 4/9] arm64: add conditional instruction simulation
 support
To: Marc Zyngier <marc.zyngier@arm.com>, Pratyush Anand <panand@redhat.com>
References: <1457501543-24197-1-git-send-email-dave.long@linaro.org>
 <1457501543-24197-5-git-send-email-dave.long@linaro.org>
 <20160313120903.54b0c8f2@arm.com> <20160314040455.GB6584@dhcppc6.redhat.com>
 <20160314073803.22c90d69@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will.deacon@arm.com>,
        Sandeepa Prabhu <sandeepa.s.prabhu@gmail.com>,
        William Cohen <wcohen@redhat.com>,
        Steve Capper <steve.capper@linaro.org>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        Dave P Martin <Dave.Martin@arm.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Robin Murphy <Robin.Murphy@arm.com>,
        Ard Biesheuvel <ard.biesheuvel@linaro.org>,
        Jens Wiklander <jens.wiklander@linaro.org>,
        Christoffer Dall <christoffer.dall@linaro.org>,
        =?UTF-8?Q?Alex_Benn=c3=a9e?= <alex.bennee@linaro.org>,
        Yang Shi <yang.shi@linaro.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Viresh Kumar <viresh.kumar@linaro.org>,
        "Suzuki K. Poulose" <suzuki.poulose@arm.com>,
        Kees Cook <keescook@chromium.org>, Zi Shen Lim <zlim.lnx@gmail.com>,
        John Blackwood <john.blackwood@ccur.com>, Feng Kan <fkan@apm.com>,
        Balamurugan Shanmugam <bshanmugam@apm.com>,
        James Morse <james.morse@arm.com>,
        Vladimir Murzin <Vladimir.Murzin@arm.com>,
        Mark Salyzyn <salyzyn@android.com>, Petr Mladek <pmladek@suse.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Mark Brown <broonie@kernel.org>
From: David Long <dave.long@linaro.org>
Message-ID: <56EFB241.80909@linaro.org>
Date: Mon, 21 Mar 2016 04:35:13 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.3.0
MIME-Version: 1.0
In-Reply-To: <20160314073803.22c90d69@arm.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1535
Lines: 43

On 03/14/2016 03:38 AM, Marc Zyngier wrote:
> On Mon, 14 Mar 2016 09:34:55 +0530
> Pratyush Anand <panand@redhat.com> wrote:
>
> Hi Pratyush,
>
>> On 13/03/2016:12:09:03 PM, Marc Zyngier wrote:
>>> On Wed,  9 Mar 2016 00:32:18 -0500
>>> David Long <dave.long@linaro.org> wrote:
>>>
>>>> +pstate_check_t * const opcode_condition_checks[16] = {
>>>> +	__check_eq, __check_ne, __check_cs, __check_cc,
>>>> +	__check_mi, __check_pl, __check_vs, __check_vc,
>>>> +	__check_hi, __check_ls, __check_ge, __check_lt,
>>>> +	__check_gt, __check_le, __check_al, __check_al
>>>
>>> The very last entry seems wrong, or is at least the opposite of what
>>> the current code has. It should be something called __check_nv(), and
>>> always return false (condition code NEVER).
>>
>> May be __check_nv() name is more appropriate as per definition, but shouldn't it
>> still return true, because TRM says:
>> "The condition code NV exists only to provide a valid disassembly of the 0b1111
>> encoding, otherwise its behavior is identical to AL"
>
> Indeed, I missed that. But this interpretation is for the A64
> instruction set, and this array is also used by the new
> arm32_check_condition. The condition code table for A32 seems to
> completely ignore the 0b1111 code (there is simply no entry for it), and
> it is only in the ConditionHolds pseudocode that you can see how this
> is actually special-cased.
>
> So I'm fine leaving the code as it is, but a comment and a pointer to
> the ARMv8 ARM wouldn't go amiss.
>
> Thanks,
>
> 	M.
>

OK.

-dl