linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/5] arm64: Prepare instruction decoder for objtool
@ 2021-01-20 17:17 Julien Thierry
  2021-01-20 17:17 ` [RFC PATCH 2/5] arm64: aarch64-insn: Add SVE instruction class Julien Thierry
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Julien Thierry @ 2021-01-20 17:17 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: mark.rutland, catalin.marinas, broonie, will, Julien Thierry

To support arm64, objtool will need to be able to decode aarch64
instructions. This patch series adds some instruction definitions needed
by objtool and moves out encoding/decoding functionalities that do not
rely on kernel code in order.

I'll post the start of the arm64 objtool backend shortly.

Thanks,

Julien

-->

Julien Thierry (5):
  arm64: Move instruction encoder/decoder under lib/
  arm64: aarch64-insn: Add SVE instruction class
  arm64: aarch64-insn: Add barrier encodings
  arm64: aarch64-insn: Add some opcodes to instruction decoder
  arm64: Add load/store decoding helpers

 arch/arm64/include/asm/aarch64-insn.h       |  552 +++++++
 arch/arm64/include/asm/alternative-macros.h |    3 -
 arch/arm64/include/asm/alternative.h        |    1 +
 arch/arm64/include/asm/debug-monitors.h     |   14 +-
 arch/arm64/include/asm/ftrace.h             |    2 +-
 arch/arm64/include/asm/insn.h               |  476 -------
 arch/arm64/include/asm/jump_label.h         |    2 +-
 arch/arm64/include/asm/uprobes.h            |    2 +-
 arch/arm64/kernel/insn.c                    | 1416 +-----------------
 arch/arm64/lib/Makefile                     |    2 +-
 arch/arm64/lib/aarch64-insn.c               | 1426 +++++++++++++++++++
 11 files changed, 1985 insertions(+), 1911 deletions(-)
 create mode 100644 arch/arm64/include/asm/aarch64-insn.h
 create mode 100644 arch/arm64/lib/aarch64-insn.c

--
2.25.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [RFC PATCH 2/5] arm64: aarch64-insn: Add SVE instruction class
  2021-01-20 17:17 [RFC PATCH 0/5] arm64: Prepare instruction decoder for objtool Julien Thierry
@ 2021-01-20 17:17 ` Julien Thierry
  2021-01-20 17:17 ` [RFC PATCH 3/5] arm64: aarch64-insn: Add barrier encodings Julien Thierry
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Julien Thierry @ 2021-01-20 17:17 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: mark.rutland, catalin.marinas, broonie, will, Julien Thierry

SVE has been public for some time now. Let the decoder acknowledge
its existence.

Signed-off-by: Julien Thierry <jthierry@redhat.com>
---
 arch/arm64/include/asm/aarch64-insn.h | 1 +
 arch/arm64/lib/aarch64-insn.c         | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/aarch64-insn.h b/arch/arm64/include/asm/aarch64-insn.h
index cead3be1a2d0..200bee726172 100644
--- a/arch/arm64/include/asm/aarch64-insn.h
+++ b/arch/arm64/include/asm/aarch64-insn.h
@@ -40,6 +40,7 @@
  */
 enum aarch64_insn_encoding_class {
 	AARCH64_INSN_CLS_UNKNOWN,	/* UNALLOCATED */
+	AARCH64_INSN_CLS_SVE,		/* SVE instructions */
 	AARCH64_INSN_CLS_DP_IMM,	/* Data processing - immediate */
 	AARCH64_INSN_CLS_DP_REG,	/* Data processing - register */
 	AARCH64_INSN_CLS_DP_FPSIMD,	/* Data processing - SIMD and FP */
diff --git a/arch/arm64/lib/aarch64-insn.c b/arch/arm64/lib/aarch64-insn.c
index df233a7790dc..95d9143aa9c6 100644
--- a/arch/arm64/lib/aarch64-insn.c
+++ b/arch/arm64/lib/aarch64-insn.c
@@ -17,7 +17,7 @@
 static const int aarch64_insn_encoding_class[] = {
 	AARCH64_INSN_CLS_UNKNOWN,
 	AARCH64_INSN_CLS_UNKNOWN,
-	AARCH64_INSN_CLS_UNKNOWN,
+	AARCH64_INSN_CLS_SVE,
 	AARCH64_INSN_CLS_UNKNOWN,
 	AARCH64_INSN_CLS_LDST,
 	AARCH64_INSN_CLS_DP_REG,
-- 
2.25.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC PATCH 3/5] arm64: aarch64-insn: Add barrier encodings
  2021-01-20 17:17 [RFC PATCH 0/5] arm64: Prepare instruction decoder for objtool Julien Thierry
  2021-01-20 17:17 ` [RFC PATCH 2/5] arm64: aarch64-insn: Add SVE instruction class Julien Thierry
@ 2021-01-20 17:17 ` Julien Thierry
  2021-02-02 11:15   ` Mark Rutland
  2021-01-20 17:17 ` [RFC PATCH 4/5] arm64: aarch64-insn: Add some opcodes to instruction decoder Julien Thierry
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Julien Thierry @ 2021-01-20 17:17 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: mark.rutland, catalin.marinas, broonie, will, Julien Thierry

Create necessary functions to encode/decode aarch64 data/instruction
barriers.

Signed-off-by: Julien Thierry <jthierry@redhat.com>
---
 arch/arm64/include/asm/aarch64-insn.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/include/asm/aarch64-insn.h b/arch/arm64/include/asm/aarch64-insn.h
index 200bee726172..d0fee47bbe6e 100644
--- a/arch/arm64/include/asm/aarch64-insn.h
+++ b/arch/arm64/include/asm/aarch64-insn.h
@@ -379,6 +379,9 @@ __AARCH64_INSN_FUNCS(eret_auth,	0xFFFFFBFF, 0xD69F0BFF)
 __AARCH64_INSN_FUNCS(mrs,	0xFFF00000, 0xD5300000)
 __AARCH64_INSN_FUNCS(msr_imm,	0xFFF8F01F, 0xD500401F)
 __AARCH64_INSN_FUNCS(msr_reg,	0xFFF00000, 0xD5100000)
+__AARCH64_INSN_FUNCS(dmb,	0xFFFFF0FF, 0xD50330BF)
+__AARCH64_INSN_FUNCS(dsb,	0xFFFFF0FF, 0xD503309F)
+__AARCH64_INSN_FUNCS(isb,	0xFFFFF0FF, 0xD50330DF)
 
 #undef	__AARCH64_INSN_FUNCS
 
@@ -390,6 +393,12 @@ static inline bool aarch64_insn_is_adr_adrp(u32 insn)
 	return aarch64_insn_is_adr(insn) || aarch64_insn_is_adrp(insn);
 }
 
+static inline bool aarch64_insn_is_barrier(u32 insn)
+{
+	return aarch64_insn_is_dmb(insn) || aarch64_insn_is_dsb(insn) ||
+	       aarch64_insn_is_isb(insn);
+}
+
 enum aarch64_insn_encoding_class aarch64_get_insn_class(u32 insn);
 bool aarch64_insn_uses_literal(u32 insn);
 bool aarch64_insn_is_branch(u32 insn);
-- 
2.25.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC PATCH 4/5] arm64: aarch64-insn: Add some opcodes to instruction decoder
  2021-01-20 17:17 [RFC PATCH 0/5] arm64: Prepare instruction decoder for objtool Julien Thierry
  2021-01-20 17:17 ` [RFC PATCH 2/5] arm64: aarch64-insn: Add SVE instruction class Julien Thierry
  2021-01-20 17:17 ` [RFC PATCH 3/5] arm64: aarch64-insn: Add barrier encodings Julien Thierry
@ 2021-01-20 17:17 ` Julien Thierry
  2021-01-20 17:17 ` [RFC PATCH 5/5] arm64: Add load/store decoding helpers Julien Thierry
       [not found] ` <20210120171745.1657762-2-jthierry@redhat.com>
  4 siblings, 0 replies; 10+ messages in thread
From: Julien Thierry @ 2021-01-20 17:17 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: mark.rutland, catalin.marinas, broonie, will, Julien Thierry

Add decoding capability for some instructions that objtool will need
to decode.

Signed-off-by: Julien Thierry <jthierry@redhat.com>
---
 arch/arm64/include/asm/aarch64-insn.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/include/asm/aarch64-insn.h b/arch/arm64/include/asm/aarch64-insn.h
index d0fee47bbe6e..3a0e0ad51f5b 100644
--- a/arch/arm64/include/asm/aarch64-insn.h
+++ b/arch/arm64/include/asm/aarch64-insn.h
@@ -305,6 +305,12 @@ __AARCH64_INSN_FUNCS(adr,	0x9F000000, 0x10000000)
 __AARCH64_INSN_FUNCS(adrp,	0x9F000000, 0x90000000)
 __AARCH64_INSN_FUNCS(prfm,	0x3FC00000, 0x39800000)
 __AARCH64_INSN_FUNCS(prfm_lit,	0xFF000000, 0xD8000000)
+__AARCH64_INSN_FUNCS(store_imm,	0x3FC00000, 0x39000000)
+__AARCH64_INSN_FUNCS(load_imm,	0x3FC00000, 0x39400000)
+__AARCH64_INSN_FUNCS(store_pre,	0x3FE00C00, 0x38000C00)
+__AARCH64_INSN_FUNCS(load_pre,	0x3FE00C00, 0x38400C00)
+__AARCH64_INSN_FUNCS(store_post,	0x3FE00C00, 0x38000400)
+__AARCH64_INSN_FUNCS(load_post,	0x3FE00C00, 0x38400400)
 __AARCH64_INSN_FUNCS(str_reg,	0x3FE0EC00, 0x38206800)
 __AARCH64_INSN_FUNCS(ldadd,	0x3F20FC00, 0x38200000)
 __AARCH64_INSN_FUNCS(ldr_reg,	0x3FE0EC00, 0x38606800)
@@ -313,6 +319,8 @@ __AARCH64_INSN_FUNCS(ldrsw_lit,	0xFF000000, 0x98000000)
 __AARCH64_INSN_FUNCS(exclusive,	0x3F800000, 0x08000000)
 __AARCH64_INSN_FUNCS(load_ex,	0x3F400000, 0x08400000)
 __AARCH64_INSN_FUNCS(store_ex,	0x3F400000, 0x08000000)
+__AARCH64_INSN_FUNCS(stp,	0x7FC00000, 0x29000000)
+__AARCH64_INSN_FUNCS(ldp,	0x7FC00000, 0x29400000)
 __AARCH64_INSN_FUNCS(stp_post,	0x7FC00000, 0x28800000)
 __AARCH64_INSN_FUNCS(ldp_post,	0x7FC00000, 0x28C00000)
 __AARCH64_INSN_FUNCS(stp_pre,	0x7FC00000, 0x29800000)
@@ -345,6 +353,7 @@ __AARCH64_INSN_FUNCS(rev64,	0x7FFFFC00, 0x5AC00C00)
 __AARCH64_INSN_FUNCS(and,	0x7F200000, 0x0A000000)
 __AARCH64_INSN_FUNCS(bic,	0x7F200000, 0x0A200000)
 __AARCH64_INSN_FUNCS(orr,	0x7F200000, 0x2A000000)
+__AARCH64_INSN_FUNCS(mov_reg,	0x7FE0FFE0, 0x2A0003E0)
 __AARCH64_INSN_FUNCS(orn,	0x7F200000, 0x2A200000)
 __AARCH64_INSN_FUNCS(eor,	0x7F200000, 0x4A000000)
 __AARCH64_INSN_FUNCS(eon,	0x7F200000, 0x4A200000)
-- 
2.25.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC PATCH 5/5] arm64: Add load/store decoding helpers
  2021-01-20 17:17 [RFC PATCH 0/5] arm64: Prepare instruction decoder for objtool Julien Thierry
                   ` (2 preceding siblings ...)
  2021-01-20 17:17 ` [RFC PATCH 4/5] arm64: aarch64-insn: Add some opcodes to instruction decoder Julien Thierry
@ 2021-01-20 17:17 ` Julien Thierry
       [not found] ` <20210120171745.1657762-2-jthierry@redhat.com>
  4 siblings, 0 replies; 10+ messages in thread
From: Julien Thierry @ 2021-01-20 17:17 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: mark.rutland, catalin.marinas, broonie, will, Julien Thierry

Provide some function to group different load/store instructions.

Signed-off-by: Julien Thierry <jthierry@redhat.com>
---
 arch/arm64/include/asm/aarch64-insn.h | 28 +++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm64/include/asm/aarch64-insn.h b/arch/arm64/include/asm/aarch64-insn.h
index 3a0e0ad51f5b..f1b5b652c400 100644
--- a/arch/arm64/include/asm/aarch64-insn.h
+++ b/arch/arm64/include/asm/aarch64-insn.h
@@ -408,6 +408,34 @@ static inline bool aarch64_insn_is_barrier(u32 insn)
 	       aarch64_insn_is_isb(insn);
 }
 
+static inline bool aarch64_insn_is_store_single(u32 insn)
+{
+	return aarch64_insn_is_store_imm(insn) ||
+	       aarch64_insn_is_store_pre(insn) ||
+	       aarch64_insn_is_store_post(insn);
+}
+
+static inline bool aarch64_insn_is_store_pair(u32 insn)
+{
+	return aarch64_insn_is_stp(insn) ||
+	       aarch64_insn_is_stp_pre(insn) ||
+	       aarch64_insn_is_stp_post(insn);
+}
+
+static inline bool aarch64_insn_is_load_single(u32 insn)
+{
+	return aarch64_insn_is_load_imm(insn) ||
+	       aarch64_insn_is_load_pre(insn) ||
+	       aarch64_insn_is_load_post(insn);
+}
+
+static inline bool aarch64_insn_is_load_pair(u32 insn)
+{
+	return aarch64_insn_is_ldp(insn) ||
+	       aarch64_insn_is_ldp_pre(insn) ||
+	       aarch64_insn_is_ldp_post(insn);
+}
+
 enum aarch64_insn_encoding_class aarch64_get_insn_class(u32 insn);
 bool aarch64_insn_uses_literal(u32 insn);
 bool aarch64_insn_is_branch(u32 insn);
-- 
2.25.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 3/5] arm64: aarch64-insn: Add barrier encodings
  2021-01-20 17:17 ` [RFC PATCH 3/5] arm64: aarch64-insn: Add barrier encodings Julien Thierry
@ 2021-02-02 11:15   ` Mark Rutland
  2021-02-03  8:47     ` Julien Thierry
  0 siblings, 1 reply; 10+ messages in thread
From: Mark Rutland @ 2021-02-02 11:15 UTC (permalink / raw)
  To: Julien Thierry
  Cc: catalin.marinas, broonie, will, linux-kernel, linux-arm-kernel

On Wed, Jan 20, 2021 at 06:17:43PM +0100, Julien Thierry wrote:
> Create necessary functions to encode/decode aarch64 data/instruction
> barriers.
> 
> Signed-off-by: Julien Thierry <jthierry@redhat.com>
> ---
>  arch/arm64/include/asm/aarch64-insn.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/aarch64-insn.h b/arch/arm64/include/asm/aarch64-insn.h
> index 200bee726172..d0fee47bbe6e 100644
> --- a/arch/arm64/include/asm/aarch64-insn.h
> +++ b/arch/arm64/include/asm/aarch64-insn.h
> @@ -379,6 +379,9 @@ __AARCH64_INSN_FUNCS(eret_auth,	0xFFFFFBFF, 0xD69F0BFF)
>  __AARCH64_INSN_FUNCS(mrs,	0xFFF00000, 0xD5300000)
>  __AARCH64_INSN_FUNCS(msr_imm,	0xFFF8F01F, 0xD500401F)
>  __AARCH64_INSN_FUNCS(msr_reg,	0xFFF00000, 0xD5100000)
> +__AARCH64_INSN_FUNCS(dmb,	0xFFFFF0FF, 0xD50330BF)
> +__AARCH64_INSN_FUNCS(dsb,	0xFFFFF0FF, 0xD503309F)
> +__AARCH64_INSN_FUNCS(isb,	0xFFFFF0FF, 0xD50330DF)

These match the encodings in ARM DDI 0487G.a, with a couple of caveats
for DSB.

Per section C6.2.82 on page C6-1000, when CRm != 0x00, the instruction
isn't considered a DSB. I believe per the "barriers" decode table on
page C4-289 that here "0x00" is actually a binary string and 'x' is a
"don't care" -- I've raised a ticket to get the documentation clarified.
I suspect we need to write a function to handle that.

There's also a secondary encoding for DSB with FEAT_XS, which we don't
currently use but might want to add.

>  #undef	__AARCH64_INSN_FUNCS
>  
> @@ -390,6 +393,12 @@ static inline bool aarch64_insn_is_adr_adrp(u32 insn)
>  	return aarch64_insn_is_adr(insn) || aarch64_insn_is_adrp(insn);
>  }
>  
> +static inline bool aarch64_insn_is_barrier(u32 insn)
> +{
> +	return aarch64_insn_is_dmb(insn) || aarch64_insn_is_dsb(insn) ||
> +	       aarch64_insn_is_isb(insn);
> +}

I assume this is meant to match the barriers instruction class, as per
the table on page C4-289? That also contains CLREX, SB, SSBB, and PSSBB,
and it might be worth adding them at the same time.

Thanks,
Mark.

>  enum aarch64_insn_encoding_class aarch64_get_insn_class(u32 insn);
>  bool aarch64_insn_uses_literal(u32 insn);
>  bool aarch64_insn_is_branch(u32 insn);
> -- 
> 2.25.4
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 1/5] arm64: Move instruction encoder/decoder under lib/
       [not found]   ` <20210202101755.GB59049@C02TD0UTHF1T.local>
@ 2021-02-03  8:26     ` Julien Thierry
  2021-02-03 11:12       ` Mark Rutland
  0 siblings, 1 reply; 10+ messages in thread
From: Julien Thierry @ 2021-02-03  8:26 UTC (permalink / raw)
  To: Mark Rutland
  Cc: catalin.marinas, broonie, will, linux-kernel, linux-arm-kernel

Hi Mark,

On 2/2/21 11:17 AM, Mark Rutland wrote:
> Hi Julien,
> 
> On Wed, Jan 20, 2021 at 06:17:41PM +0100, Julien Thierry wrote:
>> Aarch64 instruction set encoding and decoding logic can prove useful
>> for some features/tools both part of the kernel and outside the kernel.
>>
>> Isolate the function dealing only with encoding/decoding instructions,
>> with minimal dependency on kernel utilities in order to be able to reuse
>> that code.
>>
>> Code was only moved, no code should have been added, removed nor
>> modifier.
>>
>> Signed-off-by: Julien Thierry <jthierry@redhat.com>
> 
> This looks sound, but the diff is a little hard to check.
> 

Yes, I was expecting this change to be hard to digest.

> Would it be possible to split this into two patches, where:
> 
> 1) Refactoring definitions into insn.h and insn.c, leaving those files
>     in their current locations.
> 

I'm not quite sure I understand the suggestions. After this patch insn.h 
and insn.c still contain some definitions that can't really be used 
outside of kernel code which is why I split them into insn.* and 
aarch64-insn.* (the latter containing what could be used by tools).

Or do you suggest that I still create the aarch64-insn.* files next to 
the insn.* files?


> 2) Moving insn.h and insn.c out to arch/arm64/lib/, updating includes
> > With that, the second patch might be easier for git to identify as a
> rename (if using `git format-patch -M -C`), and it'd be easier to see
> that there are no unintended changes.
> 

But it is more a split than a rename (at least in this patch). But I'm 
happy to do this in another way.

Thanks,

-- 
Julien Thierry


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 3/5] arm64: aarch64-insn: Add barrier encodings
  2021-02-02 11:15   ` Mark Rutland
@ 2021-02-03  8:47     ` Julien Thierry
  0 siblings, 0 replies; 10+ messages in thread
From: Julien Thierry @ 2021-02-03  8:47 UTC (permalink / raw)
  To: Mark Rutland
  Cc: catalin.marinas, broonie, will, linux-kernel, linux-arm-kernel



On 2/2/21 12:15 PM, Mark Rutland wrote:
> On Wed, Jan 20, 2021 at 06:17:43PM +0100, Julien Thierry wrote:
>> Create necessary functions to encode/decode aarch64 data/instruction
>> barriers.
>>
>> Signed-off-by: Julien Thierry <jthierry@redhat.com>
>> ---
>>   arch/arm64/include/asm/aarch64-insn.h | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/aarch64-insn.h b/arch/arm64/include/asm/aarch64-insn.h
>> index 200bee726172..d0fee47bbe6e 100644
>> --- a/arch/arm64/include/asm/aarch64-insn.h
>> +++ b/arch/arm64/include/asm/aarch64-insn.h
>> @@ -379,6 +379,9 @@ __AARCH64_INSN_FUNCS(eret_auth,	0xFFFFFBFF, 0xD69F0BFF)
>>   __AARCH64_INSN_FUNCS(mrs,	0xFFF00000, 0xD5300000)
>>   __AARCH64_INSN_FUNCS(msr_imm,	0xFFF8F01F, 0xD500401F)
>>   __AARCH64_INSN_FUNCS(msr_reg,	0xFFF00000, 0xD5100000)
>> +__AARCH64_INSN_FUNCS(dmb,	0xFFFFF0FF, 0xD50330BF)
>> +__AARCH64_INSN_FUNCS(dsb,	0xFFFFF0FF, 0xD503309F)
>> +__AARCH64_INSN_FUNCS(isb,	0xFFFFF0FF, 0xD50330DF)
> 
> These match the encodings in ARM DDI 0487G.a, with a couple of caveats
> for DSB.
> 
> Per section C6.2.82 on page C6-1000, when CRm != 0x00, the instruction
> isn't considered a DSB. I believe per the "barriers" decode table on
> page C4-289 that here "0x00" is actually a binary string and 'x' is a
> "don't care" -- I've raised a ticket to get the documentation clarified.
> I suspect we need to write a function to handle that.
> 

Ah, I did miss that part. Thanks for pointing it out (and for clarifying 
it's probably not hexa, but the binary string makes sense since it's a 4 
bits field)

> There's also a secondary encoding for DSB with FEAT_XS, which we don't
> currently use but might want to add.
> 

Ah, yes, had to pick up a newer version of the Arm ARM! I'll add it.

>>   #undef	__AARCH64_INSN_FUNCS
>>   
>> @@ -390,6 +393,12 @@ static inline bool aarch64_insn_is_adr_adrp(u32 insn)
>>   	return aarch64_insn_is_adr(insn) || aarch64_insn_is_adrp(insn);
>>   }
>>   
>> +static inline bool aarch64_insn_is_barrier(u32 insn)
>> +{
>> +	return aarch64_insn_is_dmb(insn) || aarch64_insn_is_dsb(insn) ||
>> +	       aarch64_insn_is_isb(insn);
>> +}
> 
> I assume this is meant to match the barriers instruction class, as per
> the table on page C4-289? That also contains CLREX, SB, SSBB, and PSSBB,
> and it might be worth adding them at the same time.
> 

Yes, I have to admit I only added the ones that objtool saw and 
complained about "unreachable instruction" (mostly barriers after 
ret/eret). I'll add them as well.

Thanks,

-- 
Julien Thierry


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 1/5] arm64: Move instruction encoder/decoder under lib/
  2021-02-03  8:26     ` [RFC PATCH 1/5] arm64: Move instruction encoder/decoder under lib/ Julien Thierry
@ 2021-02-03 11:12       ` Mark Rutland
  2021-02-03 17:30         ` Julien Thierry
  0 siblings, 1 reply; 10+ messages in thread
From: Mark Rutland @ 2021-02-03 11:12 UTC (permalink / raw)
  To: Julien Thierry
  Cc: catalin.marinas, broonie, will, linux-kernel, linux-arm-kernel

On Wed, Feb 03, 2021 at 09:26:45AM +0100, Julien Thierry wrote:
> On 2/2/21 11:17 AM, Mark Rutland wrote:
> > On Wed, Jan 20, 2021 at 06:17:41PM +0100, Julien Thierry wrote:
> > > Aarch64 instruction set encoding and decoding logic can prove useful
> > > for some features/tools both part of the kernel and outside the kernel.
> > > 
> > > Isolate the function dealing only with encoding/decoding instructions,
> > > with minimal dependency on kernel utilities in order to be able to reuse
> > > that code.
> > > 
> > > Code was only moved, no code should have been added, removed nor
> > > modifier.
> > > 
> > > Signed-off-by: Julien Thierry <jthierry@redhat.com>
> > 
> > This looks sound, but the diff is a little hard to check.
> 
> Yes, I was expecting this change to be hard to digest.
> 
> > Would it be possible to split this into two patches, where:
> > 
> > 1) Refactoring definitions into insn.h and insn.c, leaving those files
> >     in their current locations.
> 
> I'm not quite sure I understand the suggestions. After this patch insn.h and
> insn.c still contain some definitions that can't really be used outside of
> kernel code which is why I split them into insn.* and aarch64-insn.* (the
> latter containing what could be used by tools).

Sorry; I hadn't appreciated what was getting left behind. With the
series applied I see that's some kernel patching logic, and AArch32 insn
bits.

How about we invert the move, and split those bits out of insn.c first,
then move the rest in one go, i.e.

1) Factor the patching bits out into new patching.{c,h} files.

2) Move insn.c to arch/arm64/lib/

3) Split insn.* into aarch64-insn.* and aarch32-insn.*

... if that makes any sense?

We might not even need to split the aarch32 bits out given they all have
an aarch32_* prefix.

> > 2) Moving insn.h and insn.c out to arch/arm64/lib/, updating includes
> > > With that, the second patch might be easier for git to identify as a
> > rename (if using `git format-patch -M -C`), and it'd be easier to see
> > that there are no unintended changes.
> 
> But it is more a split than a rename (at least in this patch). But I'm happy
> to do this in another way.

I think the above suggestion might solve that, unless I've missed
something?

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 1/5] arm64: Move instruction encoder/decoder under lib/
  2021-02-03 11:12       ` Mark Rutland
@ 2021-02-03 17:30         ` Julien Thierry
  0 siblings, 0 replies; 10+ messages in thread
From: Julien Thierry @ 2021-02-03 17:30 UTC (permalink / raw)
  To: Mark Rutland
  Cc: catalin.marinas, broonie, will, linux-kernel, linux-arm-kernel



On 2/3/21 12:12 PM, Mark Rutland wrote:
> On Wed, Feb 03, 2021 at 09:26:45AM +0100, Julien Thierry wrote:
>> On 2/2/21 11:17 AM, Mark Rutland wrote:
>>> On Wed, Jan 20, 2021 at 06:17:41PM +0100, Julien Thierry wrote:
>>>> Aarch64 instruction set encoding and decoding logic can prove useful
>>>> for some features/tools both part of the kernel and outside the kernel.
>>>>
>>>> Isolate the function dealing only with encoding/decoding instructions,
>>>> with minimal dependency on kernel utilities in order to be able to reuse
>>>> that code.
>>>>
>>>> Code was only moved, no code should have been added, removed nor
>>>> modifier.
>>>>
>>>> Signed-off-by: Julien Thierry <jthierry@redhat.com>
>>>
>>> This looks sound, but the diff is a little hard to check.
>>
>> Yes, I was expecting this change to be hard to digest.
>>
>>> Would it be possible to split this into two patches, where:
>>>
>>> 1) Refactoring definitions into insn.h and insn.c, leaving those files
>>>      in their current locations.
>>
>> I'm not quite sure I understand the suggestions. After this patch insn.h and
>> insn.c still contain some definitions that can't really be used outside of
>> kernel code which is why I split them into insn.* and aarch64-insn.* (the
>> latter containing what could be used by tools).
> 
> Sorry; I hadn't appreciated what was getting left behind. With the
> series applied I see that's some kernel patching logic, and AArch32 insn
> bits.
> 
> How about we invert the move, and split those bits out of insn.c first,
> then move the rest in one go, i.e.
> 
> 1) Factor the patching bits out into new patching.{c,h} files.
> 
> 2) Move insn.c to arch/arm64/lib/
> 
> 3) Split insn.* into aarch64-insn.* and aarch32-insn.*
> 
> ... if that makes any sense?
> 
> We might not even need to split the aarch32 bits out given they all have
> an aarch32_* prefix.
> 

Yes, that approach sounds good. I'll if the aarchxx-insn is necessary 
but as you say, all in the same file shouldn't cause trouble.

Thanks,

-- 
Julien Thierry


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-02-03 17:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-20 17:17 [RFC PATCH 0/5] arm64: Prepare instruction decoder for objtool Julien Thierry
2021-01-20 17:17 ` [RFC PATCH 2/5] arm64: aarch64-insn: Add SVE instruction class Julien Thierry
2021-01-20 17:17 ` [RFC PATCH 3/5] arm64: aarch64-insn: Add barrier encodings Julien Thierry
2021-02-02 11:15   ` Mark Rutland
2021-02-03  8:47     ` Julien Thierry
2021-01-20 17:17 ` [RFC PATCH 4/5] arm64: aarch64-insn: Add some opcodes to instruction decoder Julien Thierry
2021-01-20 17:17 ` [RFC PATCH 5/5] arm64: Add load/store decoding helpers Julien Thierry
     [not found] ` <20210120171745.1657762-2-jthierry@redhat.com>
     [not found]   ` <20210202101755.GB59049@C02TD0UTHF1T.local>
2021-02-03  8:26     ` [RFC PATCH 1/5] arm64: Move instruction encoder/decoder under lib/ Julien Thierry
2021-02-03 11:12       ` Mark Rutland
2021-02-03 17:30         ` Julien Thierry

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).