VexRiscv icon indicating copy to clipboard operation
VexRiscv copied to clipboard

{I,D}BusSimplePlugin: don't rely on `memoryTranslatorPortConfig != null`

Open lschuermann opened this issue 1 year ago • 1 comments

null can be a valid MemoryTranslator configuration parameter, for example in the case of the PmpPlugin. Furthermore, the IBusCachedPlugin and DBusCachedPlugin do not generally rely on its memoryTranslatorPortConfig to be non-null to instantiate its translator port, but instead checks whether a MemoryTranslator service exists in the pipeline. Thus it seems best to change IBusSimplePlugin and DBusSimplePlugin to behave identically to the their respective cached versions. This also prevents accidental CPU misconfigurations where an MMU or PMP plugin is instantiated and generally usable, but does not actually take effect in the memory buses.

As an unintended side-effect, when pairing this change with use of the PmpPlugin or PmpPluginOld, this will now result in a combinational loop over the skipCmd, arbitration.isValid, and MMU_FAULT signals. For now, I have simply avoided this by not setting skipCmd when there is an MMU_FAULT, given that this fault should still be caught despite a load being performed. While this seems to work for my initial tests,

  • it needs more extensive testing, and
  • I'm not sure whether there might be a better solution here.

Hence this is a draft PR and I am looking for feedback on how to resolve the combinational loop issues.

lschuermann avatar Feb 26 '23 22:02 lschuermann