[dv,mem_bkdr_util] Separate out flash_bkdir_util

[alees24]

Separate flash support into another package so that designs such as Darjeeling can employ mem_bkdr_util without depending upon the flash controller/package.

[Razer6]

Are these dependencies all needed or do the already get pulled in from the mem_bkdr_util? Or is it just a safeguard?

[alees24]

I've dropped a couple but I think the rest are all needed. I would observe that many of our core files appear not to be as minimal as they could be if they relied upon transitivity. I am a bit apprehensive about breaking an untested build, and I'm sure CI is far from complete w.r.t. that.

[Razer6]

Looks good with one question.

[matutem]

Did you consider the way this same issue was handled for otp_ctrl? It basically moved the complete specialization in otp_ctrl out to hw/ip_templates/otp_ctrl/dv/env: look at https://github.com/lowRISC/opentitan/blob/master/hw/ip_templates/otp_ctrl/dv/env/otp_ctrl_mem_bkdr_util_pkg.sv and https://github.com/lowRISC/opentitan/blob/master/hw/ip_templates/otp_ctrl/dv/env/otp_ctrl_mem_bkdr_util.core.tpl.

Perhaps for flash_ctrl the flash_ctrl_mem_bkdr_util_pkg.sv will need to be a template, but at least in the otp_ctrl all specializations are accessed via the top specific pkg files.

I think that approach is cleaner since it removes all traces of the flash_ctrl details from mem_bkdr_util, and mem_bkdr_utils simply takes care of reads and writes, which is what it does for a living.

[matutem]

I left a comment with my suggestion for how to do this. Also refer to #25891.

[alees24]

Did you consider the way this same issue was handled for otp_ctrl? It basically moved the complete specialization in otp_ctrl out to hw/ip_templates/otp_ctrl/dv/env: look at https://github.com/lowRISC/opentitan/blob/master/hw/ip_templates/otp_ctrl/dv/env/otp_ctrl_mem_bkdr_util_pkg.sv and https://github.com/lowRISC/opentitan/blob/master/hw/ip_templates/otp_ctrl/dv/env/otp_ctrl_mem_bkdr_util.core.tpl.

I am familiar with the way otp_ctrl was extracted, yes, and I think that makes sense for an ip_template that needs to be specialized for the different top levels. The single additional function required for flash support is a little different, in my opinion, since we're not specializing the flash controller for Darjeeling; it's just not present in the integrated design.

Perhaps for flash_ctrl the flash_ctrl_mem_bkdr_util_pkg.sv will need to be a template, but at least in the otp_ctrl all specializations are accessed via the top specific pkg files.

I think that approach is cleaner since it removes all traces of the flash_ctrl details from mem_bkdr_util, and mem_bkdr_utils simply takes care of reads and writes, which is what it does for a living.

The separation in this PR does achieve that too. mem_bkdr_util no longer contains any reference to the flash package or any flash-specific functionality, which allows the Darjeeling top-level to brought up alongside that of Earl Grey for the first time, and simulations of both are now possible on my local branch without crosstalk between the two environments.

If it's felt that the flash_bkdr_util package is too specific to belong in the dv/sv/ directory - and the same argument could apply to ROM- and SRAM-specific behavior - it could always be moved later since it's now self-contained.

[Razer6]

I am familiar with the way otp_ctrl was extracted, yes, and I think that makes sense for an ip_template that needs to be specialized for the different top levels. The single additional function required for flash support is a little different, in my opinion, since we're not specializing the flash controller for Darjeeling; it's just not present in the integrated design.

👍

[matutem]

I see this is a very simple change, but it adds clutter rather than addressing root causes. Basically this change will need to be undone and done the right way, so this kicks the can down the road hoping someone else will do the real fix, fully knowing what that fix should be.

[alees24]

Implementing rom_, sram_ and flash_bkdr_util as extended classes of mem_bkdr_util means that they may be accessed without requiring specialized code and without needing to know what type of memory is being accessed, as requested here some time ago:

opentitan/hw/top_earlgrey/dv/env/seq_lib/chip_sw_base_vseq.sv

Line 704 in 6efc747

virtual function void mem_bkdr_read8(input chip_mem_e mem,

Also at a few other sites in the top-level DV where you can see things become tidier.
flash_bkdr_util presently does not have specialized implementations of read/write access functions but perhaps it should, and similarly for the other memories, modeling what the hardware blocks actually do.

This PR only separates the files into different packages and core files and I should have realized that was necessary when the extended classes were introduced; mea culpa.