|
From: | Daniel Henrique Barboza |
Subject: | Re: [PATCH 14/16] target/riscv: adapt 'riscv_isa_string' for KVM |
Date: | Tue, 13 Jun 2023 07:29:23 -0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 |
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 **isa_str, 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 *cpu_obj) } }+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_Hvoid 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);--2.40.1Hmm, 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 both. 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 extensions: # cat /proc/device-tree/cpus/cpu@0/riscv,isa rv64imafdc_zicbom_zicboz_zbb_zhinx_zhinxmin_sstc 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 extensions. 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. Another idea is to make the existing isa_string() compare isa_edata_arr[] with the 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 since 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 when running KVM. We would fork the API though, which is something that we're wanting to avoid. Duplicating this code as we're doing here guarantees that the KVM isa string won't 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 until 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. Thanks, Daniel
Thanks, drew
[Prev in Thread] | Current Thread | [Next in Thread] |