BusSlaveFactory driveFlow does not honor byteEnable about spinalhdl HOT 5 CLOSED

Ok for me ^^

Else, a general fix would be to implement an AXI bridge which filter this kind of accesses, so you would just have to put the bridge in between the width adapter and peripherals.

from spinalhdl.

Flow them self aren't made for byteenable, could add additional versions which would work on specific payload type.

A real-scenario where this would matter: I am using a Axi4SlaveFactory after an AXI width converter. For a wide write request with some bytes disabled with strb, the AXI width converter issues requests that contains beats with zero strb. This then falsely triggers the downstream Flow.

Yes you are right, all bytemask being low could / should be used to not have valid = '1'

which transactions make the axi width converter make all strb going low in some transactions (example) ?

from spinalhdl.

I'm not actually using the built-in width converter / unburstifier / ..., but an external IP instead. An example of the all low strb transaction is as follows in a downsize from 512 to 64:

You can see that s_axi_wstrb is 0xff000000, which gets translated to the 4th beat having m_axi_wstrb being 0xff. The other beats have wready && wvalid but wstrb == 0.

from spinalhdl.

I would say, you may get peripherals which trigger actions when a register is written and on which byte mask make no sense, for instance the PLIC interrupt reservation.
So there is a few way to fix the situation i would say :

• patching every peripherals to not trigger side effect when byte mask is 0, but that seems the worst option to me
• but more fondamentaly, i think the main issue is the s_axi transaction we can see on the wave, with its awsize of 64 bytes.

What kind of master generates it ?

from spinalhdl.

The offending request comes from the Xilinx XDMA bridge; it issues narrow writes with 64B AWSIZE but only partial WSTRB (in this case 8 bytes). Per my understanding this is allowed by AXI (see this thread: WSTRB is allowed to go low during a transaction to signal only some bytes are valid).

Regarding a possible solution, I agree that it's bad to change behaviour of existing peripherals that expect the current behaviour. I'm working on a pull request to add an optional parameter to the method (that defaults to the old behaviour); what do you think?

from spinalhdl.