[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH 28/42] target/mips/tx79: Move RDHWR usermode kludge to tr
From: |
Maciej W. Rozycki |
Subject: |
Re: [RFC PATCH 28/42] target/mips/tx79: Move RDHWR usermode kludge to trans_SQ() |
Date: |
Tue, 16 Feb 2021 13:21:34 +0100 (CET) |
User-agent: |
Alpine 2.21 (DEB 202 2017-01-01) |
On Tue, 16 Feb 2021, Fredrik Noring wrote:
> > Not that it's odd (the final address is masked, remember), but that it a
> > store
> > to an address in the zero page.
>
> The address always resolves to 0xffffe83b (then masked) in 32-bit KSEG2,
> because rt is always $3 and rd is always $29 so -6085(zero), hence the
> last page (which is much better) rather than the first, as Maciej
> discovered:
>
> https://patchwork.kernel.org/comment/23824173/
>
> Other possible RDHWR encodings are no longer used, and can therefore be
> ignored and revert to SQ:
>
> https://patchwork.kernel.org/comment/23842167/
Or rather were never used in the general case (I can't rule out someone
using that stuff for something, but I wouldn't call it supported; I used
some of it internally while evaluating the speed of RDHWR emulation before
the use of $3 or indeed RDHWR was settled in the TLS psABI, though the
actual code that ultimately went into Linux was developed independently).
> > I would do this as
> >
> > {
> > RDHWR_user 011111 00000 ..... ..... 00000 111011 @rd_rt
> > SQ 011111 ..... ..... ................ @ldst
> > }
>
> Both rd and rt have fixed values, as mentioned.
I would suggest actually supporting variable `rt', see below. Would it
be a problem?
> For reference, RDHWR is currently done like this in the Linux kernel:
>
> if (IS_ENABLED(CONFIG_CPU_R5900)) {
> /*
> * On the R5900, a valid RDHWR instruction
> *
> * +--------+-------+----+----+-------+--------+
> * | 011111 | 00000 | rt | rd | 00000 | 111011 |
> * +--------+-------+----+----+-------+--------+
> * 6 5 5 5 5 6
> *
> * having rt $3 (v1) and rd $29 (MIPS_HWR_ULR) is
> * interpreted as the R5900 specific SQ instruction
> *
> * +--------+-------+----+---------------------+
> * | 011111 | base | rt | offset |
> * +--------+-------+----+---------------------+
> * 6 5 5 16
> *
> * with
> *
> * sq v1,-6085(zero)
> *
> * that asserts an address exception since -6085(zero)
> * always resolves to 0xffffe83b in 32-bit KSEG2.
> *
> * Other legacy values of rd, such as MIPS_HWR_CPUNUM,
> * are ignored.
> */
> if (insn.r_format.func == rdhwr_op &&
> insn.r_format.rd == MIPS_HWR_ULR &&
> insn.r_format.rt == 3 &&
I suggest leaving the `rt' check out for consistency, as changing the
register to read the value of UserLocal into from psABI-mandated $3 does
not cause any issue with the R5900 (the `rt' field overlaps between both
machine instructions, so the encoding placed there does not affect the
KSEG2 access trap caused) and those encodings are also emulated in the
slow path for other legacy ISA CPUs:
case MIPS_HWR_ULR: /* Read UserLocal register */
regs->regs[rt] = ti->tp_value;
return 0;
So e.g. `rdhwr $25, $29' is interpreted as `sq $25,-6085($0)' by the
R5900 => no issue, it still traps.
I know I have previously written that we can ignore `rt' encodings other
than $3, but they are harmless and handling them saves a couple of machine
instructions needed to make the check, so I think while we can, we do not
actually have to ignore them.
> insn.r_format.rs == 0 &&
> insn.r_format.re == 0) {
> if (compute_return_epc(regs) < 0 ||
> simulate_rdhwr(regs, insn.r_format.rd,
> insn.r_format.rt) < 0)
> goto sigill;
> return;
> }
> goto sigbus;
> } else ...
Code continuation quoted left for reference.
Maciej
- Re: [RFC PATCH 24/42] target/mips/tx79: Introduce PROT3W opcode (Parallel Rotate 3 Words), (continued)
[RFC PATCH 29/42] linux-user/mips64: Support the n32 ABI for the R5900, Philippe Mathieu-Daudé, 2021/02/14
[RFC PATCH 30/42] target/mips: Reintroduce the R5900 CPU, Philippe Mathieu-Daudé, 2021/02/14
[RFC PATCH 31/42] default-configs: Support o32 ABI with R5900 64-bit MIPS CPU, Philippe Mathieu-Daudé, 2021/02/14
[RFC PATCH 32/42] docker: Add gentoo-mipsr5900el-cross image, Philippe Mathieu-Daudé, 2021/02/14