From 5a1fcc1a396c7b1e271448be6cc361c3ce80da17 Mon Sep 17 00:00:00 2001 From: YenHaoChen Date: Fri, 2 Feb 2024 10:17:30 +0800 Subject: [PATCH] Fix hvip.VSEIP and hvip.VSTIP so they don't observe platform-specific interrupts nor CSR hgeip bits The H extension defines that bits VSEIP, VSTIP, and VSSIP of hvip are writable. (The other bits of hvip are read-only 0.) Only hip.VSSIP (mip.VSSIP) is an alias of hvip.VSSIP. The hip.VSEIP is the logical-OR of hvip.VSEIP, selected bit of hgeip by hstatus.VGEIN, and platform-specific external interrupt signals to VS-level, e.g., from AIA. The hip.VSTIP is the logical-OR of hvip.VSTIP and platform-specific timer interrupt signals to VS-level, e.g., from Sstc. Thus, the read values of hvip.VSEIP and hvip.VSTIP differ from the ones of hip.VSEIP and hip.VSTIP (mip.VSEIP and mip.VSTIP). In other words, the hvip isn't an alias (proxy) of mip. The current aliasing (proxy) implementation does not provide the desired behavior for hvip.VSEIP and hvip.VSTIP. An ISA-level behavior difference is that any platform-specific external and timer interrupt signals directed to VS-level should not be observable through the hvip. For instance, the hvip should not observe the virtual timer interrupt signal from the vstimecmp CSR (Sstc extension), which isn't true in the current implementation. Additionally, the hvip should not observe the virtual external interrupt signal from the IMSIC device (AIA extension). Another ISA-level behavior difference is that the hgeip and hstatus.VGEIN also should not affect hvip.VSEIP, which isn't true in the current implementation. This commit fixes the issue by giving the hvip a specialized class, hvip_csr_t. The hvip_csr_t aliases the hvip.VSSIP to the mip.VSSIP but decouples the hvip.VSEIP and hvip.VSTIP from mip.VSEIP and mip.VSTIP. Additionally, the commit updates the read value of mip to be the logical-OR of hvip.VSEIP and hvip.VSTIP and other sources. --- riscv/csrs.cc | 17 +++++++++++++++++ riscv/csrs.h | 13 ++++++++++++- riscv/processor.cc | 11 +---------- riscv/processor.h | 1 + 4 files changed, 31 insertions(+), 11 deletions(-) diff --git a/riscv/csrs.cc b/riscv/csrs.cc index b76b496e8e..74f07c9d9a 100644 --- a/riscv/csrs.cc +++ b/riscv/csrs.cc @@ -728,6 +728,10 @@ mip_csr_t::mip_csr_t(processor_t* const proc, const reg_t addr): mip_or_mie_csr_t(proc, addr) { } +reg_t mip_csr_t::read() const noexcept { + return val | state->hvip->basic_csr_t::read(); +} + void mip_csr_t::backdoor_write_with_mask(const reg_t mask, const reg_t val) noexcept { this->val = (this->val & ~mask) | (val & mask); } @@ -1717,3 +1721,16 @@ void srmcfg_csr_t::verify_permissions(insn_t insn, bool write) const { if (state->v) throw trap_virtual_instruction(insn.bits()); } + +hvip_csr_t::hvip_csr_t(processor_t* const proc, const reg_t addr, const reg_t init): + basic_csr_t(proc, addr, init) { +} + +reg_t hvip_csr_t::read() const noexcept { + return basic_csr_t::read() | (state->mip->read() & MIP_VSSIP); // hvip.VSSIP is an alias of mip.VSSIP +} + +bool hvip_csr_t::unlogged_write(const reg_t val) noexcept { + state->mip->write_with_mask(MIP_VSSIP, val); // hvip.VSSIP is an alias of mip.VSSIP + return basic_csr_t::unlogged_write(val & (MIP_VSEIP | MIP_VSTIP)); +} diff --git a/riscv/csrs.h b/riscv/csrs.h index ad3f7a312a..b3dfe1c526 100644 --- a/riscv/csrs.h +++ b/riscv/csrs.h @@ -349,7 +349,7 @@ typedef std::shared_ptr misa_csr_t_p; class mip_or_mie_csr_t: public csr_t { public: mip_or_mie_csr_t(processor_t* const proc, const reg_t addr); - virtual reg_t read() const noexcept override final; + virtual reg_t read() const noexcept override; void write_with_mask(const reg_t mask, const reg_t val) noexcept; @@ -364,6 +364,7 @@ class mip_or_mie_csr_t: public csr_t { class mip_csr_t: public mip_or_mie_csr_t { public: mip_csr_t(processor_t* const proc, const reg_t addr); + virtual reg_t read() const noexcept override final; // Does not log. Used by external things (clint) that wiggle bits in mip. void backdoor_write_with_mask(const reg_t mask, const reg_t val) noexcept; @@ -850,4 +851,14 @@ class srmcfg_csr_t: public masked_csr_t { srmcfg_csr_t(processor_t* const proc, const reg_t addr, const reg_t mask, const reg_t init); virtual void verify_permissions(insn_t insn, bool write) const override; }; + +class hvip_csr_t : public basic_csr_t { + public: + hvip_csr_t(processor_t* const proc, const reg_t addr, const reg_t init); + reg_t read() const noexcept override; + protected: + virtual bool unlogged_write(const reg_t val) noexcept override; +}; + +typedef std::shared_ptr hvip_csr_t_p; #endif diff --git a/riscv/processor.cc b/riscv/processor.cc index 9739ab7e77..cfce08f751 100644 --- a/riscv/processor.cc +++ b/riscv/processor.cc @@ -304,15 +304,6 @@ void state_t::reset(processor_t* const proc, reg_t max_isa) 0 // shiftamt ); - auto hvip_accr = std::make_shared( - this, - MIP_VS_MASK, // read_mask - MIP_VS_MASK, // ip_write_mask - MIP_VS_MASK, // ie_write_mask - generic_int_accessor_t::mask_mode_t::NONE, - 0 // shiftamt - ); - auto vsip_vsie_accr = std::make_shared( this, MIP_VS_MASK, // read_mask @@ -327,7 +318,7 @@ void state_t::reset(processor_t* const proc, reg_t max_isa) csrmap[CSR_VSIP] = vsip; csrmap[CSR_SIP] = std::make_shared(proc, nonvirtual_sip, vsip); csrmap[CSR_HIP] = std::make_shared(proc, CSR_HIP, hip_hie_accr); - csrmap[CSR_HVIP] = std::make_shared(proc, CSR_HVIP, hvip_accr); + csrmap[CSR_HVIP] = hvip = std::make_shared(proc, CSR_HVIP, 0); auto nonvirtual_sie = std::make_shared(proc, CSR_SIE, sip_sie_accr); auto vsie = std::make_shared(proc, CSR_VSIE, vsip_vsie_accr); diff --git a/riscv/processor.h b/riscv/processor.h index 0394e090be..f9dcf253cc 100644 --- a/riscv/processor.h +++ b/riscv/processor.h @@ -128,6 +128,7 @@ struct state_t csr_t_p htval; csr_t_p htinst; csr_t_p hgatp; + hvip_csr_t_p hvip; sstatus_csr_t_p sstatus; vsstatus_csr_t_p vsstatus; csr_t_p vstvec;