Skip to content

Commit

Permalink
bug fix: canonical check on virtual address for data accesses (#2667)
Browse files Browse the repository at this point in the history
As mentioned in the spec, we need to perform a canonical check on the virtual address for instruction fetch, load, and store. If the check fails, it will cause the page-fault exception.

This PR fixes the above two:
- Changes INSTR_ACCESS_FAULT to INSTR_PAGE_FAULT
- Adding virtual address check on data accesses as well
  • Loading branch information
fatimasaleem authored Dec 16, 2024
1 parent fd213fc commit f4355fa
Showing 1 changed file with 8 additions and 3 deletions.
11 changes: 8 additions & 3 deletions core/cva6_mmu/cva6_mmu.sv
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ module cva6_mmu
logic iaccess_err; // insufficient privilege to access this instruction page
logic i_g_st_access_err; // insufficient privilege at g stage to access this instruction page
logic daccess_err; // insufficient privilege to access this data page
logic canonical_addr_check; // canonical check on the virtual address for SV39
logic d_g_st_access_err; // insufficient privilege to access this data page
logic ptw_active; // PTW is currently walking a page table
logic walking_instr; // PTW is walking because of an ITLB miss
Expand Down Expand Up @@ -380,7 +381,7 @@ module cva6_mmu
// we work with SV39 or SV32, so if VM is enabled, check that all bits [CVA6Cfg.VLEN-1:CVA6Cfg.SV-1] are equal
if (icache_areq_i.fetch_req && !((&icache_areq_i.fetch_vaddr[CVA6Cfg.VLEN-1:CVA6Cfg.SV-1]) == 1'b1 || (|icache_areq_i.fetch_vaddr[CVA6Cfg.VLEN-1:CVA6Cfg.SV-1]) == 1'b0)) begin

icache_areq_o.fetch_exception.cause = riscv::INSTR_ACCESS_FAULT;
icache_areq_o.fetch_exception.cause = riscv::INSTR_PAGE_FAULT;
icache_areq_o.fetch_exception.valid = 1'b1;
if (CVA6Cfg.TvalEn)
icache_areq_o.fetch_exception.tval = CVA6Cfg.XLEN'(icache_areq_i.fetch_vaddr);
Expand Down Expand Up @@ -512,6 +513,10 @@ module cva6_mmu
lsu_valid_o = lsu_req_q;
lsu_exception_o = misaligned_ex_i;

// we work with SV39 or SV32, so if VM is enabled, check that all bits [CVA6Cfg.VLEN-1:CVA6Cfg.SV-1] are equal to bit [CVA6Cfg.SV]
canonical_addr_check = (lsu_req_i && en_ld_st_translation_i &&
!((&lsu_vaddr_i[CVA6Cfg.VLEN-1:CVA6Cfg.SV-1]) == 1'b1 || (|lsu_vaddr_i[CVA6Cfg.VLEN-1:CVA6Cfg.SV-1]) == 1'b0));

// Check if the User flag is set, then we may only access it in supervisor mode
// if SUM is enabled
daccess_err = en_ld_st_translation_i &&
Expand Down Expand Up @@ -578,7 +583,7 @@ module cva6_mmu
lsu_exception_o.tinst = '0;
lsu_exception_o.gva = ld_st_v_i;
end
end else if ((en_ld_st_translation_i || !CVA6Cfg.RVH) && (!dtlb_pte_q.w || daccess_err || !dtlb_pte_q.d)) begin
end else if ((en_ld_st_translation_i || !CVA6Cfg.RVH) && (!dtlb_pte_q.w || daccess_err || canonical_addr_check || !dtlb_pte_q.d)) begin
lsu_exception_o.cause = riscv::STORE_PAGE_FAULT;
lsu_exception_o.valid = 1'b1;
if (CVA6Cfg.TvalEn)
Expand Down Expand Up @@ -606,7 +611,7 @@ module cva6_mmu
lsu_exception_o.gva = ld_st_v_i;
end
// check for sufficient access privileges - throw a page fault if necessary
end else if (daccess_err) begin
end else if (daccess_err || canonical_addr_check) begin
lsu_exception_o.cause = riscv::LOAD_PAGE_FAULT;
lsu_exception_o.valid = 1'b1;
if (CVA6Cfg.TvalEn)
Expand Down

0 comments on commit f4355fa

Please sign in to comment.