riscvarchive / riscv-qemu

QEMU with RISC-V (RV64G, RV32G) Emulation Support
387 stars 154 forks source link

(WIP) Another attempt at setting tb_flags #93

Closed palmer-dabbelt closed 6 years ago

palmer-dabbelt commented 6 years ago

I've blindly applied the patch, but I think we need to refactor things a bit so we no longer look at env->misa and instead look at env->tb_flags. I'm not sure if removing env->misa is the correct thing to do, or if we should be making sure it stays in sync with tb_flags.

I doubt this compiles.

This originally came from sorear https://github.com/riscv/riscv-qemu/commit/a038a2874a3eba27650c164f4622e47a3fe95199.patch and may have missed some diff.

sorear commented 6 years ago

I'd rather keep misa around as the point of truth, because the available space in tb_flags is a limited resource and we need to use it as economically as possible (for instance, combining MXL/SXL/UXL into a single "active XLEN", and for low-use extensions like H we will want to check misa in the helper and not use a tb_flags bit at all). We could also remove F/D from the tb_flags and check those at runtime as well (OR, we should incorporate FS into the tb_flags; do one).

I see there's also a tiny bit of switchable XLEN prep work in this patch. Unclear if we want to upstream that without a complete testable implementation.

There's also a tradeoff regarding the number of in-use mmu_idx values. Data and instruction address translation is controlled by PRV, MPRV, MPP, MXR, and SUM; at one extreme we could always use a mmu_idx of 0 and tlb_flush on any change to the above; at the other we can encode those state values into tb_flags, use different mmu_idx values for each meaningful combination thereof. (I have a patch which effects this, as well: https://github.com/sorear/riscv-qemu-system/commit/30c9f4fc) Supporting the H extension will require a delegate compromise to keep the number of mmu_idx values reasonable.

sorear commented 6 years ago

This patch is no longer necessary as tb_flags is being correctly handled for the supported features. We'll need to revisit this when misa is writable but it's not needed now. @michaeljclark this PR can be closed