Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lschuermann
Copy link
Contributor

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.

`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.
This is to avoid a combinational loop.
@Dolu1990
Copy link
Member

Hi, sorry i missed that PR.
I pushed it up at the front of my todo stack, i'm just currently in a big travel. Will come back to you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants