Re: [Qemu-devel] [PULL 00/22] QOM CPUState patch queue 2015-07-06

[Peter Crosthwaite]

On Mon, Jul 6, 2015 at 8:56 PM, Bharata B Rao <address@hidden> wrote:
> On Tue, Jul 7, 2015 at 7:55 AM, Peter Crosthwaite
> <address@hidden> wrote:
>> On Mon, Jul 6, 2015 at 5:24 PM, Andreas Färber <address@hidden> wrote:
>>> Am 07.07.2015 um 01:13 schrieb Andreas Färber:
>>>> Hello Peter,
>>>>
>>>> This is my QOM CPU patch queue. Please pull.
>>>>
>>>> Note: For time reasons I did not give this queue as much testing as usual,
>>>> in particular BSD and non-x86 KVM hosts were not covered.
>>>>
>>>> Regards,
>>>> Andreas
>>>>
>>>> Cc: Peter Maydell <address@hidden>
>>>>
>>>> Cc: Eduardo Habkost <address@hidden>
>>>> Cc: Peter Crosthwaite <address@hidden>
>>>>
>>>> The following changes since commit 
>>>> 7edd8e4660beb301d527257f8e04ebec0f841cb0:
>>>>
>>>>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into 
>>>> staging (2015-07-06 14:03:44 +0100)
>>>>
>>>> are available in the git repository at:
>>>>
>>>>   git://github.com/afaerber/qemu-cpu.git tags/qom-cpu-for-peter
>>>>
>>>> for you to fetch changes up to 116382f1504d655a1afdf3eac18d276a200428b7:
>>>>
>>>>   disas: cris: QOMify target specific disas setup (2015-07-06 22:36:17 
>>>> +0200)
>>>>
>>>> ----------------------------------------------------------------
>>>> QOM CPUState and X86CPU
>>>>
>>>> * Further QOM'ification of CPU initialization
>>>> * Propagation of CPUState arguments and elimination of ENV_GET_CPU() usage
>>>> * cpu_set_pc() abstraction
>>>> * CPUClass::disas_set_info() hook
>>>>
>>>> ----------------------------------------------------------------
>>> [...]
>>>
>>> Self-nack, hurry is never good:
>>>
>>>   /aarch64/qom/xlnx-ep108:
>>> qemu-system-aarch64: Trying to use more CPUs than allowed max of 1
>>> Broken pipe
>>> FAIL
>>>
>>> Peter C., any ideas why this is regressing?
>>>
>>
>> This:
>>
>> +    if (cpu >= max_cpus) {
>> +        error_setg(errp, "Trying to use more CPUs than allowed max of %d\n",
>> +                    max_cpus);
>> +        return -1;
>>
>> xlnx-ep108 doesn't care about the -smp argument, it creates all 6 CPUs
>> regardless of -smp. This is because the number of CPUs is not flexible
>> in reality. It is also a heterogeneous arch (with R5s abd A53s) so
>> trying to limit the grand total of CPUs is ambiguous (do you remove
>> a53s or r5s for -smp < 6?).
>>
>> Can this check be dropped or is this a bug in xlnx where we should
>> overcome by just forcing smp_cpus = 6 at machine level?
>
> That check is needed to fail CPU realization when an attempt is made
> to relialize (eg. via hotplug) more than allowed max number of CPUs.
>

What was the behaviour before this patch series for this case? I can't
see any check in original code.  had another read of your commit
messages. It seems the main purpose of your change is to ensure CPU
indicies are unique rather than within any limits so this new check is
secondary to what you were trying to do. Can we just drop it?

Regards,
Peter

> Regards,
> Bharata.
>