* [PATCH 1/3] powerpc: Move PPC_HA() PPC_HI() and PPC_LO() to ppc-opcode.h
@ 2019-04-29 10:43 ` Christophe Leroy
0 siblings, 0 replies; 10+ messages in thread
From: Christophe Leroy @ 2019-04-29 10:43 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linux-kernel, linuxppc-dev
PPC_HA() PPC_HI() and PPC_LO() macros are nice macros. Move them
from module64.c to ppc-opcode.h in order to use them in other places.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/ppc-opcode.h | 7 +++++++
arch/powerpc/kernel/module_64.c | 7 -------
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index 23f7ed796f38..c5ff44400d4d 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -412,6 +412,13 @@
#define __PPC_SPR(r) ((((r) & 0x1f) << 16) | ((((r) >> 5) & 0x1f) << 11))
#define __PPC_RC21 (0x1 << 10)
+/* Both low and high 16 bits are added as SIGNED additions, so if low
+ 16 bits has high bit set, high 16 bits must be adjusted. These
+ macros do that (stolen from binutils). */
+#define PPC_LO(v) ((v) & 0xffff)
+#define PPC_HI(v) (((v) >> 16) & 0xffff)
+#define PPC_HA(v) PPC_HI ((v) + 0x8000)
+
/*
* Only use the larx hint bit on 64bit CPUs. e500v1/v2 based CPUs will treat a
* larx with EH set as an illegal instruction.
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 8661eea78503..c2e1b06253b8 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -400,13 +400,6 @@ static inline unsigned long my_r2(const Elf64_Shdr *sechdrs, struct module *me)
return (sechdrs[me->arch.toc_section].sh_addr & ~0xfful) + 0x8000;
}
-/* Both low and high 16 bits are added as SIGNED additions, so if low
- 16 bits has high bit set, high 16 bits must be adjusted. These
- macros do that (stolen from binutils). */
-#define PPC_LO(v) ((v) & 0xffff)
-#define PPC_HI(v) (((v) >> 16) & 0xffff)
-#define PPC_HA(v) PPC_HI ((v) + 0x8000)
-
/* Patch stub to reference function and correct r2 value. */
static inline int create_stub(const Elf64_Shdr *sechdrs,
struct ppc64_stub_entry *entry,
--
2.13.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 1/3] powerpc: Move PPC_HA() PPC_HI() and PPC_LO() to ppc-opcode.h
@ 2019-04-29 10:43 ` Christophe Leroy
0 siblings, 0 replies; 10+ messages in thread
From: Christophe Leroy @ 2019-04-29 10:43 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
PPC_HA() PPC_HI() and PPC_LO() macros are nice macros. Move them
from module64.c to ppc-opcode.h in order to use them in other places.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/ppc-opcode.h | 7 +++++++
arch/powerpc/kernel/module_64.c | 7 -------
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index 23f7ed796f38..c5ff44400d4d 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -412,6 +412,13 @@
#define __PPC_SPR(r) ((((r) & 0x1f) << 16) | ((((r) >> 5) & 0x1f) << 11))
#define __PPC_RC21 (0x1 << 10)
+/* Both low and high 16 bits are added as SIGNED additions, so if low
+ 16 bits has high bit set, high 16 bits must be adjusted. These
+ macros do that (stolen from binutils). */
+#define PPC_LO(v) ((v) & 0xffff)
+#define PPC_HI(v) (((v) >> 16) & 0xffff)
+#define PPC_HA(v) PPC_HI ((v) + 0x8000)
+
/*
* Only use the larx hint bit on 64bit CPUs. e500v1/v2 based CPUs will treat a
* larx with EH set as an illegal instruction.
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 8661eea78503..c2e1b06253b8 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -400,13 +400,6 @@ static inline unsigned long my_r2(const Elf64_Shdr *sechdrs, struct module *me)
return (sechdrs[me->arch.toc_section].sh_addr & ~0xfful) + 0x8000;
}
-/* Both low and high 16 bits are added as SIGNED additions, so if low
- 16 bits has high bit set, high 16 bits must be adjusted. These
- macros do that (stolen from binutils). */
-#define PPC_LO(v) ((v) & 0xffff)
-#define PPC_HI(v) (((v) >> 16) & 0xffff)
-#define PPC_HA(v) PPC_HI ((v) + 0x8000)
-
/* Patch stub to reference function and correct r2 value. */
static inline int create_stub(const Elf64_Shdr *sechdrs,
struct ppc64_stub_entry *entry,
--
2.13.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] powerpc/module32: Use symbolic instructions names.
2019-04-29 10:43 ` Christophe Leroy
@ 2019-04-29 10:43 ` Christophe Leroy
-1 siblings, 0 replies; 10+ messages in thread
From: Christophe Leroy @ 2019-04-29 10:43 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linux-kernel, linuxppc-dev
To increase readability/maintainability, replace hard coded
instructions values by symbolic names.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/kernel/module_32.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/module_32.c b/arch/powerpc/kernel/module_32.c
index 88d83771f462..c5197f856c75 100644
--- a/arch/powerpc/kernel/module_32.c
+++ b/arch/powerpc/kernel/module_32.c
@@ -170,10 +170,14 @@ int module_frob_arch_sections(Elf32_Ehdr *hdr,
return 0;
}
+ /* lis r12,sym@ha */
+#define ENTRY_JMP0(sym) (PPC_INST_ADDIS | __PPC_RT(R12) | PPC_HA(sym))
+ /* addi r12,r12,sym@l */
+#define ENTRY_JMP1(sym) (PPC_INST_ADDI | __PPC_RT(R12) | __PPC_RA(R12) | PPC_LO(sym))
+
static inline int entry_matches(struct ppc_plt_entry *entry, Elf32_Addr val)
{
- if (entry->jump[0] == 0x3d800000 + ((val + 0x8000) >> 16)
- && entry->jump[1] == 0x398c0000 + (val & 0xffff))
+ if (entry->jump[0] == ENTRY_JMP0(val) && entry->jump[1] == ENTRY_JMP1(val))
return 1;
return 0;
}
@@ -200,10 +204,10 @@ static uint32_t do_plt_call(void *location,
entry++;
}
- entry->jump[0] = 0x3d800000+((val+0x8000)>>16); /* lis r12,sym@ha */
- entry->jump[1] = 0x398c0000 + (val&0xffff); /* addi r12,r12,sym@l*/
- entry->jump[2] = 0x7d8903a6; /* mtctr r12 */
- entry->jump[3] = 0x4e800420; /* bctr */
+ entry->jump[0] = ENTRY_JMP0(val);
+ entry->jump[1] = ENTRY_JMP1(val);
+ entry->jump[2] = PPC_INST_MTCTR | __PPC_RS(R12);
+ entry->jump[3] = PPC_INST_BCTR;
pr_debug("Initialized plt for 0x%x at %p\n", val, entry);
return (uint32_t)entry;
--
2.13.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] powerpc/module32: Use symbolic instructions names.
@ 2019-04-29 10:43 ` Christophe Leroy
0 siblings, 0 replies; 10+ messages in thread
From: Christophe Leroy @ 2019-04-29 10:43 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
To increase readability/maintainability, replace hard coded
instructions values by symbolic names.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/kernel/module_32.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/module_32.c b/arch/powerpc/kernel/module_32.c
index 88d83771f462..c5197f856c75 100644
--- a/arch/powerpc/kernel/module_32.c
+++ b/arch/powerpc/kernel/module_32.c
@@ -170,10 +170,14 @@ int module_frob_arch_sections(Elf32_Ehdr *hdr,
return 0;
}
+ /* lis r12,sym@ha */
+#define ENTRY_JMP0(sym) (PPC_INST_ADDIS | __PPC_RT(R12) | PPC_HA(sym))
+ /* addi r12,r12,sym@l */
+#define ENTRY_JMP1(sym) (PPC_INST_ADDI | __PPC_RT(R12) | __PPC_RA(R12) | PPC_LO(sym))
+
static inline int entry_matches(struct ppc_plt_entry *entry, Elf32_Addr val)
{
- if (entry->jump[0] == 0x3d800000 + ((val + 0x8000) >> 16)
- && entry->jump[1] == 0x398c0000 + (val & 0xffff))
+ if (entry->jump[0] == ENTRY_JMP0(val) && entry->jump[1] == ENTRY_JMP1(val))
return 1;
return 0;
}
@@ -200,10 +204,10 @@ static uint32_t do_plt_call(void *location,
entry++;
}
- entry->jump[0] = 0x3d800000+((val+0x8000)>>16); /* lis r12,sym@ha */
- entry->jump[1] = 0x398c0000 + (val&0xffff); /* addi r12,r12,sym@l*/
- entry->jump[2] = 0x7d8903a6; /* mtctr r12 */
- entry->jump[3] = 0x4e800420; /* bctr */
+ entry->jump[0] = ENTRY_JMP0(val);
+ entry->jump[1] = ENTRY_JMP1(val);
+ entry->jump[2] = PPC_INST_MTCTR | __PPC_RS(R12);
+ entry->jump[3] = PPC_INST_BCTR;
pr_debug("Initialized plt for 0x%x at %p\n", val, entry);
return (uint32_t)entry;
--
2.13.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] powerpc/module64: Use symbolic instructions names.
2019-04-29 10:43 ` Christophe Leroy
@ 2019-04-29 10:43 ` Christophe Leroy
-1 siblings, 0 replies; 10+ messages in thread
From: Christophe Leroy @ 2019-04-29 10:43 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linux-kernel, linuxppc-dev
To increase readability/maintainability, replace hard coded
instructions values by symbolic names.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/kernel/module_64.c | 54 ++++++++++++++++++++++++++++-------------
1 file changed, 37 insertions(+), 17 deletions(-)
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index c2e1b06253b8..87097eae600b 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -135,18 +135,28 @@ struct ppc64_stub_entry
* to the TOC ptr, r2) into the stub.
*/
+ unsigned int i, num_stubs;
+ /*
+ * addis r11,r2, <high>
+ * addi r11,r11, <low>
+ * std r2,R2_STACK_OFFSET(r1)
+ * ld r12,32(r11)
+ * ld r2,40(r11)
+ * mtctr r12
+ * bctr
+ */
static u32 ppc64_stub_insns[] = {
- 0x3d620000, /* addis r11,r2, <high> */
- 0x396b0000, /* addi r11,r11, <low> */
+ PPC_INST_ADDIS | __PPC_RT(R11) | __PPC_RA(R2),
+ PPC_INST_ADDI | __PPC_RT(R11) | __PPC_RA(R11),
/* Save current r2 value in magic place on the stack. */
- 0xf8410000|R2_STACK_OFFSET, /* std r2,R2_STACK_OFFSET(r1) */
- 0xe98b0020, /* ld r12,32(r11) */
+ PPC_INST_STD | __PPC_RS(R2) | __PPC_RA(R1) | R2_STACK_OFFSET,
+ PPC_INST_LD | __PPC_RT(R12) | __PPC_RA(R11) | 32,
#ifdef PPC64_ELF_ABI_v1
/* Set up new r2 from function descriptor */
- 0xe84b0028, /* ld r2,40(r11) */
+ PPC_INST_LD | __PPC_RT(R2) | __PPC_RA(R11) | 40,
#endif
- 0x7d8903a6, /* mtctr r12 */
- 0x4e800420 /* bctr */
+ PPC_INST_MTCTR | __PPC_RS(R12),
+ PPC_INST_BCTR,
};
#ifdef CONFIG_DYNAMIC_FTRACE
@@ -704,18 +714,21 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
* ld r2, ...(r12)
* add r2, r2, r12
*/
- if ((((uint32_t *)location)[0] & ~0xfffc)
- != 0xe84c0000)
+ if ((((uint32_t *)location)[0] & ~0xfffc) !=
+ PPC_INST_LD | __PPC_RT(R2) | __PPC_RA(R12))
break;
- if (((uint32_t *)location)[1] != 0x7c426214)
+ if (((uint32_t *)location)[1] !=
+ PPC_INST_ADD | __PPC_RT(R2) | __PPC_RA(R2) | __PPC_RB(R12))
break;
/*
* If found, replace it with:
* addis r2, r12, (.TOC.-func)@ha
* addi r2, r12, (.TOC.-func)@l
*/
- ((uint32_t *)location)[0] = 0x3c4c0000 + PPC_HA(value);
- ((uint32_t *)location)[1] = 0x38420000 + PPC_LO(value);
+ ((uint32_t *)location)[0] = PPC_INST_ADDIS | __PPC_RT(R2) |
+ __PPC_RA(R12) | PPC_HA(value);
+ ((uint32_t *)location)[1] = PPC_INST_ADDI | __PPC_RT(R2) |
+ __PPC_RA(R12) | PPC_LO(value);
break;
case R_PPC64_REL16_HA:
@@ -769,12 +782,19 @@ static unsigned long create_ftrace_stub(const Elf64_Shdr *sechdrs,
{
struct ppc64_stub_entry *entry;
unsigned int i, num_stubs;
+ /*
+ * ld r12,PACATOC(r13)
+ * addis r12,r12,<high>
+ * addi r12,r12,<low>
+ * mtctr r12
+ * bctr
+ */
static u32 stub_insns[] = {
- 0xe98d0000 | PACATOC, /* ld r12,PACATOC(r13) */
- 0x3d8c0000, /* addis r12,r12,<high> */
- 0x398c0000, /* addi r12,r12,<low> */
- 0x7d8903a6, /* mtctr r12 */
- 0x4e800420, /* bctr */
+ PPC_INST_LD | __PPC_RT(R12) | __PPC_RA(R13) | PACATOC,
+ PPC_INST_ADDIS | __PPC_RT(R12) | __PPC_RA(R12),
+ PPC_INST_ADDI | __PPC_RT(R12) | __PPC_RA(R12),
+ PPC_INST_MTCTR | __PPC_RS(R12),
+ PPC_INST_BCTR,
};
long reladdr;
--
2.13.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] powerpc/module64: Use symbolic instructions names.
@ 2019-04-29 10:43 ` Christophe Leroy
0 siblings, 0 replies; 10+ messages in thread
From: Christophe Leroy @ 2019-04-29 10:43 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
To increase readability/maintainability, replace hard coded
instructions values by symbolic names.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/kernel/module_64.c | 54 ++++++++++++++++++++++++++++-------------
1 file changed, 37 insertions(+), 17 deletions(-)
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index c2e1b06253b8..87097eae600b 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -135,18 +135,28 @@ struct ppc64_stub_entry
* to the TOC ptr, r2) into the stub.
*/
+ unsigned int i, num_stubs;
+ /*
+ * addis r11,r2, <high>
+ * addi r11,r11, <low>
+ * std r2,R2_STACK_OFFSET(r1)
+ * ld r12,32(r11)
+ * ld r2,40(r11)
+ * mtctr r12
+ * bctr
+ */
static u32 ppc64_stub_insns[] = {
- 0x3d620000, /* addis r11,r2, <high> */
- 0x396b0000, /* addi r11,r11, <low> */
+ PPC_INST_ADDIS | __PPC_RT(R11) | __PPC_RA(R2),
+ PPC_INST_ADDI | __PPC_RT(R11) | __PPC_RA(R11),
/* Save current r2 value in magic place on the stack. */
- 0xf8410000|R2_STACK_OFFSET, /* std r2,R2_STACK_OFFSET(r1) */
- 0xe98b0020, /* ld r12,32(r11) */
+ PPC_INST_STD | __PPC_RS(R2) | __PPC_RA(R1) | R2_STACK_OFFSET,
+ PPC_INST_LD | __PPC_RT(R12) | __PPC_RA(R11) | 32,
#ifdef PPC64_ELF_ABI_v1
/* Set up new r2 from function descriptor */
- 0xe84b0028, /* ld r2,40(r11) */
+ PPC_INST_LD | __PPC_RT(R2) | __PPC_RA(R11) | 40,
#endif
- 0x7d8903a6, /* mtctr r12 */
- 0x4e800420 /* bctr */
+ PPC_INST_MTCTR | __PPC_RS(R12),
+ PPC_INST_BCTR,
};
#ifdef CONFIG_DYNAMIC_FTRACE
@@ -704,18 +714,21 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
* ld r2, ...(r12)
* add r2, r2, r12
*/
- if ((((uint32_t *)location)[0] & ~0xfffc)
- != 0xe84c0000)
+ if ((((uint32_t *)location)[0] & ~0xfffc) !=
+ PPC_INST_LD | __PPC_RT(R2) | __PPC_RA(R12))
break;
- if (((uint32_t *)location)[1] != 0x7c426214)
+ if (((uint32_t *)location)[1] !=
+ PPC_INST_ADD | __PPC_RT(R2) | __PPC_RA(R2) | __PPC_RB(R12))
break;
/*
* If found, replace it with:
* addis r2, r12, (.TOC.-func)@ha
* addi r2, r12, (.TOC.-func)@l
*/
- ((uint32_t *)location)[0] = 0x3c4c0000 + PPC_HA(value);
- ((uint32_t *)location)[1] = 0x38420000 + PPC_LO(value);
+ ((uint32_t *)location)[0] = PPC_INST_ADDIS | __PPC_RT(R2) |
+ __PPC_RA(R12) | PPC_HA(value);
+ ((uint32_t *)location)[1] = PPC_INST_ADDI | __PPC_RT(R2) |
+ __PPC_RA(R12) | PPC_LO(value);
break;
case R_PPC64_REL16_HA:
@@ -769,12 +782,19 @@ static unsigned long create_ftrace_stub(const Elf64_Shdr *sechdrs,
{
struct ppc64_stub_entry *entry;
unsigned int i, num_stubs;
+ /*
+ * ld r12,PACATOC(r13)
+ * addis r12,r12,<high>
+ * addi r12,r12,<low>
+ * mtctr r12
+ * bctr
+ */
static u32 stub_insns[] = {
- 0xe98d0000 | PACATOC, /* ld r12,PACATOC(r13) */
- 0x3d8c0000, /* addis r12,r12,<high> */
- 0x398c0000, /* addi r12,r12,<low> */
- 0x7d8903a6, /* mtctr r12 */
- 0x4e800420, /* bctr */
+ PPC_INST_LD | __PPC_RT(R12) | __PPC_RA(R13) | PACATOC,
+ PPC_INST_ADDIS | __PPC_RT(R12) | __PPC_RA(R12),
+ PPC_INST_ADDI | __PPC_RT(R12) | __PPC_RA(R12),
+ PPC_INST_MTCTR | __PPC_RS(R12),
+ PPC_INST_BCTR,
};
long reladdr;
--
2.13.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] powerpc/module32: Use symbolic instructions names.
2019-04-29 10:43 ` Christophe Leroy
@ 2019-04-29 11:54 ` Segher Boessenkool
-1 siblings, 0 replies; 10+ messages in thread
From: Segher Boessenkool @ 2019-04-29 11:54 UTC (permalink / raw)
To: Christophe Leroy
Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
linuxppc-dev, linux-kernel
On Mon, Apr 29, 2019 at 10:43:27AM +0000, Christophe Leroy wrote:
> To increase readability/maintainability, replace hard coded
> instructions values by symbolic names.
> + /* lis r12,sym@ha */
> +#define ENTRY_JMP0(sym) (PPC_INST_ADDIS | __PPC_RT(R12) | PPC_HA(sym))
> + /* addi r12,r12,sym@l */
> +#define ENTRY_JMP1(sym) (PPC_INST_ADDI | __PPC_RT(R12) | __PPC_RA(R12) | PPC_LO(sym))
Those aren't "jump" instructions though, as the name suggests... And you
only have names for the first two of the four insns. ("2" and "3" were
still available ;-) )
> - entry->jump[0] = 0x3d800000+((val+0x8000)>>16); /* lis r12,sym@ha */
> - entry->jump[1] = 0x398c0000 + (val&0xffff); /* addi r12,r12,sym@l*/
> - entry->jump[2] = 0x7d8903a6; /* mtctr r12 */
> - entry->jump[3] = 0x4e800420; /* bctr */
> + entry->jump[0] = ENTRY_JMP0(val);
> + entry->jump[1] = ENTRY_JMP1(val);
> + entry->jump[2] = PPC_INST_MTCTR | __PPC_RS(R12);
> + entry->jump[3] = PPC_INST_BCTR;
Deleting the comment here is not an improvement imo.
Segher
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] powerpc/module32: Use symbolic instructions names.
@ 2019-04-29 11:54 ` Segher Boessenkool
0 siblings, 0 replies; 10+ messages in thread
From: Segher Boessenkool @ 2019-04-29 11:54 UTC (permalink / raw)
To: Christophe Leroy; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
On Mon, Apr 29, 2019 at 10:43:27AM +0000, Christophe Leroy wrote:
> To increase readability/maintainability, replace hard coded
> instructions values by symbolic names.
> + /* lis r12,sym@ha */
> +#define ENTRY_JMP0(sym) (PPC_INST_ADDIS | __PPC_RT(R12) | PPC_HA(sym))
> + /* addi r12,r12,sym@l */
> +#define ENTRY_JMP1(sym) (PPC_INST_ADDI | __PPC_RT(R12) | __PPC_RA(R12) | PPC_LO(sym))
Those aren't "jump" instructions though, as the name suggests... And you
only have names for the first two of the four insns. ("2" and "3" were
still available ;-) )
> - entry->jump[0] = 0x3d800000+((val+0x8000)>>16); /* lis r12,sym@ha */
> - entry->jump[1] = 0x398c0000 + (val&0xffff); /* addi r12,r12,sym@l*/
> - entry->jump[2] = 0x7d8903a6; /* mtctr r12 */
> - entry->jump[3] = 0x4e800420; /* bctr */
> + entry->jump[0] = ENTRY_JMP0(val);
> + entry->jump[1] = ENTRY_JMP1(val);
> + entry->jump[2] = PPC_INST_MTCTR | __PPC_RS(R12);
> + entry->jump[3] = PPC_INST_BCTR;
Deleting the comment here is not an improvement imo.
Segher
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] powerpc/module32: Use symbolic instructions names.
2019-04-29 11:54 ` Segher Boessenkool
@ 2019-05-02 7:24 ` Christophe Leroy
-1 siblings, 0 replies; 10+ messages in thread
From: Christophe Leroy @ 2019-05-02 7:24 UTC (permalink / raw)
To: Segher Boessenkool
Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
linuxppc-dev, linux-kernel
Le 29/04/2019 à 13:54, Segher Boessenkool a écrit :
> On Mon, Apr 29, 2019 at 10:43:27AM +0000, Christophe Leroy wrote:
>> To increase readability/maintainability, replace hard coded
>> instructions values by symbolic names.
>
>> + /* lis r12,sym@ha */
>> +#define ENTRY_JMP0(sym) (PPC_INST_ADDIS | __PPC_RT(R12) | PPC_HA(sym))
>> + /* addi r12,r12,sym@l */
>> +#define ENTRY_JMP1(sym) (PPC_INST_ADDI | __PPC_RT(R12) | __PPC_RA(R12) | PPC_LO(sym))
>
> Those aren't "jump" instructions though, as the name suggests... And you
> only have names for the first two of the four insns. ("2" and "3" were
> still available ;-) )
Well, the idea was to say they are defining the jump destination.
Anyway, as they are used only once, let's put it directly in.
>
>> - entry->jump[0] = 0x3d800000+((val+0x8000)>>16); /* lis r12,sym@ha */
>> - entry->jump[1] = 0x398c0000 + (val&0xffff); /* addi r12,r12,sym@l*/
>> - entry->jump[2] = 0x7d8903a6; /* mtctr r12 */
>> - entry->jump[3] = 0x4e800420; /* bctr */
>> + entry->jump[0] = ENTRY_JMP0(val);
>> + entry->jump[1] = ENTRY_JMP1(val);
>> + entry->jump[2] = PPC_INST_MTCTR | __PPC_RS(R12);
>> + entry->jump[3] = PPC_INST_BCTR;
>
> Deleting the comment here is not an improvement imo.
Ok, I'll leave them in as I did for module64
Christophe
>
>
> Segher
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] powerpc/module32: Use symbolic instructions names.
@ 2019-05-02 7:24 ` Christophe Leroy
0 siblings, 0 replies; 10+ messages in thread
From: Christophe Leroy @ 2019-05-02 7:24 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
Le 29/04/2019 à 13:54, Segher Boessenkool a écrit :
> On Mon, Apr 29, 2019 at 10:43:27AM +0000, Christophe Leroy wrote:
>> To increase readability/maintainability, replace hard coded
>> instructions values by symbolic names.
>
>> + /* lis r12,sym@ha */
>> +#define ENTRY_JMP0(sym) (PPC_INST_ADDIS | __PPC_RT(R12) | PPC_HA(sym))
>> + /* addi r12,r12,sym@l */
>> +#define ENTRY_JMP1(sym) (PPC_INST_ADDI | __PPC_RT(R12) | __PPC_RA(R12) | PPC_LO(sym))
>
> Those aren't "jump" instructions though, as the name suggests... And you
> only have names for the first two of the four insns. ("2" and "3" were
> still available ;-) )
Well, the idea was to say they are defining the jump destination.
Anyway, as they are used only once, let's put it directly in.
>
>> - entry->jump[0] = 0x3d800000+((val+0x8000)>>16); /* lis r12,sym@ha */
>> - entry->jump[1] = 0x398c0000 + (val&0xffff); /* addi r12,r12,sym@l*/
>> - entry->jump[2] = 0x7d8903a6; /* mtctr r12 */
>> - entry->jump[3] = 0x4e800420; /* bctr */
>> + entry->jump[0] = ENTRY_JMP0(val);
>> + entry->jump[1] = ENTRY_JMP1(val);
>> + entry->jump[2] = PPC_INST_MTCTR | __PPC_RS(R12);
>> + entry->jump[3] = PPC_INST_BCTR;
>
> Deleting the comment here is not an improvement imo.
Ok, I'll leave them in as I did for module64
Christophe
>
>
> Segher
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-05-02 7:26 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-29 10:43 [PATCH 1/3] powerpc: Move PPC_HA() PPC_HI() and PPC_LO() to ppc-opcode.h Christophe Leroy
2019-04-29 10:43 ` Christophe Leroy
2019-04-29 10:43 ` [PATCH 2/3] powerpc/module32: Use symbolic instructions names Christophe Leroy
2019-04-29 10:43 ` Christophe Leroy
2019-04-29 11:54 ` Segher Boessenkool
2019-04-29 11:54 ` Segher Boessenkool
2019-05-02 7:24 ` Christophe Leroy
2019-05-02 7:24 ` Christophe Leroy
2019-04-29 10:43 ` [PATCH 3/3] powerpc/module64: " Christophe Leroy
2019-04-29 10:43 ` Christophe Leroy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.