Hi Igor, > From: Igor Mammedov <[email protected]> > Sent: Tuesday, October 15, 2024 3:34 PM > To: Salil Mehta <[email protected]> > > On Tue, 15 Oct 2024 09:41:24 +0000 > Salil Mehta <[email protected]> wrote: > > > HI Igor, > > > > > From: Igor Mammedov <[email protected]> > > > Sent: Tuesday, October 15, 2024 10:31 AM > > > To: Salil Mehta <[email protected]> > > > Cc: maobibo <[email protected]>; [email protected]; > Michael > > > S. Tsirkin <[email protected]>; Peter Maydell > > > <[email protected]>; zhukeqian <[email protected]>; > > > Jonathan Cameron <[email protected]>; Gavin Shan > > > <[email protected]>; Vishnu Pajjuri > <[email protected]>; > > > Xianglai Li <[email protected]>; Miguel Luis > > > <[email protected]>; Shaoqin Huang <[email protected]>; > Zhao > > > Liu <[email protected]>; Ani Sinha <[email protected]> > > > 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 <[email protected]> wrote: > > > > > > > Hi Igor, > > > > > > > > > From: [email protected] > > > <qemu- > > > > > [email protected]> On Behalf > Of > > > Igor > > Mammedov > > Sent: Monday, October 14, 2024 10:38 AM > > > > > > > On Mon, 14 Oct 2024 16:52:55 +0800 > > maobibo > > > <[email protected]> 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 <[email protected]> > > > > 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/[email protected] > > > > 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 > > > > [email protected]/ > > > > > > > > Many thanks! > > > > > > > > > > > > Best regards > > > > Salil. > > > > > > > > > >
