[PULL 02/18] i386/kvm/cpu: Init SMM cpu address space for hotplugged CPUs

Paolo Bonzini posted 18 patches 2 weeks, 3 days ago
Maintainers: Magnus Kulke <magnus.kulke@linux.microsoft.com>, Wei Liu <wei.liu@kernel.org>, "Gonglei (Arei)" <arei.gonglei@huawei.com>, Zhenwei Pi <pizhenwei@bytedance.com>, "Michael S. Tsirkin" <mst@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Amit Shah <amit@kernel.org>, Stefan Berger <stefanb@linux.vnet.ibm.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Peter Maydell <peter.maydell@linaro.org>, Igor Mitsyanko <i.mitsyanko@gmail.com>, "Clément Chigot" <chigot@adacore.com>, Frederic Konrad <konrad.frederic@yahoo.fr>, Alberto Garcia <berto@igalia.com>, Thomas Huth <huth@tuxfamily.org>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Jason Herne <jjherne@linux.ibm.com>, Yoshinori Sato <yoshinori.sato@nifty.com>, Magnus Damm <magnus.damm@gmail.com>, Nicholas Piggin <npiggin@gmail.com>, Harsh Prateek Bora <harshpb@linux.ibm.com>, "Collin L. Walling" <walling@linux.ibm.com>, Stefano Stabellini <sstabellini@kernel.org>, Anthony PERARD <anthony@xenproject.org>, Paul Durrant <paul@xen.org>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Alistair Francis <alistair@alistair23.me>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Richard Henderson <richard.henderson@linaro.org>, Helge Deller <deller@gmx.de>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Corey Minyard <minyard@acm.org>, Paul Burton <paulburton@kernel.org>, Aleksandar Rikalo <arikalo@gmail.com>, Aurelien Jarno <aurelien@aurel32.net>, Palmer Dabbelt <palmer@dabbelt.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Fam Zheng <fam@euphon.net>, Samuel Thibault <samuel.thibault@ens-lyon.org>, Michael Rolnik <mrolnik@gmail.com>, Antony Pavlov <antonynpavlov@gmail.com>, Joel Stanley <joel@jms.id.au>, Vijai Kumar K <vijai@behindbytes.com>, Samuel Tardieu <sam@rfc1149.net>, Gustavo Romero <gustavo.romero@linaro.org>, Raphael Norwitz <raphael@enfabrica.net>, Stefan Hajnoczi <stefanha@redhat.com>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.com>, Markus Armbruster <armbru@redhat.com>, Fabiano Rosas <farosas@suse.de>, "Dr. David Alan Gilbert" <dave@treblig.org>, Zhang Chen <zhangckid@gmail.com>, Li Zhijian <lizhijian@fujitsu.com>, Jason Wang <jasowang@redhat.com>, Manos Pitsidianakis <manos.pitsidianakis@linaro.org>, John Snow <jsnow@redhat.com>, Cleber Rosa <crosa@redhat.com>, Cameron Esfahani <dirty@apple.com>, Roman Bolshakov <rbolshakov@ddn.com>, Phil Dennis-Jordan <phil@philjordan.eu>, Marcelo Tosatti <mtosatti@redhat.com>, Reinoud Zandijk <reinoud@netbsd.org>, Sunil Muthuswamy <sunilmut@microsoft.com>, Max Filippov <jcmvbkbc@gmail.com>, Lukas Straub <lukasstraub2@web.de>
There is a newer version of this series
[PULL 02/18] i386/kvm/cpu: Init SMM cpu address space for hotplugged CPUs
Posted by Paolo Bonzini 2 weeks, 3 days ago
From: Xiaoyao Li <xiaoyao.li@intel.com>

The SMM cpu address space is initialized in a machine_init_done
notifier. It only runs once when QEMU starts up, which leads to the
issue that for any hotplugged CPU after the machine is ready, SMM
cpu address space doesn't get initialized.

Fix the issue by initializing the SMM cpu address space in x86_cpu_plug()
when the cpu is hotplugged.

Fixes: 591f817d819f ("target/i386: Define enum X86ASIdx for x86's address spaces")
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Closes: https://lore.kernel.org/qemu-devel/CAFEAcA_3kkZ+a5rTZGmK8W5K6J7qpYD31HkvjBnxWr-fGT2h_A@mail.gmail.com/
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
Link: https://lore.kernel.org/r/20251014094216.164306-2-xiaoyao.li@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/kvm/kvm_i386.h |  1 +
 hw/i386/x86-common.c       | 11 +++++++++++
 target/i386/kvm/kvm.c      |  6 ++++++
 3 files changed, 18 insertions(+)

diff --git a/target/i386/kvm/kvm_i386.h b/target/i386/kvm/kvm_i386.h
index 5c908fdd6a5..2b653442f4d 100644
--- a/target/i386/kvm/kvm_i386.h
+++ b/target/i386/kvm/kvm_i386.h
@@ -74,6 +74,7 @@ uint32_t kvm_x86_build_cpuid(CPUX86State *env, struct kvm_cpuid_entry2 *entries,
                              uint32_t cpuid_i);
 #endif /* CONFIG_KVM */
 
+void kvm_smm_cpu_address_space_init(X86CPU *cpu);
 void kvm_pc_setup_irq_routing(bool pci_enabled);
 
 #endif
diff --git a/hw/i386/x86-common.c b/hw/i386/x86-common.c
index 7512be64d67..5716191fff1 100644
--- a/hw/i386/x86-common.c
+++ b/hw/i386/x86-common.c
@@ -183,6 +183,17 @@ void x86_cpu_plug(HotplugHandler *hotplug_dev,
         fw_cfg_modify_i16(x86ms->fw_cfg, FW_CFG_NB_CPUS, x86ms->boot_cpus);
     }
 
+    /*
+     * Non-hotplugged CPUs get their SMM cpu address space initialized in
+     * machine init done notifier: register_smram_listener().
+     *
+     * We need initialize the SMM cpu address space for the hotplugged CPU
+     * specifically.
+     */
+    if (kvm_enabled() && dev->hotplugged && x86_machine_is_smm_enabled(x86ms)) {
+        kvm_smm_cpu_address_space_init(cpu);
+    }
+
     found_cpu = x86_find_cpu_slot(MACHINE(x86ms), cpu->apic_id, NULL);
     found_cpu->cpu = CPU(dev);
 out:
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index f7a6ef650af..4dea1ed8f0f 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -2748,6 +2748,12 @@ static void register_smram_listener(Notifier *n, void *unused)
     }
 }
 
+/* It should only be called in cpu's hotplug callback */
+void kvm_smm_cpu_address_space_init(X86CPU *cpu)
+{
+    cpu_address_space_init(CPU(cpu), X86ASIdx_SMM, "cpu-smm", &smram_as_root);
+}
+
 static void *kvm_msr_energy_thread(void *data)
 {
     KVMState *s = data;
-- 
2.51.1
Re: [PULL 02/18] i386/kvm/cpu: Init SMM cpu address space for hotplugged CPUs
Posted by Michael Tokarev 2 weeks, 1 day ago
On 10/28/25 20:34, Paolo Bonzini wrote:
> From: Xiaoyao Li <xiaoyao.li@intel.com>
> 
> The SMM cpu address space is initialized in a machine_init_done
> notifier. It only runs once when QEMU starts up, which leads to the
> issue that for any hotplugged CPU after the machine is ready, SMM
> cpu address space doesn't get initialized.
> 
> Fix the issue by initializing the SMM cpu address space in x86_cpu_plug()
> when the cpu is hotplugged.
> 
> Fixes: 591f817d819f ("target/i386: Define enum X86ASIdx for x86's address spaces")

How this commit can be fixing 591f817d819f, while technically
591f817d819f is a no-op, - it changed 0s and 1s in a few places
to symbolic names with the same 0s and 1s.

It seems the "Fixes" commit should be something else.
The way it is now, it's confusing.

Thanks,

/mjt
Re: [PULL 02/18] i386/kvm/cpu: Init SMM cpu address space for hotplugged CPUs
Posted by Xiaoyao Li 2 weeks, 1 day ago
On 10/30/2025 3:36 PM, Michael Tokarev wrote:
> On 10/28/25 20:34, Paolo Bonzini wrote:
>> From: Xiaoyao Li <xiaoyao.li@intel.com>
>>
>> The SMM cpu address space is initialized in a machine_init_done
>> notifier. It only runs once when QEMU starts up, which leads to the
>> issue that for any hotplugged CPU after the machine is ready, SMM
>> cpu address space doesn't get initialized.
>>
>> Fix the issue by initializing the SMM cpu address space in x86_cpu_plug()
>> when the cpu is hotplugged.
>>
>> Fixes: 591f817d819f ("target/i386: Define enum X86ASIdx for x86's 
>> address spaces")
> 
> How this commit can be fixing 591f817d819f, while technically
> 591f817d819f is a no-op, - it changed 0s and 1s in a few places
> to symbolic names with the same 0s and 1s.
> 
> It seems the "Fixes" commit should be something else.
> The way it is now, it's confusing.

It should be

   0516f4b70264 ("i386/cpu: Enable SMM cpu address space under KVM")

Sorry for my carelessness. Is there a way to remedy as the patch has 
been merged into the master?

> Thanks,
> 
> /mjt
>
Re: [PULL 02/18] i386/kvm/cpu: Init SMM cpu address space for hotplugged CPUs
Posted by Michael Tokarev 2 weeks, 1 day ago
On 10/30/25 10:49, Xiaoyao Li wrote:
> On 10/30/2025 3:36 PM, Michael Tokarev wrote:
>> On 10/28/25 20:34, Paolo Bonzini wrote:
>>> From: Xiaoyao Li <xiaoyao.li@intel.com>
>>>
>>> The SMM cpu address space is initialized in a machine_init_done
>>> notifier. It only runs once when QEMU starts up, which leads to the
>>> issue that for any hotplugged CPU after the machine is ready, SMM
>>> cpu address space doesn't get initialized.
>>>
>>> Fix the issue by initializing the SMM cpu address space in 
>>> x86_cpu_plug()
>>> when the cpu is hotplugged.
>>>
>>> Fixes: 591f817d819f ("target/i386: Define enum X86ASIdx for x86's 
>>> address spaces")
>>
>> How this commit can be fixing 591f817d819f, while technically
>> 591f817d819f is a no-op, - it changed 0s and 1s in a few places
>> to symbolic names with the same 0s and 1s.
>>
>> It seems the "Fixes" commit should be something else.
>> The way it is now, it's confusing.
> 
> It should be
> 
>    0516f4b70264 ("i386/cpu: Enable SMM cpu address space under KVM")
> 
> Sorry for my carelessness. Is there a way to remedy as the patch has 
> been merged into the master?

I hope it's possible still.

Paolo, please also add Cc: qemu-stable@ in there, -- because the change
which is being fixed here (Enable SMM cpu address space) has been picked
up to the stable series too.  It's best to keep all the pieces :)

Thank you Xiaoyao, Paolo!

/mjt

Re: [PULL 02/18] i386/kvm/cpu: Init SMM cpu address space for hotplugged CPUs
Posted by Zhao Liu 2 weeks, 2 days ago
> diff --git a/hw/i386/x86-common.c b/hw/i386/x86-common.c
> index 7512be64d67..5716191fff1 100644
> --- a/hw/i386/x86-common.c
> +++ b/hw/i386/x86-common.c
> @@ -183,6 +183,17 @@ void x86_cpu_plug(HotplugHandler *hotplug_dev,
>          fw_cfg_modify_i16(x86ms->fw_cfg, FW_CFG_NB_CPUS, x86ms->boot_cpus);
>      }
>  
> +    /*
> +     * Non-hotplugged CPUs get their SMM cpu address space initialized in
> +     * machine init done notifier: register_smram_listener().
> +     *
> +     * We need initialize the SMM cpu address space for the hotplugged CPU
> +     * specifically.
> +     */
> +    if (kvm_enabled() && dev->hotplugged && x86_machine_is_smm_enabled(x86ms)) {
> +        kvm_smm_cpu_address_space_init(cpu);
> +    }
> +

Unfortunately, the original KVM SMM patch caused this bug, but even
more unfortunately, CPU_FOREACH in the machine_done callback is more
fragile than I originally anticipated, now requiring more hack checks to
fix. :-(

IMO, the root of the chaos is that KVM SMM doesn't do this in the CPU
context like TCG did for a long time. I'll find time to sort all this
out.

Regards,
Zhao
Re: [PULL 02/18] i386/kvm/cpu: Init SMM cpu address space for hotplugged CPUs
Posted by Xiaoyao Li 2 weeks, 1 day ago
On 10/29/2025 3:01 PM, Zhao Liu wrote:
>> diff --git a/hw/i386/x86-common.c b/hw/i386/x86-common.c
>> index 7512be64d67..5716191fff1 100644
>> --- a/hw/i386/x86-common.c
>> +++ b/hw/i386/x86-common.c
>> @@ -183,6 +183,17 @@ void x86_cpu_plug(HotplugHandler *hotplug_dev,
>>           fw_cfg_modify_i16(x86ms->fw_cfg, FW_CFG_NB_CPUS, x86ms->boot_cpus);
>>       }
>>   
>> +    /*
>> +     * Non-hotplugged CPUs get their SMM cpu address space initialized in
>> +     * machine init done notifier: register_smram_listener().
>> +     *
>> +     * We need initialize the SMM cpu address space for the hotplugged CPU
>> +     * specifically.
>> +     */
>> +    if (kvm_enabled() && dev->hotplugged && x86_machine_is_smm_enabled(x86ms)) {
>> +        kvm_smm_cpu_address_space_init(cpu);
>> +    }
>> +
> 
> Unfortunately, the original KVM SMM patch caused this bug, but even
> more unfortunately, CPU_FOREACH in the machine_done callback is more
> fragile than I originally anticipated, now requiring more hack checks to
> fix. :-(
> 
> IMO, the root of the chaos is that KVM SMM doesn't do this in the CPU
> context like TCG did for a long time. I'll find time to sort all this
> out.

I had this idea before. It needs to move the initialization of KVM SMM 
MemoryRegion a bit earlier at least. And it seems to require more time 
than I expected, especially the effort to test the SMM functionality 
after the change. So I went with the most straightforward fix.

It will be great if you can find time to clean it up. Looking forward to it.

> Regards,
> Zhao
>