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

2018-01-14 Thread 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 
> ---
>  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(_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(_err, "KVM does not support %d threads/core.",
> +smp_threads);
> +kvmppc_hint_smt_possible(_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

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

2018-01-14 Thread Jose Ricardo Ziviani 
QEMU currently checks whether SMT passed is valid or not. However, it
doesn't check if KVM supports such mode when kvm is enabled.

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 
---
 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(_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(_err, "KVM does not support %d threads/core.",
+smp_threads);
+kvmppc_hint_smt_possible(_err);
+goto out;
+}
+}
+
 /* 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;
-- 
2.14.3