[PULL 54/61] i386/cpu: Enable SMM cpu address space under KVM

Paolo Bonzini posted 61 patches 2 weeks, 1 day ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Thomas Huth <thuth@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Cameron Esfahani <dirty@apple.com>, Roman Bolshakov <rbolshakov@ddn.com>, Phil Dennis-Jordan <phil@philjordan.eu>, Mads Ynddal <mads@ynddal.dk>, Riku Voipio <riku.voipio@iki.fi>, Warner Losh <imp@bsdimp.com>, Kyle Evans <kevans@freebsd.org>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Yanan Wang <wangyanan55@huawei.com>, Zhao Liu <zhao1.liu@intel.com>, Nicholas Piggin <npiggin@gmail.com>, Chinmay Rath <rathc@linux.ibm.com>, Harsh Prateek Bora <harshpb@linux.ibm.com>, "Michael S. Tsirkin" <mst@redhat.com>, Laurent Vivier <laurent@vivier.eu>, Brian Cain <brian.cain@oss.qualcomm.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, John Snow <jsnow@redhat.com>, Cleber Rosa <crosa@redhat.com>, Manos Pitsidianakis <manos.pitsidianakis@linaro.org>, Peter Xu <peterx@redhat.com>, David Hildenbrand <david@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Michael Rolnik <mrolnik@gmail.com>, Marcelo Tosatti <mtosatti@redhat.com>, Reinoud Zandijk <reinoud@netbsd.org>, Sunil Muthuswamy <sunilmut@microsoft.com>, Stafford Horne <shorne@gmail.com>, Yoshinori Sato <yoshinori.sato@nifty.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Artyom Tarasenko <atar4qemu@gmail.com>
There is a newer version of this series
[PULL 54/61] i386/cpu: Enable SMM cpu address space under KVM
Posted by Paolo Bonzini 2 weeks, 1 day ago
From: Xiaoyao Li <xiaoyao.li@intel.com>

Kirill Martynov reported assertation in cpu_asidx_from_attrs() being hit
when x86_cpu_dump_state() is called to dump the CPU state[*]. It happens
when the CPU is in SMM and KVM emulation failure due to misbehaving
guest.

The root cause is that QEMU i386 never enables the SMM address space for
cpu since KVM SMM support has been added.

Enable the SMM cpu address space under KVM when the SMM is enabled for
the x86machine.

[*] https://lore.kernel.org/qemu-devel/20250523154431.506993-1-stdcalllevi@yandex-team.ru/

Reported-by: Kirill Martynov <stdcalllevi@yandex-team.ru>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Tested-by: Kirill Martynov <stdcalllevi@yandex-team.ru>
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
Link: https://lore.kernel.org/r/20250730095253.1833411-2-xiaoyao.li@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 system/physmem.c          |  5 -----
 target/i386/kvm/kvm-cpu.c | 10 ++++++++++
 target/i386/kvm/kvm.c     |  5 +++++
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/system/physmem.c b/system/physmem.c
index 311011156c7..a12c7ea1956 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -793,9 +793,6 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
         cpu->as = as;
     }
 
-    /* KVM cannot currently support multiple address spaces. */
-    assert(asidx == 0 || !kvm_enabled());
-
     if (!cpu->cpu_ases) {
         cpu->cpu_ases = g_new0(CPUAddressSpace, cpu->num_ases);
         cpu->cpu_ases_count = cpu->num_ases;
@@ -818,8 +815,6 @@ void cpu_address_space_destroy(CPUState *cpu, int asidx)
 
     assert(cpu->cpu_ases);
     assert(asidx >= 0 && asidx < cpu->num_ases);
-    /* KVM cannot currently support multiple address spaces. */
-    assert(asidx == 0 || !kvm_enabled());
 
     cpuas = &cpu->cpu_ases[asidx];
     if (tcg_enabled()) {
diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
index 89a79536594..1dc1ba9b486 100644
--- a/target/i386/kvm/kvm-cpu.c
+++ b/target/i386/kvm/kvm-cpu.c
@@ -13,6 +13,7 @@
 #include "qapi/error.h"
 #include "system/system.h"
 #include "hw/boards.h"
+#include "hw/i386/x86.h"
 
 #include "kvm_i386.h"
 #include "accel/accel-cpu-target.h"
@@ -91,6 +92,15 @@ static bool kvm_cpu_realizefn(CPUState *cs, Error **errp)
         kvm_set_guest_phys_bits(cs);
     }
 
+    /*
+     * When SMM is enabled, there is 2 address spaces. Otherwise only 1.
+     *
+     * Only initialize address space 0 here, the second one for SMM is
+     * initialized at register_smram_listener() after machine init done.
+     */
+    cs->num_ases = x86_machine_is_smm_enabled(X86_MACHINE(current_machine)) ? 2 : 1;
+    cpu_address_space_init(cs, 0, "cpu-memory", cs->memory);
+
     return true;
 }
 
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 34e74f24470..d191d7177f1 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -2704,6 +2704,7 @@ static MemoryRegion smram_as_mem;
 
 static void register_smram_listener(Notifier *n, void *unused)
 {
+    CPUState *cpu;
     MemoryRegion *smram =
         (MemoryRegion *) object_resolve_path("/machine/smram", NULL);
 
@@ -2728,6 +2729,10 @@ static void register_smram_listener(Notifier *n, void *unused)
     address_space_init(&smram_address_space, &smram_as_root, "KVM-SMRAM");
     kvm_memory_listener_register(kvm_state, &smram_listener,
                                  &smram_address_space, 1, "kvm-smram");
+
+    CPU_FOREACH(cpu) {
+        cpu_address_space_init(cpu, 1, "cpu-smm", &smram_as_root);
+    }
 }
 
 static void *kvm_msr_energy_thread(void *data)
-- 
2.51.0
Re: [PULL 54/61] i386/cpu: Enable SMM cpu address space under KVM
Posted by Peter Maydell 1 day, 22 hours ago
On Sat, 13 Sept 2025 at 09:25, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> From: Xiaoyao Li <xiaoyao.li@intel.com>
>
> Kirill Martynov reported assertation in cpu_asidx_from_attrs() being hit
> when x86_cpu_dump_state() is called to dump the CPU state[*]. It happens
> when the CPU is in SMM and KVM emulation failure due to misbehaving
> guest.
>
> The root cause is that QEMU i386 never enables the SMM address space for
> cpu since KVM SMM support has been added.
>
> Enable the SMM cpu address space under KVM when the SMM is enabled for
> the x86machine.
>
> [*] https://lore.kernel.org/qemu-devel/20250523154431.506993-1-stdcalllevi@yandex-team.ru/

Hi; I just noticed that this change seems to not account
for x86 CPU hotplug...

> diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
> index 89a79536594..1dc1ba9b486 100644
> --- a/target/i386/kvm/kvm-cpu.c
> +++ b/target/i386/kvm/kvm-cpu.c
> @@ -13,6 +13,7 @@
>  #include "qapi/error.h"
>  #include "system/system.h"
>  #include "hw/boards.h"
> +#include "hw/i386/x86.h"
>
>  #include "kvm_i386.h"
>  #include "accel/accel-cpu-target.h"
> @@ -91,6 +92,15 @@ static bool kvm_cpu_realizefn(CPUState *cs, Error **errp)
>          kvm_set_guest_phys_bits(cs);
>      }
>
> +    /*
> +     * When SMM is enabled, there is 2 address spaces. Otherwise only 1.
> +     *
> +     * Only initialize address space 0 here, the second one for SMM is
> +     * initialized at register_smram_listener() after machine init done.
> +     */
> +    cs->num_ases = x86_machine_is_smm_enabled(X86_MACHINE(current_machine)) ? 2 : 1;
> +    cpu_address_space_init(cs, 0, "cpu-memory", cs->memory);

In the KVM CPU realizefn we call cpu_address_space_init()
for AS 0. This happens both for the CPUs that exist with
QEMU starts up and also for any hotplugged CPU.

> +
>      return true;
>  }
>
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index 34e74f24470..d191d7177f1 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -2704,6 +2704,7 @@ static MemoryRegion smram_as_mem;
>
>  static void register_smram_listener(Notifier *n, void *unused)
>  {
> +    CPUState *cpu;
>      MemoryRegion *smram =
>          (MemoryRegion *) object_resolve_path("/machine/smram", NULL);
>
> @@ -2728,6 +2729,10 @@ static void register_smram_listener(Notifier *n, void *unused)
>      address_space_init(&smram_address_space, &smram_as_root, "KVM-SMRAM");
>      kvm_memory_listener_register(kvm_state, &smram_listener,
>                                   &smram_address_space, 1, "kvm-smram");
> +
> +    CPU_FOREACH(cpu) {
> +        cpu_address_space_init(cpu, 1, "cpu-smm", &smram_as_root);
> +    }
>  }

However, this code is in a machine_init_done notifier, so it
runs only once when QEMU starts up. So the CPUs initially
present on QEMU startup get their AS 1 initialized, but
any CPU hot-plugged later on while QEMU is running will
not ever call cpu_address_space_init() for AS 1.

I saw this with some work-in-progress patches I have that
try to free the AddressSpaces of the CPU (which crash
because the hot-plugged CPU claims to have 2 ASes but
the second one is NULL). You can probably also get a
crash for a variation of the reported crash that this
commit is trying to fix, if the CPU that we try to
x86_cpu_dump_state() for is a hot-plugged one in SMM state.

Where should we be initing the AS for hot-plugged CPUs?

thanks
-- PMM
Re: [PULL 54/61] i386/cpu: Enable SMM cpu address space under KVM
Posted by Xiaoyao Li 9 hours ago
On 9/27/2025 1:48 AM, Peter Maydell wrote:
>> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
>> index 34e74f24470..d191d7177f1 100644
>> --- a/target/i386/kvm/kvm.c
>> +++ b/target/i386/kvm/kvm.c
>> @@ -2704,6 +2704,7 @@ static MemoryRegion smram_as_mem;
>>
>>   static void register_smram_listener(Notifier *n, void *unused)
>>   {
>> +    CPUState *cpu;
>>       MemoryRegion *smram =
>>           (MemoryRegion *) object_resolve_path("/machine/smram", NULL);
>>
>> @@ -2728,6 +2729,10 @@ static void register_smram_listener(Notifier *n, void *unused)
>>       address_space_init(&smram_address_space, &smram_as_root, "KVM-SMRAM");
>>       kvm_memory_listener_register(kvm_state, &smram_listener,
>>                                    &smram_address_space, 1, "kvm-smram");
>> +
>> +    CPU_FOREACH(cpu) {
>> +        cpu_address_space_init(cpu, 1, "cpu-smm", &smram_as_root);
>> +    }
>>   }
> However, this code is in a machine_init_done notifier, so it
> runs only once when QEMU starts up. So the CPUs initially
> present on QEMU startup get their AS 1 initialized, but
> any CPU hot-plugged later on while QEMU is running will
> not ever call cpu_address_space_init() for AS 1.
> 
> I saw this with some work-in-progress patches I have that
> try to free the AddressSpaces of the CPU (which crash
> because the hot-plugged CPU claims to have 2 ASes but
> the second one is NULL). You can probably also get a
> crash for a variation of the reported crash that this
> commit is trying to fix, if the CPU that we try to
> x86_cpu_dump_state() for is a hot-plugged one in SMM state.
> 
> Where should we be initing the AS for hot-plugged CPUs?

I think we can do it inside x86_cpu_plug(). Let me try to cook a patch 
for it.
Re: [PULL 54/61] i386/cpu: Enable SMM cpu address space under KVM
Posted by Michael Tokarev 1 week, 3 days ago
On 13.09.2025 11:09, Paolo Bonzini wrote:
> From: Xiaoyao Li <xiaoyao.li@intel.com>
> 
> Kirill Martynov reported assertation in cpu_asidx_from_attrs() being hit
> when x86_cpu_dump_state() is called to dump the CPU state[*]. It happens
> when the CPU is in SMM and KVM emulation failure due to misbehaving
> guest.
> 
> The root cause is that QEMU i386 never enables the SMM address space for
> cpu since KVM SMM support has been added.
> 
> Enable the SMM cpu address space under KVM when the SMM is enabled for
> the x86machine.
> 
> [*] https://lore.kernel.org/qemu-devel/20250523154431.506993-1-stdcalllevi@yandex-team.ru/

Hi!

Should this one be fixed for stable qemu releases too?

Also, what are the possible implications for migration
of guests between qemu versions with and without this change?

Thanks,

/mjt
Re: [PULL 54/61] i386/cpu: Enable SMM cpu address space under KVM
Posted by Paolo Bonzini 6 days, 1 hour ago
On 9/18/25 18:24, Michael Tokarev wrote:
> On 13.09.2025 11:09, Paolo Bonzini wrote:
>> From: Xiaoyao Li <xiaoyao.li@intel.com>
>>
>> Kirill Martynov reported assertation in cpu_asidx_from_attrs() being hit
>> when x86_cpu_dump_state() is called to dump the CPU state[*]. It happens
>> when the CPU is in SMM and KVM emulation failure due to misbehaving
>> guest.
>>
>> The root cause is that QEMU i386 never enables the SMM address space for
>> cpu since KVM SMM support has been added.
>>
>> Enable the SMM cpu address space under KVM when the SMM is enabled for
>> the x86machine.
>>
>> [*] https://lore.kernel.org/qemu-devel/20250523154431.506993-1- 
>> stdcalllevi@yandex-team.ru/
> 
> Hi!
> 
> Should this one be fixed for stable qemu releases too?

Yes, good idea.

> Also, what are the possible implications for migration
> of guests between qemu versions with and without this change?

No, this only affects code paths in which you would crash before.

Paolo