RE: [PULL v2 40/61] hw/acpi: Update GED _EVT method AML with CPU scan

[Salil Mehta via <qemu-devel@nongnu.org>]

Hi Igor,

>  From: qemu-devel-bounces+salil.mehta=huawei.com@nongnu.org <qemu-
>  devel-bounces+salil.mehta=huawei.com@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 <maobibo@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.mehta@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.3baf5568@imammedo.users.ipa.redhat.com/


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.mehta@huawei.com/

Many thanks!


Best regards
Salil.