lkml.org 
[lkml]   [2022]   [Oct]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH V2] LoongArch: Add unaligned access support
From
On 2022/10/17 16:59, Rui Wang wrote:
>> +void emulate_load_store_insn(struct pt_regs *regs, void __user *addr, unsigned int *pc)
>> +{
>> + bool user = user_mode(regs);
>> + unsigned int res;
>> + unsigned long origpc;
>> + unsigned long origra;
>> + unsigned long value = 0;
>> + union loongarch_instruction insn;
>> +
>> + origpc = (unsigned long)pc;
>> + origra = regs->regs[1];
>> +
>> + perf_sw_event(PERF_COUNT_SW_EMULATION_FAULTS, 1, regs, 0);
>> +
>> + /*
>> + * This load never faults.
>> + */
>> + __get_inst(&insn.word, pc, user);
>> + if (user && !access_ok(addr, 8))
>> + goto sigbus;
>> +
>> + if (insn.reg2i12_format.opcode == ldd_op ||
>> + insn.reg2i14_format.opcode == ldptrd_op ||
>> + insn.reg3_format.opcode == ldxd_op) {
>> + res = unaligned_read(addr, &value, 8, 1);
>> + if (res)
>> + goto fault;
>> + regs->regs[insn.reg2i12_format.rd] = value;
>> + } else if (insn.reg2i12_format.opcode == ldw_op ||
>> + insn.reg2i14_format.opcode == ldptrw_op ||
>> + insn.reg3_format.opcode == ldxw_op) {
>> + res = unaligned_read(addr, &value, 4, 1);
>> + if (res)
>> + goto fault;
>> + regs->regs[insn.reg2i12_format.rd] = value;
>> + } else if (insn.reg2i12_format.opcode == ldwu_op ||
>> + insn.reg3_format.opcode == ldxwu_op) {
>> + res = unaligned_read(addr, &value, 4, 0);
>> + if (res)
>> + goto fault;
>> + regs->regs[insn.reg2i12_format.rd] = value;
>> + } else if (insn.reg2i12_format.opcode == ldh_op ||
>> + insn.reg3_format.opcode == ldxh_op) {
>> + res = unaligned_read(addr, &value, 2, 1);
>> + if (res)
>> + goto fault;
>> + regs->regs[insn.reg2i12_format.rd] = value;
>> + } else if (insn.reg2i12_format.opcode == ldhu_op ||
>> + insn.reg3_format.opcode == ldxhu_op) {
>> + res = unaligned_read(addr, &value, 2, 0);
>> + if (res)
>> + goto fault;
>> + regs->regs[insn.reg2i12_format.rd] = value;
>> + } else if (insn.reg2i12_format.opcode == std_op ||
>> + insn.reg2i14_format.opcode == stptrd_op ||
>> + insn.reg3_format.opcode == stxd_op) {
>> + value = regs->regs[insn.reg2i12_format.rd];
>> + res = unaligned_write(addr, value, 8);
>> + if (res)
>> + goto fault;
>> + } else if (insn.reg2i12_format.opcode == stw_op ||
>> + insn.reg2i14_format.opcode == stptrw_op ||
>> + insn.reg3_format.opcode == stxw_op) {
>> + value = regs->regs[insn.reg2i12_format.rd];
>> + res = unaligned_write(addr, value, 4);
>> + if (res)
>> + goto fault;
>> + } else if (insn.reg2i12_format.opcode == sth_op ||
>> + insn.reg3_format.opcode == stxh_op) {
>> + value = regs->regs[insn.reg2i12_format.rd];
>> + res = unaligned_write(addr, value, 2);
>> + if (res)
>> + goto fault;
>> + } else if (insn.reg2i12_format.opcode == fldd_op ||
>> + insn.reg3_format.opcode == fldxd_op) {
>> + res = unaligned_read(addr, &value, 8, 1);
>> + if (res)
>> + goto fault;
>> + write_fpr(insn.reg2i12_format.rd, value);
>> + } else if (insn.reg2i12_format.opcode == flds_op ||
>> + insn.reg3_format.opcode == fldxs_op) {
>> + res = unaligned_read(addr, &value, 4, 1);
>> + if (res)
>> + goto fault;
>> + write_fpr(insn.reg2i12_format.rd, value);
>> + } else if (insn.reg2i12_format.opcode == fstd_op ||
>> + insn.reg3_format.opcode == fstxd_op) {
>> + value = read_fpr(insn.reg2i12_format.rd);
>> + res = unaligned_write(addr, value, 8);
>> + if (res)
>> + goto fault;
>> + } else if (insn.reg2i12_format.opcode == fsts_op ||
>> + insn.reg3_format.opcode == fstxs_op) {
>> + value = read_fpr(insn.reg2i12_format.rd);
>> + res = unaligned_write(addr, value, 4);
>> + if (res)
>> + goto fault;
>> + } else
>> + goto sigbus;
>
> So many condtional branches for linear instruction matching, use
> switch case is better?
>
> 0000000000000238 <emulate_load_store_insn>:
> ...
> 35c: 1470180f lu12i.w $t3, 229568(0x380c0)
> 360: 580141af beq $t1, $t3, 320(0x140) # 4a0
> <emulate_load_store_insn+0x268>
> 364: 1451000f lu12i.w $t3, 165888(0x28800)
> 368: 5801dd8f beq $t0, $t3, 476(0x1dc) # 544
> [snip]

The code structure here is basically several switches intermingled with
each other, each matching opcodes for one insn format, but sharing much
code once matched. Quite difficult to express with C without some kind
of code duplication.

But we can try:

{
int size;
bool is_read, is_signed;

switch (insn.reg2i12_format.opcode) {
case ldd_op:
size = 8;
is_read = true;
is_signed = true;
break;

case std_op:
size = 8;
is_read = false;
value = regs->regs[insn.reg2i12_format.rd];
break;

case ldw_op:
size = 4;
is_read = true;
is_signed = true;
break;

case ldwu_op:
size = 4;
is_read = true;
is_signed = false;
break;

/* other opcodes */

default:
/* not 2RI12 */
break;
}

switch (insn.reg2i14_format.opcode) {
case ldptrd_op:
size = 8;
is_read = true;
is_signed = true;
break;

case ldptrw_op:
size = 4;
is_read = true;
is_signed = true;
break;

// ...
}

// other formats

if (!size) {
/* no match */
goto sigbus;
}

if (is_read)
res = unaligned_read(addr, &value, size, is_signed);
else
res = unaligned_write(addr, value, size);

if (res)
goto fault;

// ...
}

This way at least the data flow is clearer, and probably the codegen
quality would benefit as well due to the clearer switch-case structures.
(It's okay if this is not the case, it's not performance-critical anyway
because if we ever hit this at all, performance would have already tanked.)

--
WANG "xen0n" Xuerui

Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/

\
 
 \ /
  Last update: 2022-10-17 11:21    [W:0.201 / U:0.248 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site