On Sat, 13 Sept 2025 at 09:25, Paolo Bonzini <[email protected]> wrote:
>
> From: Xiaoyao Li <[email protected]>
>
> 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/[email protected]/
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
