From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 9D596C43334 for ; Thu, 7 Jul 2022 17:21:56 +0000 (UTC) Received: from localhost ([::1]:56290 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1o9VCN-0005hF-7o for qemu-devel@archiver.kernel.org; Thu, 07 Jul 2022 13:21:55 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:60322) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1o9VAw-0004rl-CU for qemu-devel@nongnu.org; Thu, 07 Jul 2022 13:20:26 -0400 Received: from mail-pg1-x532.google.com ([2607:f8b0:4864:20::532]:43974) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1o9VAt-0004Ov-LY for qemu-devel@nongnu.org; Thu, 07 Jul 2022 13:20:25 -0400 Received: by mail-pg1-x532.google.com with SMTP id 73so4286526pgb.10 for ; Thu, 07 Jul 2022 10:20:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ventanamicro.com; s=google; h=message-id:subject:from:to:cc:date:in-reply-to:references :user-agent:mime-version:content-transfer-encoding; bh=7l3OyRQF4jExyMQe7zeFEVUk3lb2PZUMnFo/DDRjEwc=; b=Hd83+UDAYD0CTwaD6odq4JUa+W7eRYMOFRxT59Aa0bXpdDr3oX34gGdjs4g08xjLfk LSMvhjVmL9YhnKeQWYbPYjtkctstC7qUH58lG4F0ApmCNvQhzrjlYHQxs7Cmlzn/RIvY JNi/RLTN/cs4hUVw24xsSNOP++LwU91MBclokpaitgL7DOsVUxE5qzBNWBmbJUypElmm YKico+csVRFJ84Np7yNP18isD5w5H9Dgzt7cVzw76wRoRYiBo0EPOES+/MLLjMfWNScA 3KDl81nFK094oCydiWRU6ZwezPzHw/nMN0SjZLNs5gjwyUsBZNpJtZoyox4opGfaX9n4 raKA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=7l3OyRQF4jExyMQe7zeFEVUk3lb2PZUMnFo/DDRjEwc=; b=Q4/Sze+/wIeRXaI6kCK9DGj87TlaR9ChnI46aPor6G7FrtaKN1RG+te0HefK/lI2uc aOUT0PU9ZiKf8zD/DAT1InDevP2CrIqwssxmGbn2E8t/DVdrXGjhjuPHE86tPQVczD22 HaWZxpQemepRt/ZMyRwd7S9vE9zCIgyIdFRhiIPpgzaIC/oUXwBFzTVQTPUvlUGhwGnY yIpIQlerxdNCAPIJVrR5CpKUtgayhWeO0MDPkHYidboCSUFhgwtJuoKDpvyVUb87HiMk l6c3hfskumbeQPI0U4AHM4T6b5JRrOgoKXvtJ+ehUbpjD/FOtMoRlLxAdYFHa3kIhTvo YwVw== X-Gm-Message-State: AJIora/YyWP9GhUCMw4q70RTwssinLo1hD/DxpK7oOu1Cy0hRfZkLeEt Iuy0RqlDM6K2kXHVWi0wqwRoLw== X-Google-Smtp-Source: AGRyM1trZYFG6mQzCXqRwmYC4yoPLr60sNMXQl7XMvRaac92fdye7IzZNimRxAVQvX2zTZc6vix5pg== X-Received: by 2002:a17:90a:1485:b0:1ec:788e:a053 with SMTP id k5-20020a17090a148500b001ec788ea053mr6190733pja.16.1657214421977; Thu, 07 Jul 2022 10:20:21 -0700 (PDT) Received: from [192.168.1.18] ([182.64.106.38]) by smtp.gmail.com with ESMTPSA id f9-20020a170902684900b00168c52319c3sm28144653pln.149.2022.07.07.10.20.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Jul 2022 10:20:21 -0700 (PDT) Message-ID: Subject: Re: [RFC PATCH v5 2/4] target/riscv: smstateen check for h/senvcfg From: Mayuresh Chitale To: angell1518 Cc: qemu-devel@nongnu.org, qemu-riscv@nongnu.org, alistair.francis@wdc.com Date: Thu, 07 Jul 2022 22:50:14 +0530 In-Reply-To: <35f3afa1.21b5.181be7970fc.Coremail.angell1518@163.com> References: <20220603160425.3667456-1-mchitale@ventanamicro.com> <20220603160425.3667456-3-mchitale@ventanamicro.com> <35f3afa1.21b5.181be7970fc.Coremail.angell1518@163.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.36.5-0ubuntu1 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Received-SPF: pass client-ip=2607:f8b0:4864:20::532; envelope-from=mchitale@ventanamicro.com; helo=mail-pg1-x532.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On Sat, 2022-07-02 at 18:33 +0800, angell1518 wrote: > At 2022-06-04 00:04:23, "Mayuresh Chitale" > wrote: > >Accesses to henvcfg, henvcfgh and senvcfg are allowed > >only if corresponding bit in mstateen0/hstateen0 is > >enabled. Otherwise an illegal instruction trap is > >generated. > > > >Signed-off-by: Mayuresh Chitale > >--- > > target/riscv/csr.c | 84 ++++++++++++++++++++++++++++++++++++++++++- > --- > > 1 file changed, 78 insertions(+), 6 deletions(-) > > > >diff --git a/target/riscv/csr.c b/target/riscv/csr.c > >index 324fefce59..ae91ae1f7e 100644 > >--- a/target/riscv/csr.c > >+++ b/target/riscv/csr.ccounteren > >@@ -39,6 +39,37 @@ void riscv_set_csr_ops(int csrno, > riscv_csr_operations *ops) > > } > > > > /* Predicates */ > >+static RISCVException smstateen_acc_ok(CPURISCVState *env, int > mode, int bit) > >+{ > >+ CPUState *cs = env_cpu(env); > >+ RISCVCPU *cpu = RISCV_CPU(cs); > >+ bool virt = riscv_cpu_virt_enabled(env); > >+ > >+ if (!cpu->cfg.ext_smstateen) { > >+ return RISCV_EXCP_NONE; > >+ } > >+ > >+#if !defined(CONFIG_USER_ONLY) > >+ if (!(env->mstateen[0] & 1UL << bit)) { > >+ return RISCV_EXCP_ILLEGAL_INST; > >+ } > >+ > I think mstateen only control access in priv mode under M mode. > So we should take the current priv node into consideration here. For any curent priv mode if the required bit is not set in mstateen we can return here itself. For e.g if current priv mode is S then we only check the required bit in mstateen. If current priv mode is 'U', we need to check the required bit in mstateen and sstateen > >+ if (virt) { > >+ if (!(env->hstateen[0] & 1UL << bit)) { > >+ return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > >+ } > >+ } > >+ > >+ if (mode == PRV_U) { > >+ if (!(env->sstateen[0] & 1UL << bit)) { > >+ return RISCV_EXCP_ILLEGAL_INST; > >+ } > I think there are two problem here: > The first is that we should also take virt mode into consideration > here too. Actually virt mode is considered above for both priv modes S and U. > As the spec said: > "if executing in VS or VU mode and the circumstances for a virtual > instruction > exception apply, raises a virtual instruction exception instead of an > illegal instruction exception" > So it will generate RISCV_EXCP_VIRT_INSTRUCTION_FAULT in VU mode. > > The second is that sstateen csr works only when 'S' extension is > enabled > I will fix it in the next version. > Regards, > Weiwei Li > >+ } > >+#endif > >+ > >+ return RISCV_EXCP_NONE; > >+} > >+ > > static RISCVException fs(CPURISCVState *env, int csrno) > > { > > #if !defined(CONFIG_USER_ONLY) > >@@ -1557,6 +1588,13 @@ static RISCVException > write_menvcfgh(CPURISCVState *env, int csrno, > > static RISCVException read_senvcfg(CPURISCVState *env, int csrno, > > target_ulong *val) > > { > >+ RISCVException ret; > >+ > >+ ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG); > >+ if (ret != RISCV_EXCP_NONE) { > >+ return ret; > >+ } > >+ > > *val = env->senvcfg; > > return RISCV_EXCP_NONE; > > } > >@@ -1565,15 +1603,27 @@ static RISCVException > write_senvcfg(CPURISCVState *env, int csrno, > > target_ulong val) > > { > > uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE | > SENVCFG_CBZE; > >+ RISCVException ret; > > > >- env->senvcfg = (env->senvcfg & ~mask) | (val & mask); > >+ ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG); > >+ if (ret != RISCV_EXCP_NONE) { > >+ return ret; > >+ } > > > >+ env->senvcfg = (env->senvcfg & ~mask) | (val & mask); > > return RISCV_EXCP_NONE; > > } > > > > static RISCVException read_henvcfg(CPURISCVState *env, int csrno, > > target_ulong *val) > > { > >+ RISCVException ret; > >+ > >+ ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG); > >+ if (ret != RISCV_EXCP_NONE) { > >+ return ret; > >+ } > >+ > > *val = env->henvcfg; > > return RISCV_EXCP_NONE; > > } > >@@ -1582,6 +1632,12 @@ static RISCVException > write_henvcfg(CPURISCVState *env, int csrno, > > target_ulong val) > > { > > uint64_t mask = HENVCFG_FIOM | HENVCFG_CBIE | HENVCFG_CBCFE | > HENVCFG_CBZE; > >+ RISCVException ret; > >+ > >+ ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG); > >+ if (ret != RISCV_EXCP_NONE) { > >+ return ret; > >+ } > > > > if (riscv_cpu_mxl(env) == MXL_RV64) { > > mask |= HENVCFG_PBMTE | HENVCFG_STCE; > >@@ -1595,6 +1651,13 @@ static RISCVException > write_henvcfg(CPURISCVState *env, int csrno, > > static RISCVException read_henvcfgh(CPURISCVState *env, int csrno, > > target_ulong *val) > > { > >+ RISCVException ret; > >+ > >+ ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG); > >+ if (ret != RISCV_EXCP_NONE) { > >+ return ret; > >+ } > >+ > > *val = env->henvcfg >> 32; > > return RISCV_EXCP_NONE; > > } > >@@ -1604,9 +1667,14 @@ static RISCVException > write_henvcfgh(CPURISCVState *env, int csrno, > > { > > uint64_t mask = HENVCFG_PBMTE | HENVCFG_STCE; > > uint64_t valh = (uint64_t)val << 32; > >+ RISCVException ret; > > > >- env->henvcfg = (env->henvcfg & ~mask) | (valh & mask); > >+ ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG); > >+ if (ret != RISCV_EXCP_NONE) { > >+ return ret; > >+ } > > > >+ env->henvcfg = (env->henvcfg & ~mask) | (valh & mask); > > return RISCV_EXCP_NONE; > > } > > > >@@ -1628,7 +1696,8 @@ static RISCVException > write_mstateen(CPURISCVState *env, int csrno, > > target_ulong new_val) > > { > > uint64_t *reg; > >- uint64_t wr_mask = 1UL << SMSTATEEN_STATEN; > >+ uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) | > >+ (1UL << SMSTATEEN0_HSENVCFG); > > > > reg = &env->mstateen[csrno - CSR_MSTATEEN0]; > > write_smstateen(env, reg, wr_mask, new_val); > >@@ -1649,7 +1718,8 @@ static RISCVException > write_mstateenh(CPURISCVState *env, int csrno, > > { > > uint64_t *reg; > > uint64_t val; > >- uint64_t wr_mask = 1UL << SMSTATEEN_STATEN; > >+ uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) | > >+ (1UL << SMSTATEEN0_HSENVCFG); > > > > reg = &env->mstateen[csrno - CSR_MSTATEEN0H]; > > val = (uint64_t)new_val << 32; > >@@ -1671,7 +1741,8 @@ static RISCVException > write_hstateen(CPURISCVState *env, int csrno, > > target_ulong new_val) > > { > > uint64_t *reg; > >- uint64_t wr_mask = 1UL << SMSTATEEN_STATEN; > >+ uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) | > >+ (1UL << SMSTATEEN0_HSENVCFG); > > int index = csrno - CSR_HSTATEEN0; > > > > reg = &env->hstateen[index]; > >@@ -1694,8 +1765,9 @@ static RISCVException > write_hstateenh(CPURISCVState *env, int csrno, > > { > > uint64_t *reg; > > uint64_t val; > >- uint64_t wr_mask = 1UL << SMSTATEEN_STATEN; > > int index = csrno - CSR_HSTATEEN0H; > >+ uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) | > >+ (1UL << SMSTATEEN0_HSENVCFG); > > > > reg = &env->hstateen[index]; > > val = (uint64_t)new_val << 32; > >-- > >2.25.1 > >