Re: [Qemu-ppc] [PATCH v2 0/1] Exploit settable KVM_CAP_PPC_SMT

[David Gibson]

On Mon, Aug 14, 2017 at 01:40:20PM +1000, Sam Bobroff wrote:
> Hello QEMU PPC people,
> 
> This is v2 of my set, and it is significantly changed since v1:
> * Patch 1 fixed a bug that has been fixed upstream, so it is dropped.
> * Patch 2 moved kvm_cap_smt into sPAPRMachineState, but I no longer think that
>   this is the best solution (see below), so this patch is dropped.
> * Patch 4 introduced a way to query the VSMT value but it's a "nice to have"
>   rather than a requirement and I would like to discuss it more before
>   implementing it. So I'll attempt it in a follow up set and it's dropped from
>   this one.

Erm.. seems like this cover letter needs an update.  There's only one
patch in this series.

> I've also re-written the coverletter to (hopefully) be clearer about what the
> set needs to do, and how that's implemented, as below:
> 
> My core objective with the set is to provide a way for QEMU to configure the
> newly writeable KVM capability 'KVM_CAP_PPC_SMT', because without it Power 9
> hosts can only run VMs with a single thread per core. (With this capability 
> they
> are able to run VMs with 1, 2, 4 or 8 threads per core.) KVM also now 
> contains a
> new read-only property ('KVM_CAP_PPC_SMT_POSSIBLE') to expose the possible
> valid values of KVM_CAP_PPC_SMT, although this is only used in a hint to the
> user at this stage. This new capability is already upstream and the QEMU
> headers have already been updated to include it.
> 
> The new way KVM_CAP_PPC_SMT works is that, when set, it causes KVM to act as 
> if
> the host's native number of threads per core were the value of the capability.
> 
> I've implemented this by adding a new property to pseries machines, which is
> stored in sPAPRMachineState as 'vsmt'. This provides a way to set it from the
> command line (and a way to add it to the VMState if we later decide to do so)
> and makes the property available only when using SPAPR (pseries) machines. I
> use this value to call in to KVM and set the capability when necessary.
> 
> For pseries machines, the vsmt value will be a duplicate of the KVM capability
> value, and in version 1 I tried to remove this duplication by replacing
> cap_ppc_smt completely with references to spapr->vsmt. Unfortunately, that
> forced generic code (e.g. translate_init.c) to have knowledge of the
> sPAPRMachine state which didn't seem conceptually clean. In version 2, I've
> left the capability on the KVM side which keeps the KVM and generic code clear
> of SPAPR concepts. I think on the whole this is a better solution.
> 
> Notes/Questions:
> * I've moved the code that validates smp_threads out of ppc_cpu_realizefn()
>   because it only needs to be done once, not once per CPU.
> * The intialization of KVM_CAP_PPC_SMT has changed slightly because it has
>   changed (in KVM) from a global capability to a VM-specific one. This won't
>   cause a problem on older KVMs because VM capabilities fall back to global
>   ones.
> 
> Patch set changelog follows:
> 
> ====== Version 1 -> version 2: ======
> 
> Patch 1/1: PPC: KVM: Support machine option to set VSMT mode
> * Reworked to keep SPAPR dependencies out of KVM code.
> 
> 
> Sam Bobroff (1):
>   PPC: KVM: Support machine option to set VSMT mode
> 
>  hw/ppc/spapr.c              | 105 
> ++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/ppc/spapr.h      |   1 +
>  target/ppc/kvm.c            |  20 ++++++++-
>  target/ppc/kvm_ppc.h        |  12 +++++
>  target/ppc/translate_init.c |  14 ------
>  5 files changed, 137 insertions(+), 15 deletions(-)
> 

-- 
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