VUnit/vunit

Unaligned access to AXI read slave yields incorrect data

ru551n opened this issue · 4 comments

When doing multi-beat accesses to AXI read slave with an unaligned start address, every beat except the first one will yield wrong data due to the address offsetting

Line 116 in axi_read_slave.vhd:

for j in 0 to burst.size-1 loop
  idx := (address + j) mod self.data_size;
  rdata(8*idx+7 downto 8*idx) <= std_logic_vector(to_unsigned(read_byte(axi_slave.p_memory, address+j), 8));
end loop;

Example:
Bus width: 32 bits
Address: 1
Burst length: 2 beats, 7 bytes

For the first beat, due to the modulo operation, the byte with index 1, 2 and 3 will be correctly, set. However the byte with index with index 0 will be fetched from memory index 4. For the first beat, this does not matter, since that address is "invalid" anyway.

But for the second beat and onwards, the same will occur, byte with address 4 (and index 0) will be fetched from address 8.
In the case of unallocated data this will fail the allocation check, but otherwise this will return the wrong data.

This should be relatively easily fixed by looping over address the address to the next aligned address and also aligning all subsequent addresses. This fix will also yield undefined data (if this is configured via generic) on the bytes of the first beat which are "below" the starting address, which is probably a good thing since those bytes are not of interest, should the user decide to configure the drive_invalid generic.

I will post a patch later.

Is this the same issue as the one addressed in this PR? #936

Is this the same issue as the one addressed in this PR? #936

It is the same issue yes, but in that case reading from an unaligned address you don't really care if it is only one beat.
Reading multiple beats you will actually get the wrong data from the second beat onwards.

@ru551n Can you build on the solution provided in #936 to get it fully correct? And use the specified invalid value for the bytes that we don't care about?

@LarsAsplund, I can check it when I get the time. The solution is similiar to what I had in mind.