On Sat, May 27, 2017 at 2:42 AM 王翔 <merle@tya.email> wrote:
# Some qustion about riscv implement
## 1. SMP
Coreboot for riscv does not support SMP, now.
why does the secondary hart not halt in **src/arch/riscv/bootblock.S**?
Now, secondary hart halts in **src/arch/riscv/trap_util.S**.
This may affect playload implementation.
To be honest, I don't know, Jonathan, do you? I have not looked at that code in some time.
## 2. Privilege level for hypervisor
Privilege level2 is Reserved In newest **rescv-privileged-v1.10.pdf**.
According to the latest **rescv-privileged-v1.10.pdf**, the privilege level 2 is
reserved. However, it appears in the source code.
yes, this is a recent change with the removal of hypervisor mode.
## 3. Exception
Both **supervisor_trap_entry** and **trap_entry** in
**src/arch/riscv/trap_util.S** have similar functionality. The only difference
is the latter halts the secondary hart. **supervisor_trap_entry** is unused
currently, what's the purpose of that?
The two restore context handlers **supervisor_call_return**
**machine_call_return** in **src/arch/riscv/trap_util.S** are the same and
they're in another section. Why not put these codes right after
**supervisor_trap_entry**/**trap_entry** so there's no need to insert assembly
codes in **src/arch/riscv/trap_handler.c**.
We would welcome a patch so we could see what you'd like to change.
## 4. CSR(mtime mtimecmp)
Hart-local storage records the address of mtime/mtimecmp. However, I could not
find information regarding CSR mapped to memory space in the latest document
**rescv-privileged-v1.10.pdf**.
That's weird, but this may be because mtime/mtimecmp are not really CSRs any more, and hence are optional?
You used to use the config string to find these resources, but that is possibly deprecated too. I don't know how to locate these resources at present. There seems to be a move to the FDT, which is regrettable, but at least it's not ACPI like ARM V8 :-)
## 5. S/M Privilege level use same stack
Function **riscvpayload** in **src/arch/riscv/payload.S** does not create a new
stack for S privilege level. This may destroy the stack of M privilege level.
The M privilege level is not really a level in any normal sense, or at least it was not. Supervisor level can examine and rewrite all of M memory. Also, we assume the payload will create its own stack if needed, as it should. Finally, it makes no sense for M level to create a stack for S level; this would imply the M level has knowledge of the S level activities and that's just plain wrong. S level should always set up its own stack.
Overall the M level / S level interactions had some real problems in the earlier spec, and I'll have to look at the new spec to see what's changed.
Thank you for looking at this code and if you have changes to make we'd be interested in seeing them.
ron