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 X-Spam-Level: X-Spam-Status: No, score=-12.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 958CCC2B9F8 for ; Tue, 25 May 2021 07:23:37 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 78B936141C for ; Tue, 25 May 2021 07:23:36 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 78B936141C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=c-sky.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:36054 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1llRPb-0004x6-Ke for qemu-devel@archiver.kernel.org; Tue, 25 May 2021 03:23:35 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:37760) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1llROV-0003zZ-Pr; Tue, 25 May 2021 03:22:27 -0400 Received: from out28-170.mail.aliyun.com ([115.124.28.170]:40214) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1llROR-0000Ll-Pi; Tue, 25 May 2021 03:22:27 -0400 X-Alimail-AntiSpam: AC=CONTINUE; BC=0.07176567|-1; CH=green; DM=|CONTINUE|false|; DS=CONTINUE|ham_regular_dialog|0.0126492-8.3722e-05-0.987267; FP=0|0|0|0|0|-1|-1|-1; HT=ay29a033018047188; MF=zhiwei_liu@c-sky.com; NM=1; PH=DS; RN=7; RT=7; SR=0; TI=SMTPD_---.KIP.QLB_1621927328; Received: from 172.27.117.59(mailfrom:zhiwei_liu@c-sky.com fp:SMTPD_---.KIP.QLB_1621927328) by smtp.aliyun-inc.com(10.147.42.253); Tue, 25 May 2021 15:22:09 +0800 Subject: Re: [PATCH] target/riscv: hardwire bits in hideleg and hedeleg To: Jose Martins , qemu-devel@nongnu.org References: <20210522155902.374439-1-josemartins90@gmail.com> From: LIU Zhiwei Message-ID: <434d8825-8bd6-5344-aea2-e176fad6183e@c-sky.com> Date: Tue, 25 May 2021 15:22:07 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <20210522155902.374439-1-josemartins90@gmail.com> Content-Type: multipart/alternative; boundary="------------3A719C23A2C0B1D4B11E73BA" Content-Language: en-US Received-SPF: none client-ip=115.124.28.170; envelope-from=zhiwei_liu@c-sky.com; helo=out28-170.mail.aliyun.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, HTML_MESSAGE=0.001, NICE_REPLY_A=-0.001, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_NONE=0.001, UNPARSEABLE_RELAY=0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Bastian Koppelmann , Alistair Francis , "open list:RISC-V TCG CPUs" , Palmer Dabbelt , Sagar Karandikar Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" This is a multi-part message in MIME format. --------------3A719C23A2C0B1D4B11E73BA Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit On 5/22/21 11:59 PM, Jose Martins wrote: > The specification mandates for certain bits to be hardwired in the > hypervisor delegation registers. This was not being enforced. > > Signed-off-by: Jose Martins > --- > target/riscv/csr.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > index d2585395bf..9b74a00cc9 100644 > --- a/target/riscv/csr.c > +++ b/target/riscv/csr.c > @@ -394,6 +394,7 @@ static int read_timeh(CPURISCVState *env, int csrno, target_ulong *val) > > static const target_ulong delegable_ints = S_MODE_INTERRUPTS | > VS_MODE_INTERRUPTS; > +static const target_ulong vs_delegable_ints = VS_MODE_INTERRUPTS; We can use it directly if only one macro VS_MODE_INTERRUPTS. > static const target_ulong all_ints = M_MODE_INTERRUPTS | S_MODE_INTERRUPTS | > VS_MODE_INTERRUPTS; > static const target_ulong delegable_excps = > @@ -416,6 +417,14 @@ static const target_ulong delegable_excps = > (1ULL << (RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT)) | > (1ULL << (RISCV_EXCP_VIRT_INSTRUCTION_FAULT)) | > (1ULL << (RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT)); > +static const target_ulong vs_delegable_excps = delegable_excps & > + ~((1ULL << (RISCV_EXCP_S_ECALL)) | > + (1ULL << (RISCV_EXCP_VS_ECALL)) | I didn't find that the RISCV_EXCP_VS_ECALL should be read-only 0 from the specification. > + (1ULL << (RISCV_EXCP_M_ECALL)) | > + (1ULL << (RISCV_EXCP_INST_GUEST_PAGE_FAULT)) | > + (1ULL << (RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT)) | > + (1ULL << (RISCV_EXCP_VIRT_INSTRUCTION_FAULT)) | > + (1ULL << (RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT))); > static const target_ulong sstatus_v1_10_mask = SSTATUS_SIE | SSTATUS_SPIE | > SSTATUS_UIE | SSTATUS_UPIE | SSTATUS_SPP | SSTATUS_FS | SSTATUS_XS | > SSTATUS_SUM | SSTATUS_MXR | SSTATUS_SD; > @@ -963,7 +972,7 @@ static int read_hedeleg(CPURISCVState *env, int csrno, target_ulong *val) > > static int write_hedeleg(CPURISCVState *env, int csrno, target_ulong val) > { > - env->hedeleg = val; > + env->hedeleg = val & vs_delegable_excps; I think it's OK here. However, as hedeleg is WARL, you had better reserve the other fields like medeleg: env->medeleg = (env->medeleg & ~delegable_excps) | (val & delegable_excps); I really don't understand why medeleg codes this way. Is there anyone can give a better explanation? > return 0; > } > > @@ -975,7 +984,7 @@ static int read_hideleg(CPURISCVState *env, int csrno, target_ulong *val) > > static int write_hideleg(CPURISCVState *env, int csrno, target_ulong val) > { > - env->hideleg = val; > + env->hideleg = val & vs_delegable_ints; > return 0; > } > The similar comments for hedeleg. Zhiwei --------------3A719C23A2C0B1D4B11E73BA Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 7bit


On 5/22/21 11:59 PM, Jose Martins wrote:
The specification mandates for certain bits to be hardwired in the
hypervisor delegation registers. This was not being enforced.

Signed-off-by: Jose Martins <josemartins90@gmail.com>
---
 target/riscv/csr.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d2585395bf..9b74a00cc9 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -394,6 +394,7 @@ static int read_timeh(CPURISCVState *env, int csrno, target_ulong *val)
 
 static const target_ulong delegable_ints = S_MODE_INTERRUPTS |
                                            VS_MODE_INTERRUPTS;
+static const target_ulong vs_delegable_ints = VS_MODE_INTERRUPTS;
We can use it directly if only one macro VS_MODE_INTERRUPTS.
 static const target_ulong all_ints = M_MODE_INTERRUPTS | S_MODE_INTERRUPTS |
                                      VS_MODE_INTERRUPTS;
 static const target_ulong delegable_excps =
@@ -416,6 +417,14 @@ static const target_ulong delegable_excps =
     (1ULL << (RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT)) |
     (1ULL << (RISCV_EXCP_VIRT_INSTRUCTION_FAULT)) |
     (1ULL << (RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT));
+static const target_ulong vs_delegable_excps = delegable_excps &
+    ~((1ULL << (RISCV_EXCP_S_ECALL)) |
+    (1ULL << (RISCV_EXCP_VS_ECALL)) |
I didn't find that the RISCV_EXCP_VS_ECALL should be read-only 0 from the specification.
+    (1ULL << (RISCV_EXCP_M_ECALL)) |
+    (1ULL << (RISCV_EXCP_INST_GUEST_PAGE_FAULT)) |
+    (1ULL << (RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT)) |
+    (1ULL << (RISCV_EXCP_VIRT_INSTRUCTION_FAULT)) |
+    (1ULL << (RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT)));
 static const target_ulong sstatus_v1_10_mask = SSTATUS_SIE | SSTATUS_SPIE |
     SSTATUS_UIE | SSTATUS_UPIE | SSTATUS_SPP | SSTATUS_FS | SSTATUS_XS |
     SSTATUS_SUM | SSTATUS_MXR | SSTATUS_SD;
@@ -963,7 +972,7 @@ static int read_hedeleg(CPURISCVState *env, int csrno, target_ulong *val)
 
 static int write_hedeleg(CPURISCVState *env, int csrno, target_ulong val)
 {
-    env->hedeleg = val;
+    env->hedeleg = val & vs_delegable_excps;

I think it's OK here.

However, as hedeleg is WARL, you had better reserve the other fields like medeleg:

	env->medeleg = (env->medeleg & ~delegable_excps) | (val & delegable_excps);

I really don't understand why medeleg codes this way. Is there anyone can give a better explanation?

     return 0;
 }
 
@@ -975,7 +984,7 @@ static int read_hideleg(CPURISCVState *env, int csrno, target_ulong *val)
 
 static int write_hideleg(CPURISCVState *env, int csrno, target_ulong val)
 {
-    env->hideleg = val;
+    env->hideleg = val & vs_delegable_ints;
     return 0;
 }

The similar comments for hedeleg.

Zhiwei


    

  


--------------3A719C23A2C0B1D4B11E73BA--


From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from list by lists.gnu.org with archive (Exim 4.90_1)
	id 1llROX-000401-R3
	for mharc-qemu-riscv@gnu.org; Tue, 25 May 2021 03:22:29 -0400
Received: from eggs.gnu.org ([2001:470:142:3::10]:37760)
 by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from )
 id 1llROV-0003zZ-Pr; Tue, 25 May 2021 03:22:27 -0400
Received: from out28-170.mail.aliyun.com ([115.124.28.170]:40214)
 by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from )
 id 1llROR-0000Ll-Pi; Tue, 25 May 2021 03:22:27 -0400
X-Alimail-AntiSpam: AC=CONTINUE; BC=0.07176567|-1; CH=green;
 DM=|CONTINUE|false|;
 DS=CONTINUE|ham_regular_dialog|0.0126492-8.3722e-05-0.987267;
 FP=0|0|0|0|0|-1|-1|-1; HT=ay29a033018047188; MF=zhiwei_liu@c-sky.com; NM=1;
 PH=DS; RN=7; RT=7; SR=0; TI=SMTPD_---.KIP.QLB_1621927328; 
Received: from 172.27.117.59(mailfrom:zhiwei_liu@c-sky.com
 fp:SMTPD_---.KIP.QLB_1621927328)
 by smtp.aliyun-inc.com(10.147.42.253);
 Tue, 25 May 2021 15:22:09 +0800
Subject: Re: [PATCH] target/riscv: hardwire bits in hideleg and hedeleg
To: Jose Martins , qemu-devel@nongnu.org
Cc: "open list:RISC-V TCG CPUs" ,
 Sagar Karandikar ,
 Bastian Koppelmann ,
 Alistair Francis ,
 Palmer Dabbelt 
References: <20210522155902.374439-1-josemartins90@gmail.com>
From: LIU Zhiwei 
Message-ID: <434d8825-8bd6-5344-aea2-e176fad6183e@c-sky.com>
Date: Tue, 25 May 2021 15:22:07 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.8.1
MIME-Version: 1.0
In-Reply-To: <20210522155902.374439-1-josemartins90@gmail.com>
Content-Type: multipart/alternative;
 boundary="------------3A719C23A2C0B1D4B11E73BA"
Content-Language: en-US
Received-SPF: none client-ip=115.124.28.170; envelope-from=zhiwei_liu@c-sky.com;
 helo=out28-170.mail.aliyun.com
X-Spam_score_int: -18
X-Spam_score: -1.9
X-Spam_bar: -
X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, HTML_MESSAGE=0.001,
 NICE_REPLY_A=-0.001, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H4=0.001,
 RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_NONE=0.001,
 UNPARSEABLE_RELAY=0.001 autolearn=ham autolearn_force=no
X-Spam_action: no action
X-BeenThere: qemu-riscv@nongnu.org
X-Mailman-Version: 2.1.23
Precedence: list
List-Id: 
List-Unsubscribe: ,
 
List-Archive: 
List-Post: 
List-Help: 
List-Subscribe: ,
 
X-List-Received-Date: Tue, 25 May 2021 07:22:28 -0000

This is a multi-part message in MIME format.
--------------3A719C23A2C0B1D4B11E73BA
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit


On 5/22/21 11:59 PM, Jose Martins wrote:
> The specification mandates for certain bits to be hardwired in the
> hypervisor delegation registers. This was not being enforced.
>
> Signed-off-by: Jose Martins 
> ---
>   target/riscv/csr.c | 13 +++++++++++--
>   1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index d2585395bf..9b74a00cc9 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -394,6 +394,7 @@ static int read_timeh(CPURISCVState *env, int csrno, target_ulong *val)
>   
>   static const target_ulong delegable_ints = S_MODE_INTERRUPTS |
>                                              VS_MODE_INTERRUPTS;
> +static const target_ulong vs_delegable_ints = VS_MODE_INTERRUPTS;
We can use it directly if only one macro VS_MODE_INTERRUPTS.
>   static const target_ulong all_ints = M_MODE_INTERRUPTS | S_MODE_INTERRUPTS |
>                                        VS_MODE_INTERRUPTS;
>   static const target_ulong delegable_excps =
> @@ -416,6 +417,14 @@ static const target_ulong delegable_excps =
>       (1ULL << (RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT)) |
>       (1ULL << (RISCV_EXCP_VIRT_INSTRUCTION_FAULT)) |
>       (1ULL << (RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT));
> +static const target_ulong vs_delegable_excps = delegable_excps &
> +    ~((1ULL << (RISCV_EXCP_S_ECALL)) |
> +    (1ULL << (RISCV_EXCP_VS_ECALL)) |
I didn't find that the RISCV_EXCP_VS_ECALL should be read-only 0 from 
the specification.
> +    (1ULL << (RISCV_EXCP_M_ECALL)) |
> +    (1ULL << (RISCV_EXCP_INST_GUEST_PAGE_FAULT)) |
> +    (1ULL << (RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT)) |
> +    (1ULL << (RISCV_EXCP_VIRT_INSTRUCTION_FAULT)) |
> +    (1ULL << (RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT)));
>   static const target_ulong sstatus_v1_10_mask = SSTATUS_SIE | SSTATUS_SPIE |
>       SSTATUS_UIE | SSTATUS_UPIE | SSTATUS_SPP | SSTATUS_FS | SSTATUS_XS |
>       SSTATUS_SUM | SSTATUS_MXR | SSTATUS_SD;
> @@ -963,7 +972,7 @@ static int read_hedeleg(CPURISCVState *env, int csrno, target_ulong *val)
>   
>   static int write_hedeleg(CPURISCVState *env, int csrno, target_ulong val)
>   {
> -    env->hedeleg = val;
> +    env->hedeleg = val & vs_delegable_excps;

I think it's OK here.

However, as hedeleg is WARL, you had better reserve the other fields 
like medeleg:

	env->medeleg = (env->medeleg & ~delegable_excps) | (val & delegable_excps);

I really don't understand why medeleg codes this way. Is there anyone 
can give a better explanation?

>       return 0;
>   }
>   
> @@ -975,7 +984,7 @@ static int read_hideleg(CPURISCVState *env, int csrno, target_ulong *val)
>   
>   static int write_hideleg(CPURISCVState *env, int csrno, target_ulong val)
>   {
> -    env->hideleg = val;
> +    env->hideleg = val & vs_delegable_ints;
>       return 0;
>   }
>   

The similar comments for hedeleg.

Zhiwei


--------------3A719C23A2C0B1D4B11E73BA
Content-Type: text/html; charset=utf-8
Content-Transfer-Encoding: 7bit


  
    
  
  
    


    

    
On 5/22/21 11:59 PM, Jose Martins
      wrote:

    

    

      
The specification mandates for certain bits to be hardwired in the
hypervisor delegation registers. This was not being enforced.

Signed-off-by: Jose Martins <josemartins90@gmail.com>
---
 target/riscv/csr.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d2585395bf..9b74a00cc9 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -394,6 +394,7 @@ static int read_timeh(CPURISCVState *env, int csrno, target_ulong *val)
 
 static const target_ulong delegable_ints = S_MODE_INTERRUPTS |
                                            VS_MODE_INTERRUPTS;
+static const target_ulong vs_delegable_ints = VS_MODE_INTERRUPTS;

    

    We can use it directly if only one macro VS_MODE_INTERRUPTS.

    

      
 static const target_ulong all_ints = M_MODE_INTERRUPTS | S_MODE_INTERRUPTS |
                                      VS_MODE_INTERRUPTS;
 static const target_ulong delegable_excps =
@@ -416,6 +417,14 @@ static const target_ulong delegable_excps =
     (1ULL << (RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT)) |
     (1ULL << (RISCV_EXCP_VIRT_INSTRUCTION_FAULT)) |
     (1ULL << (RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT));
+static const target_ulong vs_delegable_excps = delegable_excps &
+    ~((1ULL << (RISCV_EXCP_S_ECALL)) |
+    (1ULL << (RISCV_EXCP_VS_ECALL)) |

    

    I didn't find that the RISCV_EXCP_VS_ECALL should be read-only 0
    from the specification.

    

      
+    (1ULL << (RISCV_EXCP_M_ECALL)) |
+    (1ULL << (RISCV_EXCP_INST_GUEST_PAGE_FAULT)) |
+    (1ULL << (RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT)) |
+    (1ULL << (RISCV_EXCP_VIRT_INSTRUCTION_FAULT)) |
+    (1ULL << (RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT)));
 static const target_ulong sstatus_v1_10_mask = SSTATUS_SIE | SSTATUS_SPIE |
     SSTATUS_UIE | SSTATUS_UPIE | SSTATUS_SPP | SSTATUS_FS | SSTATUS_XS |
     SSTATUS_SUM | SSTATUS_MXR | SSTATUS_SD;
@@ -963,7 +972,7 @@ static int read_hedeleg(CPURISCVState *env, int csrno, target_ulong *val)
 
 static int write_hedeleg(CPURISCVState *env, int csrno, target_ulong val)
 {
-    env->hedeleg = val;
+    env->hedeleg = val & vs_delegable_excps;

    

    
I think it's OK here.

    

    
However, as hedeleg is WARL, you had better reserve the other
      fields like medeleg:

    
	env->medeleg = (env->medeleg & ~delegable_excps) | (val & delegable_excps);



    
I really don't understand why medeleg codes this way. Is there
      anyone can give a better explanation?

    

    

      
     return 0;
 }
 
@@ -975,7 +984,7 @@ static int read_hideleg(CPURISCVState *env, int csrno, target_ulong *val)
 
 static int write_hideleg(CPURISCVState *env, int csrno, target_ulong val)
 {
-    env->hideleg = val;
+    env->hideleg = val & vs_delegable_ints;
     return 0;
 }
 

    

    
The similar comments for hedeleg.

    
Zhiwei

    

    

      

    

  


--------------3A719C23A2C0B1D4B11E73BA--