Hi Igor,
> From: Igor Mammedov <imamm...@redhat.com>
> Sent: Tuesday, October 15, 2024 3:34 PM
> To: Salil Mehta <salil.me...@huawei.com>
>
> On Tue, 15 Oct 2024 09:41:24 +0000
> Salil Mehta <salil.me...@huawei.com> wrote:
>
> > 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/
>
> ACPI name segment is limited to 4 characters only.
I see.
>
> ECPU - Event handler for CPU
> it could be something else though
>
> the point is not confuse it with CSCN (apparently different namespace but
> still it could lead to confusion as above shows )
No problem. I'll send a patch today.
Thanks!
>
> >
> >
> > 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.me
> > > ht
> > > > a...@huawei.com/
> > > >
> > > > Many thanks!
> > > >
> > > >
> > > > Best regards
> > > > Salil.
> > > >
> > >
> >
>
