Re: [Qemu-ppc] [PATCH] ppc/pnv: fix cores per chip for multiple cpus

[Nikunj A Dadhania]

David Gibson <address@hidden> writes:

> On Fri, Sep 15, 2017 at 01:53:15PM +0530, Nikunj A Dadhania wrote:
>> David Gibson <address@hidden> writes:
>> 
>> >> 
>> >> I thought, I am doing the same here for PowerNV, number of online cores
>> >> is equal to initial online vcpus / threads per core
>> >> 
>> >>    int boot_cores_nr = smp_cpus / smp_threads;
>> >> 
>> >> Only difference that I see in PowerNV is that we have multiple chips
>> >> (max 2, at the moment)
>> >> 
>> >>         cores_per_chip = smp_cpus / (smp_threads * pnv->num_chips);
>> >
>> > This doesn't make sense to me.  Cores per chip should *always* equal
>> > smp_cores, you shouldn't need another calculation for it.
>> >
>> >> And in case user has provided sane smp_cores, we use it.
>> >
>> > If smp_cores isn't sane, you should simply reject it, not try to fix
>> > it.  That's just asking for confusion.
>> 
>> This is the case where the user does not provide a topology(which is a
>> valid scenario), not sure we should reject it. So qemu defaults
>> smp_cores/smt_threads to 1. I think it makes sense to over-ride.
>
> If you can find a way to override it by altering smp_cores when it's
> not explicitly specified, then ok.

Should I change the global smp_cores here as well ?

> But overriding smp_cores with a different variable that's the "real"
> number of cores is not acceptable. If that means the user has to
> specify cores explicitly, so be it.

Right, we would error out in case there is mismatch.

> Slight awkwardness in command line is preferable to breaking the
> assumption that smp_cores == (# of cores per next level up cpu object)
> which is used all over the place.

Regards
Nikunj