22.12.2023 15:22, Daniel Henrique Barboza wrote:
> To turn cbom_blocksize and cboz_blocksize into class properties we need
> KVM specific changes.
>
> KVM is creating its own version of these options with a customized
> setter() that prevents users from picking an invalid value during init()
> time. This comes at the cost of duplicating each option that KVM
> supports. This will keep happening for each new shared option KVM
> implements in the future.
>
> We can avoid that by using the same property TCG uses and adding
> specific KVM handling during finalize() time, like TCG already does with
> riscv_tcg_cpu_finalize_features(). To do that, the common CPU property
> offers a way of knowing if an option was user set or not, sparing us
> from doing unneeded syscalls.
>
> riscv_kvm_cpu_finalize_features() is then created using the same
> KVMScratch CPU we already use during init() time, since finalize() time
> is still too early to use the official KVM CPU for it. cbom_blocksize
> and cboz_blocksize are then handled during finalize() in the same way
> they're handled by their KVM specific setter.
>
> With this change we can proceed with the blocksize changes in the common
> code without breaking the KVM driver.
>
> Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
> ---
> target/riscv/cpu.c | 16 +++++++---
> target/riscv/cpu.h | 1 +
> target/riscv/kvm/kvm-cpu.c | 59 ++++++++++++++++++++++++++++++++++++
> target/riscv/kvm/kvm_riscv.h | 1 +
> 4 files changed, 72 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 8be619b6f1..f49d31d753 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -63,6 +63,11 @@ static void cpu_option_add_user_setting(const char
> *optname, uint32_t value)
> GUINT_TO_POINTER(value));
> }
>
> +bool riscv_cpu_option_set(const char *optname)
> +{
> + return g_hash_table_contains(general_user_opts, optname);
> +}
> +
This function may work in unexpected way for future developer since we can
check just somehow restricted
number of options using it, like vlen/elen/cbom/cboz size, but not vext_spec or
pmu-num/mask.
> #define ISA_EXT_DATA_ENTRY(_name, _min_ver, _prop) \
> {#_name, _min_ver, CPU_CFG_OFFSET(_prop)}
>
> @@ -1056,17 +1061,18 @@ void riscv_cpu_finalize_features(RISCVCPU *cpu, Error
> **errp)
> {
> Error *local_err = NULL;
>
> - /*
> - * KVM accel does not have a specialized finalize()
> - * callback because its extensions are validated
> - * in the get()/set() callbacks of each property.
> - */
> if (tcg_enabled()) {
> riscv_tcg_cpu_finalize_features(cpu, &local_err);
> if (local_err != NULL) {
> error_propagate(errp, local_err);
> return;
> }
> + } else if (kvm_enabled()) {
> + riscv_kvm_cpu_finalize_features(cpu, &local_err);
> + if (local_err != NULL) {
> + error_propagate(errp, local_err);
> + return;
> + }
> }
>
> #ifndef CONFIG_USER_ONLY
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 53101b82c5..988471c7ba 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -495,6 +495,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
> size,
> bool probe, uintptr_t retaddr);
> char *riscv_isa_string(RISCVCPU *cpu);
> void riscv_cpu_list(void);
> +bool riscv_cpu_option_set(const char *optname);
>
> #define cpu_list riscv_cpu_list
> #define cpu_mmu_index riscv_cpu_mmu_index
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 62a1e51f0a..70fb075846 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -1490,6 +1490,65 @@ static void kvm_cpu_instance_init(CPUState *cs)
> }
> }
>
> +void riscv_kvm_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
> +{
> + CPURISCVState *env = &cpu->env;
> + KVMScratchCPU kvmcpu;
> + struct kvm_one_reg reg;
> + uint64_t val;
> + int ret;
> +
> + /* short-circuit without spinning the scratch CPU */
> + if (!cpu->cfg.ext_zicbom && !cpu->cfg.ext_zicboz) {
> + return;
> + }
> +
> + if (!kvm_riscv_create_scratch_vcpu(&kvmcpu)) {
> + error_setg(errp, "Unable to create scratch KVM cpu");
> + return;
> + }
> +
> + if (cpu->cfg.ext_zicbom &&
> + riscv_cpu_option_set(kvm_cbom_blocksize.name)) {
> +
> + reg.id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CONFIG,
> + kvm_cbom_blocksize.kvm_reg_id);
> + reg.addr = (uint64_t)&val;
> + ret = ioctl(kvmcpu.cpufd, KVM_GET_ONE_REG, ®);
> + if (ret != 0) {
> + error_setg(errp, "Unable to read cbom_blocksize, error %d",
> errno);
> + return;
> + }
> +
> + if (cpu->cfg.cbom_blocksize != val) {
> + error_setg(errp, "Unable to set cbom_blocksize to a different "
> + "value than the host (%lu)", val);
> + return;
> + }
> + }
> +
> + if (cpu->cfg.ext_zicboz &&
> + riscv_cpu_option_set(kvm_cboz_blocksize.name)) {
> +
> + reg.id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CONFIG,
> + kvm_cboz_blocksize.kvm_reg_id);
> + reg.addr = (uint64_t)&val;
> + ret = ioctl(kvmcpu.cpufd, KVM_GET_ONE_REG, ®);
> + if (ret != 0) {
> + error_setg(errp, "Unable to read cbom_blocksize, error %d",
> errno);
> + return;
> + }
> +
> + if (cpu->cfg.cboz_blocksize != val) {
> + error_setg(errp, "Unable to set cboz_blocksize to a different "
> + "value than the host (%lu)", val);
> + return;
> + }
> + }
> +
> + kvm_riscv_destroy_scratch_vcpu(&kvmcpu);
> +}
> +
> static void kvm_cpu_accel_class_init(ObjectClass *oc, void *data)
> {
> AccelCPUClass *acc = ACCEL_CPU_CLASS(oc);
> diff --git a/target/riscv/kvm/kvm_riscv.h b/target/riscv/kvm/kvm_riscv.h
> index 8329cfab82..4bd98fddc7 100644
> --- a/target/riscv/kvm/kvm_riscv.h
> +++ b/target/riscv/kvm/kvm_riscv.h
> @@ -27,5 +27,6 @@ void kvm_riscv_aia_create(MachineState *machine, uint64_t
> group_shift,
> uint64_t guest_num);
> void riscv_kvm_aplic_request(void *opaque, int irq, int level);
> int kvm_riscv_sync_mpstate_to_kvm(RISCVCPU *cpu, int state);
> +void riscv_kvm_cpu_finalize_features(RISCVCPU *cpu, Error **errp);
>
> #endif
> --
> 2.43.0
>
>
