Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

REG_TLB code path untangling #116

Open
joelsmithTT opened this issue Oct 4, 2024 · 3 comments
Open

REG_TLB code path untangling #116

joelsmithTT opened this issue Oct 4, 2024 · 3 comments
Assignees
Labels
P1 Major issues with workarounds available, fix soon

Comments

@joelsmithTT
Copy link
Contributor

tt_SiliconDevice::write_mmio_device_register will pad the input buffer if size isn't a multiple of four. If the buffer is padded, the value of the padding is undefined but will be written to the chip. Callers attempting to use "REG_TLB" as a mechanism to perform e.g. 2 byte write of device memory via UC mapping may find this behavior surprising.

Suggestions:

  • Stronger programmatic distinction between register reads/writes and memory reads/writes. Do not select the behavior based on a magic string.
  • Stronger programmatic distinction between device access via UC vs WC mappings. Byte access through UC mapping is fine if it is accessing memory.
  • Allow only 32-bit reads/writes in the register code paths.
@joelsmithTT joelsmithTT self-assigned this Oct 4, 2024
@pjanevskiTT pjanevskiTT added P1 Major issues with workarounds available, fix soon bug and removed bug labels Nov 5, 2024
@broskoTT broskoTT changed the title REG_TLB code path has surprising behavior REG_TLB code path untangling Nov 6, 2024
@broskoTT
Copy link
Contributor

broskoTT commented Nov 6, 2024

We should untangle this REG_TLB path.
tt_debuda just hit an issue where write_to_device accepts 64bit address, which can then call write_mmio_device_register which also accepts 64bit address, but if write_device_memory is called it accepts 32bit address. Specifically on ARC the L1 address space sits high in 36bit address space (on Wormhole), so they're unable to hit those addresses without fallback_tlb "REG_TLB".

@joelsmithTT
Copy link
Contributor Author

Ouch. That's broken. We should use uint64_t for addresses everywhere, uint32_t is a bug. There are plenty of locations in Blackhole that require 64 bits to address.

@broskoTT
Copy link
Contributor

broskoTT commented Nov 7, 2024

I'll fix that issue as part of #281

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 Major issues with workarounds available, fix soon
Projects
None yet
Development

No branches or pull requests

3 participants