Add `vCLIC` interface and logic #44

alex96295 · 2024-03-04T14:34:17Z

This PR adds support for virtualized interrupts handled by the RISC-V CLIC interface.

@ezelioli this is the reference PR for this feature, to be updated in Cheshire and other systems using CVA6 + CLIC with virtual extensions enabled for the latter

It would be good if you could use this branch to run your tests, so we see if I missed something during cherry-picking (there were a lot of conflicts because the current codebase has different indentation, causing false negatives)

@ezelioli @niwis we should try to converge on this ASAP

core/branch_unit.sv

core/commit_stage.sv

core/csr_regfile.sv

core/cva6_clic_controller.sv

core/load_store_unit.sv

core/mmu_sv39x4/cva6_mmu_sv39x4.sv

core/include/ariane_pkg.sv

core/commit_stage.sv

core/csr_regfile.sv

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

niwis · 2024-03-05T13:28:08Z

FPGA implementation fails because priv_lvl and trap_to_v are missing in this assignment

cva6/core/acc_dispatcher.sv

Lines 268 to 275 in eff9be5

    
           assign acc_exception_o = '{ 
        
                   cause: riscv::ILLEGAL_INSTR, 
        
                   tval : '0, 
        
                   tval2 : '0, 
        
                   tinst : '0, 
        
                   gva : '0, 
        
                   valid: acc_resp_i.error 
        
               };

alex96295 · 2024-03-05T14:54:37Z

FPGA implementation fails because priv_lvl and trap_to_v are missing in this assignment

cva6/core/acc_dispatcher.sv

Lines 268 to 275 in eff9be5

assign acc_exception_o = '{

cause: riscv::ILLEGAL_INSTR,

tval : '0,

tval2 : '0,

tinst : '0,

gva : '0,

valid: acc_resp_i.error

};

Thanks, will fix :)

alex96295 · 2024-03-07T15:47:08Z

LGTM for me @niwis, maybe we can do some squashing or squash-and-merge, but functionally it's ready for merge

alex96295 · 2024-03-11T07:58:49Z

@niwis rfc when you have time, so that we can proceed also in cheshire

niwis

Thanks a lot @ezelioli and @alex96295 for the extensions and rebase! I have a few general comments and suggestions, the rest in-line.

It seems like of the changes would generally be useful for H extension (also without CLIC). In particular, I'm thinking of a) tracing virtualisation mode, and b) Fixing H interrupt delegation. Do you think you could open dedicated PRs for these changes to this repo and Bruno's "upstream" H-ext branch?
We're still experimenting with the new "integrate" action, so this is not very obvious. But the current integration causes a port mismatch. Can you point the base-ref in integration.yml to a cheshire ref which integrates vCLIC? See CONTRIBUTING-PULP.md
A more open question: Should we have an explicit VCLIC parameter or similar to enable/disable non-specified features? My concern is that H and CLIC have official RISC-V specs whereas vCLIC does not, so implicitly activating vCLIC when H and CLIC are enabled might lead to unexpected behavior.

niwis · 2024-03-16T14:05:45Z

common/local/util/ex_trace_item.svh

@@ -35,6 +35,7 @@ class ex_trace_item;
            riscv::ST_ACCESS_FAULT:       this.cause_s = "Store Access Fault";
            riscv::ENV_CALL_UMODE:        this.cause_s = "Environment Call User Mode";
            riscv::ENV_CALL_SMODE:        this.cause_s = "Environment Call Supervisor Mode";
+            riscv::ENV_CALL_VSMODE:       this.cause_s = "Environment Call Virtual Supervisor Mode";


I believe this is generally useful for H ext... can we commit this separately so that we can also merge it into upstream H ext?

niwis · 2024-03-16T14:06:28Z

common/local/util/instr_trace_item.svh

Same here, seems more related to H ext than to vCLIC

niwis · 2024-03-16T14:07:18Z

common/local/util/instr_tracer.sv

Same for the changes in this file (vCLIC->H)

niwis · 2024-03-16T14:07:57Z

common/local/util/instr_tracer_if.sv

Also here (vCLIC->H)

niwis · 2024-03-16T14:53:47Z

core/include/ariane_pkg.sv

+    riscv::priv_lvl_t priv_lvl;  // In CLIC mode, keeps information about
+                                 // incoming interrupt target privilege level
+    logic trap_to_v;


These fields might be a bit misleading since they are only used in CLIC mode. Would it be possible to pack them together with the other CLIC-specific data into the cause field? There should be enough space. This would also reduce the overall diff. We can also define a new struct type for better readability if you prefer.

cva6/core/cva6_clic_controller.sv

Lines 69 to 79 in 876c325

// ------------------

// Interrupt Packager

// ------------------

// Pack interrupt cause to be inserted into the pipeline

assign clic_irq_cause_o = {

1'b1, // This is an irq

{riscv::XLEN - 25{1'b0}}, // XLEN-2...24

clic_irq_level_i, // to mintstatus.mil

{16 - $clog2(CVA6Cfg.CLICNumInterruptSrc) {1'b0}}, // 15...IDWidth

clic_irq_id_i

}; // to mcause

niwis · 2024-03-16T14:55:26Z

core/include/ariane_pkg.sv

+                                                    | riscv::HSTATUS_VGEIN
                                                    | riscv::HSTATUS_VTVM
                                                    | riscv::HSTATUS_VTW
                                                    | riscv::HSTATUS_VTSR;

  // hypervisor delegable interrupts
-  localparam logic [riscv::XLEN-1:0] HS_DELEG_INTERRUPTS = riscv::MIP_VSSIP
+  localparam logic [riscv::XLEN-1:0] HS_DELEG_INTERRUPTS = riscv::MIP_VSSIP;
+
+  localparam logic [63:0] HIE_MASK                = riscv::MIP_VSSIP
                                                    | riscv::MIP_VSTIP
-                                                    | riscv::MIP_VSEIP;
+                                                    | riscv::MIP_VSEIP
+                                                    | riscv::MIP_SGEIP;
+


Do these apply to H ext in general?

niwis · 2024-03-16T14:56:38Z

core/cva6_clic_controller.sv

@@ -55,12 +64,67 @@ module cva6_clic_controller #(
          clic_irq_req_o = clic_irq_valid_i;
          // Take S-mode interrupts with higher level
        end else if (clic_irq_priv_i == riscv::PRIV_LVL_S) begin
-          clic_irq_req_o = (clic_irq_level_i > max_sthresh) && (clic_irq_valid_i) && irq_ctrl_i.sie;
+          // clic_irq_req_o = (clic_irq_level_i > max_sthresh) && (clic_irq_valid_i) && irq_ctrl_i.sie;


I assume this line can be removed

niwis · 2024-03-16T14:58:34Z

core/cva6_clic_controller.sv

There are several ToDos in this file. Should we discuss them before merging?

niwis · 2024-03-16T15:02:08Z

core/csr_regfile.sv

@@ -531,7 +536,7 @@ module csr_regfile
        if (CVA6Cfg.RVH) csr_rdata = htinst_q;
        else read_access_exception = 1'b1;
        riscv::CSR_HGEIE:
-        if (CVA6Cfg.RVH) csr_rdata = '0;
+        if (CVA6Cfg.RVH) csr_rdata = {hgeie_q[riscv::XLEN-1:1], 1'b0};


These also look generally useful for H ext

niwis · 2024-03-16T15:05:40Z

core/cva6.sv

@@ -1478,6 +1494,7 @@ module cva6
  assign tracer_if.exception      = commit_stage_i.exception_o;
  // assign current privilege level
  assign tracer_if.priv_lvl       = priv_lvl;
+  assign tracer_if.v              = v;


Can be combined with tracer H-support changes above

niwis · 2024-10-31T03:49:19Z

Superseded by #55

Enrico Zelioli added 2 commits March 4, 2024 10:32

[ADD] Support for Virtual Supervisor mode tracing

0abae30

[ADD] S-mode trap delegation when in CLIC mode

44a75d6

alex96295 requested a review from niwis as a code owner March 4, 2024 14:34

github-actions bot reviewed Mar 4, 2024

View reviewed changes

alex96295 mentioned this pull request Mar 4, 2024

treewide: Add clic virtualization support pulp-platform/cheshire#102

Draft

github-actions bot reviewed Mar 4, 2024

View reviewed changes

core/include/ariane_pkg.sv Outdated Show resolved Hide resolved

core/include/ariane_pkg.sv Outdated Show resolved Hide resolved

github-actions bot reviewed Mar 4, 2024

View reviewed changes

core/commit_stage.sv Outdated Show resolved Hide resolved

core/csr_regfile.sv Outdated Show resolved Hide resolved

core/csr_regfile.sv Show resolved Hide resolved

Enrico Zelioli and others added 7 commits March 5, 2024 09:09

[ADD] Support for virtualised CLIC

67f4acc

[FIX] CLIC interrupts when in U-mode

a502ac9

[FIX] Remove unnecessary vsie signal from irq_ctrl_t

2c8dad0

[FIX] Uninitialized signals in irq_ctrl_t struct

a238d82

[FIX] CLIC interrupts in U-mode

959a034

Update core/branch_unit.sv

fd1ce4b

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Update core/commit_stage.sv

eff9be5

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

alex96295 force-pushed the aottaviano/vclic branch from e604564 to eff9be5 Compare March 5, 2024 08:10

ezelioli added 2 commits March 5, 2024 15:16

fix lint errors

40ef357

add missing assignments in core/acc_dispatcher.sv

e5139fb

RVH: fix interrupt delegation

9b56333

niwis requested changes Mar 16, 2024

View reviewed changes

niwis closed this Oct 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `vCLIC` interface and logic #44

Add `vCLIC` interface and logic #44

alex96295 commented Mar 4, 2024

niwis commented Mar 5, 2024

alex96295 commented Mar 5, 2024

alex96295 commented Mar 7, 2024

alex96295 commented Mar 11, 2024

niwis left a comment

niwis Mar 16, 2024

niwis Mar 16, 2024

niwis Mar 16, 2024

niwis Mar 16, 2024

niwis Mar 16, 2024

niwis Mar 16, 2024

niwis Mar 16, 2024

niwis Mar 16, 2024

niwis Mar 16, 2024

niwis Mar 16, 2024

niwis commented Oct 31, 2024

	// ------------------
	// Interrupt Packager
	// ------------------
	// Pack interrupt cause to be inserted into the pipeline
	assign clic_irq_cause_o = {
	1'b1, // This is an irq
	{riscv::XLEN - 25{1'b0}}, // XLEN-2...24
	clic_irq_level_i, // to mintstatus.mil
	{16 - $clog2(CVA6Cfg.CLICNumInterruptSrc) {1'b0}}, // 15...IDWidth
	clic_irq_id_i
	}; // to mcause

Add vCLIC interface and logic #44

Add vCLIC interface and logic #44

Conversation

alex96295 commented Mar 4, 2024

niwis commented Mar 5, 2024

alex96295 commented Mar 5, 2024

alex96295 commented Mar 7, 2024

alex96295 commented Mar 11, 2024

niwis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

niwis commented Oct 31, 2024

Add `vCLIC` interface and logic #44

Add `vCLIC` interface and logic #44