On Tue, 3 Jun 2025 15:08:49 -0400 Annie Li <annie...@oracle.com> wrote:
> Hello Igor, > > On 6/3/2025 8:31 AM, Igor Mammedov wrote: > > On Wed, 28 May 2025 12:38:34 -0400 > > Annie Li<annie...@oracle.com> wrote: > > > >> The fixed hardware sleep button isn't appropriate for hardware > >> reduced platform. This patch implements the control method sleep > >> button in a separate source file so that the button can be added > >> for various platforms. > >> > >> Co-developed-by: Miguel Luis<miguel.l...@oracle.com> > >> Signed-off-by: Annie Li<annie...@oracle.com> > >> --- > >> hw/acpi/control_method_device.c | 38 +++++++++++++++++++++++++ > >> hw/acpi/meson.build | 1 + > >> include/hw/acpi/control_method_device.h | 21 ++++++++++++++ > >> 3 files changed, 60 insertions(+) > >> > >> diff --git a/hw/acpi/control_method_device.c > >> b/hw/acpi/control_method_device.c > > sleep_button would be more to the point > Was thinking of more control method devices may be added in future, so > choose this general name(control_method_device) just in case. > I'll rename it to control_method_sleep_button then. > > > >> new file mode 100644 > >> index 0000000000..f8d691ee04 > >> --- /dev/null > >> +++ b/hw/acpi/control_method_device.c > >> @@ -0,0 +1,38 @@ > >> +/* > >> + * Control Method Device > >> + * > >> + * Copyright (c) 2023 Oracle and/or its affiliates. > >> + * > >> + * > >> + * Authors: > >> + * Annie Li<annie...@oracle.com> > >> + * > >> + * SPDX-License-Identifier: GPL-2.0-or-later > >> + */ > >> + > >> +#include "qemu/osdep.h" > >> +#include "hw/acpi/control_method_device.h" > >> +#include "hw/acpi/aml-build.h" > >> + > >> +/* > >> + * The control method sleep button[ACPI v6.5 Section 4.8.2.2.2.2] > >> + * resides in generic hardware address spaces. The sleep button > >> + * is defined as _HID("PNP0C0E") that associates with device "SLPB". > >> + */ > >> +void acpi_dsdt_add_sleep_button(Aml *scope) > >> +{ > >> + Aml *dev = aml_device(ACPI_SLEEP_BUTTON_DEVICE); > >> + aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C0E"))); > >> + /* > >> + * No _PRW, the sleep button device is always tied to GPE L07 > >> + * event handler for x86 platform, or a GED event for other > >> + * platforms such as virt, ARM, microvm, etc. > >> + */ > >> + aml_append(dev, aml_operation_region("\\SLP", AML_SYSTEM_IO, > >> + aml_int(0x201), 0x1)); > > ^^^^^^ > > where does this come from? > Got it from an example in ACPI spec[ACPI v6.5 Section 4.8.2.2.2.2]. "• > Creates an operational region for the control method sleep button’s > programming model: System I/O space at 0x201." Any suggestions are welcome. rather than giving answer, I'd ask * what it's supposed to do * does QEMU has this register or its alternative answers to above likely will help with answering my original question or ideas how to fix patch > > > > > >> + Aml *field = aml_field("\\SLP", AML_BYTE_ACC, AML_NOLOCK, > >> + AML_WRITE_AS_ZEROS); > >> + aml_append(field, aml_named_field("SBP", 1)); > >> + aml_append(dev, field); > >> + aml_append(scope, dev); > >> +} > >> diff --git a/hw/acpi/meson.build b/hw/acpi/meson.build > >> index 73f02b9691..a62e625cef 100644 > >> --- a/hw/acpi/meson.build > >> +++ b/hw/acpi/meson.build > >> @@ -17,6 +17,7 @@ acpi_ss.add(when: 'CONFIG_ACPI_CXL', if_true: > >> files('cxl.c'), if_false: files('c > >> acpi_ss.add(when: 'CONFIG_ACPI_VMGENID', if_true: files('vmgenid.c')) > >> acpi_ss.add(when: 'CONFIG_ACPI_VMCLOCK', if_true: files('vmclock.c')) > >> acpi_ss.add(when: 'CONFIG_ACPI_HW_REDUCED', if_true: > >> files('generic_event_device.c')) > >> +acpi_ss.add(when: 'CONFIG_ACPI_HW_REDUCED', if_true: > >> files('control_method_device.c')) > > that would build only for microvm + arm, and pc/q35 wouldn't get it > > if microvm were disabled. > True. > How about the following? > acpi_ss.add(files( > 'acpi_interface.c', > 'aml-build.c', > 'bios-linker-loader.c', > 'core.c', > 'utils.c', > +'control_method_device.c', > )) perhaps this is better > > Thanks > > Annie > > > > > >> acpi_ss.add(when: 'CONFIG_ACPI_HMAT', if_true: files('hmat.c')) > >> acpi_ss.add(when: 'CONFIG_ACPI_APEI', if_true: files('ghes.c'), > >> if_false: files('ghes-stub.c')) > >> acpi_ss.add(when: 'CONFIG_ACPI_PIIX4', if_true: files('piix4.c')) > >> diff --git a/include/hw/acpi/control_method_device.h > >> b/include/hw/acpi/control_method_device.h > >> new file mode 100644 > >> index 0000000000..079f1a74dd > >> --- /dev/null > >> +++ b/include/hw/acpi/control_method_device.h > >> @@ -0,0 +1,21 @@ > >> +/* > >> + * Control Method Device > >> + * > >> + * Copyright (c) 2023 Oracle and/or its affiliates. > >> + * > >> + * > >> + * Authors: > >> + * Annie Li<annie...@oracle.com> > >> + * > >> + * SPDX-License-Identifier: GPL-2.0-or-later > >> + */ > >> + > >> + > >> +#ifndef HW_ACPI_CONTROL_METHOD_DEVICE_H > >> +#define HW_ACPI_CONTROL_NETHOD_DEVICE_H > >> + > >> +#define ACPI_SLEEP_BUTTON_DEVICE "SLPB" > >> + > >> +void acpi_dsdt_add_sleep_button(Aml *scope); > >> + > >> +#endif