search

chipsalliance / rocket-chip

Rocket Chip Generator
Other
3.18k stars 1.12k forks source link

[Bug Report] Incorrect behavior of TileLinktoAHB module #2909

Open johnny-wang16 opened 2 years ago

johnny-wang16 commented 2 years ago

Type of issue: bug report

Impact: API modification

Development Phase: request

Other information

If the current behavior is a bug, please provide the steps to reproduce the problem: I use the TileLinktoAHB bus module to convert all AXI bus to AHB and run Gemmini with this configuration. Gemmini outputs correct values but a bug is encountered in the TileLinktoAHB process and noticed this bug that might affect others who are also using this module.

What is the current behavior? The TileLinktoAHB module ignores the bit mask information and directly outputs whatever is on the a_bits_address. What is the expected behavior? The hsize and haddr should be affected by a_bits_mask which contains the bit mask information.

TileLinktoAHB module is not taking into account of the bit mask information. From the below waveform graph (top module is from Gemmini, bottom is TileLinktoAHB), the correct behavior should be: hsize/haddrchanges according to a_bits_mask so that only the portion of 8000_c9a0 that's not masked by a_bits_mask is output to haddr. but you can see that TileLinktoAHB is not taking into account of the bit mask information. The scala code that needs to be modified is probably somewhere here : https://github.com/chipsalliance/rocket-chip/blob/400f99530790a0e149f4d9b52b7ddbcafd41c653/src/main/scala/tilelink/ToAHB.scala#L153 . MicrosoftTeams-image (13)

michael-etzkorn commented 2 years ago

To more accurately describe the expected behavior: because there's no mask signal on the AHB end, we're expecting in translating to AHB from TL to only transfer the data on the byte lanes with a corresponding mask enable bit and hsize to match the number of byte lanes we've enabled. We should also ensure the address respects the new hsize alignment (a_addr << (addr_width - a_size))[addr_width:0]. Just confirming @johnny-wang16 that this is your expected behavior?

Looking at your waveform, it's not clear your example presents a problem given the address is still aligned (the width is cut off but I'm assuming at least 32 bits) and the data lane is still 64 bits with the top 32 bits cleared to zero. But perhaps AHB uses an implicit mask based on size and so a_size of 0x3 with a_mask of 0xf needs to result in an hsize of 0x2. A test transaction item with data 0xffff_ffff_f7e5_f101 certainly fails.