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 81E75EB64DA for ; Mon, 19 Jun 2023 09:55:02 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qBBas-0000Ca-Qg; Mon, 19 Jun 2023 05:54:42 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qBBar-0000CD-Q4 for qemu-devel@nongnu.org; Mon, 19 Jun 2023 05:54:41 -0400 Received: from mail-wm1-x336.google.com ([2a00:1450:4864:20::336]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qBBap-0001oj-R4 for qemu-devel@nongnu.org; Mon, 19 Jun 2023 05:54:41 -0400 Received: by mail-wm1-x336.google.com with SMTP id 5b1f17b1804b1-3f907f311ccso21946155e9.1 for ; Mon, 19 Jun 2023 02:54:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ventanamicro.com; s=google; t=1687168478; x=1689760478; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=k3hsCDH9FCjdfg1dQGr6jtaDvMP9TxbCpSrdm1wg3ME=; b=jR+UrdglSy7WLIVE/2RxlmQ6P3DAQvp532nXbt3TJnCEQtYt3fLA4fROoG43qEeRcR MpcvWbf9a8j/Gyw9S2Jh8SesBhjGvu8mO5Xe4PN1zj8vBBVFUFozowMmgO4Fbvg1Ckii +vBr0eLlmAfEe3GL0CHCMVzQZ1bvRyESX7iFKbDGnsXKyyUnXTm/5aCeYMTLigUSQRHm LOuakHyWYasQ1o88k+e+rjjsh6Ylq/jk2cwQGgmgyLksuPVGOg0syuUXOmjB8cRyzhn4 S4trWEzwfP9WVz4NFJNyNefMnRbJFEDYHI8+U0vdFNOnow/MRO60a4qsdHxoEu52+FvO 2BYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687168478; x=1689760478; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=k3hsCDH9FCjdfg1dQGr6jtaDvMP9TxbCpSrdm1wg3ME=; b=TCRZkBAkJzYC6FAswpRZwCkq40ljw7wMwwh2m5VPlVjRSSaktmuOCwlRjjd7xW/0sH crbvpvnF9xGvUBsBGaxrI0x5wuaCiBPrvq/uKw83on0+LlwoStjEmPkZ4LXQqwIUomCG UuqY/XbDACWRi4RMLhamlaaFKJ9eBFJjBCD3k2ZrbXwbpDFKIjMayp7W/KuyaXS7RG3y R9DkKH8q4uzOL4u+To3OrU8sH2GUDCA2Dt9+EKHE1GeLN45e/HSOEhk4yfQsLW/XL1PH o67vAYGQVWy+Nc/x9xhAGJOJwSXSEIL3cpbJj09i+ZquWWV5C4T5LUqaKybZw5xxE9X5 n+YA== X-Gm-Message-State: AC+VfDxrDuwjyvSOCbwj/KhCopaBxc/4fLziu/rZ+VGBHBWmstVUXkRV 2DtJ/dv4U9D/SUm+MyWshnpoLA== X-Google-Smtp-Source: ACHHUZ6M7ns/ShAQtP4sVojT2o8gwcxIH81oqkTmH03yfNHxZl3d32JyrSa3fy0giTKxwOSzLP6kqA== X-Received: by 2002:a1c:f70d:0:b0:3f7:3685:1e10 with SMTP id v13-20020a1cf70d000000b003f736851e10mr7505881wmh.40.1687168478360; Mon, 19 Jun 2023 02:54:38 -0700 (PDT) Received: from localhost (cst2-173-16.cust.vodafone.cz. [31.30.173.16]) by smtp.gmail.com with ESMTPSA id o1-20020a05600c378100b003f50d6ee334sm10087750wmr.47.2023.06.19.02.54.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 19 Jun 2023 02:54:37 -0700 (PDT) Date: Mon, 19 Jun 2023 11:54:36 +0200 From: Andrew Jones To: Daniel Henrique Barboza Cc: qemu-devel@nongnu.org, qemu-riscv@nongnu.org, alistair.francis@wdc.com, bmeng@tinylab.org, liweiwei@iscas.ac.cn, zhiwei_liu@linux.alibaba.com, palmer@rivosinc.com Subject: Re: [PATCH v2 15/18] target/riscv: make riscv_isa_string_ext() KVM compatible Message-ID: <20230619-30e78979c13df598ca2e4493@orel> References: <20230613205857.495165-1-dbarboza@ventanamicro.com> <20230613205857.495165-16-dbarboza@ventanamicro.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230613205857.495165-16-dbarboza@ventanamicro.com> Received-SPF: pass client-ip=2a00:1450:4864:20::336; envelope-from=ajones@ventanamicro.com; helo=mail-wm1-x336.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=unavailable 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-bounces+qemu-devel=archiver.kernel.org@nongnu.org On Tue, Jun 13, 2023 at 05:58:54PM -0300, Daniel Henrique Barboza wrote: > The isa_edata_arr[] is used by riscv_isa_string_ext() to create the > riscv,isa DT attribute. isa_edata_arr[] is kept in sync with the TCG > property vector riscv_cpu_extensions[], i.e. all TCG properties from > this vector that has a riscv,isa representation are included in > isa_edata_arr[]. > > KVM doesn't implement all TCG properties, but allow them to be created > anyway to not force an API change between TCG and KVM guests. Some of > these TCG-only extensions are defaulted to 'true', and users are also > allowed to enable them. KVM doesn't care, but riscv_isa_string_ext() > does. The result is that these TCG-only enabled extensions will appear > in the riscv,isa DT string under KVM. > > To avoid code repetition and re-use riscv_isa_string_ext() for KVM > guests we'll make a couple of tweaks: > > - set env->priv_ver to 'LATEST' for the KVM 'host' CPU. This is needed > because riscv_isa_string_ext() makes a priv check for each extension > before including them in the ISA string. KVM doesn't care about > env->priv_ver, since it's part of the TCG-only CPU validation, so this > change is benign for KVM; > > - add a new 'kvm_available' flag in isa_ext_data struct. This flag is > set via a new ISA_EXT_DATA_ENTRY_KVM macro to report that, for a given > extension, KVM also supports it. riscv_isa_string_ext() then can check > if a given extension is known by KVM and skip it if it's not. > > This will allow us to re-use riscv_isa_string_ext() for KVM guests. > > Signed-off-by: Daniel Henrique Barboza > --- > target/riscv/cpu.c | 28 ++++++++++++++++++++-------- > 1 file changed, 20 insertions(+), 8 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index a4f3ed0c17..a773c09645 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -44,11 +44,15 @@ struct isa_ext_data { > const char *name; > int min_version; > int ext_enable_offset; > + bool kvm_available; > }; > > #define ISA_EXT_DATA_ENTRY(_name, _min_ver, _prop) \ > {#_name, _min_ver, offsetof(struct RISCVCPUConfig, _prop)} > > +#define ISA_EXT_DATA_ENTRY_KVM(_name, _min_ver, _prop) \ > + {#_name, _min_ver, offsetof(struct RISCVCPUConfig, _prop), true} > + > /* > * Here are the ordering rules of extension naming defined by RISC-V > * specification : > @@ -68,14 +72,17 @@ struct isa_ext_data { > * > * Single letter extensions are checked in riscv_cpu_validate_misa_priv() > * instead. > + * > + * ISA_EXT_DATA_ENTRY_KVM() is used to indicate that the extension is > + * also known by the KVM driver. If unsure, use ISA_EXT_DATA_ENTRY(). > */ > static const struct isa_ext_data isa_edata_arr[] = { > - ISA_EXT_DATA_ENTRY(zicbom, PRIV_VERSION_1_12_0, ext_icbom), > - ISA_EXT_DATA_ENTRY(zicboz, PRIV_VERSION_1_12_0, ext_icboz), > + ISA_EXT_DATA_ENTRY_KVM(zicbom, PRIV_VERSION_1_12_0, ext_icbom), > + ISA_EXT_DATA_ENTRY_KVM(zicboz, PRIV_VERSION_1_12_0, ext_icboz), > ISA_EXT_DATA_ENTRY(zicond, PRIV_VERSION_1_12_0, ext_zicond), > ISA_EXT_DATA_ENTRY(zicsr, PRIV_VERSION_1_10_0, ext_icsr), > ISA_EXT_DATA_ENTRY(zifencei, PRIV_VERSION_1_10_0, ext_ifencei), > - ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause), > + ISA_EXT_DATA_ENTRY_KVM(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause), > ISA_EXT_DATA_ENTRY(zawrs, PRIV_VERSION_1_12_0, ext_zawrs), > ISA_EXT_DATA_ENTRY(zfh, PRIV_VERSION_1_11_0, ext_zfh), > ISA_EXT_DATA_ENTRY(zfhmin, PRIV_VERSION_1_11_0, ext_zfhmin), > @@ -89,7 +96,7 @@ static const struct isa_ext_data isa_edata_arr[] = { > ISA_EXT_DATA_ENTRY(zcmp, PRIV_VERSION_1_12_0, ext_zcmp), > ISA_EXT_DATA_ENTRY(zcmt, PRIV_VERSION_1_12_0, ext_zcmt), > ISA_EXT_DATA_ENTRY(zba, PRIV_VERSION_1_12_0, ext_zba), > - ISA_EXT_DATA_ENTRY(zbb, PRIV_VERSION_1_12_0, ext_zbb), > + ISA_EXT_DATA_ENTRY_KVM(zbb, PRIV_VERSION_1_12_0, ext_zbb), > ISA_EXT_DATA_ENTRY(zbc, PRIV_VERSION_1_12_0, ext_zbc), > ISA_EXT_DATA_ENTRY(zbkb, PRIV_VERSION_1_12_0, ext_zbkb), > ISA_EXT_DATA_ENTRY(zbkc, PRIV_VERSION_1_12_0, ext_zbkc), > @@ -114,13 +121,13 @@ static const struct isa_ext_data isa_edata_arr[] = { > ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin), > ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia), > ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen), > - ISA_EXT_DATA_ENTRY(ssaia, PRIV_VERSION_1_12_0, ext_ssaia), > + ISA_EXT_DATA_ENTRY_KVM(ssaia, PRIV_VERSION_1_12_0, ext_ssaia), > ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf), > - ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc), > + ISA_EXT_DATA_ENTRY_KVM(sstc, PRIV_VERSION_1_12_0, ext_sstc), > ISA_EXT_DATA_ENTRY(svadu, PRIV_VERSION_1_12_0, ext_svadu), > - ISA_EXT_DATA_ENTRY(svinval, PRIV_VERSION_1_12_0, ext_svinval), > + ISA_EXT_DATA_ENTRY_KVM(svinval, PRIV_VERSION_1_12_0, ext_svinval), > ISA_EXT_DATA_ENTRY(svnapot, PRIV_VERSION_1_12_0, ext_svnapot), > - ISA_EXT_DATA_ENTRY(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt), > + ISA_EXT_DATA_ENTRY_KVM(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt), This approach looks a bit difficult to maintain (it's hard to even see the _KVM macro uses). The approach will be even more difficult if we add more accelerators. I feel like we need an extension class where objects of that class can be passed to functions like riscv_isa_string_ext(). And then we also need tcg-extension and kvm-extension classes which inherit from the extension class. We can then keep the lists of extensions separate, as you originally proposed, as each list will be of its own type. Thanks, drew