On 11/07/2024 08:34, Igor Mammedov wrote:
On Thu, 11 Jul 2024 03:29:40 +0000
Salil Mehta <salil.me...@opnsrc.net> wrote:

Hi Igor,


On 06/07/2024 14:28, Igor Mammedov wrote:
On Fri, 7 Jun 2024 12:56:45 +0100
Salil Mehta <salil.me...@huawei.com> wrote:
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 \\_SB.CPUS.CSCN

Also, macro CPU_SCAN_METHOD might be referred in other places like during GED
intialization so it makes sense to have its definition placed in some common
header file like cpu_hotplug.h. But doing this can cause compilation break
because of the conflicting macro definitions present in cpu.c and cpu_hotplug.c
one of the reasons is that you reusing legacy hw/acpi/cpu_hotplug.h,
see below for suggestion.
and because both these files get compiled due to historic reasons of x86 world
i.e. decision to use legacy(GPE.2)/modern(GED) CPU hotplug interface happens
during runtime [1]. To mitigate above, for now, declare a new common macro
ACPI_CPU_SCAN_METHOD for CPU scan method instead.
(This needs a separate discussion later on for clean-up)

Reference:
[1] 
https://lore.kernel.org/qemu-devel/1463496205-251412-24-git-send-email-imamm...@redhat.com/

Co-developed-by: Keqian Zhu <zhukeqi...@huawei.com>
Signed-off-by: Keqian Zhu <zhukeqi...@huawei.com>
Signed-off-by: Salil Mehta <salil.me...@huawei.com>
Reviewed-by: Jonathan Cameron <jonathan.came...@huawei.com>
Reviewed-by: Gavin Shan <gs...@redhat.com>
Tested-by: Vishnu Pajjuri <vis...@os.amperecomputing.com>
Tested-by: Xianglai Li <lixiang...@loongson.cn>
Tested-by: Miguel Luis <miguel.l...@oracle.com>
Reviewed-by: Shaoqin Huang <shahu...@redhat.com>
Tested-by: Zhao Liu <zhao1....@intel.com>

[...]

diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index 2d6e91b124..33addb6275 100644
--- a/hw/acpi/generic_event_device.c
+++ b/hw/acpi/generic_event_device.c
@@ -117,6 +117,9 @@ void build_ged_aml(Aml *table, const char *name, 
HotplugHandler *hotplug_dev,
                             aml_notify(aml_name("\\_SB.NVDR"),
                                        aml_int(0x80)));
                  break;
+            case ACPI_GED_CPU_HOTPLUG_EVT:
+                aml_append(if_ctx, aml_call0("\\_SB.GED.CPEV"));
+                break
              default:
                  /*
                   * Please make sure all the events in ged_supported_events[]
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index f4e366f64f..8b4f422652 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1536,7 +1536,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
              .fw_unplugs_cpu = pm->smi_on_cpu_unplug,
          };
          build_cpus_aml(dsdt, machine, opts, pc_madt_cpu_entry,
-                       pm->cpu_hp_io_base, "\\_SB.PCI0", "\\_GPE._E02");
+                       pm->cpu_hp_io_base, "\\_SB.PCI0", "\\_SB.GED.CPEV");
      }
this way build_cpus_aml() will create and populate with scan
\\_SB.GED.CPEV method.


I see. Perhaps, I failed to realise the semantics in the AML
sense i.e. exporting a call to a method belonging to one scope
to another (\\_SB.CPUS to \\_SB.GED). Yes, with above change
we can avoid calling \\_SB.CPUS.CSCN method direclty from
within GED _EVT. Sorry for the noise.

I will do the change.

Thanks


PS:
For legacy cphp handling acpi/cpu.c has only _INI method
that is created when opts.has_legacy_cphp is true.
we should be able to get rid of it when 2.6 machine type is removed.
But ARM [or anything else] don't have to be aware of it
if you use static initializer like it's done in hw/i386/acpi-build.c
and just ignore non relevant fields.

ok. thanks for the clarification.

Best regards
Salil.

Reply via email to