From: Andrew Jones <ajones@ventanamicro.com>
To: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
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
Date: Mon, 19 Jun 2023 11:54:36 +0200 [thread overview]
Message-ID: <20230619-30e78979c13df598ca2e4493@orel> (raw)
In-Reply-To: <20230613205857.495165-16-dbarboza@ventanamicro.com>
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 <dbarboza@ventanamicro.com>
> ---
> 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
next prev parent reply other threads:[~2023-06-19 9:55 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-13 20:58 [PATCH v2 00/18] target/riscv, KVM: fixes and enhancements Daniel Henrique Barboza
2023-06-13 20:58 ` [PATCH v2 01/18] target/riscv: skip features setup for KVM CPUs Daniel Henrique Barboza
2023-06-13 20:58 ` [PATCH v2 02/18] hw/riscv/virt.c: skip 'mmu-type' FDT if satp mode not set Daniel Henrique Barboza
2023-06-13 20:58 ` [PATCH v2 03/18] target/riscv/cpu.c: restrict 'mvendorid' value Daniel Henrique Barboza
2023-06-22 1:07 ` Alistair Francis
2023-06-13 20:58 ` [PATCH v2 04/18] target/riscv/cpu.c: restrict 'mimpid' value Daniel Henrique Barboza
2023-06-22 1:08 ` Alistair Francis
2023-06-13 20:58 ` [PATCH v2 05/18] target/riscv/cpu.c: restrict 'marchid' value Daniel Henrique Barboza
2023-06-22 1:10 ` Alistair Francis
2023-06-13 20:58 ` [PATCH v2 06/18] target/riscv: use KVM scratch CPUs to init KVM properties Daniel Henrique Barboza
2023-06-13 20:58 ` [PATCH v2 07/18] target/riscv: read marchid/mimpid in kvm_riscv_init_machine_ids() Daniel Henrique Barboza
2023-06-13 20:58 ` [PATCH v2 08/18] target/riscv: handle mvendorid/marchid/mimpid for KVM CPUs Daniel Henrique Barboza
2023-06-22 1:16 ` Alistair Francis
2023-06-13 20:58 ` [PATCH v2 09/18] linux-headers: Update to v6.4-rc1 Daniel Henrique Barboza
2023-06-22 1:18 ` Alistair Francis
2023-06-13 20:58 ` [PATCH v2 10/18] target/riscv/kvm.c: init 'misa_ext_mask' with scratch CPU Daniel Henrique Barboza
2023-06-22 1:34 ` Alistair Francis
2023-06-13 20:58 ` [PATCH v2 11/18] target/riscv/cpu: add misa_ext_infos[] Daniel Henrique Barboza
2023-06-19 9:05 ` Andrew Jones
2023-06-13 20:58 ` [PATCH v2 12/18] target/riscv: add KVM specific MISA properties Daniel Henrique Barboza
2023-06-19 9:17 ` Andrew Jones
2023-06-13 20:58 ` [PATCH v2 13/18] target/riscv/kvm.c: update KVM MISA bits Daniel Henrique Barboza
2023-06-19 9:30 ` Andrew Jones
2023-06-13 20:58 ` [PATCH v2 14/18] target/riscv/kvm.c: add multi-letter extension KVM properties Daniel Henrique Barboza
2023-06-19 9:44 ` Andrew Jones
2023-06-13 20:58 ` [PATCH v2 15/18] target/riscv: make riscv_isa_string_ext() KVM compatible Daniel Henrique Barboza
2023-06-19 9:54 ` Andrew Jones [this message]
2023-06-20 22:05 ` Daniel Henrique Barboza
2023-06-21 8:20 ` Andrew Jones
2023-06-21 9:13 ` Andrew Jones
2023-06-21 9:43 ` Daniel Henrique Barboza
2023-06-13 20:58 ` [PATCH v2 16/18] target/riscv: update multi-letter extension KVM properties Daniel Henrique Barboza
2023-06-13 20:58 ` [PATCH v2 17/18] target/riscv/kvm.c: add kvmconfig_get_cfg_addr() helper Daniel Henrique Barboza
2023-06-19 9:55 ` Andrew Jones
2023-06-13 20:58 ` [PATCH v2 18/18] target/riscv/kvm.c: read/write (cbom|cboz)_blocksize in KVM Daniel Henrique Barboza
2023-06-19 12:33 ` Andrew Jones
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230619-30e78979c13df598ca2e4493@orel \
--to=ajones@ventanamicro.com \
--cc=alistair.francis@wdc.com \
--cc=bmeng@tinylab.org \
--cc=dbarboza@ventanamicro.com \
--cc=liweiwei@iscas.ac.cn \
--cc=palmer@rivosinc.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-riscv@nongnu.org \
--cc=zhiwei_liu@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).