Re: [Qemu-devel] [PATCH v4 11/21] s390x: allow only 1 CPU with TCG

[David Hildenbrand <david@redhat.com>]

On 13.09.2017 09:19, Igor Mammedov wrote:
> On Tue, 12 Sep 2017 17:42:35 +0200
> David Hildenbrand <david@redhat.com> wrote:
> 
>> On 12.09.2017 14:43, Igor Mammedov wrote:
>>> On Mon, 11 Sep 2017 17:21:40 +0200
>>> David Hildenbrand <david@redhat.com> wrote:
>>>   
>>>> Specifying more than 1 CPU (e.g. -smp 5) leads to SIGP errors (the
>>>> guest tries to bring these CPUs up but fails), because we don't support
>>>> multiple CPUs on s390x under TCG.
>>>>
>>>> Let's bail out if more than 1 is specified, so we don't raise people's
>>>> hope. Make it a define, so we can easily bump it up later.
>>>>
>>>> Tested-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
>>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>>> ---
>>>>  hw/s390x/s390-virtio-ccw.c | 10 ++++++++++
>>>>  1 file changed, 10 insertions(+)
>>>>
>>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>>> index f67b4b5d58..f1198b2745 100644
>>>> --- a/hw/s390x/s390-virtio-ccw.c
>>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>>> @@ -23,6 +23,7 @@
>>>>  #include "hw/s390x/css.h"
>>>>  #include "virtio-ccw.h"
>>>>  #include "qemu/config-file.h"
>>>> +#include "qemu/error-report.h"
>>>>  #include "s390-pci-bus.h"
>>>>  #include "hw/s390x/storage-keys.h"
>>>>  #include "hw/s390x/storage-attributes.h"
>>>> @@ -47,6 +48,8 @@ S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
>>>>      return cpu_states[cpu_addr];
>>>>  }
>>>>  
>>>> +/* #define S390_TCG_SMP_SUPPORT */  
>>> I'd drop define and ifdef for something that doesn't exists  
>>
>> Conny requested it as we might see some work on that area (supporting
>> smp) soon. So as long as there are no other opinions, I'll stick to the
>> current version.
>>
>> Thanks!
>>
> I've just removed a bunch of TODO cpu models in PPC with
> corresponding macro, my guess they were also introduced
> with similar intent but ended up as dead code.
> 
> So it's better to add new code when it actually is needed,
> instead of just in case.
> 
v1 of this patch had no ifdef at all. So I'll let Conny decide whether
to keep it like this or whether to drop the ifdef again.

-- 

Thanks,

David