mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Robin Murphy <robin.murphy@arm.com>
To: Fredrik Strupe <fredrik@strupe.net>,
	Russell King - ARM Linux admin <linux@armlinux.org.uk>
Cc: Kate Stewart <kstewart@linuxfoundation.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, Oleg Nesterov <oleg@redhat.com>,
	Richard Fontana <rfontana@redhat.com>,
	Allison Randal <allison@lohutok.net>,
	Thomas Gleixner <tglx@linutronix.de>,
	Enrico Weigelt <info@metux.net>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [RFC PATCH] arm: Don't trap conditional UDF instructions
Date: Wed, 13 May 2020 21:57:54 +0100	[thread overview]
Message-ID: <d2171942-e4b4-4441-b3ba-b5c16c06c2a2@arm.com> (raw)
In-Reply-To: <9fba4d17-d826-6a3a-86fc-60977aa6cc9e@strupe.net>

On 2020-05-13 7:56 pm, Fredrik Strupe wrote:
> On 13.05.2020 20:12, Russell King - ARM Linux admin wrote:
>> On Wed, May 13, 2020 at 05:41:58PM +0200, Fredrik Strupe wrote:
>>> Hi,
>>>
>>> This is more of a question than a patch, but I hope the attached patch makes
>>> the issue a bit clearer.
>>>
>>> The arm port of Linux supports hooking/trapping of undefined instructions. Some
>>> parts of the code use this to trap UDF instructions with certain immediates in
>>> order to use them for other purposes, like 'UDF #16' which is equivalent to a
>>> BKPT instruction in A32.
>>>
>>> Moreover, most of the undef hooks on UDF instructions assume that UDF is
>>> conditional and mask out the condition prefix during matching. The attached
>>> patch shows the locations where this happens. However, the Arm architecture
>>> reference manual explicitly states that UDF is *not* conditional, making
>>> any instruction encoding with a condition prefix other than 0xe (always
>>> execute) unallocated.
>>
>> The latest version of the ARM architecture reference manual may say
>> that, but earlier versions say different things. The latest reference
>> manual does not apply to earlier architectures, so if you're writing
>> code to cover multiple different architectures, you must have an
>> understanding of each of those architectures.
>>
>> So, from the code:
>>
>> 	ARM:   xxxx 0111 1111 xxxx xxxx xxxx 1111 xxxx
>>
>>  From DDI0100E:
>>
>> 3.13.1 Undefined instruction space
>>         Instructions with the following opcodes are undefined
>>         instruction space:
>>
>>         opcode[27:25] = 0b011
>>         opcode[4] = 1
>>
>>         31 28 27 26 25 24                                     5 4 3     0
>>         cond  0  1  1  x  x x x x x x x x x x x x x x x x x x x 1 x x x x
>>
>> So, in this version of the architecture, undefined instructions may
>> be conditional - and indeed that used to be the case.  The condition
>> code was always respected, and cond=1111 meant "never" (NV).
>>
>> Hence, trapping them if the condition code is not 1110 (AL) is
>> entirely reasonable, legal and safe.  If an ARM CPU defines an
>> instruction coding that matches the above, then it won't take the
>> undefined instruction trap, and we'll never see it.
>>
>> Now, as for UDF usage in the kernel, it may be quite correct that we
>> always use the AL condition code for them, but it would be very odd
>> for there to be an instruction implemented with a different (non-NV)
>> condition code that can't also have it's AL condition code encoding.
>> You could never execute such an instruction unconditionally.
>>
> 
> That makes sense. Thank you very much for a great answer!

The other subtlety is that UDF is a *mnemonic* for a specific encoding 
with cond=0b1110, thus "conditional UDF" is an oxymoron. You can still 
have a conditional instruction from the "permanently undefined" space 
(although on Armv7-A it's implementation-defined whether it causes an 
exception regardless of the condition), it's just not a UDF.

Robin.

      reply	other threads:[~2020-05-13 20:58 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-13 15:41 Fredrik Strupe
2020-05-13 18:12 ` Russell King - ARM Linux admin
2020-05-13 18:56   ` Fredrik Strupe
2020-05-13 20:57     ` Robin Murphy [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d2171942-e4b4-4441-b3ba-b5c16c06c2a2@arm.com \
    --to=robin.murphy@arm.com \
    --cc=allison@lohutok.net \
    --cc=fredrik@strupe.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=info@metux.net \
    --cc=kstewart@linuxfoundation.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=oleg@redhat.com \
    --cc=rfontana@redhat.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®