From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id DDCFD3858D33 for ; Mon, 29 Apr 2024 10:01:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DDCFD3858D33 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org DDCFD3858D33 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1714384906; cv=none; b=mH1JNzqX934NxriUdgp8l+MLYT4TUqc15D+bm0exodsepqlOBIw/qrASji1lqJyFJhJB1hiI7+MfvoR+LquhmwPFAkaYIjMPHW64rdjSDvAp7WET3aw5JKAt1oextc2zZTza2Ec4gvnZ0l6m2HIUt8Gh3WkAN/dnRYpdghasRaY= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1714384906; c=relaxed/simple; bh=965VBcFUy+4yyZuHzR0NqfXEvWltMl4mEry0SSWZ/aw=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=Isk9Pw0yD/tyeP+pxyinMck61vRkZ0yTIn16u8VDY5i1HrzwB64/ZtBz986hh/2e+7d5CXps/RbPAzVqBVZaBZN86po4gwQ/6g9IqhqpbvcQeqNJS7qR2O9EtA6tOrMCrnik4Gga5Zv1m6r+WzQkkIprffHpToibE3qh3yJFKmA= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1714384903; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=+2RuV5d/A/2PrtrsUyHHk82kXoho5Ks9dIMk1cnqb1g=; b=QVWiTPgruMddDDnLFcs28+zMb70ce+3nra9Bxdlagu1CXx+fjVAnhNOv+PF2IqJywLnkU+ Iu3PsKYUMB0sVK34QKvJ14xzjIEyCkbhgdzSHwK20fGvgR4EKB06pfA3CS1fUKjgF1cvHU NflQP8mS/qdVIBDjpfO5LYzU4Qzfb00= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-425-oqxuFqTiNW6_6kaB1YllhQ-1; Mon, 29 Apr 2024 06:01:39 -0400 X-MC-Unique: oqxuFqTiNW6_6kaB1YllhQ-1 Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-34d1089a3feso446206f8f.2 for ; Mon, 29 Apr 2024 03:01:39 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714384897; x=1714989697; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=+2RuV5d/A/2PrtrsUyHHk82kXoho5Ks9dIMk1cnqb1g=; b=QxyJL5YhR/aeWho4b4Ct+U+tQBuLHJnQVhHa2TD/aGp/mgw3DmO/Hb8Zkv2rZl2L4v QTL+23/wUuGzahliV6zYN1RvW2jnXBVeDSe8xo9wU5GIbQ8o+SyQwHhgRqdvKRsMGoYw apo2qPIYZqSx7Im5bOB5f1BZsFW4n3lQF3gBs5frdtTrB0KrJtUAcM/S6q2124S1FBj1 C4VAJnQPd8R0ezTEohWmsS5PyrZgZkE7buSZTsR8Dx2WRXi+Dfw3RDC67QykXiBiCCGZ kKo019yp4UmTE3VamnasakuGIuJ8a8GStboOJfsCHoYLcIlGKsnNatDKQ5SQ9rYZ0bAy K2Qw== X-Forwarded-Encrypted: i=1; AJvYcCXBClpXU0Uf0aGYJQCo+Y15646OjnMh9bxxlxn9dvIZ+A1cWfSmE9+o01q+kXSbPAvEQkKM19yKKMmzvCUgQLybQZKfHO3TCZy6mg== X-Gm-Message-State: AOJu0YxETeK+lGNuEd2TfWV+x3r5J2ei2Cczb0py2WHy9e9hGWT5OYo5 U/yyxEMDBBzbs7FcmNlshs7iLbbFsleYPhr1cwUweY0KufcYfxVknUTYGwzlIbDNFRZiH8/5EHt cqXXOru8w7HfvHzM+KyaCPN6wHEu0UN1ONbLSkRodoHpkT8rHYXjq03IufQgQv1fNKRI= X-Received: by 2002:a05:6000:e4c:b0:34b:66be:3199 with SMTP id dy12-20020a0560000e4c00b0034b66be3199mr7621102wrb.62.1714384897138; Mon, 29 Apr 2024 03:01:37 -0700 (PDT) X-Google-Smtp-Source: AGHT+IH5hEbXxTaYr1hNGU6xI58qWIowazmBjPXghXXGbQ8WNx9BUsfR8lmrDk9fGkE2LBh2ECDi7g== X-Received: by 2002:a05:6000:e4c:b0:34b:66be:3199 with SMTP id dy12-20020a0560000e4c00b0034b66be3199mr7621072wrb.62.1714384896541; Mon, 29 Apr 2024 03:01:36 -0700 (PDT) Received: from localhost (185.223.159.143.dyn.plus.net. [143.159.223.185]) by smtp.gmail.com with ESMTPSA id j1-20020adfb301000000b0034d29b87fcasm1397665wrd.87.2024.04.29.03.01.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 29 Apr 2024 03:01:36 -0700 (PDT) From: Andrew Burgess To: Bernd Edlinger , "gdb-patches@sourceware.org" Subject: Re: [PATCH] sim: riscv: Fix Zicsr and fence instructions In-Reply-To: References: Date: Mon, 29 Apr 2024 11:01:35 +0100 Message-ID: <87il00pjwg.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-12.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Bernd Edlinger writes: > The Zicsr instructions were totally broken, and > some instructions like fence.tso were missing. > > Since the test coverage is not very good, add some > basic tests for fence and csrrw instructions. > --- > sim/riscv/sim-main.c | 74 ++++++++++++++++++++++++++++++++----- > sim/testsuite/riscv/fence.s | 17 +++++++++ > sim/testsuite/riscv/zicsr.s | 24 ++++++++++++ > 3 files changed, 106 insertions(+), 9 deletions(-) > create mode 100644 sim/testsuite/riscv/fence.s > create mode 100644 sim/testsuite/riscv/zicsr.s > > diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c > index 1815d7f2a6c..69007d3108e 100644 > --- a/sim/riscv/sim-main.c > +++ b/sim/riscv/sim-main.c > @@ -535,37 +535,57 @@ execute_i (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op) > break; > > case MATCH_CSRRC: > - TRACE_INSN (cpu, "csrrc"); > + TRACE_INSN (cpu, "csrrc %s, %#x, %s;", > + rd_name, csr, rs1_name); > switch (csr) > { > #define DECLARE_CSR(name, num, ...) \ > case num: \ > - store_rd (cpu, rd, \ > - fetch_csr (cpu, #name, num, &riscv_cpu->csr.name)); \ > + tmp = fetch_csr (cpu, #name, num, &riscv_cpu->csr.name); \ > store_csr (cpu, #name, num, &riscv_cpu->csr.name, \ > - riscv_cpu->csr.name & !riscv_cpu->regs[rs1]); \ > + riscv_cpu->csr.name & ~riscv_cpu->regs[rs1]); \ > + store_rd (cpu, rd, tmp); \ I know that store_csr doesn't support many CSRs, and doesn't do any checks for things like writing to read-only CSRs, but .... ... I think it might be worth adding a check here for rs1 == x0. The docs say: For both CSRRS and CSRRC, if rs1=x0, then the instruction will not write to the CSR at all, and so shall not cause any of the side effects that might otherwise occur on a CSR write, such as raising illegal instruction exceptions on accesses to read-only CSRs. Adding these checks now might mean things "just work" if we add support for more CSRs at a later date. > break; > #include "opcode/riscv-opc.h" > #undef DECLARE_CSR > } > break; > case MATCH_CSRRS: > - TRACE_INSN (cpu, "csrrs"); > + TRACE_INSN (cpu, "csrrs %s, %#x, %s;", > + rd_name, csr, rs1_name); > switch (csr) > { > #define DECLARE_CSR(name, num, ...) \ > case num: \ > - store_rd (cpu, rd, \ > - fetch_csr (cpu, #name, num, &riscv_cpu->csr.name)); \ > + tmp = fetch_csr (cpu, #name, num, &riscv_cpu->csr.name); \ > store_csr (cpu, #name, num, &riscv_cpu->csr.name, \ > riscv_cpu->csr.name | riscv_cpu->regs[rs1]); \ > + store_rd (cpu, rd, tmp); \ > break; > #include "opcode/riscv-opc.h" > #undef DECLARE_CSR > } > break; > case MATCH_CSRRW: > - TRACE_INSN (cpu, "csrrw"); > + TRACE_INSN (cpu, "csrrw %s, %#x, %s;", > + rd_name, csr, rs1_name); > + switch (csr) > + { > +#define DECLARE_CSR(name, num, ...) \ > + case num: \ > + tmp = fetch_csr (cpu, #name, num, &riscv_cpu->csr.name); \ > + store_csr (cpu, #name, num, &riscv_cpu->csr.name, \ > + riscv_cpu->regs[rs1]); \ > + store_rd (cpu, rd, tmp); \ > + break; > +#include "opcode/riscv-opc.h" > +#undef DECLARE_CSR > + } > + break; > + > + case MATCH_CSRRCI: > + TRACE_INSN (cpu, "csrrci %s, %#x, %#x;", > + rd_name, csr, rs1); > switch (csr) > { > #define DECLARE_CSR(name, num, ...) \ > @@ -573,7 +593,38 @@ execute_i (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op) > store_rd (cpu, rd, \ > fetch_csr (cpu, #name, num, &riscv_cpu->csr.name)); \ > store_csr (cpu, #name, num, &riscv_cpu->csr.name, \ > - riscv_cpu->regs[rs1]); \ > + riscv_cpu->csr.name & ~rs1); \ I think there should be a similar check around the store_csr call for the case where rs1 == 0: For CSRRSI and CSRRCI, if the uimm[4:0] field is zero, then these instructions will not write to the CSR, and shall not cause any of the side effects that might otherwise occur on a CSR write. Thanks, Andrew > + break; > +#include "opcode/riscv-opc.h" > +#undef DECLARE_CSR > + } > + break; > + case MATCH_CSRRSI: > + TRACE_INSN (cpu, "csrrsi %s, %#x, %#x;", > + rd_name, csr, rs1); > + switch (csr) > + { > +#define DECLARE_CSR(name, num, ...) \ > + case num: \ > + store_rd (cpu, rd, \ > + fetch_csr (cpu, #name, num, &riscv_cpu->csr.name)); \ > + store_csr (cpu, #name, num, &riscv_cpu->csr.name, \ > + riscv_cpu->csr.name | rs1); \ > + break; > +#include "opcode/riscv-opc.h" > +#undef DECLARE_CSR > + } > + break; > + case MATCH_CSRRWI: > + TRACE_INSN (cpu, "csrrwi %s, %#x, %#x;", > + rd_name, csr, rs1); > + switch (csr) > + { > +#define DECLARE_CSR(name, num, ...) \ > + case num: \ > + store_rd (cpu, rd, \ > + fetch_csr (cpu, #name, num, &riscv_cpu->csr.name)); \ > + store_csr (cpu, #name, num, &riscv_cpu->csr.name, rs1); \ > break; > #include "opcode/riscv-opc.h" > #undef DECLARE_CSR > @@ -622,6 +673,9 @@ execute_i (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op) > case MATCH_FENCE_I: > TRACE_INSN (cpu, "fence.i;"); > break; > + case MATCH_FENCE_TSO: > + TRACE_INSN (cpu, "fence.tso;"); > + break; > case MATCH_EBREAK: > TRACE_INSN (cpu, "ebreak;"); > sim_engine_halt (sd, cpu, NULL, riscv_cpu->pc, sim_stopped, SIM_SIGTRAP); > @@ -1349,6 +1403,8 @@ execute_one (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op) > SIM_SIGILL); > } > case INSN_CLASS_I: > + case INSN_CLASS_ZICSR: > + case INSN_CLASS_ZIFENCEI: > return execute_i (cpu, iw, op); > case INSN_CLASS_M: > case INSN_CLASS_ZMMUL: > diff --git a/sim/testsuite/riscv/fence.s b/sim/testsuite/riscv/fence.s > new file mode 100644 > index 00000000000..25200891161 > --- /dev/null > +++ b/sim/testsuite/riscv/fence.s > @@ -0,0 +1,17 @@ > +# Check that various fence instructions run without any faults. > +# mach: riscv32 riscv64 > + > +.include "testutils.inc" > + > + start > + > + fence > + fence rw,rw > + fence rw,w > + fence r,r > + fence w,w > + fence r,rw > + fence.i > + fence.tso > + > + pass > diff --git a/sim/testsuite/riscv/zicsr.s b/sim/testsuite/riscv/zicsr.s > new file mode 100644 > index 00000000000..7f1bd740230 > --- /dev/null > +++ b/sim/testsuite/riscv/zicsr.s > @@ -0,0 +1,24 @@ > +# Check that the Zicsr instructions run without any faults. > +# mach: riscv32 riscv64 > + > +.include "testutils.inc" > + > + start > + > + csrrs a0,frm,x0 > + csrrw a1,frm,a0 > + bne a1,a0,bad > + csrrc a2,frm,a1 > + bne a2,x0,bad > + csrrsi a0,frm,1 > + bne a0,x0,bad > + csrrci a0,frm,1 > + li a1,1 > + bne a0,a1,bad > + csrrwi a0,frm,1 > + bne a0,x0,bad > + > + pass > + > +bad: > + fail > -- > 2.39.2