[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-riscv] [Qemu-devel] [PULL 10/34] RISC-V: Fix a PMP check with
|
From: |
Richard Henderson |
|
Subject: |
Re: [Qemu-riscv] [Qemu-devel] [PULL 10/34] RISC-V: Fix a PMP check with the correct access size |
|
Date: |
Thu, 27 Jun 2019 20:23:20 +0200 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0 |
On 6/27/19 7:44 PM, Jonathan Behrens wrote:
> I think this patch is slightly incorrect. If the PMP region is valid for
> the size of the access, but not the rest of the page then a few lines down
> in this function the entire page should not be placed into the TLB. Instead
> only the portion of the page that passed the access check should be
> included. To give an example of where this goes wrong, in the code below
> access to address 0x90000008 should always fail due to PMP rules, but if
> the TLB has already been primed by loading the (allowed) address 0x90000000
> then no fault will be triggered. Notably, this code also executes
> improperly without the patch because the first `ld` instruction traps when
> it shouldn't.
>
> li t0, 0x0000000024000000 // region[0]: 0x90000000..0x90000007
> csrw pmpaddr0, t0
>
> li t0, 0x00000000240001FF // region[1]: 0x90000000..0x90000fff
> csrw pmpaddr1, t0
>
> li t0, 0x1F0000000000989F // cfg[0] = LXRW, cfg[1] = L
> csrw pmpcfg0, t0
>
> sfence.vma
>
> li t0, 0x90000000
> ld s0, 0(t0)
> ld s1, 8(t0) // NO TRAP: address is incorrectly in TLB!
Nice test case.
> I think that the proper fix would be to first do a PMP check for the full
> PAGE_SIZE and execute normally if it passes. Then in the event the full
> page fails, there could be a more granular PMP check with only the accessed
> region inserted as an entry in the TLB.
This feature looks to be almost identical to the ARM m-profile MPU.
The fix is:
If the PMP check is valid for the entire page, then continue to call
tlb_set_page with size=TARGET_PAGE_SIZE.
If the PMP check is valid for the current access, but not for the entire page,
then call tlb_set_page with any size < TARGET_PAGE_SIZE. This change alone is
sufficient, even though the full argument tuple (paddr, vaddr, size) no longer
quite make perfect sense. (For the arm mpu, we compute some 1 << rsize, but
the actual value is never used; setting size=1 would be sufficient.)
Any size < TARGET_PAGE_SIZE will cause TLB_RECHECK to be set for the page,
which will force all accesses to the page to go back through riscv_cpu_tlb_fill
for re-validation.
> Unrelated question: should I be sending "Reviewed By" lines if I read
> through patches that seem reasonable? Or there some formal process I'd have
> to go through first to be approved?
No formal process. More eyes are always welcome.
r~
- [Qemu-riscv] [PULL] RISC-V Patches for the 4.1 Soft Freeze, Part 2, Palmer Dabbelt, 2019/06/27
- [Qemu-riscv] [PULL 01/34] target/riscv: Allow setting ISA extensions via CPU props, Palmer Dabbelt, 2019/06/27
- [Qemu-riscv] [PULL 02/34] sifive_prci: Read and write PRCI registers, Palmer Dabbelt, 2019/06/27
- [Qemu-riscv] [PULL 05/34] RISC-V: Only Check PMP if MMU translation succeeds, Palmer Dabbelt, 2019/06/27
- [Qemu-riscv] [PULL 03/34] target/riscv: Fix PMP range boundary address bug, Palmer Dabbelt, 2019/06/27
- [Qemu-riscv] [PULL 04/34] target/riscv: Implement riscv_cpu_unassigned_access, Palmer Dabbelt, 2019/06/27
- [Qemu-riscv] [PULL 19/34] target/riscv: Remove user version information, Palmer Dabbelt, 2019/06/27
- [Qemu-riscv] [PULL 10/34] RISC-V: Fix a PMP check with the correct access size, Palmer Dabbelt, 2019/06/27
- [Qemu-riscv] [PULL 27/34] disas/riscv: Fix `rdinstreth` constraint, Palmer Dabbelt, 2019/06/27
- [Qemu-riscv] [PULL 08/34] RISC-V: Check PMP during Page Table Walks, Palmer Dabbelt, 2019/06/27
- [Qemu-riscv] [PULL 21/34] RISC-V: Add support for the Zifencei extension, Palmer Dabbelt, 2019/06/27
- [Qemu-riscv] [PULL 31/34] hw/riscv: Add support for loading a firmware, Palmer Dabbelt, 2019/06/27
- [Qemu-riscv] [PULL 13/34] target/riscv: Restructure deprecatd CPUs, Palmer Dabbelt, 2019/06/27
- [Qemu-riscv] [PULL 07/34] RISC-V: Check for the effective memory privilege mode during PMP checks, Palmer Dabbelt, 2019/06/27
- [Qemu-riscv] [PULL 20/34] target/riscv: Add support for disabling/enabling Counters, Palmer Dabbelt, 2019/06/27
- [Qemu-riscv] [PULL 22/34] RISC-V: Add support for the Zicsr extension, Palmer Dabbelt, 2019/06/27
- [Qemu-riscv] [PULL 15/34] target/riscv: Add the mcountinhibit CSR, Palmer Dabbelt, 2019/06/27
- [Qemu-riscv] [PULL 12/34] RISC-V: Fix a memory leak when realizing a sifive_e, Palmer Dabbelt, 2019/06/27