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.
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 )
>
>
> 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.
> > >
> >
>
