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=-10.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 5266CC07E95 for ; Tue, 13 Jul 2021 07:17:15 +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 B7847611F1 for ; Tue, 13 Jul 2021 07:17:14 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B7847611F1 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=sifive.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:44990 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1m3CfJ-0005TQ-Se for qemu-devel@archiver.kernel.org; Tue, 13 Jul 2021 03:17:13 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:56644) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m3CeB-00045N-FF for qemu-devel@nongnu.org; Tue, 13 Jul 2021 03:16:04 -0400 Received: from mail-pj1-x1029.google.com ([2607:f8b0:4864:20::1029]:52081) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1m3Ce8-0005dU-BW for qemu-devel@nongnu.org; Tue, 13 Jul 2021 03:16:03 -0400 Received: by mail-pj1-x1029.google.com with SMTP id n11so11601164pjo.1 for ; Tue, 13 Jul 2021 00:16:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sifive.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=ZIxC97vo3D/9ttg5bUmdmcZs3xyDx1jpdRBLzqRnrn4=; b=L/qp8DgRV3vXBa7RXmsw8KdAM7mX6XIj2Bdirec//LXfwez+T/sRnL+utnaBW/StI3 wZ8TFgZhZryx0fq2seaiBOivAbOPremRbzkzpY0Gsf3Aml9cRb2zJOpCnCPp5TEtyzTS YF2m7hwB5kCdCwByjnp/o5pKyAp80Z8DT2MT6gPFifkB38I4OsrkseJhD71BVJjrtYG8 svYY4I4zRc/dbu/1oNSce/VcS+Qh6xUFixOXtr1iQyh5pUedSjh8nzGZTG2pDU5hArSf 0w3EqAnVSm18Si6t4HGIflPHovFLEJ0zRds6KfLipyZPmO31XCD/LeYOolhsrFLvehJy Um9A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ZIxC97vo3D/9ttg5bUmdmcZs3xyDx1jpdRBLzqRnrn4=; b=Dm2JhsBCId+ybVeA6m3EJJcewNt34zYBfKAV6ANHc77/EEgsjI5pE4jroCVMqRRkk8 rPFvo9GwUDCnJGqm5nK7bkqrNJBg/sTm3QvUkULAoOxW6C5iz5imcupicEy/cIWUiCM4 fE+Yoqf0CVc9S4psEf7q/6Am+pilgSb3RsMlNuWepcQE5O4cJmj2e8/WTpW2xOPUjvOu VR3RYaBJqM6jbYwHXvSgX/WXwcPR9XgLOTsLwF2o4eyOzOlxO4Id+dRNki2JqqzOH2ww uFw3Klpz1+CP8KUFGTyjUvExayoBPkJKs1mqZejD/Aveo6YE2SaB6+JwB3YOjpLBZfs1 bTKA== X-Gm-Message-State: AOAM532CsNrzHbHKhJMAbvOAQpkf1yRqw8DYB/axeaz81ZfqQuQFKg6d IvIEmH5QHa2CAxr+he9svUwN0yFoCmW8WwqX X-Google-Smtp-Source: ABdhPJykeWLaas46TZ9by8/73KbjyoSaUJYQ3ZYi35yKlGlMNQs04jpMzxjKcwg7gjQDooVrAH/dcg== X-Received: by 2002:a17:902:bd96:b029:129:24e:43b5 with SMTP id q22-20020a170902bd96b0290129024e43b5mr2507112pls.66.1626160558340; Tue, 13 Jul 2021 00:15:58 -0700 (PDT) Received: from mail-pf1-f169.google.com (mail-pf1-f169.google.com. [209.85.210.169]) by smtp.gmail.com with ESMTPSA id d13sm17865411pfn.136.2021.07.13.00.15.57 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 13 Jul 2021 00:15:57 -0700 (PDT) Received: by mail-pf1-f169.google.com with SMTP id q10so18688468pfj.12; Tue, 13 Jul 2021 00:15:57 -0700 (PDT) X-Received: by 2002:aa7:938c:0:b029:32a:1725:a3d7 with SMTP id t12-20020aa7938c0000b029032a1725a3d7mr3073973pfe.64.1626160557533; Tue, 13 Jul 2021 00:15:57 -0700 (PDT) MIME-Version: 1.0 References: <20210409074857.166082-1-zhiwei_liu@c-sky.com> <20210409074857.166082-12-zhiwei_liu@c-sky.com> In-Reply-To: <20210409074857.166082-12-zhiwei_liu@c-sky.com> From: Frank Chang Date: Tue, 13 Jul 2021 15:15:46 +0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [RFC PATCH 11/11] target/riscv: Update interrupt return in CLIC mode To: LIU Zhiwei Content-Type: multipart/alternative; boundary="000000000000612c9f05c6fc0211" Received-SPF: pass client-ip=2607:f8b0:4864:20::1029; envelope-from=frank.chang@sifive.com; helo=mail-pj1-x1029.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, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable 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: Palmer Dabbelt , Alistair Francis , "open list:RISC-V" , "qemu-devel@nongnu.org Developers" , wxy194768@alibaba-inc.com Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --000000000000612c9f05c6fc0211 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable LIU Zhiwei =E6=96=BC 2021=E5=B9=B44=E6=9C=889=E6=97= =A5 =E9=80=B1=E4=BA=94 =E4=B8=8B=E5=8D=883:55=E5=AF=AB=E9=81=93=EF=BC=9A > When a vectored interrupt is selected and serviced, the hardware will > automatically clear the corresponding pending bit in edge-triggered mode. > This may lead to a lower priviledge interrupt pending forever. > > Therefore when interrupts return, pull a pending interrupt to service. > > Signed-off-by: LIU Zhiwei > --- > target/riscv/op_helper.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c > index 1eddcb94de..42563b22ba 100644 > --- a/target/riscv/op_helper.c > +++ b/target/riscv/op_helper.c > @@ -24,6 +24,10 @@ > #include "exec/exec-all.h" > #include "exec/helper-proto.h" > > +#if !defined(CONFIG_USER_ONLY) > +#include "hw/intc/riscv_clic.h" > +#endif > + > /* Exceptions processing helpers */ > void QEMU_NORETURN riscv_raise_exception(CPURISCVState *env, > uint32_t exception, uintptr_t > pc) > @@ -130,6 +134,17 @@ target_ulong helper_sret(CPURISCVState *env, > target_ulong cpu_pc_deb) > mstatus =3D set_field(mstatus, MSTATUS_SPIE, 1); > mstatus =3D set_field(mstatus, MSTATUS_SPP, PRV_U); > env->mstatus =3D mstatus; > + > + if (riscv_clic_is_clic_mode(env)) { > + CPUState *cs =3D env_cpu(env); > + target_ulong spil =3D get_field(env->scause, SCAUSE_SPIL); > + env->mintstatus =3D set_field(env->mintstatus, MINTSTATUS_SI= L, > spil); > + env->scause =3D set_field(env->scause, SCAUSE_SPIE, 0); > + env->scause =3D set_field(env->scause, SCAUSE_SPP, PRV_U); > + qemu_mutex_lock_iothread(); > + riscv_clic_get_next_interrupt(env->clic, cs->cpu_index); > + qemu_mutex_unlock_iothread(); > + } > } > > riscv_cpu_set_mode(env, prev_priv); > @@ -172,6 +187,16 @@ target_ulong helper_mret(CPURISCVState *env, > target_ulong cpu_pc_deb) > riscv_cpu_set_virt_enabled(env, prev_virt); > } > > + if (riscv_clic_is_clic_mode(env)) { > + CPUState *cs =3D env_cpu(env); > + target_ulong mpil =3D get_field(env->mcause, MCAUSE_MPIL); > + env->mintstatus =3D set_field(env->mintstatus, MINTSTATUS_MIL, > mpil); > + env->mcause =3D set_field(env->mcause, MCAUSE_MPIE, 0); > + env->mcause =3D set_field(env->mcause, MCAUSE_MPP, PRV_U); > + qemu_mutex_lock_iothread(); > + riscv_clic_get_next_interrupt(env->clic, cs->cpu_index); > + qemu_mutex_unlock_iothread(); > + } > return retpc; > } > > -- > 2.25.1 > > > A little note here. According to spec, for nesting interrupts: To take advantage of hardware preemption in the new CLIC, inline handlers must save and restore xepc and xcause before enabling interrupts. (Section 9.2) However, xstatus.xpp will be set to U-mode when xret instruction is executed. Which will incorrectly switch to U-mode when executing xret instruction second time in first ISR. E.g. ISR 1 --------------------------------------------------------- xret =3D> Current privilege is incorrectly set to U-mode. ISR 2 --------------- xret =3D> xstatus.xpp is set to U-mod= e. Therefore, in our SiFive CLIC hardware implementation. xstatus.xpp is set to the privilege mode when xret instruction is executed (i.e. current privilege mode) under CLIC mode. But this behavior is not documented in CLIC spec explicitly. Regards, Frank Chang --000000000000612c9f05c6fc0211 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
LIU Zhiwei <zhiwei_liu@c-sky.com> =E6=96=BC 2021=E5=B9=B44=E6=9C= =889=E6=97=A5 =E9=80=B1=E4=BA=94 =E4=B8=8B=E5=8D=883:55=E5=AF=AB=E9=81=93= =EF=BC=9A
When a vectored interrupt is selected and serviced, th= e hardware will
automatically clear the corresponding pending bit in edge-triggered mode. This may lead to a lower priviledge interrupt pending forever.

Therefore when interrupts return, pull a pending interrupt to service.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
=C2=A0target/riscv/op_helper.c | 25 +++++++++++++++++++++++++
=C2=A01 file changed, 25 insertions(+)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 1eddcb94de..42563b22ba 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -24,6 +24,10 @@
=C2=A0#include "exec/exec-all.h"
=C2=A0#include "exec/helper-proto.h"

+#if !defined(CONFIG_USER_ONLY)
+#include "hw/intc/riscv_clic.h"
+#endif
+
=C2=A0/* Exceptions processing helpers */
=C2=A0void QEMU_NORETURN riscv_raise_exception(CPURISCVState *env,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0uint32_t exception, uintptr_t pc)
@@ -130,6 +134,17 @@ target_ulong helper_sret(CPURISCVState *env, target_ul= ong cpu_pc_deb)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0mstatus =3D set_field(mstatus, MSTATUS_SP= IE, 1);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0mstatus =3D set_field(mstatus, MSTATUS_SP= P, PRV_U);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0env->mstatus =3D mstatus;
+
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (riscv_clic_is_clic_mode(env)) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 CPUState *cs =3D env_cpu(env); +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 target_ulong spil =3D get_field(= env->scause, SCAUSE_SPIL);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 env->mintstatus =3D set_field= (env->mintstatus, MINTSTATUS_SIL, spil);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 env->scause =3D set_field(env= ->scause, SCAUSE_SPIE, 0);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 env->scause =3D set_field(env= ->scause, SCAUSE_SPP, PRV_U);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 qemu_mutex_lock_iothread();
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 riscv_clic_get_next_interrupt(en= v->clic, cs->cpu_index);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 qemu_mutex_unlock_iothread(); +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
=C2=A0 =C2=A0 =C2=A0}

=C2=A0 =C2=A0 =C2=A0riscv_cpu_set_mode(env, prev_priv);
@@ -172,6 +187,16 @@ target_ulong helper_mret(CPURISCVState *env, target_ul= ong cpu_pc_deb)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0riscv_cpu_set_virt_enabled(env, prev_virt= );
=C2=A0 =C2=A0 =C2=A0}

+=C2=A0 =C2=A0 if (riscv_clic_is_clic_mode(env)) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 CPUState *cs =3D env_cpu(env);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 target_ulong mpil =3D get_field(env->mcause= , MCAUSE_MPIL);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 env->mintstatus =3D set_field(env->mints= tatus, MINTSTATUS_MIL, mpil);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 env->mcause =3D set_field(env->mcause, M= CAUSE_MPIE, 0);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 env->mcause =3D set_field(env->mcause, M= CAUSE_MPP, PRV_U);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 qemu_mutex_lock_iothread();
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 riscv_clic_get_next_interrupt(env->clic, cs= ->cpu_index);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 qemu_mutex_unlock_iothread();
+=C2=A0 =C2=A0 }
=C2=A0 =C2=A0 =C2=A0return retpc;
=C2=A0}

--
2.25.1



A little note here.

According to spec, for nesting interrupts:
=C2=A0 To take = advantage of hardware preemption in the new CLIC,
=C2=A0 inline h= andlers must save and restore xepc and xcause before enabling interrupts.
=C2=A0 (Section 9.2)

However, xstatus= .xpp will be set to U-mode when xret instruction is executed.
Whi= ch will incorrectly switch to U-mode when executing xret instruction second= time in first ISR.
E.g.

ISR 1 ---------= ------------------------------------------------ xret =3D> Current privi= lege is incorrectly set to U-mode.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 ISR 2 --------------- xret =3D> xstatus.xpp is set = to U-mode.

Therefore, in our SiFive CLIC hardware = implementation.
xstatus.xpp is set to the privilege mode when xre= t instruction is executed
(i.e. current privilege mode) under CLI= C mode.
But this behavior is not documented in CLIC spec explicit= ly.

Regards,
Frank Chang
--000000000000612c9f05c6fc0211-- From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1m3CeD-00047d-7g for mharc-qemu-riscv@gnu.org; Tue, 13 Jul 2021 03:16:05 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:56642) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m3CeA-000451-Vl for qemu-riscv@nongnu.org; Tue, 13 Jul 2021 03:16:03 -0400 Received: from mail-pl1-x632.google.com ([2607:f8b0:4864:20::632]:40654) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1m3Ce8-0005d3-5G for qemu-riscv@nongnu.org; Tue, 13 Jul 2021 03:16:02 -0400 Received: by mail-pl1-x632.google.com with SMTP id j3so8701235plx.7 for ; Tue, 13 Jul 2021 00:15:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sifive.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=ZIxC97vo3D/9ttg5bUmdmcZs3xyDx1jpdRBLzqRnrn4=; b=L/qp8DgRV3vXBa7RXmsw8KdAM7mX6XIj2Bdirec//LXfwez+T/sRnL+utnaBW/StI3 wZ8TFgZhZryx0fq2seaiBOivAbOPremRbzkzpY0Gsf3Aml9cRb2zJOpCnCPp5TEtyzTS YF2m7hwB5kCdCwByjnp/o5pKyAp80Z8DT2MT6gPFifkB38I4OsrkseJhD71BVJjrtYG8 svYY4I4zRc/dbu/1oNSce/VcS+Qh6xUFixOXtr1iQyh5pUedSjh8nzGZTG2pDU5hArSf 0w3EqAnVSm18Si6t4HGIflPHovFLEJ0zRds6KfLipyZPmO31XCD/LeYOolhsrFLvehJy Um9A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ZIxC97vo3D/9ttg5bUmdmcZs3xyDx1jpdRBLzqRnrn4=; b=qOvnJE4OhxzOWDy7vqK681zQbWsjbaKH9pT0oMJKtD+6IFwMiEnC8FYJh84zYGs4Ob BMwS3GFclSenVdW1kEJ8KCRROkmoRxxxSgMFbBMdtV9thjQzt9h2G9L0O+sGZr0GyDkj B4aA3TSlYchjosN8px40uRnWedy4iGp3w29dOmF6kFjB7gJnZEJnLLURZjVQ9XlusiRa kQLoh08g/Ktkk+sM2ujzQI56FAnxPjzRqC5BJ1MrU3uXsydiCgrO+N9Yn2dE3C0HDl4M ItPUsDOiVSdgYcP0nX1maT6FpckvAkK/WoVH9fngqrTRS66yXXdcNgYlplNuNjlfvj4t DzhA== X-Gm-Message-State: AOAM532dYyV0esPkZXHA8kXwcNqFj4w7zY8aH/dkdilVyDQpWBvYOwwk wxND0gVCzOCGQNT0fzO4cNKnRg== X-Google-Smtp-Source: ABdhPJykeWLaas46TZ9by8/73KbjyoSaUJYQ3ZYi35yKlGlMNQs04jpMzxjKcwg7gjQDooVrAH/dcg== X-Received: by 2002:a17:902:bd96:b029:129:24e:43b5 with SMTP id q22-20020a170902bd96b0290129024e43b5mr2507112pls.66.1626160558340; Tue, 13 Jul 2021 00:15:58 -0700 (PDT) Received: from mail-pf1-f169.google.com (mail-pf1-f169.google.com. [209.85.210.169]) by smtp.gmail.com with ESMTPSA id d13sm17865411pfn.136.2021.07.13.00.15.57 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 13 Jul 2021 00:15:57 -0700 (PDT) Received: by mail-pf1-f169.google.com with SMTP id q10so18688468pfj.12; Tue, 13 Jul 2021 00:15:57 -0700 (PDT) X-Received: by 2002:aa7:938c:0:b029:32a:1725:a3d7 with SMTP id t12-20020aa7938c0000b029032a1725a3d7mr3073973pfe.64.1626160557533; Tue, 13 Jul 2021 00:15:57 -0700 (PDT) MIME-Version: 1.0 References: <20210409074857.166082-1-zhiwei_liu@c-sky.com> <20210409074857.166082-12-zhiwei_liu@c-sky.com> In-Reply-To: <20210409074857.166082-12-zhiwei_liu@c-sky.com> From: Frank Chang Date: Tue, 13 Jul 2021 15:15:46 +0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [RFC PATCH 11/11] target/riscv: Update interrupt return in CLIC mode To: LIU Zhiwei Cc: "qemu-devel@nongnu.org Developers" , "open list:RISC-V" , Palmer Dabbelt , Alistair Francis , wxy194768@alibaba-inc.com Content-Type: multipart/alternative; boundary="000000000000612c9f05c6fc0211" Received-SPF: pass client-ip=2607:f8b0:4864:20::632; envelope-from=frank.chang@sifive.com; helo=mail-pl1-x632.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, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-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, 13 Jul 2021 07:16:03 -0000 --000000000000612c9f05c6fc0211 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable LIU Zhiwei =E6=96=BC 2021=E5=B9=B44=E6=9C=889=E6=97= =A5 =E9=80=B1=E4=BA=94 =E4=B8=8B=E5=8D=883:55=E5=AF=AB=E9=81=93=EF=BC=9A > When a vectored interrupt is selected and serviced, the hardware will > automatically clear the corresponding pending bit in edge-triggered mode. > This may lead to a lower priviledge interrupt pending forever. > > Therefore when interrupts return, pull a pending interrupt to service. > > Signed-off-by: LIU Zhiwei > --- > target/riscv/op_helper.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c > index 1eddcb94de..42563b22ba 100644 > --- a/target/riscv/op_helper.c > +++ b/target/riscv/op_helper.c > @@ -24,6 +24,10 @@ > #include "exec/exec-all.h" > #include "exec/helper-proto.h" > > +#if !defined(CONFIG_USER_ONLY) > +#include "hw/intc/riscv_clic.h" > +#endif > + > /* Exceptions processing helpers */ > void QEMU_NORETURN riscv_raise_exception(CPURISCVState *env, > uint32_t exception, uintptr_t > pc) > @@ -130,6 +134,17 @@ target_ulong helper_sret(CPURISCVState *env, > target_ulong cpu_pc_deb) > mstatus =3D set_field(mstatus, MSTATUS_SPIE, 1); > mstatus =3D set_field(mstatus, MSTATUS_SPP, PRV_U); > env->mstatus =3D mstatus; > + > + if (riscv_clic_is_clic_mode(env)) { > + CPUState *cs =3D env_cpu(env); > + target_ulong spil =3D get_field(env->scause, SCAUSE_SPIL); > + env->mintstatus =3D set_field(env->mintstatus, MINTSTATUS_SI= L, > spil); > + env->scause =3D set_field(env->scause, SCAUSE_SPIE, 0); > + env->scause =3D set_field(env->scause, SCAUSE_SPP, PRV_U); > + qemu_mutex_lock_iothread(); > + riscv_clic_get_next_interrupt(env->clic, cs->cpu_index); > + qemu_mutex_unlock_iothread(); > + } > } > > riscv_cpu_set_mode(env, prev_priv); > @@ -172,6 +187,16 @@ target_ulong helper_mret(CPURISCVState *env, > target_ulong cpu_pc_deb) > riscv_cpu_set_virt_enabled(env, prev_virt); > } > > + if (riscv_clic_is_clic_mode(env)) { > + CPUState *cs =3D env_cpu(env); > + target_ulong mpil =3D get_field(env->mcause, MCAUSE_MPIL); > + env->mintstatus =3D set_field(env->mintstatus, MINTSTATUS_MIL, > mpil); > + env->mcause =3D set_field(env->mcause, MCAUSE_MPIE, 0); > + env->mcause =3D set_field(env->mcause, MCAUSE_MPP, PRV_U); > + qemu_mutex_lock_iothread(); > + riscv_clic_get_next_interrupt(env->clic, cs->cpu_index); > + qemu_mutex_unlock_iothread(); > + } > return retpc; > } > > -- > 2.25.1 > > > A little note here. According to spec, for nesting interrupts: To take advantage of hardware preemption in the new CLIC, inline handlers must save and restore xepc and xcause before enabling interrupts. (Section 9.2) However, xstatus.xpp will be set to U-mode when xret instruction is executed. Which will incorrectly switch to U-mode when executing xret instruction second time in first ISR. E.g. ISR 1 --------------------------------------------------------- xret =3D> Current privilege is incorrectly set to U-mode. ISR 2 --------------- xret =3D> xstatus.xpp is set to U-mod= e. Therefore, in our SiFive CLIC hardware implementation. xstatus.xpp is set to the privilege mode when xret instruction is executed (i.e. current privilege mode) under CLIC mode. But this behavior is not documented in CLIC spec explicitly. Regards, Frank Chang --000000000000612c9f05c6fc0211 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
LIU Zhiwei <zhiwei_liu@c-sky.com> =E6=96=BC 2021=E5=B9=B44=E6=9C= =889=E6=97=A5 =E9=80=B1=E4=BA=94 =E4=B8=8B=E5=8D=883:55=E5=AF=AB=E9=81=93= =EF=BC=9A
When a vectored interrupt is selected and serviced, th= e hardware will
automatically clear the corresponding pending bit in edge-triggered mode. This may lead to a lower priviledge interrupt pending forever.

Therefore when interrupts return, pull a pending interrupt to service.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
=C2=A0target/riscv/op_helper.c | 25 +++++++++++++++++++++++++
=C2=A01 file changed, 25 insertions(+)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 1eddcb94de..42563b22ba 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -24,6 +24,10 @@
=C2=A0#include "exec/exec-all.h"
=C2=A0#include "exec/helper-proto.h"

+#if !defined(CONFIG_USER_ONLY)
+#include "hw/intc/riscv_clic.h"
+#endif
+
=C2=A0/* Exceptions processing helpers */
=C2=A0void QEMU_NORETURN riscv_raise_exception(CPURISCVState *env,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0uint32_t exception, uintptr_t pc)
@@ -130,6 +134,17 @@ target_ulong helper_sret(CPURISCVState *env, target_ul= ong cpu_pc_deb)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0mstatus =3D set_field(mstatus, MSTATUS_SP= IE, 1);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0mstatus =3D set_field(mstatus, MSTATUS_SP= P, PRV_U);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0env->mstatus =3D mstatus;
+
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (riscv_clic_is_clic_mode(env)) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 CPUState *cs =3D env_cpu(env); +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 target_ulong spil =3D get_field(= env->scause, SCAUSE_SPIL);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 env->mintstatus =3D set_field= (env->mintstatus, MINTSTATUS_SIL, spil);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 env->scause =3D set_field(env= ->scause, SCAUSE_SPIE, 0);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 env->scause =3D set_field(env= ->scause, SCAUSE_SPP, PRV_U);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 qemu_mutex_lock_iothread();
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 riscv_clic_get_next_interrupt(en= v->clic, cs->cpu_index);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 qemu_mutex_unlock_iothread(); +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
=C2=A0 =C2=A0 =C2=A0}

=C2=A0 =C2=A0 =C2=A0riscv_cpu_set_mode(env, prev_priv);
@@ -172,6 +187,16 @@ target_ulong helper_mret(CPURISCVState *env, target_ul= ong cpu_pc_deb)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0riscv_cpu_set_virt_enabled(env, prev_virt= );
=C2=A0 =C2=A0 =C2=A0}

+=C2=A0 =C2=A0 if (riscv_clic_is_clic_mode(env)) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 CPUState *cs =3D env_cpu(env);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 target_ulong mpil =3D get_field(env->mcause= , MCAUSE_MPIL);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 env->mintstatus =3D set_field(env->mints= tatus, MINTSTATUS_MIL, mpil);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 env->mcause =3D set_field(env->mcause, M= CAUSE_MPIE, 0);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 env->mcause =3D set_field(env->mcause, M= CAUSE_MPP, PRV_U);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 qemu_mutex_lock_iothread();
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 riscv_clic_get_next_interrupt(env->clic, cs= ->cpu_index);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 qemu_mutex_unlock_iothread();
+=C2=A0 =C2=A0 }
=C2=A0 =C2=A0 =C2=A0return retpc;
=C2=A0}

--
2.25.1



A little note here.

According to spec, for nesting interrupts:
=C2=A0 To take = advantage of hardware preemption in the new CLIC,
=C2=A0 inline h= andlers must save and restore xepc and xcause before enabling interrupts.
=C2=A0 (Section 9.2)

However, xstatus= .xpp will be set to U-mode when xret instruction is executed.
Whi= ch will incorrectly switch to U-mode when executing xret instruction second= time in first ISR.
E.g.

ISR 1 ---------= ------------------------------------------------ xret =3D> Current privi= lege is incorrectly set to U-mode.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 ISR 2 --------------- xret =3D> xstatus.xpp is set = to U-mode.

Therefore, in our SiFive CLIC hardware = implementation.
xstatus.xpp is set to the privilege mode when xre= t instruction is executed
(i.e. current privilege mode) under CLI= C mode.
But this behavior is not documented in CLIC spec explicit= ly.

Regards,
Frank Chang
--000000000000612c9f05c6fc0211--