[dvsim] Simulation reports moved when specifying proj_root

[desorya]

Greetings @rswarbrick ,

By default, the simulation reports(html, json) are under the folder scratch/<branch>/<IP>_sim_<tool>/reports/.

However, if the switch --proj-root is set, the scratch folder will be under proj_root, but the reports folder disappears, and the reports will be directly under where the sim_cfg.hjson file locates.

Taking uart as an example:

// $(pwd) = opentitan/
python3 util/dvsim/dvsim.py hw/ip/uart/dv/uart_sim_cfg.hjson -i uart_smoke -pr $(pwd)

Now the simulation reports are no longer under scratch and is under opentitan/hw/ip/uart/dv/ and the reports folder will not be created.

[desorya]

Suddenly resolved with no reason after more than 30 tries.

[desorya]

Found how this happened. @rswarbrick

The results_dir generated by SimCfg(FlowCfg) uses os.path.abspath() to resolve the hjson sim cfg file. Where the proj_root and scratch_root could be a hyperlink or shortcut to another path.

For example, if there is a path /home/user/opentitan/ where /user/ is a shortcut to /home2/user/, then the $(pwd) can not be used here because it will cause the resolved proj_root be the /home/user/opentitan/ instead of /home2/user/opentitan/.

Hence, the results_dir in FlowCfg

 self.rel_path = os.path.dirname(self.flow_cfg_file).replace(self.proj_root + '/', '')

will not cut the expected proj_root and stay unchanged.

So I think maybe it is possible to add a process to resolved the abspath of input proj_root.

Thank you.

[rswarbrick]

Thanks for the debugging. The path hacking in the code here doesn't look great, and it dates back to 2020! Fortunately, I'm not sure an abspath is required. Will something like this work in FlowCfg.py?

        if not self.rel_path:
            flow_cfg_dir = os.path.dirname(self.flow_cfg_file)
            self.rel_path = os.path.relpath(flow_cfg_dir, self.proj_root)

[desorya]

Not work on my side.

The point is that the path of the hjson(flow_cfg_file) is resolved by abspath, but for the user-defined proj_root, it is just returned by resolve_proj_root(args) without treatments. Then if you haven't specify a scratch_root as well, the scratch_root that default_scratch_root = proj_root + "/scratch" will be returned. In this case, the scratch_root is also not the abspath but it will be if you specified -sr. Therefore, the treatments to specified/non-specified scratch_root are different, and so do which to proj_root and flow_cfg_file.

The difference between proj_root and self.flow_cfg_file may skip the creation of reports folder because the path results_dir may trace back by adding self.rel_path and will end up in latest. And if the input proj_root path contains any shortcut, it will not be resolved and will not trigger any warnings.

The difficulty here I think is that the code can not recognize whether the proj_root contains a shortcut, and for many users, they may also don't know if their user's path is a shortcut, especially when they are using NFS shares.

[rswarbrick]

Oh, I see! I'm convinced that we should stay consistent between the variables. I suspect that the abspath calls were really added so that the path (flow_cfg_file here) can be passed to processes that might be running with a different working directory.

That's a bit silly: we should probably have a "render this path for that subprocess" method (which could be abspath) and avoid messing around with paths "within" the dvsim process. But we haven't done that, and doing it consistently will be a bit fiddly: probably not a change we should be making right now, at any rate.

Do we just need to add a call to abspath to how we use self.proj_root, like this?

        if not self.rel_path:
            flow_cfg_dir = os.path.dirname(self.flow_cfg_file)
            abs_proj_root = os.path.abspath(self.proj_root)
            self.rel_path = os.path.relpath(flow_cfg_dir, abs_proj_root)

[desorya]

Not exactly. The modification here doesn't resolve the path correctly. The reports folder is created now but empty.

Taking /home/user/<mid_path>/opentitan as an example of proj_root, and /user/ is a shortcut to /home2/user/: The reports now are under /home/user/<abspath(self.flow_cfg_file)>/latest, which indicates that the self.rel_path has wrong backtrack level and a whole new path has been created.

I think that we should keep the scratch_root, proj_root and flow_cfg_file in the same way of treatment, no matter abspath or linked path(personal preferred) itself but should stay consistent because the scratch_root comes from proj_root(in dvsim.py), and flow_cfg_file will participate in the self.rel_path then the results_dir, and for results_dir, it also calls self.scratch_base_path so there is a coupling.

Once we used the abspath to any variable it will no longer go back to linked path in further usage. So if we use the abspath in FlowCfg to resolve self.flow_cfg_file, the results_dir will never under scratch_root if the proj_root contains any shortcut.