* [PATCH 0/2] Cleanup x86 instruction decoder
@ 2024-02-22 11:16 Nikolay Borisov
2024-02-22 11:16 ` [PATCH 1/2] x86/insn: Remove superfluous checks from instruction decoding routines Nikolay Borisov
2024-02-22 11:16 ` [PATCH 2/2] x86/insn: Directly assign x86_64 state in init_ins Nikolay Borisov
0 siblings, 2 replies; 5+ messages in thread
From: Nikolay Borisov @ 2024-02-22 11:16 UTC (permalink / raw)
To: bp; +Cc: linux-kernel, x86, Nikolay Borisov
First patch removes some duplicated checks in caller/callee second one is
trivial to remove a check in init_ins()
Nikolay Borisov (2):
x86/insn: Remove superfluous checks from instruction decoding routines
x86/insn: Directly assign x86_64 state in init_ins
arch/x86/lib/insn.c | 58 +++++++++++++++------------------------
tools/arch/x86/lib/insn.c | 58 +++++++++++++++------------------------
2 files changed, 44 insertions(+), 72 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] x86/insn: Remove superfluous checks from instruction decoding routines
2024-02-22 11:16 [PATCH 0/2] Cleanup x86 instruction decoder Nikolay Borisov
@ 2024-02-22 11:16 ` Nikolay Borisov
2024-02-22 11:31 ` [tip: x86/asm] " tip-bot2 for Nikolay Borisov
2024-02-22 11:16 ` [PATCH 2/2] x86/insn: Directly assign x86_64 state in init_ins Nikolay Borisov
1 sibling, 1 reply; 5+ messages in thread
From: Nikolay Borisov @ 2024-02-22 11:16 UTC (permalink / raw)
To: bp; +Cc: linux-kernel, x86, Nikolay Borisov
It's pointless checking if a particular part of an instruction is
decoded before calling the routine responsible for decoding it as this
check is duplicated in the routines itself. Streamline the code by
removing the superfluous checks. No functional difference.
Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
---
arch/x86/lib/insn.c | 56 +++++++++++++++------------------------
tools/arch/x86/lib/insn.c | 56 +++++++++++++++------------------------
2 files changed, 42 insertions(+), 70 deletions(-)
diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
index 55e371cc69fd..3946bdc75087 100644
--- a/arch/x86/lib/insn.c
+++ b/arch/x86/lib/insn.c
@@ -268,11 +268,9 @@ int insn_get_opcode(struct insn *insn)
if (opcode->got)
return 0;
- if (!insn->prefixes.got) {
- ret = insn_get_prefixes(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_prefixes(insn);
+ if (ret)
+ return ret;
/* Get first opcode */
op = get_next(insn_byte_t, insn);
@@ -339,11 +337,9 @@ int insn_get_modrm(struct insn *insn)
if (modrm->got)
return 0;
- if (!insn->opcode.got) {
- ret = insn_get_opcode(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_opcode(insn);
+ if (ret)
+ return ret;
if (inat_has_modrm(insn->attr)) {
mod = get_next(insn_byte_t, insn);
@@ -386,11 +382,9 @@ int insn_rip_relative(struct insn *insn)
if (!insn->x86_64)
return 0;
- if (!modrm->got) {
- ret = insn_get_modrm(insn);
- if (ret)
- return 0;
- }
+ ret = insn_get_modrm(insn);
+ if (ret)
+ return 0;
/*
* For rip-relative instructions, the mod field (top 2 bits)
* is zero and the r/m field (bottom 3 bits) is 0x5.
@@ -417,11 +411,9 @@ int insn_get_sib(struct insn *insn)
if (insn->sib.got)
return 0;
- if (!insn->modrm.got) {
- ret = insn_get_modrm(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_modrm(insn);
+ if (ret)
+ return ret;
if (insn->modrm.nbytes) {
modrm = insn->modrm.bytes[0];
@@ -460,11 +452,9 @@ int insn_get_displacement(struct insn *insn)
if (insn->displacement.got)
return 0;
- if (!insn->sib.got) {
- ret = insn_get_sib(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_sib(insn);
+ if (ret)
+ return ret;
if (insn->modrm.nbytes) {
/*
@@ -628,11 +618,9 @@ int insn_get_immediate(struct insn *insn)
if (insn->immediate.got)
return 0;
- if (!insn->displacement.got) {
- ret = insn_get_displacement(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_displacement(insn);
+ if (ret)
+ return ret;
if (inat_has_moffset(insn->attr)) {
if (!__get_moffset(insn))
@@ -703,11 +691,9 @@ int insn_get_length(struct insn *insn)
if (insn->length)
return 0;
- if (!insn->immediate.got) {
- ret = insn_get_immediate(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_immediate(insn);
+ if (ret)
+ return ret;
insn->length = (unsigned char)((unsigned long)insn->next_byte
- (unsigned long)insn->kaddr);
diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c
index 8fd63a067308..5d81924478d9 100644
--- a/tools/arch/x86/lib/insn.c
+++ b/tools/arch/x86/lib/insn.c
@@ -268,11 +268,9 @@ int insn_get_opcode(struct insn *insn)
if (opcode->got)
return 0;
- if (!insn->prefixes.got) {
- ret = insn_get_prefixes(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_prefixes(insn);
+ if (ret)
+ return ret;
/* Get first opcode */
op = get_next(insn_byte_t, insn);
@@ -339,11 +337,9 @@ int insn_get_modrm(struct insn *insn)
if (modrm->got)
return 0;
- if (!insn->opcode.got) {
- ret = insn_get_opcode(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_opcode(insn);
+ if (ret)
+ return ret;
if (inat_has_modrm(insn->attr)) {
mod = get_next(insn_byte_t, insn);
@@ -386,11 +382,9 @@ int insn_rip_relative(struct insn *insn)
if (!insn->x86_64)
return 0;
- if (!modrm->got) {
- ret = insn_get_modrm(insn);
- if (ret)
- return 0;
- }
+ ret = insn_get_modrm(insn);
+ if (ret)
+ return 0;
/*
* For rip-relative instructions, the mod field (top 2 bits)
* is zero and the r/m field (bottom 3 bits) is 0x5.
@@ -417,11 +411,9 @@ int insn_get_sib(struct insn *insn)
if (insn->sib.got)
return 0;
- if (!insn->modrm.got) {
- ret = insn_get_modrm(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_modrm(insn);
+ if (ret)
+ return ret;
if (insn->modrm.nbytes) {
modrm = insn->modrm.bytes[0];
@@ -460,11 +452,9 @@ int insn_get_displacement(struct insn *insn)
if (insn->displacement.got)
return 0;
- if (!insn->sib.got) {
- ret = insn_get_sib(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_sib(insn);
+ if (ret)
+ return ret;
if (insn->modrm.nbytes) {
/*
@@ -628,11 +618,9 @@ int insn_get_immediate(struct insn *insn)
if (insn->immediate.got)
return 0;
- if (!insn->displacement.got) {
- ret = insn_get_displacement(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_displacement(insn);
+ if (ret)
+ return ret;
if (inat_has_moffset(insn->attr)) {
if (!__get_moffset(insn))
@@ -703,11 +691,9 @@ int insn_get_length(struct insn *insn)
if (insn->length)
return 0;
- if (!insn->immediate.got) {
- ret = insn_get_immediate(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_immediate(insn);
+ if (ret)
+ return ret;
insn->length = (unsigned char)((unsigned long)insn->next_byte
- (unsigned long)insn->kaddr);
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] x86/insn: Directly assign x86_64 state in init_ins
2024-02-22 11:16 [PATCH 0/2] Cleanup x86 instruction decoder Nikolay Borisov
2024-02-22 11:16 ` [PATCH 1/2] x86/insn: Remove superfluous checks from instruction decoding routines Nikolay Borisov
@ 2024-02-22 11:16 ` Nikolay Borisov
2024-02-22 11:31 ` [tip: x86/asm] x86/insn: Directly assign x86_64 state in insn_init() tip-bot2 for Nikolay Borisov
1 sibling, 1 reply; 5+ messages in thread
From: Nikolay Borisov @ 2024-02-22 11:16 UTC (permalink / raw)
To: bp; +Cc: linux-kernel, x86, Nikolay Borisov
No point in checking again as this was already done by the caller.
Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
---
arch/x86/lib/insn.c | 2 +-
tools/arch/x86/lib/insn.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
index 3946bdc75087..1bb155a0955b 100644
--- a/arch/x86/lib/insn.c
+++ b/arch/x86/lib/insn.c
@@ -71,7 +71,7 @@ void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64)
insn->kaddr = kaddr;
insn->end_kaddr = kaddr + buf_len;
insn->next_byte = kaddr;
- insn->x86_64 = x86_64 ? 1 : 0;
+ insn->x86_64 = x86_64;
insn->opnd_bytes = 4;
if (x86_64)
insn->addr_bytes = 8;
diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c
index 5d81924478d9..ada4b4a79dd4 100644
--- a/tools/arch/x86/lib/insn.c
+++ b/tools/arch/x86/lib/insn.c
@@ -71,7 +71,7 @@ void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64)
insn->kaddr = kaddr;
insn->end_kaddr = kaddr + buf_len;
insn->next_byte = kaddr;
- insn->x86_64 = x86_64 ? 1 : 0;
+ insn->x86_64 = x86_64;
insn->opnd_bytes = 4;
if (x86_64)
insn->addr_bytes = 8;
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip: x86/asm] x86/insn: Directly assign x86_64 state in insn_init()
2024-02-22 11:16 ` [PATCH 2/2] x86/insn: Directly assign x86_64 state in init_ins Nikolay Borisov
@ 2024-02-22 11:31 ` tip-bot2 for Nikolay Borisov
0 siblings, 0 replies; 5+ messages in thread
From: tip-bot2 for Nikolay Borisov @ 2024-02-22 11:31 UTC (permalink / raw)
To: linux-tip-commits
Cc: Nikolay Borisov, Ingo Molnar, Josh Poimboeuf, Peter Zijlstra,
x86, linux-kernel
The following commit has been merged into the x86/asm branch of tip:
Commit-ID: 07a5d4bcbf8e70a430431991f185eb29e74ae533
Gitweb: https://git.kernel.org/tip/07a5d4bcbf8e70a430431991f185eb29e74ae533
Author: Nikolay Borisov <nik.borisov@suse.com>
AuthorDate: Thu, 22 Feb 2024 13:16:36 +02:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Thu, 22 Feb 2024 12:23:27 +01:00
x86/insn: Directly assign x86_64 state in insn_init()
No point in checking again as this was already done by the caller.
Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: https://lore.kernel.org/r/20240222111636.2214523-3-nik.borisov@suse.com
---
arch/x86/lib/insn.c | 2 +-
tools/arch/x86/lib/insn.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
index 3946bdc..1bb155a 100644
--- a/arch/x86/lib/insn.c
+++ b/arch/x86/lib/insn.c
@@ -71,7 +71,7 @@ void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64)
insn->kaddr = kaddr;
insn->end_kaddr = kaddr + buf_len;
insn->next_byte = kaddr;
- insn->x86_64 = x86_64 ? 1 : 0;
+ insn->x86_64 = x86_64;
insn->opnd_bytes = 4;
if (x86_64)
insn->addr_bytes = 8;
diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c
index 5d81924..ada4b4a 100644
--- a/tools/arch/x86/lib/insn.c
+++ b/tools/arch/x86/lib/insn.c
@@ -71,7 +71,7 @@ void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64)
insn->kaddr = kaddr;
insn->end_kaddr = kaddr + buf_len;
insn->next_byte = kaddr;
- insn->x86_64 = x86_64 ? 1 : 0;
+ insn->x86_64 = x86_64;
insn->opnd_bytes = 4;
if (x86_64)
insn->addr_bytes = 8;
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip: x86/asm] x86/insn: Remove superfluous checks from instruction decoding routines
2024-02-22 11:16 ` [PATCH 1/2] x86/insn: Remove superfluous checks from instruction decoding routines Nikolay Borisov
@ 2024-02-22 11:31 ` tip-bot2 for Nikolay Borisov
0 siblings, 0 replies; 5+ messages in thread
From: tip-bot2 for Nikolay Borisov @ 2024-02-22 11:31 UTC (permalink / raw)
To: linux-tip-commits
Cc: Nikolay Borisov, Ingo Molnar, Josh Poimboeuf, Peter Zijlstra,
x86, linux-kernel
The following commit has been merged into the x86/asm branch of tip:
Commit-ID: 427e1646f1ef6c714a5bade30ca0302edc5d46a0
Gitweb: https://git.kernel.org/tip/427e1646f1ef6c714a5bade30ca0302edc5d46a0
Author: Nikolay Borisov <nik.borisov@suse.com>
AuthorDate: Thu, 22 Feb 2024 13:16:35 +02:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Thu, 22 Feb 2024 12:23:04 +01:00
x86/insn: Remove superfluous checks from instruction decoding routines
It's pointless checking if a particular part of an instruction is
decoded before calling the routine responsible for decoding it as this
check is duplicated in the routines itself. Streamline the code by
removing the superfluous checks. No functional difference.
Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: https://lore.kernel.org/r/20240222111636.2214523-2-nik.borisov@suse.com
---
arch/x86/lib/insn.c | 56 ++++++++++++++------------------------
tools/arch/x86/lib/insn.c | 56 ++++++++++++++------------------------
2 files changed, 42 insertions(+), 70 deletions(-)
diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
index 55e371c..3946bdc 100644
--- a/arch/x86/lib/insn.c
+++ b/arch/x86/lib/insn.c
@@ -268,11 +268,9 @@ int insn_get_opcode(struct insn *insn)
if (opcode->got)
return 0;
- if (!insn->prefixes.got) {
- ret = insn_get_prefixes(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_prefixes(insn);
+ if (ret)
+ return ret;
/* Get first opcode */
op = get_next(insn_byte_t, insn);
@@ -339,11 +337,9 @@ int insn_get_modrm(struct insn *insn)
if (modrm->got)
return 0;
- if (!insn->opcode.got) {
- ret = insn_get_opcode(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_opcode(insn);
+ if (ret)
+ return ret;
if (inat_has_modrm(insn->attr)) {
mod = get_next(insn_byte_t, insn);
@@ -386,11 +382,9 @@ int insn_rip_relative(struct insn *insn)
if (!insn->x86_64)
return 0;
- if (!modrm->got) {
- ret = insn_get_modrm(insn);
- if (ret)
- return 0;
- }
+ ret = insn_get_modrm(insn);
+ if (ret)
+ return 0;
/*
* For rip-relative instructions, the mod field (top 2 bits)
* is zero and the r/m field (bottom 3 bits) is 0x5.
@@ -417,11 +411,9 @@ int insn_get_sib(struct insn *insn)
if (insn->sib.got)
return 0;
- if (!insn->modrm.got) {
- ret = insn_get_modrm(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_modrm(insn);
+ if (ret)
+ return ret;
if (insn->modrm.nbytes) {
modrm = insn->modrm.bytes[0];
@@ -460,11 +452,9 @@ int insn_get_displacement(struct insn *insn)
if (insn->displacement.got)
return 0;
- if (!insn->sib.got) {
- ret = insn_get_sib(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_sib(insn);
+ if (ret)
+ return ret;
if (insn->modrm.nbytes) {
/*
@@ -628,11 +618,9 @@ int insn_get_immediate(struct insn *insn)
if (insn->immediate.got)
return 0;
- if (!insn->displacement.got) {
- ret = insn_get_displacement(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_displacement(insn);
+ if (ret)
+ return ret;
if (inat_has_moffset(insn->attr)) {
if (!__get_moffset(insn))
@@ -703,11 +691,9 @@ int insn_get_length(struct insn *insn)
if (insn->length)
return 0;
- if (!insn->immediate.got) {
- ret = insn_get_immediate(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_immediate(insn);
+ if (ret)
+ return ret;
insn->length = (unsigned char)((unsigned long)insn->next_byte
- (unsigned long)insn->kaddr);
diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c
index 8fd63a0..5d81924 100644
--- a/tools/arch/x86/lib/insn.c
+++ b/tools/arch/x86/lib/insn.c
@@ -268,11 +268,9 @@ int insn_get_opcode(struct insn *insn)
if (opcode->got)
return 0;
- if (!insn->prefixes.got) {
- ret = insn_get_prefixes(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_prefixes(insn);
+ if (ret)
+ return ret;
/* Get first opcode */
op = get_next(insn_byte_t, insn);
@@ -339,11 +337,9 @@ int insn_get_modrm(struct insn *insn)
if (modrm->got)
return 0;
- if (!insn->opcode.got) {
- ret = insn_get_opcode(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_opcode(insn);
+ if (ret)
+ return ret;
if (inat_has_modrm(insn->attr)) {
mod = get_next(insn_byte_t, insn);
@@ -386,11 +382,9 @@ int insn_rip_relative(struct insn *insn)
if (!insn->x86_64)
return 0;
- if (!modrm->got) {
- ret = insn_get_modrm(insn);
- if (ret)
- return 0;
- }
+ ret = insn_get_modrm(insn);
+ if (ret)
+ return 0;
/*
* For rip-relative instructions, the mod field (top 2 bits)
* is zero and the r/m field (bottom 3 bits) is 0x5.
@@ -417,11 +411,9 @@ int insn_get_sib(struct insn *insn)
if (insn->sib.got)
return 0;
- if (!insn->modrm.got) {
- ret = insn_get_modrm(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_modrm(insn);
+ if (ret)
+ return ret;
if (insn->modrm.nbytes) {
modrm = insn->modrm.bytes[0];
@@ -460,11 +452,9 @@ int insn_get_displacement(struct insn *insn)
if (insn->displacement.got)
return 0;
- if (!insn->sib.got) {
- ret = insn_get_sib(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_sib(insn);
+ if (ret)
+ return ret;
if (insn->modrm.nbytes) {
/*
@@ -628,11 +618,9 @@ int insn_get_immediate(struct insn *insn)
if (insn->immediate.got)
return 0;
- if (!insn->displacement.got) {
- ret = insn_get_displacement(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_displacement(insn);
+ if (ret)
+ return ret;
if (inat_has_moffset(insn->attr)) {
if (!__get_moffset(insn))
@@ -703,11 +691,9 @@ int insn_get_length(struct insn *insn)
if (insn->length)
return 0;
- if (!insn->immediate.got) {
- ret = insn_get_immediate(insn);
- if (ret)
- return ret;
- }
+ ret = insn_get_immediate(insn);
+ if (ret)
+ return ret;
insn->length = (unsigned char)((unsigned long)insn->next_byte
- (unsigned long)insn->kaddr);
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-02-22 11:31 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-22 11:16 [PATCH 0/2] Cleanup x86 instruction decoder Nikolay Borisov
2024-02-22 11:16 ` [PATCH 1/2] x86/insn: Remove superfluous checks from instruction decoding routines Nikolay Borisov
2024-02-22 11:31 ` [tip: x86/asm] " tip-bot2 for Nikolay Borisov
2024-02-22 11:16 ` [PATCH 2/2] x86/insn: Directly assign x86_64 state in init_ins Nikolay Borisov
2024-02-22 11:31 ` [tip: x86/asm] x86/insn: Directly assign x86_64 state in insn_init() tip-bot2 for Nikolay Borisov
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®