[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH v2 0/1] Exploit settable KVM_CAP_PPC_SMT
From: |
Greg Kurz |
Subject: |
Re: [Qemu-ppc] [PATCH v2 0/1] Exploit settable KVM_CAP_PPC_SMT |
Date: |
Mon, 14 Aug 2017 14:52:54 +0200 |
On Mon, 14 Aug 2017 16:28:57 +1000
David Gibson <address@hidden> wrote:
> 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:
since RFC actually :)
> > * 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.
>
IIUC, the patches mentioned above were in the previous 4-patch version and have
been dropped. Only patch 3 was kept.
> > 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(-)
> >
>
pgp5E9v9YWjmR.pgp
Description: OpenPGP digital signature