[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 14/16] target/riscv: adapt 'riscv_isa_string' for KVM

From: Daniel Henrique Barboza
Subject: Re: [PATCH 14/16] target/riscv: adapt 'riscv_isa_string' for KVM
Date: Tue, 13 Jun 2023 15:19:20 -0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0

On 6/13/23 07:29, Daniel Henrique Barboza wrote:

On 6/7/23 09:21, Andrew Jones wrote:
On Tue, May 30, 2023 at 04:46:21PM -0300, Daniel Henrique Barboza wrote:
KVM is not using the same attributes as TCG, i.e. it doesn't use
isa_edata_arr[]. Add a new kvm_riscv_isa_string_ext() helper that does
basically the same thing, but using KVM internals instead.

The decision to add this helper target/riscv/kvm.c is to foster the
separation between KVM and TCG logic, while still using
riscv_isa_string_ext() from target/riscv/cpu.c to retrieve the string
to not overcomplicate things.

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
  target/riscv/cpu.c       |  5 +++++
  target/riscv/kvm.c       | 19 +++++++++++++++++++
  target/riscv/kvm_riscv.h |  2 ++
  3 files changed, 26 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 3c348049a3..ec1d0c621a 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1956,6 +1956,11 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char 
      char *new = *isa_str;
      int i;
+    if (riscv_running_KVM()) {
+        kvm_riscv_isa_string_ext(cpu, isa_str, max_str_len);
+        return;
+    }
      for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) {
          if (cpu->env.priv_ver >= isa_edata_arr[i].min_version &&
              isa_ext_is_enabled(cpu, &isa_edata_arr[i])) {
diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
index b4193a10d8..675e18df3b 100644
--- a/target/riscv/kvm.c
+++ b/target/riscv/kvm.c
@@ -320,6 +320,25 @@ static void kvm_riscv_add_cpu_user_properties(Object 
+void kvm_riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str,
+                              int max_str_len)
+    char *old = *isa_str;
+    char *new = *isa_str;
+    int i;
+    for (i = 0; i < ARRAY_SIZE(kvm_multi_ext_cfgs); i++) {
+        RISCVCPUMultiExtConfig *multi_ext_cfg = &kvm_multi_ext_cfgs[i];
+        if (kvm_cpu_cfg_get(cpu, multi_ext_cfg)) {
+            new = g_strconcat(old, "_", multi_ext_cfg->name, NULL);
+            g_free(old);
+            old = new;
+        }
+    }
+    *isa_str = new;
  static int kvm_riscv_get_regs_core(CPUState *cs)
      int ret = 0;
diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm_riscv.h
index e3ba935808..1a12efa8db 100644
--- a/target/riscv/kvm_riscv.h
+++ b/target/riscv/kvm_riscv.h
@@ -20,6 +20,8 @@
  #define QEMU_KVM_RISCV_H
  void kvm_riscv_init_user_properties(Object *cpu_obj);
+void kvm_riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str,
+                              int max_str_len);
  void kvm_riscv_reset_vcpu(RISCVCPU *cpu);
  void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level);

Hmm, more duplication. I think we need an abstraction which can support
both TCG and KVM extension lists. Allowing functions like
riscv_isa_string_ext() to be shared for them.

I tried to play around a bit and didn't manage to find a solution that covers

The root cause is that the TCG only options are being ignored by KVM, but they
are still around. I made an attempt of re-using the existing isa_string()
function with KVM by changing all TCG-only extensions default to 'false'. This
doesn't change the fact that, with KVM, I can do:

sudo ./qemu/build/qemu-system-riscv64  -machine virt,accel=kvm \
-cpu host,zhinx=true,zhinxmin=true (...)

Note that zhinx and zhinxmin are TCG-only. And the ISA string showed these 2

# cat /proc/device-tree/cpus/cpu@0/riscv,isa

Alternatives would be to change TCG code to allow for extra fields for KVM 
(e.g. the
'supported' flag) to allow the isa_string() function to ignore the TCG-only 
Bear in mind that TCG has 63 extensions, so we would do 63 ioctls for each CPU 
in this
extension discovery and KVM only 8 support extensions ATM.

... or we can add an extra flag in isa_edata_arr[] 'kvm_available' to indicate 
if a
given extension is also present in KVM, set it manually for each KVM-capable
entry of the array and check for that during riscv_isa_string_ext().

This will avoid the code duplication while not allowing TCG-only extensions
to appear in the riscv,isa when running KVM.

I made this change in v2. I'll send it up shortly. Thanks,


Another idea is to make the existing isa_string() compare isa_edata_arr[] with 
KVM counterpart kvm_multi_ext_cfgs[] and, if running KVM, check if the extension
in isa_edata_arr[] is also in the KVM array. This also seems a bit inefficient 
we're adding a search loop for 55 extensions when creating the string.

Another alternative is to exclude all TCG-only extensions from the command line 
running KVM. We would fork the API though, which is something that we're 
wanting to

Duplicating this code as we're doing here guarantees that the KVM isa string 
have anything that KVM doesn't know about, regardless of the user input. I am 
not a
fan of duplication, but at this moment it seems plausible to keep it. At least 
we sort a way of unifying both TCG and KVM options in a satisfying manner.

I mean, at least as far as a I can see. Suggestions always welcome.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]