Re: [Qemu-devel] [PATCH 13/13] target-ppc: Give a meaningful error if too many threads are specified

[Alexander Graf]

On 13.12.2012, at 14:17, David Gibson wrote:

> On Thu, Dec 13, 2012 at 01:57:16PM +0100, Alexander Graf wrote:
>> 
>> On 04.12.2012, at 04:11, David Gibson wrote:
>> 
>>> On Tue, Dec 04, 2012 at 01:42:17PM +1100, David Gibson wrote:
>>>> From: Mike Qiu <address@hidden>
>>> 
>>> Oops, messed this one up, build error for user only builds.  Fixed
>>> version below.
>>> 
>>> From d2cc6bcee4737647c4bbbf65d3c0129d4a119dd3 Mon Sep 17 00:00:00 2001
>>> From: Mike Qiu <address@hidden>
>>> Date: Tue, 4 Dec 2012 10:52:14 +1100
>>> Subject: [PATCH] target-ppc: Give a meaningful error if too many threads are
>>> specified
>>> 
>>> Currently the target-ppc tcg code only supports a single thread.  You can
>>> specify more, but they're treated identically to multiple cores.  On KVM
>>> we obviously can't support more threads than the hardware; if more are
>>> specified it will cause strange and cryptic errors.
>>> 
>>> This patch clarifies the situation by giving a simple meaningful error if
>>> more threads are specified than we can support.
>>> 
>>> Signed-off-by: Mike Qiu <address@hidden>
>>> Signed-off-by: David Gibson <address@hidden>
>>> ---
>>> target-ppc/translate_init.c |   12 ++++++++++++
>>> 1 file changed, 12 insertions(+)
>>> 
>>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>>> index dba572f..bb054a7 100644
>>> --- a/target-ppc/translate_init.c
>>> +++ b/target-ppc/translate_init.c
>>> @@ -28,6 +28,7 @@
>>> #include <kvm.h>
>>> #include "kvm_ppc.h"
>>> #include "arch_init.h"
>>> +#include "cpus.h"
>>> 
>>> //#define PPC_DUMP_CPU
>>> //#define PPC_DEBUG_SPR
>>> @@ -10037,6 +10038,10 @@ static int ppc_fixup_cpu(CPUPPCState *env)
>>> 
>>> int cpu_ppc_register_internal (CPUPPCState *env, const ppc_def_t *def)
>>> {
>>> +#if !defined(CONFIG_USER_ONLY)
>>> +    int max_smt = kvmppc_smt_threads();
>> 
>> What if I am using TCG on PPC?
> 
> Then the stub version of kvmppc_smt_threads() returns 1, and since the
> PPC TCG doesn't do any SMT emulation, that would seem to be correct...

int kvmppc_smt_threads(void)
{
    return cap_ppc_smt ? cap_ppc_smt : 1;
}

So because we don't call kvm_arch_init, the cap is 0 and thus we return 1? 
That's quite hard to follow :).

Please make this explicit. Only call kvmppc_smt_threads() when kvm_enabled().


Alex