search

intel / device-modeling-language

Mozilla Public License 2.0
92 stars 47 forks source link

Limited scope for function_io_memory #284

Closed e-malitsky closed 5 months ago

e-malitsky commented 5 months ago

Legacy pci bus maps pci_config banks as Device/Subdevice + function number instead of bank objects. When we have 2+ PCI devices in 1 DML device (e.g. by using subdevice) such pcI_config bank can be unreachable because pci bank function number is hardcoded to 255. This PR introduces a limited scope for function_io_memory - closest port object parent.

syssimics commented 5 months ago

Verification #13410854: pass

e-malitsky commented 5 months ago

Approved for my part. I'm curious what kind of model you're working with that combines such a new feature as subdevices together with such a legacy feature as function-mapped banks. How did that happen?

Hopefully we will migrate to new pci lib sometime

mandolaerik commented 5 months ago

Legacy pci bus maps pci_config banks as Device/Subdevice + function number instead of bank objects.

pci-bus maps banks on function number only if some device requests to be mapped on a function number through the method pci_bus.add_map. This used to be the default behaviour in Simics 5, but since Simics 6 the default is to map the bank object (method add_map in src/devices/dml-lib/1.4/pci/common.dml). We have seen use cases where dml-lib's add_map was overridden with a modified copy within the 5 timeframe.

If this is the case for you, then I would suggest that you also update your copy of add_map to resemble what the lib does in 6, so that your banks get mapped on object. Function mapped banks make it harder to inspect the system.

The PR itself is still valid, it does fix a bug

syssimics commented 5 months ago

Verification #13411170: pass

e-malitsky commented 5 months ago

Legacy pci bus maps pci_config banks as Device/Subdevice + function number instead of bank objects.

pci-bus maps banks on function number only if some device requests to be mapped on a function number through the method pci_bus.add_map. This used to be the default behaviour in Simics 5, but since Simics 6 the default is to map the bank object (method add_map in src/devices/dml-lib/1.4/pci/common.dml). We have seen use cases where dml-lib's add_map was overridden with a modified copy within the 5 timeframe.

If this is the case for you, then I would suggest that you also update your copy of add_map to resemble what the lib does in 6, so that your banks get mapped on object. Function mapped banks make it harder to inspect the system.

The PR itself is still valid, it does fix a bug

I was under the impression that we can somewhat customize io/mem memory spaces of pci_bus but conf is hardcoded to 255. Thanks, will take a proper look

mandolaerik commented 5 months ago

Indeed, that's a problem in pci-bus. Wrote a PR: https://github.com/intel-innersource/applications.simulators.simics.simics-base/pull/6767