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 12/18] target/riscv: add KVM specific MISA properties
Date: Mon, 19 Jun 2023 11:17:45 +0200 [thread overview]
Message-ID: <20230619-8ea679b59cf7df0b0dbd213b@orel> (raw)
In-Reply-To: <20230613205857.495165-13-dbarboza@ventanamicro.com>
On Tue, Jun 13, 2023 at 05:58:51PM -0300, Daniel Henrique Barboza wrote:
> Using all TCG user properties in KVM is tricky. First because KVM
> supports only a small subset of what TCG provides, so most of the
> cpu->cfg flags do nothing for KVM.
>
> Second, and more important, we don't have a way of telling if any given
> value is an user input or not. For TCG this has a small impact since we
> just validating everything and error out if needed. But for KVM it would
> be good to know if a given value was set by the user or if it's a value
> already provided by KVM. Otherwise we don't know how to handle failed
> kvm_set_one_regs() when writing the configurations back.
>
> These characteristics make it overly complicated to use the same user
> facing flags for both KVM and TCG. A simpler approach is to create KVM
> specific properties that have specialized logic, forking KVM and TCG use
> cases for those cases only. Fully separating KVM/TCG properties is
> unneeded at this point - in fact we want the user experience to be as
> equal as possible, regardless of the acceleration chosen.
>
> We'll start this fork with the MISA properties, adding the MISA bits
> that the KVM driver currently supports. A new KVMCPUConfig type is
> introduced. It'll hold general information about an extension. For MISA
> extensions we're going to use the newly created misa_ext_infos[] to
> populate their name and description. 'offset' holds the MISA bit (RVA,
> RVC, ...). We're calling it 'offset' instead of 'misa_bit' because this
> same KVMCPUConfig struct will be used to multi-letter extensions later
> on.
>
> This new type also holds a 'user_set' flag. This flag will be set when
> the user set an option that's different than what is already configured
> in the host, requiring KVM intervention to write the regs back during
> kvm_arch_init_vcpu(). Similar mechanics will be implemented for
> multi-letter extensions as well.
>
> There is no need to duplicate more code than necessary, so we're going
> to use the existing kvm_riscv_init_user_properties() to add the KVM
> specific properties. Any code that is adding a TCG user prop is then
> changed slightly to verify first if there's a KVM prop with the same
> name already added.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/cpu.c | 10 ++++++
> target/riscv/kvm.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 86 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index edaf052f25..a4f3ed0c17 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1590,6 +1590,11 @@ static void riscv_cpu_add_misa_properties(Object *cpu_obj)
> for (i = 0; i < ARRAY_SIZE(misa_ext_cfgs); i++) {
> const RISCVCPUMisaExtConfig *misa_cfg = &misa_ext_cfgs[i];
>
> + /* Check if KVM didn't create the property already */
> + if (object_property_find(cpu_obj, misa_cfg->name)) {
> + continue;
> + }
> +
> object_property_add(cpu_obj, misa_cfg->name, "bool",
> cpu_get_misa_ext_cfg,
> cpu_set_misa_ext_cfg,
> @@ -1713,6 +1718,11 @@ static void riscv_cpu_add_user_properties(Object *obj)
> riscv_cpu_add_misa_properties(obj);
>
> for (prop = riscv_cpu_extensions; prop && prop->name; prop++) {
> + /* Check if KVM didn't create the property already */
> + if (object_property_find(obj, prop->name)) {
> + continue;
> + }
> +
> qdev_property_add_static(dev, prop);
> }
>
> diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
> index 4d0808cb9a..53042a0e86 100644
> --- a/target/riscv/kvm.c
> +++ b/target/riscv/kvm.c
> @@ -22,8 +22,10 @@
> #include <linux/kvm.h>
>
> #include "qemu/timer.h"
> +#include "qapi/error.h"
> #include "qemu/error-report.h"
> #include "qemu/main-loop.h"
> +#include "qapi/visitor.h"
> #include "sysemu/sysemu.h"
> #include "sysemu/kvm.h"
> #include "sysemu/kvm_int.h"
> @@ -105,6 +107,79 @@ static uint64_t kvm_riscv_reg_id(CPURISCVState *env, uint64_t type,
> } \
> } while (0)
>
> +typedef struct KVMCPUConfig {
> + const char *name;
> + const char *description;
> + target_ulong offset;
> + int kvm_reg_id;
> + bool user_set;
> +} KVMCPUConfig;
> +
> +#define KVM_MISA_CFG(_bit, _reg_id) \
> + {.name = misa_ext_infos[_bit].name, \
> + .description = misa_ext_infos[_bit].description, \
> + .offset = _bit, .kvm_reg_id = _reg_id}
> +
> +/* KVM ISA extensions */
> +static KVMCPUConfig kvm_misa_ext_cfgs[] = {
> + KVM_MISA_CFG(RVA, KVM_RISCV_ISA_EXT_A),
> + KVM_MISA_CFG(RVC, KVM_RISCV_ISA_EXT_C),
> + KVM_MISA_CFG(RVD, KVM_RISCV_ISA_EXT_D),
> + KVM_MISA_CFG(RVF, KVM_RISCV_ISA_EXT_F),
> + KVM_MISA_CFG(RVH, KVM_RISCV_ISA_EXT_H),
> + KVM_MISA_CFG(RVI, KVM_RISCV_ISA_EXT_I),
> + KVM_MISA_CFG(RVM, KVM_RISCV_ISA_EXT_M),
> +};
> +
> +static void kvm_cpu_set_misa_ext_cfg(Object *obj, Visitor *v,
> + const char *name,
> + void *opaque, Error **errp)
> +{
> + KVMCPUConfig *misa_ext_cfg = opaque;
> + target_ulong misa_bit = misa_ext_cfg->offset;
> + RISCVCPU *cpu = RISCV_CPU(obj);
> + CPURISCVState *env = &cpu->env;
> + bool value, host_bit;
> +
> + if (!visit_type_bool(v, name, &value, errp)) {
> + return;
> + }
> +
> + host_bit = env->misa_ext_mask & misa_bit;
> +
> + if (value == host_bit) {
> + return;
> + }
> +
> + if (!value) {
> + misa_ext_cfg->user_set = true;
> + return;
> + }
> +
> + /*
> + * Forbid users to enable extensions that aren't
> + * available in the hart.
> + */
> + error_setg(errp, "Enabling MISA bit '%s' is not allowed: it's not "
> + "enabled in the host", misa_ext_cfg->name);
> +}
> +
> +static void kvm_riscv_add_cpu_user_properties(Object *cpu_obj)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(kvm_misa_ext_cfgs); i++) {
> + KVMCPUConfig *misa_cfg = &kvm_misa_ext_cfgs[i];
> +
> + object_property_add(cpu_obj, misa_cfg->name, "bool",
> + NULL,
> + kvm_cpu_set_misa_ext_cfg,
> + NULL, misa_cfg);
> + object_property_set_description(cpu_obj, misa_cfg->name,
> + misa_cfg->description);
> + }
> +}
> +
> static int kvm_riscv_get_regs_core(CPUState *cs)
> {
> int ret = 0;
> @@ -427,6 +502,7 @@ void kvm_riscv_init_user_properties(Object *cpu_obj)
> return;
> }
>
> + kvm_riscv_add_cpu_user_properties(cpu_obj);
> kvm_riscv_init_machine_ids(cpu, &kvmcpu);
> kvm_riscv_init_misa_ext_mask(cpu, &kvmcpu);
>
> --
> 2.40.1
>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
next prev parent reply other threads:[~2023-06-19 9:18 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 [this message]
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
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-8ea679b59cf7df0b0dbd213b@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).