Currently, on a P10 KVM guest, the mitigations seen in the output of
"lscpu" command are different from the host. The reason for this
behaviour is that when the KVM guest makes the "h_get_cpu_characteristics"
hcall, QEMU does not consider the data it received from the host via the
KVM_PPC_GET_CPU_CHAR ioctl, and just uses the values present in
spapr->eff.caps[], which in turn just contain the default values set in
spapr_machine_class_init().

Fix this behaviour by making sure that h_get_cpu_characteristics()
returns the data received from the KVM ioctl for a KVM guest.

Perf impact:
With null syscall benchmark[1], ~45% improvement is observed.

1. Vanilla QEMU
$ ./null_syscall
132.19 ns     456.54 cycles

2. With this patch
$ ./null_syscall
91.18 ns     314.57 cycles

[1]: https://ozlabs.org/~anton/junkcode/null_syscall.c

Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
---
v1 -> v2:
Handle the case where KVM_PPC_GET_CPU_CHAR ioctl fails

 hw/ppc/spapr_hcall.c   |  6 ++++++
 include/hw/ppc/spapr.h |  1 +
 target/ppc/kvm.c       | 13 ++++++++++---
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 5e1d020e3d..d6db1bdab8 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1402,6 +1402,12 @@ static target_ulong h_get_cpu_characteristics(PowerPCCPU *cpu,
     uint8_t count_cache_flush_assist = spapr_get_cap(spapr,
                                                      SPAPR_CAP_CCF_ASSIST);
 
+    if (kvm_enabled() && spapr->chars.character) {
+        args[0] = spapr->chars.character;
+        args[1] = spapr->chars.behaviour;
+        return H_SUCCESS;
+    }
+
     switch (safe_cache) {
     case SPAPR_CAP_WORKAROUND:
         characteristics |= H_CPU_CHAR_L1D_FLUSH_ORI30;
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index af4aa1cb0f..c41da8cb82 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -280,6 +280,7 @@ struct SpaprMachineState {
     Error *fwnmi_migration_blocker;
 
     SpaprWatchdog wds[WDT_MAX_WATCHDOGS];
+    struct kvm_ppc_cpu_char chars;
 };
 
 #define H_SUCCESS         0
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 3efc28f18b..fec7bcc347 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -2499,7 +2499,8 @@ bool kvmppc_has_cap_xive(void)
 
 static void kvmppc_get_cpu_characteristics(KVMState *s)
 {
-    struct kvm_ppc_cpu_char c;
+    SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
+    struct kvm_ppc_cpu_char c = {0};
     int ret;
 
     /* Assume broken */
@@ -2509,18 +2510,24 @@ static void kvmppc_get_cpu_characteristics(KVMState *s)
 
     ret = kvm_vm_check_extension(s, KVM_CAP_PPC_GET_CPU_CHAR);
     if (!ret) {
-        return;
+        goto err;
     }
     ret = kvm_vm_ioctl(s, KVM_PPC_GET_CPU_CHAR, &c);
     if (ret < 0) {
-        return;
+        goto err;
     }
 
+    spapr->chars = c;
     cap_ppc_safe_cache = parse_cap_ppc_safe_cache(c);
     cap_ppc_safe_bounds_check = parse_cap_ppc_safe_bounds_check(c);
     cap_ppc_safe_indirect_branch = parse_cap_ppc_safe_indirect_branch(c);
     cap_ppc_count_cache_flush_assist =
         parse_cap_ppc_count_cache_flush_assist(c);
+
+    return;
+
+err:
+    memset(&(spapr->chars), 0, sizeof(struct kvm_ppc_cpu_char));
 }
 
 int kvmppc_get_cap_safe_cache(void)
-- 
2.49.0

Gautam,

Thanks for this patch

Gautam Menghani <gautam@linux.ibm.com> writes:

> Currently, on a P10 KVM guest, the mitigations seen in the output of
> "lscpu" command are different from the host. The reason for this
> behaviour is that when the KVM guest makes the "h_get_cpu_characteristics"
> hcall, QEMU does not consider the data it received from the host via the
> KVM_PPC_GET_CPU_CHAR ioctl, and just uses the values present in
> spapr->eff.caps[], which in turn just contain the default values set in
> spapr_machine_class_init().
>
> Fix this behaviour by making sure that h_get_cpu_characteristics()
> returns the data received from the KVM ioctl for a KVM guest.
>
> Perf impact:
> With null syscall benchmark[1], ~45% improvement is observed.
>
> 1. Vanilla QEMU
> $ ./null_syscall
> 132.19 ns     456.54 cycles
>
> 2. With this patch
> $ ./null_syscall
> 91.18 ns     314.57 cycles
>
> [1]: https://ozlabs.org/~anton/junkcode/null_syscall.c
>
> Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> ---
<snip>

LGTM,

Reviewed-by: Vaibhav Jain <vaibhav@linux.ibm.com>

-- 
Cheers
~ Vaibhav

+ Nicholas M (Forgot to include the text in the prev mail, please ignore
that)

On Thu, Apr 17, 2025 at 04:49:03PM +0530, Gautam Menghani wrote:
> Currently, on a P10 KVM guest, the mitigations seen in the output of
> "lscpu" command are different from the host. The reason for this
> behaviour is that when the KVM guest makes the "h_get_cpu_characteristics"
> hcall, QEMU does not consider the data it received from the host via the
> KVM_PPC_GET_CPU_CHAR ioctl, and just uses the values present in
> spapr->eff.caps[], which in turn just contain the default values set in
> spapr_machine_class_init().
> 
> Fix this behaviour by making sure that h_get_cpu_characteristics()
> returns the data received from the KVM ioctl for a KVM guest.
> 
> Perf impact:
> With null syscall benchmark[1], ~45% improvement is observed.
> 
> 1. Vanilla QEMU
> $ ./null_syscall
> 132.19 ns     456.54 cycles
> 
> 2. With this patch
> $ ./null_syscall
> 91.18 ns     314.57 cycles
> 
> [1]: https://ozlabs.org/~anton/junkcode/null_syscall.c
> 
> Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> ---
> v1 -> v2:
> Handle the case where KVM_PPC_GET_CPU_CHAR ioctl fails
> 
>  hw/ppc/spapr_hcall.c   |  6 ++++++
>  include/hw/ppc/spapr.h |  1 +
>  target/ppc/kvm.c       | 13 ++++++++++---
>  3 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 5e1d020e3d..d6db1bdab8 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1402,6 +1402,12 @@ static target_ulong h_get_cpu_characteristics(PowerPCCPU *cpu,
>      uint8_t count_cache_flush_assist = spapr_get_cap(spapr,
>                                                       SPAPR_CAP_CCF_ASSIST);
>  
> +    if (kvm_enabled() && spapr->chars.character) {
> +        args[0] = spapr->chars.character;
> +        args[1] = spapr->chars.behaviour;
> +        return H_SUCCESS;
> +    }
> +
>      switch (safe_cache) {
>      case SPAPR_CAP_WORKAROUND:
>          characteristics |= H_CPU_CHAR_L1D_FLUSH_ORI30;
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index af4aa1cb0f..c41da8cb82 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -280,6 +280,7 @@ struct SpaprMachineState {
>      Error *fwnmi_migration_blocker;
>  
>      SpaprWatchdog wds[WDT_MAX_WATCHDOGS];
> +    struct kvm_ppc_cpu_char chars;
>  };
>  
>  #define H_SUCCESS         0
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 3efc28f18b..fec7bcc347 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -2499,7 +2499,8 @@ bool kvmppc_has_cap_xive(void)
>  
>  static void kvmppc_get_cpu_characteristics(KVMState *s)
>  {
> -    struct kvm_ppc_cpu_char c;
> +    SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> +    struct kvm_ppc_cpu_char c = {0};
>      int ret;
>  
>      /* Assume broken */
> @@ -2509,18 +2510,24 @@ static void kvmppc_get_cpu_characteristics(KVMState *s)
>  
>      ret = kvm_vm_check_extension(s, KVM_CAP_PPC_GET_CPU_CHAR);
>      if (!ret) {
> -        return;
> +        goto err;
>      }
>      ret = kvm_vm_ioctl(s, KVM_PPC_GET_CPU_CHAR, &c);
>      if (ret < 0) {
> -        return;
> +        goto err;
>      }
>  
> +    spapr->chars = c;
>      cap_ppc_safe_cache = parse_cap_ppc_safe_cache(c);
>      cap_ppc_safe_bounds_check = parse_cap_ppc_safe_bounds_check(c);
>      cap_ppc_safe_indirect_branch = parse_cap_ppc_safe_indirect_branch(c);
>      cap_ppc_count_cache_flush_assist =
>          parse_cap_ppc_count_cache_flush_assist(c);
> +
> +    return;
> +
> +err:
> +    memset(&(spapr->chars), 0, sizeof(struct kvm_ppc_cpu_char));
>  }
>  
>  int kvmppc_get_cap_safe_cache(void)
> -- 
> 2.49.0
>

+ Nicholas

Hi Nick,

Any comments on this? Can this go in if it looks ok?

Thanks,
Gautam

Hello,

Please let me know if any changes are needed in this patch.

Thanks,
Gautam