> From: Zhao Liu <zhao1....@intel.com> > Sent: Monday, May 6, 2024 10:06 AM > > Hi Salil, > > On Fri, May 03, 2024 at 07:59:32PM +0000, Salil Mehta wrote: > > Date: Fri, 3 May 2024 19:59:32 +0000 > > From: Salil Mehta <salil.me...@huawei.com> > > Subject: RE: [PATCH V8 3/8] hw/acpi: Update ACPI GED framework to > > support vCPU Hotplug > > > > Hello, > > > > Sorry, I missed this earlier. > > > > > From: Zhao Liu <zhao1....@intel.com> > > > Sent: Wednesday, March 13, 2024 6:14 AM > > > To: Salil Mehta <salil.me...@huawei.com> > > > > > > Hi Salil, > > > > > > It seems my comment [1] in v7 was missed, but I still hit the same > > > issue. Pls let me paste the previous comment here again. > > > > > > [1]: https://lore.kernel.org/qemu- devel/zxcqp32ggifvu...@intel.com/ > > > > Yes, I have this in my mind. > > > > > > > > [snip] > > > > > > > @@ -400,6 +411,12 @@ static void acpi_ged_initfn(Object *obj) > > > > memory_region_init_io(&ged_st->regs, obj, &ged_regs_ops, ged_st, > > > > TYPE_ACPI_GED "-regs", ACPI_GED_REG_COUNT); > > > > sysbus_init_mmio(sbd, &ged_st->regs); > > > > + > > > > + memory_region_init(&s->container_cpuhp, OBJECT(dev), "cpuhp > > > container", > > > > + ACPI_CPU_HOTPLUG_REG_LEN); > > > > + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container_cpuhp); > > > > + cpu_hotplug_hw_init(&s->container_cpuhp, OBJECT(dev), > > > > + &s->cpuhp_state, 0); > > > > } > > > > > > > > > > I find this cpu_hotplug_hw_init() can still cause qtest errors (for > > > v8) on x86 platforms as you mentioned in v6: > > > > https://lore.kernel.org/qemu-devel/15e70616-6abb-63a4-17d0-820f4a254...@opnsrc.net/T/#m108f102b2fe92b7dd7218f2f942f7b233a9d6af3 > > > > > > IIUC, microvm machine has its own 'possible_cpus_arch_ids' and that > > > is inherited from its parent x86 machine. > > > > > > The above error is because device-introspect-test sets the none-machine: > > > > > > # starting QEMU: exec ./qemu-system-i386 -qtest unix:/tmp/qtest- > > > 3094820.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest- > > > 3094820.qmp,id=char0 -mon chardev=char0,mode=control -display none > > > - audio none -nodefaults -machine none -accel qtest > > > > > > So what about just checking mc->possible_cpu_arch_ids instead of an > > > assert in cpu_hotplug_hw_init()? > > > > > > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c index > > > 4b24a2500361..303f1f1f57bc 100644 > > > --- a/hw/acpi/cpu.c > > > +++ b/hw/acpi/cpu.c > > > @@ -221,7 +221,10 @@ void cpu_hotplug_hw_init(MemoryRegion *as, > > > Object *owner, > > > const CPUArchIdList *id_list; > > > int i; > > > > > > - assert(mc->possible_cpu_arch_ids); > > > + if (!mc->possible_cpu_arch_ids) { > > > + return; > > > + } > > > + > > > > > > Yes, we can do this with some debug print or trace maybe. > > Here it is just to return early without touching mc->possible_cpu_arch_ids(). > If you adopt this workaround, then in the meantime I suggest adding a > comment to this "if" to clarify that it is for compatibility with certain > machines that do not implement mc->possible_cpu_arch_ids().
sure. > > > > id_list = mc->possible_cpu_arch_ids(machine); > > > state->dev_count = id_list->len; > > > state->devs = g_new0(typeof(*state->devs), state->dev_count); > > > > > > This check seems to be acceptable in the general code path? Not all > > > machines have possible_cpu_arch_ids, after all. > > > > True. BTW, have you tested this with Qtest? > > Yes, by "make check" on x86 platform. This workaround can help us pass the > x86 tests. thanks. Best Salil.
