HI Igor,
> From: Igor Mammedov <imamm...@redhat.com>
> Sent: Tuesday, October 15, 2024 10:31 AM
> To: Salil Mehta <salil.me...@huawei.com>
> Cc: maobibo <maob...@loongson.cn>; qemu-devel@nongnu.org; Michael
> S. Tsirkin <m...@redhat.com>; Peter Maydell <peter.mayd...@linaro.org>;
> zhukeqian <zhukeqi...@huawei.com>; Jonathan Cameron
> <jonathan.came...@huawei.com>; Gavin Shan <gs...@redhat.com>;
> Vishnu Pajjuri <vis...@os.amperecomputing.com>; Xianglai Li
> <lixiang...@loongson.cn>; Miguel Luis <miguel.l...@oracle.com>; Shaoqin
> Huang <shahu...@redhat.com>; Zhao Liu <zhao1....@intel.com>; Ani Sinha
> <anisi...@redhat.com>
> Subject: Re: [PULL v2 40/61] hw/acpi: Update GED _EVT method AML with
> CPU scan
>
> On Mon, 14 Oct 2024 20:05:58 +0000
> Salil Mehta <salil.me...@huawei.com> wrote:
>
> > Hi Igor,
> >
> > > From: qemu-devel-bounces+salil.mehta=huawei....@nongnu.org
> <qemu-
> > > devel-bounces+salil.mehta=huawei....@nongnu.org> On Behalf Of
> Igor
> > > Mammedov
> > > Sent: Monday, October 14, 2024 10:38 AM
> > >
> > > On Mon, 14 Oct 2024 16:52:55 +0800
> > > maobibo <maob...@loongson.cn> wrote:
> > >
> > > > Hi Salil,
> > > >
> > > > When I debug cpu hotplug on LoongArch system, It reports error like
> this:
> > > > ACPI BIOS Error (bug): Could not resolve symbol [\_SB.GED.CSCN],
> > > > AE_NOT_FOUND
> > > > ACPI Error: Aborting method \_SB.GED._EVT due to previous error
> > > > (AE_NOT_FOUND)
> > > > acpi-ged ACPI0013:00: IRQ method execution failed
> > > >
> > > >
> > > > With this patch, GED CPU call method is "\\_SB.GED.CSCN", however
> > > in > function build_cpus_aml(), its method name is "\\_SB.CPUS.CSCN".
> > > > method = aml_method(event_handler_method, 0,
> > > AML_NOTSERIALIZED);
> > > > aml_append(method, aml_call0("\\_SB.CPUS."
> CPU_SCAN_METHOD));
> > > > aml_append(table, method);
> > > >
> > > > It seems that CPU scanning method name is not consistent between
> > > > function build_cpus_aml() and build_ged_aml().
> > > >
> > > > Regards
> > > > Bibo Mao
> > > >
> > > > On 2024/7/23 下午6:59, Michael S. Tsirkin wrote:
> > > > > From: Salil Mehta <salil.me...@huawei.com> > > > > OSPM
> > > evaluates _EVT method to map the event. The CPU hotplug event > >
> > > eventually results in start of the CPU scan. Scan figures out the >
> > > > CPU and the kind of > > event(plug/unplug) and notifies it back
> > > to the guest. Update the GED > > AML _EVT method with the call to
> > > method \\_SB.CPUS.CSCN (via > > \\_SB.GED.CSCN) > > > >
> > > Architecture specific code [1] might initialize its CPUs AML code by
> > > > > calling common function build_cpus_aml() like below for ARM:
> > > > >
> > > > > build_cpus_aml(scope, ms, opts, xx_madt_cpu_entry,
> > > memmap[VIRT_CPUHP_ACPI].base,
> > > > > "\\_SB", "\\_SB.GED.CSCN", AML_SYSTEM_MEMORY);
> > >
> > > it should be \\_SB.CPUS.CSCN
> >
> >
> > I guess we are getting back to where we started then?
> >
> > https://lore.kernel.org/qemu-
> devel/20240706162845.3baf5...@imammedo.us
> > ers.ipa.redhat.com/
> >
>
> Indeed, CSCN in name had me confused,
> perhaps it would be better to rename that something else.
> maybe something like s/_SB.GED.CSCN/_SB.GED.ECPU/
Sure, we can definitely improve. But we are indeed triggering CPU Scan
here so I don’t understand how will `ECPU` be more intuitive than
`CSCN`. what about below?
s/_SB.GED.CSCN/_SB.GED.CPUSCAN/
Thanks
Salil.
>
> > Excerpt from above discussion and your suggestion:
> > [...]
> >
> > I don't particularly like exposing cpu hotplug internals for outside
> > code and then making that code do plumbing hoping that nothing will
> > explode in the future.
> >
> > build_cpus_aml() takes event_handler_method to create a method that
> > can be called by platform. What I suggest is to call that method here
> > instead of trying to expose CPU hotplug internals and manually
> > building call path here.
> > aka:
> > build_cpus_aml(event_handler_method =
> PATH_TO_GED_DEVICE.CSCN) and
> > then call here
> > aml_append(if_ctx, aml_call0(CSCN)); which will call CSCN in GED
> > scope, that was be populated by
> > build_cpus_aml() to do cpu scan properly without need to expose cpu
> > hotplug internal names and then trying to fixup conflicts caused by that.
> >
> > PS:
> > we should do the same for memory hotplug, we see in context above
> >
> > [...]
> >
> >
> > Solution:
> > I've avoided above error in different way and keeping exactly what you
> > suggested \_SB.PATH_TO_GED_DEVICE.CSCN i.e. \_SB.GED.CSCN Please
> have
> > a look:
> >
> > https://lore.kernel.org/qemu-devel/20241009031815.250096-16-salil.meht
> > a...@huawei.com/
> >
> > Many thanks!
> >
> >
> > Best regards
> > Salil.
> >
>
