[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 18/21] s390x: implement query-hotpluggable-cp
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [PATCH v4 18/21] s390x: implement query-hotpluggable-cpus |
Date: |
Tue, 12 Sep 2017 16:24:11 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 12.09.2017 16:03, David Hildenbrand wrote:
> On 12.09.2017 15:43, Igor Mammedov wrote:
>> On Mon, 11 Sep 2017 17:21:47 +0200
>> David Hildenbrand <address@hidden> wrote:
>>
>>> CPU hotplug is only possible on a per core basis on s390x.
>>>
>>> As we now have ms->possible_cpus, we can get rid of the global variable
>>> cpu_states.
>>>
>>> While rewriting s390_cpu_addr2state() completely to be based on
>>> possible_cpus, move it to cpu.c, as it is independent of the virtio-ccw
>>> machine.
>> I'd split patch on
>> 1) introduce possible cpus
>> 2) rewrite s390_cpu_addr2state() using #2
>
> Than I have to keep the global variable + setting it for one patch.
> Might not be worth the trouble. Will have a look.
>
> [...]
>>>
>>> +static CPUArchId *s390_find_cpu_slot(MachineState *ms, uint32_t core_id,
>>> + int *idx)
>>> +{
>>> + if (core_id >= ms->possible_cpus->len) {
>>> + return NULL;
>>> + }
>>> + /* core_id corresponds to the index */
>>> + if (idx) {
>>> + *idx = core_id;
>>> + }
>>> + return &ms->possible_cpus->cpus[core_id];
>>> +}
>> it looks like cpu_index == core_id == idx in possible_cpus,
>> is this helper really necessary?
>> (we have it in x86 because of possible not 1:1 mapping)
>>
>> I'd drop it and just access array directly
>
> Just kept this because the other architectures also have this. I can of
> course drop it. The nice thing about this helper is the comment :)
>
> [...]
>>> }
>>> +
>>> +S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
>>> +{
>> target/cpu.c and cpu itself preferably shouldn't pull in
>> or depend on machine, so I'd keep s390_cpu_addr2state() where it's now
>> or somewhere in board related files
>
> Thomas requested this. I actually don't care., but it looks like a
> generic "get_cpu_by_arch_id" function. But I don't really want to go
> that additional path now. And also I don't want to move this back and forth.
>
> Thomas, what's your opinion?
Hmm, since the new version uses MachineState now, it's maybe really
better to leave it in the hw/s390x/ directory, I guess. Sorry for the
inconvenience ...
Thomas
- [Qemu-devel] [PATCH v4 14/21] target/s390x: rename next_cpu_id to next_core_id, (continued)
- [Qemu-devel] [PATCH v4 14/21] target/s390x: rename next_cpu_id to next_core_id, David Hildenbrand, 2017/09/11
- [Qemu-devel] [PATCH v4 16/21] s390x: allow cpu hotplug via device_add, David Hildenbrand, 2017/09/11
- [Qemu-devel] [PATCH v4 17/21] s390x: CPU hot unplug via device_del cannot work for now, David Hildenbrand, 2017/09/11
- [Qemu-devel] [PATCH v4 15/21] s390x: print CPU definitions in sorted order, David Hildenbrand, 2017/09/11
- [Qemu-devel] [PATCH v4 18/21] s390x: implement query-hotpluggable-cpus, David Hildenbrand, 2017/09/11
- [Qemu-devel] [PATCH v4 19/21] s390x: get rid of cpu_s390x_create(), David Hildenbrand, 2017/09/11
- [Qemu-devel] [PATCH v4 21/21] s390x: allow CPU hotplug in random core-id order, David Hildenbrand, 2017/09/11
- [Qemu-devel] [PATCH v4 20/21] s390x: generate sclp cpu information from possible_cpus, David Hildenbrand, 2017/09/11
- Re: [Qemu-devel] [PATCH v4 00/21] s390x cleanups and CPU hotplug via device_add, Igor Mammedov, 2017/09/12