opentitan icon indicating copy to clipboard operation
opentitan copied to clipboard

[rv_core_ibex] Reads using address_translation modes do not work if read is in a different 64kb aligned address

Open jettr opened this issue 1 year ago • 2 comments

Description

When we enabled address_translation in our manifest, we started to see errors, and it took a little while to figure out what was going wrong. I traced it down to the loading of initial values of the static variables in early code. The problem seems to be in the lw read of addresses that are not within the same 64kb (0x1_0000) boundary as the program counter. I have demonstrated this with the following simplified _start function attached to a debugging on the FPGA

The code I am running is as follows:

a0010798 <.Ltmp3>: 
a0010798: 98 42        lw·····a4, 0(a3)
a001079a: 91 06        addi···a3, a3, 4
a001079c: f5 bf        j······0xa0010798 <.Ltmp3>

where I loaded up a3 with and address 0x8 before the 0xa0020000 boundary. Then I just ran the loop three times while dumping out flash and register contents below:

(gdb) p/x *($a3 + 0x80000000) == $a4
$1 = 0x1
(gdb) i r a3 a4
a3             0xa001fff8       -1610481672
a4             0x44924422       1150436386
(gdb) si 3
0xa001079a in ?? ()
(gdb) p/x *($a3 + 0x80000000) == $a4
$2 = 0x1
(gdb) i r a3 a4
a3             0xa001fffc       -1610481668
a4             0x80820141       -2138963647
(gdb) si 3
0xa001079a in ?? ()
(gdb) p/x *($a3 + 0x80000000) == $a4
$3 = 0x0
(gdb) i r a3 a4
a3             0xa0020000       -1610481664
a4             0x4238282        69436034
(gdb) p/x *($a3 + 0x80000000)
$4 = 0xce061101
(gdb) x/3x 0x2001fff8
0x2001fff8:     0x44924422      0x80820141      0xce061101

If p/x *($a3 + 0x80000000) == $a4 returns true (0x1), that means the lw read operation worked and the contents in register a4 matches the value read directly from the memory mapped flash location (e.g. 0x2001fff8) -- since gdb cannot read the address_translated address for some reason.

I then ran the entire loop si 3 (single step 3 times), a few times and performed the check again while dumping out the register contents.

Notice the last check p/x *($a3 + 0x80000000) == $a4 fails, and the contents in flash do not match the contents in a4 after the load. This also happens at the 0x10000 boundary, so it really appears that something is not mapped correctly for address_translation outside of the program counters (or maybe even start vector -- I didn't experiment) 64kb window.

jettr avatar Apr 29 '24 16:04 jettr

I believe this is an implementation error in the rv_core_ibex peripheral here: https://cs.opensource.google/opentitan/opentitan/+/master:hw/ip/rv_core_ibex/rtl/rv_core_addr_trans.sv;l=76

I think the expression should be:

  assign addr_o = sel_match ? addr_i & sel_region.output_mask | sel_region.remap_addr & ~sel_region.output_mask:
                              addr_i;

Fortunately, the desired effect can be achieved by masking off the lower bits of the remap address in software before programming the REMAP_ADDR register.

cfrantz avatar Apr 29 '24 20:04 cfrantz

@GregAC could you PTAL?

andreaskurth avatar May 03 '24 15:05 andreaskurth

@jettr could you confirm that the suggested workaround from @cfrantz (setting the lower bits of REMAP_ADDR to 0, such that the number of 0s corresponds to the size of the region, e.g. for a 64 kb region the bottom 16 bits must be 0) solves this issue?

Additionally could you tell me the settings of the DBUS_x registers that control the address mapping for the scenario where you've seen the issue?

In your GDB session you're adding 0x80000000 to a3 when accessing the address, are you expecting some region starting from 0x0 to be remapped to a region starting 0x80000000?

Note that GDB likely uses the debug module system bus access interface meaning any read/writes from there do no go through address translation.

It looks likely the issue @cfrantz has highlighted is the problem here but it's important I can confirm that and the above information will help me do that.

GregAC avatar May 16 '24 14:05 GregAC

@jettr could you confirm that the suggested workaround from @cfrantz (setting the lower bits of REMAP_ADDR to 0, such that the number of 0s corresponds to the size of the region, e.g. for a 64 kb region the bottom 16 bits must be 0) solves this issue?

Yes, Chris's changes works around/fixes the issues (I am using ROM_EXT version 0.1 with success).

In your GDB session you're adding 0x80000000 to a3 when accessing the address, are you expecting some region starting from 0x0 to be remapped to a region starting 0x80000000?

The addition of 0x80000000 turn the virtual address of 0xAxxxxxxx into the physical address of 0x2xxxxxxx so GDB can read the correct value from flash to compare the results of the lw instruction.

jettr avatar May 16 '24 15:05 jettr

I have an RTL fix for the issue here: https://github.com/lowRISC/opentitan/pull/23178

GregAC avatar May 17 '24 15:05 GregAC

Closing as completed in #23178.

andreaskurth avatar May 21 '24 16:05 andreaskurth