Re: [PATCH 03/13] acpi: Support Control Method sleep button for x86

[Annie Li <annie.li@oracle.com>]

Hi Igor,

On 6/3/2025 8:52 AM, Igor Mammedov wrote:
> On Wed, 28 May 2025 12:39:17 -0400
> Annie Li <annie.li@oracle.com> wrote:
>
>> Add Control Method Sleep button and its GPE event handler for
>> x86 platform. The GPE event handler notifies OSPM when the
>> Sleep button event is triggered.
>>
>> Signed-off-by: Annie Li <annie.li@oracle.com>
>> ---
>>   hw/i386/acpi-build.c | 24 +++++++++++++++++++++++-
>>   1 file changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 3fffa4a332..2ddf669006 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -40,6 +40,7 @@
>>   #include "hw/acpi/acpi_aml_interface.h"
>>   #include "hw/input/i8042.h"
>>   #include "hw/acpi/memory_hotplug.h"
>> +#include "hw/acpi/control_method_device.h"
>>   #include "system/tpm.h"
>>   #include "hw/acpi/tpm.h"
>>   #include "hw/acpi/vmgenid.h"
>> @@ -1359,7 +1360,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>>                                                        NULL);
>>       Object *q35 = object_resolve_type_unambiguous(TYPE_Q35_HOST_DEVICE, NULL);
>>       CrsRangeEntry *entry;
>> -    Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs;
>> +    Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs, *condition;
>>       CrsRangeSet crs_range_set;
>>       PCMachineState *pcms = PC_MACHINE(machine);
>>       PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine);
>> @@ -1465,6 +1466,27 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>>       }
>>       aml_append(dsdt, scope);
>>   
>> +    sb_scope = aml_scope("_SB");
>> +    acpi_dsdt_add_sleep_button(sb_scope);
>> +    aml_append(dsdt, sb_scope);
>> +
>> +    /*
>> +     * The event handler for the control method sleep button is generated
>> +     * for notifying OSPM (ACPI v6.5, Section 4.8.2.2.2.2).
>> +     */
>> +    scope =  aml_scope("\\_GPE");
>> +    method = aml_method("_L07", 0, AML_NOTSERIALIZED);
>> +    condition = aml_if(aml_name("\\_SB."ACPI_SLEEP_BUTTON_DEVICE".SBP"));
>         s/condition/if_ctx/
> also use full form 'if something == something' for condtion
Implemented based on the spec, will look into it.
>
>> +    aml_append(condition,
>> +               aml_store(aml_int(1),
>> +                         aml_name("\\_SB."ACPI_SLEEP_BUTTON_DEVICE".SBP")));
> so what is handling this write on qemu side?
Qemu only triggers the GPE event for x86 or GED event for reduced
hardware platforms to notify OSPM to go to sleep. However, no writing
operation has been done corresponding to the above clearing.
Per the spec,
"When this bit is set it is the responsibility of the AML code to clear 
it ".
Since GPE/GED events are being sent to notify the OSPM, I am wondering
if handling this write is needed on qemu side? or just removing this
clearing operation above?
> and why it's here to begin with? (commit says that it sends event to OSMP but nothing about this write)

Added this to clear sleep button status per the spec.

Thanks
Annie

>
>> +    aml_append(condition,
>> +               aml_notify(aml_name("\\_SB."ACPI_SLEEP_BUTTON_DEVICE),
>> +                                    aml_int(0x80)));
>> +    aml_append(method, condition);
>> +    aml_append(scope, method);
>> +    aml_append(dsdt, scope);
>> +
>>       if (pcmc->legacy_cpu_hotplug) {
>>           build_legacy_cpu_hotplug_aml(dsdt, machine, pm->cpu_hp_io_base);
>>       } else {