Do not set parallel_cpus if there is only one cpu instantiated.
This will allow tcg to use serial code to implement atomics.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
softmmu/cpus.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index a802e899ab..e3b98065c9 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -1895,6 +1895,16 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
if (!tcg_region_inited) {
tcg_region_inited = 1;
tcg_region_init();
+ /*
+ * If MTTCG, and we will create multiple cpus,
+ * then we will have cpus running in parallel.
+ */
+ if (qemu_tcg_mttcg_enabled()) {
+ MachineState *ms = MACHINE(qdev_get_machine());
+ if (ms->smp.max_cpus > 1) {
+ parallel_cpus = true;
+ }
+ }
}
if (qemu_tcg_mttcg_enabled() || !single_tcg_cpu_thread) {
@@ -1904,7 +1914,6 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
if (qemu_tcg_mttcg_enabled()) {
/* create a thread per vCPU with TCG (MTTCG) */
- parallel_cpus = true;
snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/TCG",
cpu->cpu_index);
--
2.25.1
Hi Richard,
currently rebasing on top of this one,
just a question, why is the patch not directly using "current_machine"?
Is using MACHINE(qdev_get_machine()) preferrable here?
Thanks,
Claudio
On 9/3/20 11:40 PM, Richard Henderson wrote:
> Do not set parallel_cpus if there is only one cpu instantiated.
> This will allow tcg to use serial code to implement atomics.
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> softmmu/cpus.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
> index a802e899ab..e3b98065c9 100644
> --- a/softmmu/cpus.c
> +++ b/softmmu/cpus.c
> @@ -1895,6 +1895,16 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
> if (!tcg_region_inited) {
> tcg_region_inited = 1;
> tcg_region_init();
> + /*
> + * If MTTCG, and we will create multiple cpus,
> + * then we will have cpus running in parallel.
> + */
> + if (qemu_tcg_mttcg_enabled()) {
> + MachineState *ms = MACHINE(qdev_get_machine());
MachineState *ms = current_machine;
?
> + if (ms->smp.max_cpus > 1) {
> + parallel_cpus = true;
> + }
> + }
> }
>
> if (qemu_tcg_mttcg_enabled() || !single_tcg_cpu_thread) {
> @@ -1904,7 +1914,6 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
>
> if (qemu_tcg_mttcg_enabled()) {
> /* create a thread per vCPU with TCG (MTTCG) */
> - parallel_cpus = true;
> snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/TCG",
> cpu->cpu_index);
>
>
On 9/7/20 12:05 PM, Claudio Fontana wrote:
> Hi Richard,
>
> currently rebasing on top of this one,
> just a question, why is the patch not directly using "current_machine"?
For user mode?
>
> Is using MACHINE(qdev_get_machine()) preferrable here?
>
> Thanks,
>
> Claudio
>
> On 9/3/20 11:40 PM, Richard Henderson wrote:
>> Do not set parallel_cpus if there is only one cpu instantiated.
>> This will allow tcg to use serial code to implement atomics.
>>
>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> softmmu/cpus.c | 11 ++++++++++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
>> index a802e899ab..e3b98065c9 100644
>> --- a/softmmu/cpus.c
>> +++ b/softmmu/cpus.c
>> @@ -1895,6 +1895,16 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
>> if (!tcg_region_inited) {
>> tcg_region_inited = 1;
>> tcg_region_init();
>> + /*
>> + * If MTTCG, and we will create multiple cpus,
>> + * then we will have cpus running in parallel.
>> + */
>> + if (qemu_tcg_mttcg_enabled()) {
>> + MachineState *ms = MACHINE(qdev_get_machine());
>
> MachineState *ms = current_machine;
> ?
>
>
>> + if (ms->smp.max_cpus > 1) {
>> + parallel_cpus = true;
>> + }
>> + }
>> }
>>
>> if (qemu_tcg_mttcg_enabled() || !single_tcg_cpu_thread) {
>> @@ -1904,7 +1914,6 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
>>
>> if (qemu_tcg_mttcg_enabled()) {
>> /* create a thread per vCPU with TCG (MTTCG) */
>> - parallel_cpus = true;
>> snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/TCG",
>> cpu->cpu_index);
>>
>>
>
>
On 9/7/20 12:20 PM, Philippe Mathieu-Daudé wrote:
> On 9/7/20 12:05 PM, Claudio Fontana wrote:
>> Hi Richard,
>>
>> currently rebasing on top of this one,
>> just a question, why is the patch not directly using "current_machine"?
>
> For user mode?
In user mode I'd not expect softmmu/cpus.c to be built at all...
Ciao,
Claudio
>
>>
>> Is using MACHINE(qdev_get_machine()) preferrable here?
>>
>> Thanks,
>>
>> Claudio
>>
>> On 9/3/20 11:40 PM, Richard Henderson wrote:
>>> Do not set parallel_cpus if there is only one cpu instantiated.
>>> This will allow tcg to use serial code to implement atomics.
>>>
>>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>> softmmu/cpus.c | 11 ++++++++++-
>>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
>>> index a802e899ab..e3b98065c9 100644
>>> --- a/softmmu/cpus.c
>>> +++ b/softmmu/cpus.c
>>> @@ -1895,6 +1895,16 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
>>> if (!tcg_region_inited) {
>>> tcg_region_inited = 1;
>>> tcg_region_init();
>>> + /*
>>> + * If MTTCG, and we will create multiple cpus,
>>> + * then we will have cpus running in parallel.
>>> + */
>>> + if (qemu_tcg_mttcg_enabled()) {
>>> + MachineState *ms = MACHINE(qdev_get_machine());
>>
>> MachineState *ms = current_machine;
>> ?
>>
>>
>>> + if (ms->smp.max_cpus > 1) {
>>> + parallel_cpus = true;
>>> + }
>>> + }
>>> }
>>>
>>> if (qemu_tcg_mttcg_enabled() || !single_tcg_cpu_thread) {
>>> @@ -1904,7 +1914,6 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
>>>
>>> if (qemu_tcg_mttcg_enabled()) {
>>> /* create a thread per vCPU with TCG (MTTCG) */
>>> - parallel_cpus = true;
>>> snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/TCG",
>>> cpu->cpu_index);
>>>
>>>
>>
>>
On 9/7/20 6:30 PM, Claudio Fontana wrote:
> On 9/7/20 12:20 PM, Philippe Mathieu-Daudé wrote:
>> On 9/7/20 12:05 PM, Claudio Fontana wrote:
>>> Hi Richard,
>>>
>>> currently rebasing on top of this one,
>>> just a question, why is the patch not directly using "current_machine"?
>>
>> For user mode?
>
> In user mode I'd not expect softmmu/cpus.c to be built at all...
Which is why :) current_machine is NULL in user-mode.
>
> Ciao,
>
> Claudio
>
>>
>>>
>>> Is using MACHINE(qdev_get_machine()) preferrable here?
>>>
>>> Thanks,
>>>
>>> Claudio
>>>
>>> On 9/3/20 11:40 PM, Richard Henderson wrote:
>>>> Do not set parallel_cpus if there is only one cpu instantiated.
>>>> This will allow tcg to use serial code to implement atomics.
>>>>
>>>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>>> ---
>>>> softmmu/cpus.c | 11 ++++++++++-
>>>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
>>>> index a802e899ab..e3b98065c9 100644
>>>> --- a/softmmu/cpus.c
>>>> +++ b/softmmu/cpus.c
>>>> @@ -1895,6 +1895,16 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
>>>> if (!tcg_region_inited) {
>>>> tcg_region_inited = 1;
>>>> tcg_region_init();
>>>> + /*
>>>> + * If MTTCG, and we will create multiple cpus,
>>>> + * then we will have cpus running in parallel.
>>>> + */
>>>> + if (qemu_tcg_mttcg_enabled()) {
>>>> + MachineState *ms = MACHINE(qdev_get_machine());
>>>
>>> MachineState *ms = current_machine;
>>> ?
>>>
>>>
>>>> + if (ms->smp.max_cpus > 1) {
>>>> + parallel_cpus = true;
>>>> + }
>>>> + }
>>>> }
>>>>
>>>> if (qemu_tcg_mttcg_enabled() || !single_tcg_cpu_thread) {
>>>> @@ -1904,7 +1914,6 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
>>>>
>>>> if (qemu_tcg_mttcg_enabled()) {
>>>> /* create a thread per vCPU with TCG (MTTCG) */
>>>> - parallel_cpus = true;
>>>> snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/TCG",
>>>> cpu->cpu_index);
>>>>
>>>>
>>>
>>>
>
>
On 9/7/20 6:49 PM, Philippe Mathieu-Daudé wrote:
> On 9/7/20 6:30 PM, Claudio Fontana wrote:
>> On 9/7/20 12:20 PM, Philippe Mathieu-Daudé wrote:
>>> On 9/7/20 12:05 PM, Claudio Fontana wrote:
>>>> Hi Richard,
>>>>
>>>> currently rebasing on top of this one,
>>>> just a question, why is the patch not directly using "current_machine"?
>>>
>>> For user mode?
>>
>> In user mode I'd not expect softmmu/cpus.c to be built at all...
>
> Which is why :) current_machine is NULL in user-mode.
Ciao Philippe,
then why does the patch change softmmu/cpus.c in a way that accounts for user mode?
The function that the patch changes is never called in user mode.
The patch could instead use current_machine without any concern of it being NULL, it will always be set in vl.c .
Ciao,
Claudio
>
>>
>> Ciao,
>>
>> Claudio
>>
>>>
>>>>
>>>> Is using MACHINE(qdev_get_machine()) preferrable here?
>>>>
>>>> Thanks,
>>>>
>>>> Claudio
>>>>
>>>> On 9/3/20 11:40 PM, Richard Henderson wrote:
>>>>> Do not set parallel_cpus if there is only one cpu instantiated.
>>>>> This will allow tcg to use serial code to implement atomics.
>>>>>
>>>>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>>>> ---
>>>>> softmmu/cpus.c | 11 ++++++++++-
>>>>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
>>>>> index a802e899ab..e3b98065c9 100644
>>>>> --- a/softmmu/cpus.c
>>>>> +++ b/softmmu/cpus.c
>>>>> @@ -1895,6 +1895,16 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
>>>>> if (!tcg_region_inited) {
>>>>> tcg_region_inited = 1;
>>>>> tcg_region_init();
>>>>> + /*
>>>>> + * If MTTCG, and we will create multiple cpus,
>>>>> + * then we will have cpus running in parallel.
>>>>> + */
>>>>> + if (qemu_tcg_mttcg_enabled()) {
>>>>> + MachineState *ms = MACHINE(qdev_get_machine());
>>>>
>>>> MachineState *ms = current_machine;
>>>> ?
>>>>
>>>>
>>>>> + if (ms->smp.max_cpus > 1) {
>>>>> + parallel_cpus = true;
>>>>> + }
>>>>> + }
>>>>> }
>>>>>
>>>>> if (qemu_tcg_mttcg_enabled() || !single_tcg_cpu_thread) {
>>>>> @@ -1904,7 +1914,6 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
>>>>>
>>>>> if (qemu_tcg_mttcg_enabled()) {
>>>>> /* create a thread per vCPU with TCG (MTTCG) */
>>>>> - parallel_cpus = true;
>>>>> snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/TCG",
>>>>> cpu->cpu_index);
>>>>>
>>>>>
>>>>
>>>>
>>
>>
+Laurent
On 9/8/20 9:09 AM, Claudio Fontana wrote:
> On 9/7/20 6:49 PM, Philippe Mathieu-Daudé wrote:
>> On 9/7/20 6:30 PM, Claudio Fontana wrote:
>>> On 9/7/20 12:20 PM, Philippe Mathieu-Daudé wrote:
>>>> On 9/7/20 12:05 PM, Claudio Fontana wrote:
>>>>> Hi Richard,
>>>>>
>>>>> currently rebasing on top of this one,
>>>>> just a question, why is the patch not directly using "current_machine"?
>>>>
>>>> For user mode?
>>>
>>> In user mode I'd not expect softmmu/cpus.c to be built at all...
>>
>> Which is why :) current_machine is NULL in user-mode.
>
> Ciao Philippe,
>
> then why does the patch change softmmu/cpus.c in a way that accounts for user mode?
>
> The function that the patch changes is never called in user mode.
>
> The patch could instead use current_machine without any concern of it being NULL, it will always be set in vl.c .
Better send a patch to prove your point :)
I have bad remember about the last time I tried to understand/modify
that part, because IIRC the user-mode creates some fake machine to
satisfy various QEMU generic code assumptions. Sincerely I now prefer
stay away from this; too many unmerged patches there.
>
> Ciao,
>
> Claudio
>
>>
>>>
>>> Ciao,
>>>
>>> Claudio
>>>
>>>>
>>>>>
>>>>> Is using MACHINE(qdev_get_machine()) preferrable here?
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Claudio
>>>>>
>>>>> On 9/3/20 11:40 PM, Richard Henderson wrote:
>>>>>> Do not set parallel_cpus if there is only one cpu instantiated.
>>>>>> This will allow tcg to use serial code to implement atomics.
>>>>>>
>>>>>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>>>>> ---
>>>>>> softmmu/cpus.c | 11 ++++++++++-
>>>>>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
>>>>>> index a802e899ab..e3b98065c9 100644
>>>>>> --- a/softmmu/cpus.c
>>>>>> +++ b/softmmu/cpus.c
>>>>>> @@ -1895,6 +1895,16 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
>>>>>> if (!tcg_region_inited) {
>>>>>> tcg_region_inited = 1;
>>>>>> tcg_region_init();
>>>>>> + /*
>>>>>> + * If MTTCG, and we will create multiple cpus,
>>>>>> + * then we will have cpus running in parallel.
>>>>>> + */
>>>>>> + if (qemu_tcg_mttcg_enabled()) {
>>>>>> + MachineState *ms = MACHINE(qdev_get_machine());
>>>>>
>>>>> MachineState *ms = current_machine;
>>>>> ?
>>>>>
>>>>>
>>>>>> + if (ms->smp.max_cpus > 1) {
>>>>>> + parallel_cpus = true;
>>>>>> + }
>>>>>> + }
>>>>>> }
>>>>>>
>>>>>> if (qemu_tcg_mttcg_enabled() || !single_tcg_cpu_thread) {
>>>>>> @@ -1904,7 +1914,6 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
>>>>>>
>>>>>> if (qemu_tcg_mttcg_enabled()) {
>>>>>> /* create a thread per vCPU with TCG (MTTCG) */
>>>>>> - parallel_cpus = true;
>>>>>> snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/TCG",
>>>>>> cpu->cpu_index);
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>
>>>
>
>
Hi Philippe,
On 9/8/20 9:56 AM, Philippe Mathieu-Daudé wrote:
> +Laurent
>
> On 9/8/20 9:09 AM, Claudio Fontana wrote:
>> On 9/7/20 6:49 PM, Philippe Mathieu-Daudé wrote:
>>> On 9/7/20 6:30 PM, Claudio Fontana wrote:
>>>> On 9/7/20 12:20 PM, Philippe Mathieu-Daudé wrote:
>>>>> On 9/7/20 12:05 PM, Claudio Fontana wrote:
>>>>>> Hi Richard,
>>>>>>
>>>>>> currently rebasing on top of this one,
>>>>>> just a question, why is the patch not directly using "current_machine"?
>>>>>
>>>>> For user mode?
>>>>
>>>> In user mode I'd not expect softmmu/cpus.c to be built at all...
>>>
>>> Which is why :) current_machine is NULL in user-mode.
>>
>> Ciao Philippe,
>>
>> then why does the patch change softmmu/cpus.c in a way that accounts for user mode?
>>
>> The function that the patch changes is never called in user mode.
>>
>> The patch could instead use current_machine without any concern of it being NULL, it will always be set in vl.c .
>
> Better send a patch to prove your point :)
Yes, I am already testing without problems the cpus-refactoring series with this applied:
commit 53f6db772f1522025650441102b16be17773bdc9
Author: Claudio Fontana <cfontana@suse.de>
Date: Tue Sep 8 10:59:07 2020 +0200
accel/tcg: use current_machine as it is always set for softmmu
current_machine is always set before accelerators are initialized,
so use that instead of MACHINE(qdev_get_machine()).
Signed-off-by: Claudio Fontana <cfontana@suse.de>
diff --git a/accel/tcg/tcg-cpus.c b/accel/tcg/tcg-cpus.c
index ec7158b55e..05af1168a2 100644
--- a/accel/tcg/tcg-cpus.c
+++ b/accel/tcg/tcg-cpus.c
@@ -484,7 +484,7 @@ static void tcg_start_vcpu_thread(CPUState *cpu)
* then we will have cpus running in parallel.
*/
if (qemu_tcg_mttcg_enabled()) {
- MachineState *ms = MACHINE(qdev_get_machine());
+ MachineState *ms = current_machine;
if (ms->smp.max_cpus > 1) {
parallel_cpus = true;
}
>
> I have bad remember about the last time I tried to understand/modify
> that part, because IIRC the user-mode creates some fake machine to
> satisfy various QEMU generic code assumptions. Sincerely I now prefer
> stay away from this; too many unmerged patches there.
linux-user/main.c and bsd-user/main.c seem to use cpu_create() to create the cpus,
then they have their own cpu_loop(), they do not use any of the cpus.c code.
By contrast, softmmu/vl.c initializes current_machine in qemu_init(),
even before accelerator is chosen and configured.
So there is no chance currently that current_machine is not set correctly when
the accelerator vcpu startup code in is called.
Ciao,
CLaudio
>
>>
>> Ciao,
>>
>> Claudio
>>
>>>
>>>>
>>>> Ciao,
>>>>
>>>> Claudio
>>>>
>>>>>
>>>>>>
>>>>>> Is using MACHINE(qdev_get_machine()) preferrable here?
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Claudio
>>>>>>
>>>>>> On 9/3/20 11:40 PM, Richard Henderson wrote:
>>>>>>> Do not set parallel_cpus if there is only one cpu instantiated.
>>>>>>> This will allow tcg to use serial code to implement atomics.
>>>>>>>
>>>>>>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>>>>>> ---
>>>>>>> softmmu/cpus.c | 11 ++++++++++-
>>>>>>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
>>>>>>> index a802e899ab..e3b98065c9 100644
>>>>>>> --- a/softmmu/cpus.c
>>>>>>> +++ b/softmmu/cpus.c
>>>>>>> @@ -1895,6 +1895,16 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
>>>>>>> if (!tcg_region_inited) {
>>>>>>> tcg_region_inited = 1;
>>>>>>> tcg_region_init();
>>>>>>> + /*
>>>>>>> + * If MTTCG, and we will create multiple cpus,
>>>>>>> + * then we will have cpus running in parallel.
>>>>>>> + */
>>>>>>> + if (qemu_tcg_mttcg_enabled()) {
>>>>>>> + MachineState *ms = MACHINE(qdev_get_machine());
>>>>>>
>>>>>> MachineState *ms = current_machine;
>>>>>> ?
>>>>>>
>>>>>>
>>>>>>> + if (ms->smp.max_cpus > 1) {
>>>>>>> + parallel_cpus = true;
>>>>>>> + }
>>>>>>> + }
>>>>>>> }
>>>>>>>
>>>>>>> if (qemu_tcg_mttcg_enabled() || !single_tcg_cpu_thread) {
>>>>>>> @@ -1904,7 +1914,6 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
>>>>>>>
>>>>>>> if (qemu_tcg_mttcg_enabled()) {
>>>>>>> /* create a thread per vCPU with TCG (MTTCG) */
>>>>>>> - parallel_cpus = true;
>>>>>>> snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/TCG",
>>>>>>> cpu->cpu_index);
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>>
>>
On 9/10/20 10:28 AM, Claudio Fontana wrote:
> Hi Philippe,
>
> On 9/8/20 9:56 AM, Philippe Mathieu-Daudé wrote:
>> +Laurent
>>
>> On 9/8/20 9:09 AM, Claudio Fontana wrote:
>>> On 9/7/20 6:49 PM, Philippe Mathieu-Daudé wrote:
>>>> On 9/7/20 6:30 PM, Claudio Fontana wrote:
>>>>> On 9/7/20 12:20 PM, Philippe Mathieu-Daudé wrote:
>>>>>> On 9/7/20 12:05 PM, Claudio Fontana wrote:
>>>>>>> Hi Richard,
>>>>>>>
>>>>>>> currently rebasing on top of this one,
>>>>>>> just a question, why is the patch not directly using "current_machine"?
>>>>>>
>>>>>> For user mode?
>>>>>
>>>>> In user mode I'd not expect softmmu/cpus.c to be built at all...
>>>>
>>>> Which is why :) current_machine is NULL in user-mode.
>>>
>>> Ciao Philippe,
>>>
>>> then why does the patch change softmmu/cpus.c in a way that accounts for user mode?
>>>
>>> The function that the patch changes is never called in user mode.
>>>
>>> The patch could instead use current_machine without any concern of it being NULL, it will always be set in vl.c .
>>
>> Better send a patch to prove your point :)
>
> Yes, I am already testing without problems the cpus-refactoring series with this applied:
>
> commit 53f6db772f1522025650441102b16be17773bdc9
> Author: Claudio Fontana <cfontana@suse.de>
> Date: Tue Sep 8 10:59:07 2020 +0200
>
> accel/tcg: use current_machine as it is always set for softmmu
>
> current_machine is always set before accelerators are initialized,
> so use that instead of MACHINE(qdev_get_machine()).
>
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>
> diff --git a/accel/tcg/tcg-cpus.c b/accel/tcg/tcg-cpus.c
> index ec7158b55e..05af1168a2 100644
> --- a/accel/tcg/tcg-cpus.c
> +++ b/accel/tcg/tcg-cpus.c
> @@ -484,7 +484,7 @@ static void tcg_start_vcpu_thread(CPUState *cpu)
> * then we will have cpus running in parallel.
> */
> if (qemu_tcg_mttcg_enabled()) {
> - MachineState *ms = MACHINE(qdev_get_machine());
> + MachineState *ms = current_machine;
> if (ms->smp.max_cpus > 1) {
> parallel_cpus = true;
> }
>
>
>
>>
>> I have bad remember about the last time I tried to understand/modify
>> that part, because IIRC the user-mode creates some fake machine to
>> satisfy various QEMU generic code assumptions. Sincerely I now prefer
>> stay away from this; too many unmerged patches there.
>
>
> linux-user/main.c and bsd-user/main.c seem to use cpu_create() to create the cpus,
> then they have their own cpu_loop(), they do not use any of the cpus.c code.
>
> By contrast, softmmu/vl.c initializes current_machine in qemu_init(),
> even before accelerator is chosen and configured.
>
> So there is no chance currently that current_machine is not set correctly when
> the accelerator vcpu startup code in is called.
Thanks for checking and correcting me!
We are good then :)
>
> Ciao,
>
> CLaudio
© 2016 - 2025 Red Hat, Inc.