search

riscv-software-src / riscof

BSD 3-Clause "New" or "Revised" License
61 stars 39 forks source link

Misaligned tests failing #37

Closed jurevreca12 closed 2 years ago

jurevreca12 commented 2 years ago

Hello everyone, I am trying to build a simple risc-v processor that implements the RV32IZicsr instruction set (and only M mode). I integrated the riscof framework for testing purposes, and discovered that the unprivileged instructions are all passing the tests, but many misaligned tests are failing. Specifically: lb-align-01.S, lbu-align-01.S, lh-align-01.S, lhu-align-01.S, lw-align-01.S, sb-align-01.S, sh-align-01.S, misalign-beq-01.S, misalign-bge-01.S, misalign-bgeu-01.S, misalign-blt-01.S, misalign-bltu-01.S, misalign-bne-01.S, misalign-jal-01.S, misalign-lh-01.S, misalign-lhu-01.S, misalign-lw-01.S, misalign-sh-01.S, misalign-sw-01.S, misalign2-jalr-01.S.

I browsed the issues and discovered that this has been reported here https://github.com/riscv-non-isa/riscv-arch-test/issues/186 . But they claim that riscof will have solved these issues. So I have the following questions.

  1. On misaligned loads and stores the risc-v manual says (Sec:2.6) that: Loads and stores where the effective address is not naturally aligned to the referenced datatype have behavior dependent on the EEI. I intend to run the FreeRTOS operating system on the processor, which doesn't handle misaligned loads/stores in its trap handler. So my question is, do I have to handle misaligned loads/stores? And if I do have to handle them, does that mean that the FreeRTOS port is broken? Here is a link to the FreeRTOS port https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/portable/GCC/RISC-V/portASM.S

  2. If the answer to the previous question is no (load/stores don't need to be handled), then is perhaps my riscof configuration wrong, or incomplete?

  3. Since I don't support compressed instructions, I have to throw an address misaligned on jump and branch instructions that aren't 4 byte aligned. My question is, do the tests assume that this exception is implemented (i.e. not fatal)? and is this a requirement of the spec? or is it a drawback of the testing framework?

  4. If the answer to the previous question is that the spec does not require handling of misaligned jumps/branches then, is my riscof configuration wrong or incomplete?

Here is my riscof configuration for the core:

hart_ids: [0]
hart0:
  ISA: RV32IZicsr
  physical_addr_sz: 32
  User_Spec_Version: '2.3'
  supported_xlen: [32]
  misa:
   reset-val: 0x40000100
   rv32:
     accessible: true
     mxl:
       implemented: true
       type:
           warl:
              dependency_fields: []
              legal:
                - mxl[1:0] in [0x1]
              wr_illegal:
                - Unchanged
     extensions:
       implemented: true
       type:
           warl:
              dependency_fields: []
              legal:
                - extensions[25:0] bitmask [0x0001104, 0x0000000]
              wr_illegal:
                - Unchanged

and for the platform:

mtime:
  implemented: true
  address: 0xbff8
mtimecmp:
  implemented: true
  address: 0x4000
nmi:
  label: nmi_vector
reset:
  label: reset_vector
pawks commented 2 years ago

@jurevreca12

When you say fail , I assume that the signatures generated by your model and the reference models do not match.

jurevreca12 commented 2 years ago

Thank you very much for the quick answer @pawks . I guess I made an over generalized assumption about these tests.

Thanks again for the answers.

pawks commented 2 years ago
  • With regards to the tests misalign-beq-01.S, misalign-bge-01.S.... Are you claiming that if there is code that branches or jumps to an misaligned address, that the processor MUST support it? Or is it ok, that the exception is fatal? Do the riscof tests assume it is handled?

Yes, according to the spec, the processor must support it and the tests work on that assumption that an exception will be raised and execution is redirected to the trap handling routine.

  • For the tests misalign-lh-01.S, misalign-lhu-01.S... Are you saying that the tests shouldn't even run, because my hardware doesn't support it. The configuration field "hw_data_misaligned_support" is there only as a setting, that doesn't do anything yet, but later versions should support it. Am I correct?

The tests will be chosen and run, however the functionality in the reference model to support this behaviour is missing. Hence the signatures from the model and DUT will not match. It is a known scenario and is being actively worked on. The hw_misaligned_support does help in configuring the test correctly(i.e no trap handler is compiled into the test), but the configurability in the reference model is lacking.

jurevreca12 commented 2 years ago

I've managed to fix the problem my processor had with instructions: sb-align-01.S, lbu-align-01.S, lh-align-01.S, lhu-align-01.S, lw-align-01.S, sb-align-01.S, sh-align-01.S. Thank you again for the help @pawks .

But now I am trying to wrap my head around the misaligned instruction jumps/branches, and I just can't make sense of it. Lets say I encounter a jump to an instruction that is at address 6 (so the bytes are in addresses 6, 7, 8, 9). Then suppose I throw an exception because the address is not aligned. What could I then possibly do to load the correct instruction? One possible solution would be to use two half-word load instructions, to load bytes 6, 7 and 8,9 separately, then combine the half-words to form the instruction, save that instruction somewhere in memory (stack idk.) and then jump to that instruction, and immediately after this instruction there would have to be a jump back to instruction at address 10. This just seems ridiculous, and It would be much easier to solve this in hardware, but then I wouldn't be raising an exception as the spec demands.

Am I missing something? I've been searching the internet and I can't find any example of such an exception handler being implemented.

Any help is appreciated.

pawks commented 2 years ago

You need not have an exception handler implemented in hardware. All the core needs to do is raise an exception when the target address is 2 byte aligned and jump to the address pointed to by mtvec. It is the responsibility of the trap handler in software to handle it correctly and return to normal execution flow.

jurevreca12 commented 2 years ago

My concern is with the actual implementation of the trap handler. I've looked at the Zephyr and FreeRTOS trap handlers:

Neither of them seems to handle the instruction address misaligned exception. This is why I am having trouble understanding how exactly I would write such an exception handler.

neelgala commented 2 years ago

@jurevreca12 I believe it would be best to pose the questions regarding Zephyr/FreeRTOS to those respective repos/projects. A general purpose solution to handling instruction-misaligned can be very tricky. After loading bytes 6,7 (from your example) you will need to check if it points to a compressed instruction (probing the lower 2 bits) or not, only then decide to load the remaining bytes. Now even this fails, if the original jump instruction tried to jump in the middle of a 4-byte instruction - youmay get a false positive decision that its compressed. So as I said its not going to be easy to have a general purpose solution for this.

The arch-tests however maintain a strict control - so the test trap handler has a way of returning back to execution correctly. We don't execute the trapping instruction, but rather ignore it and resume to a next reasonable aligned-instruction.

If you haven't implemented "C" extension and there is a jump/branch to a non-4 byte boundary then your implementation needs to take an instruction-misaligned trap with mepc = PC of the jump/branch and mtval = target misaligned address.

Hope this helps.

jurevreca12 commented 2 years ago

Thanks for the answer @neelgala . I do however believe this could be a problem with the riscof testing framework. I have found some additional information regarding this question. In section 2.2 the Risc-v spec says:

In the base RV32I ISA, there are four core instruction formats (R/I/S/U), as shown in Figure 2.2. All are a fixed 32 bits in length and must be aligned on a four-byte boundary in memory.

If I am interpreting this correctly, this means that the test, such as the misalign-jal-01.S test should not run if the C extension is not enabled. In other words, if I am running software that is legal for RV32I, then I think I should be able to handle the instruction-misaligned-exception as a fatal exception, as by definition the unaligned address can not contain a legal instruction.

pawks commented 2 years ago

If you look at the way the effective addresses are calculated, there is a possibility that a 2-byte aligned target address can be generated. A correctly written and compiled software will never generate these scenarios, however the spec(Section 2.5) is clear on the behaviour if such a case occurs. The core needs to recognise this as a misaligned instruction exception and trap.

jurevreca12 commented 2 years ago

Yes, I agree. But as this would then be a incorrect instruction, there is no way of knowing how to handle this in the trap, right? hence it should be a fatal trap?

neelgala commented 2 years ago

@jurevreca12 all misaligned tests fall under the privilege test suite because they test the csrs and privilege spec not the unprivilege spec that you have quoted. The signatures generated will be different for cores with compressed support and those without. Anyhow, those tests will be run on both types of cores - since spec defines the behavior for each explicitly.

jurevreca12 commented 2 years ago

@neelgala I see. Yes that makes more sense. However, that would entail, that for my configuration (no C extension support), that the unaligned jump would cause an exception, from which it could not recover, since the code violates the 4-byte memory alignment constraints. Practically, speaking, the simulation should end in an infinite loop in the trap handler, meaning that the signatures would be meaningless.

neelgala commented 2 years ago

the trap handler in the test will not endlessly loop it will simply jump to the next reasonable aligned instructin - note these are not random tests they have been architected along with the trap-handler and thus the test will exit gracefully in either case. You can run these on spike/sail without enabling compressed and have a look at thie behavior.

jurevreca12 commented 2 years ago

Oh. I see. I understand now. Thank you for the answer.