Re: [PATCH v5 20/21] powerpc sstep: Add support for prefixed load/stores
Hi Jordan,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on v5.6]
[cannot apply to powerpc/next kvm-ppc/kvm-ppc-next scottwood/next next-20200406]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Jordan-Niethe/Initial-Prefixed-Instruction-support/20200406-165215
base:7111951b8d4973bda27ff663f2cf18b663d15b48
config: powerpc-allnoconfig (attached as .config)
compiler: powerpc-linux-gcc (GCC) 9.3.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=9.3.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
All errors (new ones prefixed by >>):
In file included from arch/powerpc/include/asm/code-patching.h:14,
from arch/powerpc/include/asm/kprobes.h:24,
from include/linux/kprobes.h:30,
from arch/powerpc/lib/sstep.c:8:
arch/powerpc/include/asm/inst.h:69:38: error: unknown type name 'ppc_inst'
69 | static inline bool ppc_inst_prefixed(ppc_inst x)
| ^~~~
arch/powerpc/include/asm/inst.h:79:19: error: redefinition of 'ppc_inst_val'
79 | static inline u32 ppc_inst_val(struct ppc_inst x)
| ^~~~
arch/powerpc/include/asm/inst.h:21:19: note: previous definition of
'ppc_inst_val' was here
21 | static inline u32 ppc_inst_val(struct ppc_inst x)
| ^~~~
arch/powerpc/include/asm/inst.h: In function 'ppc_inst_len':
arch/powerpc/include/asm/inst.h:103:10: error: implicit declaration of
function 'ppc_inst_prefixed'; did you mean 'ppc_inst_write'?
[-Werror=implicit-function-declaration]
103 | return (ppc_inst_prefixed(x)) ? 8 : 4;
| ^
| ppc_inst_write
arch/powerpc/lib/sstep.c: In function 'analyse_instr':
arch/powerpc/lib/sstep.c:1215:11: error: implicit declaration of function
'ppc_inst_suffix'; did you mean 'ppc_inst_swab'?
[-Werror=implicit-function-declaration]
1215 | suffix = ppc_inst_suffix(instr);
| ^~~
| ppc_inst_swab
>> arch/powerpc/lib/sstep.c:1207:41: error: unused variable 'prefix_r'
>> [-Werror=unused-variable]
1207 | unsigned int suffixopcode, prefixtype, prefix_r;
| ^~~~
>> arch/powerpc/lib/sstep.c:1207:29: error: unused variable 'prefixtype'
>> [-Werror=unused-variable]
1207 | unsigned int suffixopcode, prefixtype, prefix_r;
| ^~
>> arch/powerpc/lib/sstep.c:1207:15: error: unused variable 'suffixopcode'
>> [-Werror=unused-variable]
1207 | unsigned int suffixopcode, prefixtype, prefix_r;
| ^~~~
cc1: all warnings being treated as errors
vim +/prefix_r +1207 arch/powerpc/lib/sstep.c
1191
1192 /*
1193 * Decode an instruction, and return information about it in *op
1194 * without changing *regs.
1195 * Integer arithmetic and logical instructions, branches, and barrier
1196 * instructions can be emulated just using the information in *op.
1197 *
1198 * Return value is 1 if the instruction can be emulated just by
1199 * updating *regs with the information in *op, -1 if we need the
1200 * GPRs but *regs doesn't contain the full register set, or 0
1201 * otherwise.
1202 */
1203 int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
1204struct ppc_inst instr)
1205 {
1206 unsigned int opcode, ra, rb, rc, rd, spr, u;
> 1207 unsigned int suffixopcode, prefixtype, prefix_r;
1208 unsigned long int imm;
1209 unsigned long int val, val2;
1210 unsigned int mb, me, sh;
1211 unsigned int word, suffix;
1212 long ival;
1213
1214 word = ppc_inst_val(instr);
1215 suffix = ppc_inst_suffix(instr);
1216
1217 op->type = COMPUTE;
1218
1219 opcode = word >> 26;
1220 switch (opcode) {
1221 case 16:/* bc */
1222 op->type = BRANCH;
1223 imm = (signed short)(word & 0xfffc);
1224 if ((word & 2) == 0)
1225 imm += regs->nip;
1226 op->val = truncate_if_32bit(regs->msr, imm);
1227 if (word & 1)
1228 op->type |= SETLK;
1229 if
[PATCH v5 20/21] powerpc sstep: Add support for prefixed load/stores
This adds emulation support for the following prefixed integer
load/stores:
* Prefixed Load Byte and Zero (plbz)
* Prefixed Load Halfword and Zero (plhz)
* Prefixed Load Halfword Algebraic (plha)
* Prefixed Load Word and Zero (plwz)
* Prefixed Load Word Algebraic (plwa)
* Prefixed Load Doubleword (pld)
* Prefixed Store Byte (pstb)
* Prefixed Store Halfword (psth)
* Prefixed Store Word (pstw)
* Prefixed Store Doubleword (pstd)
* Prefixed Load Quadword (plq)
* Prefixed Store Quadword (pstq)
the follow prefixed floating-point load/stores:
* Prefixed Load Floating-Point Single (plfs)
* Prefixed Load Floating-Point Double (plfd)
* Prefixed Store Floating-Point Single (pstfs)
* Prefixed Store Floating-Point Double (pstfd)
and for the following prefixed VSX load/stores:
* Prefixed Load VSX Scalar Doubleword (plxsd)
* Prefixed Load VSX Scalar Single-Precision (plxssp)
* Prefixed Load VSX Vector [0|1] (plxv, plxv0, plxv1)
* Prefixed Store VSX Scalar Doubleword (pstxsd)
* Prefixed Store VSX Scalar Single-Precision (pstxssp)
* Prefixed Store VSX Vector [0|1] (pstxv, pstxv0, pstxv1)
Reviewed-by: Balamuruhan S
Signed-off-by: Jordan Niethe
---
v2: - Combine all load/store patches
- Fix the name of Type 01 instructions
- Remove sign extension flag from pstd/pld
- Rename sufx -> suffix
v3: - Move prefixed loads and stores into the switch statement
---
arch/powerpc/include/asm/sstep.h | 4 +
arch/powerpc/lib/sstep.c | 159 +++
2 files changed, 163 insertions(+)
diff --git a/arch/powerpc/include/asm/sstep.h b/arch/powerpc/include/asm/sstep.h
index c3ce903ac488..9b200a5f8794 100644
--- a/arch/powerpc/include/asm/sstep.h
+++ b/arch/powerpc/include/asm/sstep.h
@@ -90,11 +90,15 @@ enum instruction_type {
#define VSX_LDLEFT 4 /* load VSX register from left */
#define VSX_CHECK_VEC 8 /* check MSR_VEC not MSR_VSX for reg >= 32 */
+/* Prefixed flag, ORed in with type */
+#define PREFIXED 0x800
+
/* Size field in type word */
#define SIZE(n)((n) << 12)
#define GETSIZE(w) ((w) >> 12)
#define GETTYPE(t) ((t) & INSTR_TYPE_MASK)
+#define GETLENGTH(t) (((t) & PREFIXED) ? 8 : 4)
#define MKOP(t, f, s) ((t) | (f) | SIZE(s))
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 8b285bf11218..8b6aee0ee636 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -187,6 +187,44 @@ static nokprobe_inline unsigned long xform_ea(unsigned int
instr,
return ea;
}
+/*
+ * Calculate effective address for a MLS:D-form / 8LS:D-form
+ * prefixed instruction
+ */
+static nokprobe_inline unsigned long mlsd_8lsd_ea(unsigned int instr,
+ unsigned int suffix,
+ const struct pt_regs *regs)
+{
+ int ra, prefix_r;
+ unsigned int dd;
+ unsigned long ea, d0, d1, d;
+
+ prefix_r = instr & (1ul << 20);
+ ra = (suffix >> 16) & 0x1f;
+
+ d0 = instr & 0x3;
+ d1 = suffix & 0x;
+ d = (d0 << 16) | d1;
+
+ /*
+* sign extend a 34 bit number
+*/
+ dd = (unsigned int)(d >> 2);
+ ea = (signed int)dd;
+ ea = (ea << 2) | (d & 0x3);
+
+ if (!prefix_r && ra)
+ ea += regs->gpr[ra];
+ else if (!prefix_r && !ra)
+ ; /* Leave ea as is */
+ else if (prefix_r && !ra)
+ ea += regs->nip;
+ else if (prefix_r && ra)
+ ; /* Invalid form. Should already be checked for by caller! */
+
+ return ea;
+}
+
/*
* Return the largest power of 2, not greater than sizeof(unsigned long),
* such that x is a multiple of it.
@@ -1166,6 +1204,7 @@ int analyse_instr(struct instruction_op *op, const struct
pt_regs *regs,
struct ppc_inst instr)
{
unsigned int opcode, ra, rb, rc, rd, spr, u;
+ unsigned int suffixopcode, prefixtype, prefix_r;
unsigned long int imm;
unsigned long int val, val2;
unsigned int mb, me, sh;
@@ -2652,6 +2691,126 @@ int analyse_instr(struct instruction_op *op, const
struct pt_regs *regs,
break;
}
break;
+ case 1: /* Prefixed instructions */
+ prefix_r = word & (1ul << 20);
+ ra = (suffix >> 16) & 0x1f;
+ op->update_reg = ra;
+ rd = (suffix >> 21) & 0x1f;
+ op->reg = rd;
+ op->val = regs->gpr[rd];
+
+ suffixopcode = suffix >> 26;
+ prefixtype = (word >> 24) & 0x3;
+ switch (prefixtype) {
+ case 0: /* Type 00 Eight-Byte Load/Store */
+ if (prefix_r && ra)
+ break;
+ op->ea = mlsd_8lsd_ea(word, suffix, regs);
+ switch (suffixopcode) {
+
