Top-level UART redux#47
Top-level UART redux#47elliotb-lowrisc merged 9 commits intolowRISC:mainfrom elliotb-lowrisc:uart_redux
Conversation
marnovandermaas
left a comment
marnovandermaas
left a comment
There was a problem hiding this comment.
Nice work Elliot! When I run this code, I get an error that openpty is an undefined symbol. Is this because I'm missing a package?
| // The first byte must be FF so we can differentiate this blob from ASCII sent | ||
| // by the ROM when it starts. FF is not UTF-8 / ASCII. | ||
| static const uint8_t kUartTxData[UART_DATASET_SIZE] = { | ||
| static volatile const uint8_t kUartTxData[UART_DATASET_SIZE] = { |
There was a problem hiding this comment.
Why does this need to be volatile? It's constant right?
There was a problem hiding this comment.
It is supposed to be overwritten by the DV, so is not so constant.
There was a problem hiding this comment.
Interesting, so is this something we should change upstream as well? https://github.com/lowRISC/opentitan/blob/3470eb09c57bc2b08994e50bc6a10979dc034f21/sw/device/tests/uart_tx_rx_test.c#L93
There was a problem hiding this comment.
Possibly, though in the case of the OT UART there are separate test programs for use in SiVal. I wonder if they could be merged with the DV ones, but have not yet looked into it. This may be one such thing that would need to change.
There was a problem hiding this comment.
Ok, the change looks reasonable to me. This comment shouldn't block the merging of this PR.
This is a workaround for CHERIoT using interrupt-disabling backward sentries to return from `irq_global_ctrl()`, thus removing the ability to enable interrupts for more than a few cycles at a time. Inlining the function removes the jump-and-link and associated interrupt-disabling backward sentry, allowing the enabling of interrupts to persist in the context of the caller and any inheriting functions. See CHERIoT-Platform/llvm-project#120
Correct the base address of the UART1 block and comment-out some unused and clashing address `#define`s.
Mapping of interrupt ID to peripheral. Hardcoded for now as we have don't have auto generation set-up.
A few changes were required to get `uart_tx_rx_test` to work: - Gate clkmgr code with `#if` to avoid errors due to lacking clkmgr DIF. - Rework interrupt handler to check the UART block for interrupt specifics due to PLIC having less detailed interrupts. - Tweak console configuration due to lack of fast/backdoor logging.
No point in having these top-level monitors enabled by default. They cannot do any meaningful checking without being provided with configuration information telling what clock period, parity, etc. to expect from the DUT. It would be nice if one day the agents could figure out most of this for themselves from the raw output.
No point in wasting runtime clocking the UART DPI model connected to UART0 when it has been disconnected in favour of a UART UVM agent.
- Fix `kUartIdxDv` capitalisation error. - Allow `uart_idx` to randomise. - Prepare for future baud rate randomisation.
Switch from the initial hacky top-level UART test programs to the ones ported from OpenTitan for increased randomisation. Also introduce a new test that randomises baud rate and a new `uart` regression test group.
|
Removed |
Not sure what the problem is for you there. The only mention of |
|
Hmmm, interesting. You're right. I saw this issue on Ubuntu 20.04. I tried fixing it by running https://opentitan.org/book/doc/getting_started/index.html#step-2-install-dependencies-using-the-package-manager but that didn't help. |
marnovandermaas
left a comment
marnovandermaas
left a comment
There was a problem hiding this comment.
Ok, this runs successfully for me with the following command:
hw/vendor/lowrisc_ip/util/dvsim/dvsim.py hw/top_chip/dv/top_chip_sim_cfg.hjson -i uart --max-parallel 32
I've changed the default reseed to 10 in an additional commit, so I'll leave it up to you to press the merge button if you're happy with that.
|
I think 10 is a bit high for the global default for me. I'll remove your commit and merge my changes. |
2 participants
Rework top-level UART tests to use ported OpenTitan test programs and introduce more randomisation.
See individual commits for details.