diff mbox series

[v4,07/33] linux-user/nios2: Trim target_pc_regs to sp and pc

Message ID 20220308072005.307955-8-richard.henderson@linaro.org
State New
Headers show
Series target/nios2: Shadow register set, EIC and VIC | expand

Commit Message

Richard Henderson March 8, 2022, 7:19 a.m. UTC
The only thing this struct is used for is passing startup values
from elfload.c to the cpu.  We do not need all registers to be
represented, we do not need the kernel internal stack slots.

The userland argc, argv, and envp values are passed on
the stack, so only SP and PC need updating.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/nios2/target_syscall.h | 25 ++-----------------------
 linux-user/elfload.c              |  3 +--
 linux-user/nios2/cpu_loop.c       | 24 +-----------------------
 3 files changed, 4 insertions(+), 48 deletions(-)

Comments

Peter Maydell March 8, 2022, 10 a.m. UTC | #1
On Tue, 8 Mar 2022 at 07:20, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The only thing this struct is used for is passing startup values
> from elfload.c to the cpu.  We do not need all registers to be
> represented, we do not need the kernel internal stack slots.
>
> The userland argc, argv, and envp values are passed on
> the stack, so only SP and PC need updating.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/nios2/target_syscall.h | 25 ++-----------------------
>  linux-user/elfload.c              |  3 +--
>  linux-user/nios2/cpu_loop.c       | 24 +-----------------------
>  3 files changed, 4 insertions(+), 48 deletions(-)

Well, I guess we're not using it for anything else currently,
but if you do this then it's not the target arch's pt_regs
struct any more. And all our other target archs seem to define
the struct to follow the kernel definition even if we don't
happen to use it all.

-- PMM
Richard Henderson March 8, 2022, 7:34 p.m. UTC | #2
On 3/8/22 00:00, Peter Maydell wrote:
> On Tue, 8 Mar 2022 at 07:20, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> The only thing this struct is used for is passing startup values
>> from elfload.c to the cpu.  We do not need all registers to be
>> represented, we do not need the kernel internal stack slots.
>>
>> The userland argc, argv, and envp values are passed on
>> the stack, so only SP and PC need updating.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   linux-user/nios2/target_syscall.h | 25 ++-----------------------
>>   linux-user/elfload.c              |  3 +--
>>   linux-user/nios2/cpu_loop.c       | 24 +-----------------------
>>   3 files changed, 4 insertions(+), 48 deletions(-)
> 
> Well, I guess we're not using it for anything else currently,
> but if you do this then it's not the target arch's pt_regs
> struct any more. And all our other target archs seem to define
> the struct to follow the kernel definition even if we don't
> happen to use it all.

Yes, something I've meant to clean up for ages.

My real goal here was removing the reference to estatus, which then does not need to be 
converted to a different form in the coming patches.

(Assigning to estatus is unusable in user-mode, because rdctl and eret are supervisor 
insns.  This code appears to be mirroring the kernel code in which it is trying to get the 
right bits ready for the context switch to user-mode.  For qemu, we've done that for 
user-only during reset, and with proper symbolic constants, so no need to do it again here.)

I could just remove the two mentions of estatus in init_thread and copy_regs, if that 
seems better.  Leave the target_pt_regs cleanup to a larger patch set touching them all.


r~
diff mbox series

Patch

diff --git a/linux-user/nios2/target_syscall.h b/linux-user/nios2/target_syscall.h
index 561b28d281..0999ce25fd 100644
--- a/linux-user/nios2/target_syscall.h
+++ b/linux-user/nios2/target_syscall.h
@@ -5,29 +5,8 @@ 
 #define UNAME_MINIMUM_RELEASE "3.19.0"
 
 struct target_pt_regs {
-    unsigned long  r8;    /* r8-r15 Caller-saved GP registers */
-    unsigned long  r9;
-    unsigned long  r10;
-    unsigned long  r11;
-    unsigned long  r12;
-    unsigned long  r13;
-    unsigned long  r14;
-    unsigned long  r15;
-    unsigned long  r1;    /* Assembler temporary */
-    unsigned long  r2;    /* Retval LS 32bits */
-    unsigned long  r3;    /* Retval MS 32bits */
-    unsigned long  r4;    /* r4-r7 Register arguments */
-    unsigned long  r5;
-    unsigned long  r6;
-    unsigned long  r7;
-    unsigned long  orig_r2;    /* Copy of r2 ?? */
-    unsigned long  ra;    /* Return address */
-    unsigned long  fp;    /* Frame pointer */
-    unsigned long  sp;    /* Stack pointer */
-    unsigned long  gp;    /* Global pointer */
-    unsigned long  estatus;
-    unsigned long  ea;    /* Exception return address (pc) */
-    unsigned long  orig_r7;
+    target_ulong sp;
+    target_ulong pc;
 };
 
 #define TARGET_MCL_CURRENT 1
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 23ff9659a5..cb14c5f786 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1094,9 +1094,8 @@  static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUMBState *env
 
 static void init_thread(struct target_pt_regs *regs, struct image_info *infop)
 {
-    regs->ea = infop->entry;
+    regs->pc = infop->entry;
     regs->sp = infop->start_stack;
-    regs->estatus = 0x3;
 }
 
 #define LO_COMMPAGE  TARGET_PAGE_SIZE
diff --git a/linux-user/nios2/cpu_loop.c b/linux-user/nios2/cpu_loop.c
index 7b20c024db..37e1dfecfd 100644
--- a/linux-user/nios2/cpu_loop.c
+++ b/linux-user/nios2/cpu_loop.c
@@ -132,28 +132,6 @@  void cpu_loop(CPUNios2State *env)
 
 void target_cpu_copy_regs(CPUArchState *env, struct target_pt_regs *regs)
 {
-    env->regs[0] = 0;
-    env->regs[1] = regs->r1;
-    env->regs[2] = regs->r2;
-    env->regs[3] = regs->r3;
-    env->regs[4] = regs->r4;
-    env->regs[5] = regs->r5;
-    env->regs[6] = regs->r6;
-    env->regs[7] = regs->r7;
-    env->regs[8] = regs->r8;
-    env->regs[9] = regs->r9;
-    env->regs[10] = regs->r10;
-    env->regs[11] = regs->r11;
-    env->regs[12] = regs->r12;
-    env->regs[13] = regs->r13;
-    env->regs[14] = regs->r14;
-    env->regs[15] = regs->r15;
-    /* TODO: unsigned long  orig_r2; */
-    env->regs[R_RA] = regs->ra;
-    env->regs[R_FP] = regs->fp;
     env->regs[R_SP] = regs->sp;
-    env->regs[R_GP] = regs->gp;
-    env->regs[CR_ESTATUS] = regs->estatus;
-    env->pc = regs->ea;
-    /* TODO: unsigned long  orig_r7; */
+    env->pc = regs->pc;
 }