Re: [Qemu-devel] [PATCH v2 2/2] ppc: spapr: Check if thread argument is supported by host KVM

[David Gibson]

On Sun, Jan 14, 2018 at 05:23:49PM -0200, Jose Ricardo Ziviani wrote:
> QEMU currently checks whether SMT passed is valid or not. However, it
> doesn't check if KVM supports such mode when kvm is enabled.

That's not really true - the attempt to actually set the vsmt mode in
KVM later on in spapr_set_vsmt_mode() will fail if KVM can't support
the number of threads.

The error added here might be a bit easier to understand, since it
doesn't refer to vsmt modes, which might just confuse the issue.

The change isn't urgent, though.

> This patch relies on KVM_CAP_PPC_SMT_POSSIBLE to make it sure that QEMU
> will either set a valid SMT mode or warn an error message and quit.
> 
> Signed-off-by: Jose Ricardo Ziviani <address@hidden>
> ---
>  hw/ppc/spapr.c       | 10 ++++++++++
>  target/ppc/kvm.c     |  5 +++++
>  target/ppc/kvm_ppc.h |  6 ++++++
>  3 files changed, 21 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d1acfe8858..aed4d25fc4 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2261,12 +2261,22 @@ static void spapr_set_vsmt_mode(sPAPRMachineState 
> *spapr, Error **errp)
>                       "on a pseries machine");
>          goto out;
>      }
> +
>      if (!is_power_of_2(smp_threads)) {
>          error_setg(&local_err, "Cannot support %d threads/core on a pseries "
>                       "machine because it must be a power of 2", smp_threads);
>          goto out;
>      }
>  
> +    if (kvm_enabled() && kvmppc_cap_smt_possible() > 0) {
> +        if ((kvmppc_cap_smt_possible() & smp_threads) != smp_threads) {
> +            error_setg(&local_err, "KVM does not support %d threads/core.",
> +                    smp_threads);
> +            kvmppc_hint_smt_possible(&local_err);
> +            goto out;
> +        }
> +    }

I'd like to see a fallback for kernels that don't support the
smt_possible cap and vsmt mode setting (for those, we must have
smp_threads <= kvm_smt).

> +
>      /* Detemine the VSMT mode to use: */
>      if (vsmt_user) {
>          if (spapr->vsmt < smp_threads) {
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 914be687e7..4a8ff4d63c 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -2456,6 +2456,11 @@ bool kvmppc_has_cap_mmu_hash_v3(void)
>      return cap_mmu_hash_v3;
>  }
>  
> +int kvmppc_cap_smt_possible(void)
> +{
> +    return cap_ppc_smt_possible;
> +}
> +
>  PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void)
>  {
>      uint32_t host_pvr = mfpvr();
> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> index ecb55493cc..2221850723 100644
> --- a/target/ppc/kvm_ppc.h
> +++ b/target/ppc/kvm_ppc.h
> @@ -59,6 +59,7 @@ bool kvmppc_has_cap_fixup_hcalls(void);
>  bool kvmppc_has_cap_htm(void);
>  bool kvmppc_has_cap_mmu_radix(void);
>  bool kvmppc_has_cap_mmu_hash_v3(void);
> +int kvmppc_cap_smt_possible(void);
>  int kvmppc_enable_hwrng(void);
>  int kvmppc_put_books_sregs(PowerPCCPU *cpu);
>  PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void);
> @@ -290,6 +291,11 @@ static inline bool kvmppc_has_cap_mmu_hash_v3(void)
>      return false;
>  }
>  
> +static inline int kvmppc_cap_smt_possible(void)
> +{
> +    return 0;
> +}
> +
>  static inline int kvmppc_enable_hwrng(void)
>  {
>      return -1;

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature