lowRISC / opentitan

OpenTitan: Open source silicon root of trust
https://www.opentitan.org
Apache License 2.0
2.53k stars 754 forks source link

DV/Ibex: Keep vendoring in Ibex DV into OT? #2513

Open imphil opened 4 years ago

imphil commented 4 years ago

Currently, we perform DV on Ibex in the Ibex repository. At the same time, we also vendor in the DV code from Ibex into OT. This is sometimes tricky, as we have duplicated some of our common DV infrastructure into Ibex, which give us, if we're not careful, slightly diverged versions of the same file. #2511 is one example.

The easiest solution to all of that would be the removal of all DV code when we vendor Ibex into OT, and do Ibex DV solely in the ibex repository.

However, this makes it harder for us to run Ibex DV in OT, if we wish to do so. That's the question: do we want to do that? For full chip regressions? Or for post-[synthesis/layout/...] regressions?

@sriyerg @sjgitty @rswarbrick

rswarbrick commented 4 years ago

Personally, I think that any "block-level" Ibex testing should be done in the Ibex repo, so removing the infrastructure for that when you vendor seems like a reasonable thing to do. Maybe others will feel differently.

sjgitty commented 4 years ago

I think that any "block-level" Ibex testing should be done in the Ibex repo

That feels like the right answer, but there might be issues of combined code coverage or other subtleties that @udinator or @sriyerg might know of

On Tue, Jun 16, 2020 at 6:50 AM Rupert Swarbrick notifications@github.com wrote:

Personally, I think that any "block-level" Ibex testing should be done in the Ibex repo, so removing the infrastructure for that when you vendor seems like a reasonable thing to do. Maybe others will feel differently.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/lowRISC/opentitan/issues/2513#issuecomment-644778740, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJZQKVNG2W6XUU56N6C6WVDRW52B3ANCNFSM4N7SY55A .

sriyerg commented 4 years ago

TL; DR; very nice to have, but not absolutely necessary.

The original motivation was to be able to run it just like other IPs in OpenTitan, so that it also shows up in our summary page here: https://reports.opentitan.org/hw/top_earlgrey/dv/summary.html

This will then become the complete dashboard for the regression results of everything the SoC is composed of, including the full chip level.

How about we vendor in Ibex-specific DV code only (and leave out the common stuff, since those can be picked up from the OT repo natively)?

sriyerg commented 4 years ago

The single-dashboard-for-all argument applies for lint / fpv / synthesis as well.

udinator commented 4 years ago

Agree with @sriyerg, maybe just vendoring in the Ibex-specific DV code could be a possible solution.

imphil commented 4 years ago

Thanks for your input!

How about we vendor in Ibex-specific DV code only (and leave out the common stuff, since those can be picked up from the OT repo natively)?

We already (try to) do that. However, we sometimes miss duplicated files, like top_pkg.sv recently, which causes problems.

Since we do not have a pressing need that would require removing the Ibex DV code currently, we can leave it in and revisit if we stumble over it again.

moidx commented 2 years ago

We will need to evaluate this again once we have a more up to date repository strategy.

GregAC commented 1 year ago

We don't want to disturb things for M2.5 (indeed we may want to use dvsim to run Ibex regressions so will want things vendored in). Worth revisting the strategy here for integrated.

johngt commented 3 months ago

I believe that we are currently vendoring in at appropriate windows. This is more of a long term fix - so deprioritising for now and should consider moving to backlog / future releases. @moidx / @GregAC