| Message ID | 20240911030922.877259-2-maobibo@loongson.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add FDT table support with acpi ged pm register | expand |
On Wed, 11 Sep 2024 11:09:21 +0800 Bibo Mao <maobibo@loongson.cn> wrote: > Macro definition is added for acpi sleep control register, so that > ged emulation driver can use this, also it can be used in FDT table if > ged is exposed with FDT table. > > Signed-off-by: Bibo Mao <maobibo@loongson.cn> > --- > hw/acpi/generic_event_device.c | 6 +++--- > hw/i386/acpi-microvm.c | 2 +- > hw/loongarch/acpi-build.c | 2 +- > include/hw/acpi/generic_event_device.h | 9 +++++++-- > 4 files changed, 12 insertions(+), 7 deletions(-) > > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c > index 15b4c3ebbf..94992e6119 100644 > --- a/hw/acpi/generic_event_device.c > +++ b/hw/acpi/generic_event_device.c > @@ -201,9 +201,9 @@ static void ged_regs_write(void *opaque, hwaddr addr, uint64_t data, > > switch (addr) { > case ACPI_GED_REG_SLEEP_CTL: > - slp_typ = (data >> 2) & 0x07; > - slp_en = (data >> 5) & 0x01; > - if (slp_en && slp_typ == 5) { > + slp_typ = (data & ACPI_GED_SLP_TYPx_MASK) >> ACPI_GED_SLP_TYPx_POS; this makes a bit more complex expression once macros are expanded, but doesn't really helps to clarity. If I have to touch/share this code, I'd replace magic numbers above with corresponding simple numeric macro but keep the same expressions. > + slp_en = !!(data & ACPI_GED_SLP_EN); > + if (slp_en && slp_typ == ACPI_GED_SLP_TYPx_S5) { > qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); > } > return; > diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c > index 279da6b4aa..1e424076d2 100644 > --- a/hw/i386/acpi-microvm.c > +++ b/hw/i386/acpi-microvm.c > @@ -131,7 +131,7 @@ build_dsdt_microvm(GArray *table_data, BIOSLinker *linker, > /* ACPI 5.0: Table 7-209 System State Package */ > scope = aml_scope("\\"); > pkg = aml_package(4); > - aml_append(pkg, aml_int(ACPI_GED_SLP_TYP_S5)); > + aml_append(pkg, aml_int(ACPI_GED_SLP_TYPx_S5)); what's the point of renaming this? > aml_append(pkg, aml_int(0)); /* ignored */ > aml_append(pkg, aml_int(0)); /* reserved */ > aml_append(pkg, aml_int(0)); /* reserved */ > diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c > index 2638f87434..974519a347 100644 > --- a/hw/loongarch/acpi-build.c > +++ b/hw/loongarch/acpi-build.c > @@ -418,7 +418,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, MachineState *machine) > /* System State Package */ > scope = aml_scope("\\"); > pkg = aml_package(4); > - aml_append(pkg, aml_int(ACPI_GED_SLP_TYP_S5)); > + aml_append(pkg, aml_int(ACPI_GED_SLP_TYPx_S5)); > aml_append(pkg, aml_int(0)); /* ignored */ > aml_append(pkg, aml_int(0)); /* reserved */ > aml_append(pkg, aml_int(0)); /* reserved */ > diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h > index 40af3550b5..41741e94ea 100644 > --- a/include/hw/acpi/generic_event_device.h > +++ b/include/hw/acpi/generic_event_device.h > @@ -81,8 +81,13 @@ OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED) > /* ACPI_GED_REG_RESET value for reset*/ > #define ACPI_GED_RESET_VALUE 0x42 > > -/* ACPI_GED_REG_SLEEP_CTL.SLP_TYP value for S5 (aka poweroff) */ > -#define ACPI_GED_SLP_TYP_S5 0x05 > +/* [ACPI 5.0+ FADT] Sleep Control Register */ > +/* 3-bit field defines the type of hardware sleep state */ > +#define ACPI_GED_SLP_TYPx_POS 0x2 > +#define ACPI_GED_SLP_TYPx_MASK (0x07 << ACPI_GED_SLP_TYPx_POS) > +#define ACPI_GED_SLP_TYPx_S5 0x05 /* System \_S5 State (Soft Off) */ > +/* Write-only, Set this bit causes system to enter SLP_TYPx sleeping state */ > +#define ACPI_GED_SLP_EN 0x20 > > #define GED_DEVICE "GED" > #define AML_GED_EVT_REG "EREG"
On 2024/9/13 下午8:41, Igor Mammedov wrote: > On Wed, 11 Sep 2024 11:09:21 +0800 > Bibo Mao <maobibo@loongson.cn> wrote: > >> Macro definition is added for acpi sleep control register, so that >> ged emulation driver can use this, also it can be used in FDT table if >> ged is exposed with FDT table. >> >> Signed-off-by: Bibo Mao <maobibo@loongson.cn> >> --- >> hw/acpi/generic_event_device.c | 6 +++--- >> hw/i386/acpi-microvm.c | 2 +- >> hw/loongarch/acpi-build.c | 2 +- >> include/hw/acpi/generic_event_device.h | 9 +++++++-- >> 4 files changed, 12 insertions(+), 7 deletions(-) >> >> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c >> index 15b4c3ebbf..94992e6119 100644 >> --- a/hw/acpi/generic_event_device.c >> +++ b/hw/acpi/generic_event_device.c >> @@ -201,9 +201,9 @@ static void ged_regs_write(void *opaque, hwaddr addr, uint64_t data, >> >> switch (addr) { >> case ACPI_GED_REG_SLEEP_CTL: >> - slp_typ = (data >> 2) & 0x07; >> - slp_en = (data >> 5) & 0x01; >> - if (slp_en && slp_typ == 5) { >> + slp_typ = (data & ACPI_GED_SLP_TYPx_MASK) >> ACPI_GED_SLP_TYPx_POS; > this makes a bit more complex expression once macros are expanded, > but doesn't really helps to clarity. > > If I have to touch/share this code, I'd replace magic numbers above > with corresponding simple numeric macro but keep the same expressions. That sounds reasonable, it is better to keep the same expression such as: slp_typ = (data >> ACPI_GED_SLP_TYPx_POS) & ACPI_GED_SLP_TYPx_MASK; However what about for this sentence? slp_en = (data >> 5) & 0x01; I think the modification like this is better slp_en = !!(data & ACPI_GED_SLP_EN); > >> + slp_en = !!(data & ACPI_GED_SLP_EN); >> + if (slp_en && slp_typ == ACPI_GED_SLP_TYPx_S5) { >> qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); >> } >> return; >> diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c >> index 279da6b4aa..1e424076d2 100644 >> --- a/hw/i386/acpi-microvm.c >> +++ b/hw/i386/acpi-microvm.c >> @@ -131,7 +131,7 @@ build_dsdt_microvm(GArray *table_data, BIOSLinker *linker, >> /* ACPI 5.0: Table 7-209 System State Package */ >> scope = aml_scope("\\"); >> pkg = aml_package(4); >> - aml_append(pkg, aml_int(ACPI_GED_SLP_TYP_S5)); >> + aml_append(pkg, aml_int(ACPI_GED_SLP_TYPx_S5)); > > what's the point of renaming this? ACPI spec set name with SLP_TYPx. I am ok with both, it seems less modification is better. Regards Bibo Mao > >> aml_append(pkg, aml_int(0)); /* ignored */ >> aml_append(pkg, aml_int(0)); /* reserved */ >> aml_append(pkg, aml_int(0)); /* reserved */ >> diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c >> index 2638f87434..974519a347 100644 >> --- a/hw/loongarch/acpi-build.c >> +++ b/hw/loongarch/acpi-build.c >> @@ -418,7 +418,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, MachineState *machine) >> /* System State Package */ >> scope = aml_scope("\\"); >> pkg = aml_package(4); >> - aml_append(pkg, aml_int(ACPI_GED_SLP_TYP_S5)); >> + aml_append(pkg, aml_int(ACPI_GED_SLP_TYPx_S5)); >> aml_append(pkg, aml_int(0)); /* ignored */ >> aml_append(pkg, aml_int(0)); /* reserved */ >> aml_append(pkg, aml_int(0)); /* reserved */ >> diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h >> index 40af3550b5..41741e94ea 100644 >> --- a/include/hw/acpi/generic_event_device.h >> +++ b/include/hw/acpi/generic_event_device.h >> @@ -81,8 +81,13 @@ OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED) >> /* ACPI_GED_REG_RESET value for reset*/ >> #define ACPI_GED_RESET_VALUE 0x42 >> >> -/* ACPI_GED_REG_SLEEP_CTL.SLP_TYP value for S5 (aka poweroff) */ >> -#define ACPI_GED_SLP_TYP_S5 0x05 >> +/* [ACPI 5.0+ FADT] Sleep Control Register */ >> +/* 3-bit field defines the type of hardware sleep state */ >> +#define ACPI_GED_SLP_TYPx_POS 0x2 >> +#define ACPI_GED_SLP_TYPx_MASK (0x07 << ACPI_GED_SLP_TYPx_POS) >> +#define ACPI_GED_SLP_TYPx_S5 0x05 /* System \_S5 State (Soft Off) */ >> +/* Write-only, Set this bit causes system to enter SLP_TYPx sleeping state */ >> +#define ACPI_GED_SLP_EN 0x20 >> >> #define GED_DEVICE "GED" >> #define AML_GED_EVT_REG "EREG" >
On Sat, 14 Sep 2024 10:25:45 +0800 maobibo <maobibo@loongson.cn> wrote: > On 2024/9/13 下午8:41, Igor Mammedov wrote: > > On Wed, 11 Sep 2024 11:09:21 +0800 > > Bibo Mao <maobibo@loongson.cn> wrote: > > > >> Macro definition is added for acpi sleep control register, so that > >> ged emulation driver can use this, also it can be used in FDT table if > >> ged is exposed with FDT table. > >> > >> Signed-off-by: Bibo Mao <maobibo@loongson.cn> > >> --- > >> hw/acpi/generic_event_device.c | 6 +++--- > >> hw/i386/acpi-microvm.c | 2 +- > >> hw/loongarch/acpi-build.c | 2 +- > >> include/hw/acpi/generic_event_device.h | 9 +++++++-- > >> 4 files changed, 12 insertions(+), 7 deletions(-) > >> > >> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c > >> index 15b4c3ebbf..94992e6119 100644 > >> --- a/hw/acpi/generic_event_device.c > >> +++ b/hw/acpi/generic_event_device.c > >> @@ -201,9 +201,9 @@ static void ged_regs_write(void *opaque, hwaddr addr, uint64_t data, > >> > >> switch (addr) { > >> case ACPI_GED_REG_SLEEP_CTL: > >> - slp_typ = (data >> 2) & 0x07; > >> - slp_en = (data >> 5) & 0x01; > >> - if (slp_en && slp_typ == 5) { > >> + slp_typ = (data & ACPI_GED_SLP_TYPx_MASK) >> ACPI_GED_SLP_TYPx_POS; > > this makes a bit more complex expression once macros are expanded, > > but doesn't really helps to clarity. > > > > If I have to touch/share this code, I'd replace magic numbers above > > with corresponding simple numeric macro but keep the same expressions. > That sounds reasonable, it is better to keep the same expression such as: > slp_typ = (data >> ACPI_GED_SLP_TYPx_POS) & ACPI_GED_SLP_TYPx_MASK; > > However what about for this sentence? > slp_en = (data >> 5) & 0x01; > I think the modification like this is better > slp_en = !!(data & ACPI_GED_SLP_EN); then one has to got and check what ACPI_GED_SLP_EN is and why it's that specific value. while keeping it as is would be consistent with slp_typ line right above it. But it's stylistic, I don't really care wrt it. > > > > >> + slp_en = !!(data & ACPI_GED_SLP_EN); > >> + if (slp_en && slp_typ == ACPI_GED_SLP_TYPx_S5) { > >> qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); > >> } > >> return; > >> diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c > >> index 279da6b4aa..1e424076d2 100644 > >> --- a/hw/i386/acpi-microvm.c > >> +++ b/hw/i386/acpi-microvm.c > >> @@ -131,7 +131,7 @@ build_dsdt_microvm(GArray *table_data, BIOSLinker *linker, > >> /* ACPI 5.0: Table 7-209 System State Package */ > >> scope = aml_scope("\\"); > >> pkg = aml_package(4); > >> - aml_append(pkg, aml_int(ACPI_GED_SLP_TYP_S5)); > >> + aml_append(pkg, aml_int(ACPI_GED_SLP_TYPx_S5)); > > > > what's the point of renaming this? > ACPI spec set name with SLP_TYPx. I am ok with both, it seems less > modification is better. I'd avoid renaming, if one need to reference spec we usually add a comment about field/value that points to earliest spec where it was introduced and chapter in it. Also for fields we also add a comment with _verbatim_ field name from spec, so that one can copy/past/search it in spec when needed. > > Regards > Bibo Mao > > > >> aml_append(pkg, aml_int(0)); /* ignored */ > >> aml_append(pkg, aml_int(0)); /* reserved */ > >> aml_append(pkg, aml_int(0)); /* reserved */ > >> diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c > >> index 2638f87434..974519a347 100644 > >> --- a/hw/loongarch/acpi-build.c > >> +++ b/hw/loongarch/acpi-build.c > >> @@ -418,7 +418,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, MachineState *machine) > >> /* System State Package */ > >> scope = aml_scope("\\"); > >> pkg = aml_package(4); > >> - aml_append(pkg, aml_int(ACPI_GED_SLP_TYP_S5)); > >> + aml_append(pkg, aml_int(ACPI_GED_SLP_TYPx_S5)); > >> aml_append(pkg, aml_int(0)); /* ignored */ > >> aml_append(pkg, aml_int(0)); /* reserved */ > >> aml_append(pkg, aml_int(0)); /* reserved */ > >> diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h > >> index 40af3550b5..41741e94ea 100644 > >> --- a/include/hw/acpi/generic_event_device.h > >> +++ b/include/hw/acpi/generic_event_device.h > >> @@ -81,8 +81,13 @@ OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED) > >> /* ACPI_GED_REG_RESET value for reset*/ > >> #define ACPI_GED_RESET_VALUE 0x42 > >> > >> -/* ACPI_GED_REG_SLEEP_CTL.SLP_TYP value for S5 (aka poweroff) */ > >> -#define ACPI_GED_SLP_TYP_S5 0x05 > >> +/* [ACPI 5.0+ FADT] Sleep Control Register */ > >> +/* 3-bit field defines the type of hardware sleep state */ > >> +#define ACPI_GED_SLP_TYPx_POS 0x2 > >> +#define ACPI_GED_SLP_TYPx_MASK (0x07 << ACPI_GED_SLP_TYPx_POS) > >> +#define ACPI_GED_SLP_TYPx_S5 0x05 /* System \_S5 State (Soft Off) */ > >> +/* Write-only, Set this bit causes system to enter SLP_TYPx sleeping state */ > >> +#define ACPI_GED_SLP_EN 0x20 > >> > >> #define GED_DEVICE "GED" > >> #define AML_GED_EVT_REG "EREG" > > >
On 2024/9/17 下午3:44, Igor Mammedov wrote: > On Sat, 14 Sep 2024 10:25:45 +0800 > maobibo <maobibo@loongson.cn> wrote: > >> On 2024/9/13 下午8:41, Igor Mammedov wrote: >>> On Wed, 11 Sep 2024 11:09:21 +0800 >>> Bibo Mao <maobibo@loongson.cn> wrote: >>> >>>> Macro definition is added for acpi sleep control register, so that >>>> ged emulation driver can use this, also it can be used in FDT table if >>>> ged is exposed with FDT table. >>>> >>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn> >>>> --- >>>> hw/acpi/generic_event_device.c | 6 +++--- >>>> hw/i386/acpi-microvm.c | 2 +- >>>> hw/loongarch/acpi-build.c | 2 +- >>>> include/hw/acpi/generic_event_device.h | 9 +++++++-- >>>> 4 files changed, 12 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c >>>> index 15b4c3ebbf..94992e6119 100644 >>>> --- a/hw/acpi/generic_event_device.c >>>> +++ b/hw/acpi/generic_event_device.c >>>> @@ -201,9 +201,9 @@ static void ged_regs_write(void *opaque, hwaddr addr, uint64_t data, >>>> >>>> switch (addr) { >>>> case ACPI_GED_REG_SLEEP_CTL: >>>> - slp_typ = (data >> 2) & 0x07; >>>> - slp_en = (data >> 5) & 0x01; >>>> - if (slp_en && slp_typ == 5) { >>>> + slp_typ = (data & ACPI_GED_SLP_TYPx_MASK) >> ACPI_GED_SLP_TYPx_POS; >>> this makes a bit more complex expression once macros are expanded, >>> but doesn't really helps to clarity. >>> >>> If I have to touch/share this code, I'd replace magic numbers above >>> with corresponding simple numeric macro but keep the same expressions. >> That sounds reasonable, it is better to keep the same expression such as: >> slp_typ = (data >> ACPI_GED_SLP_TYPx_POS) & ACPI_GED_SLP_TYPx_MASK; >> >> However what about for this sentence? >> slp_en = (data >> 5) & 0x01; >> I think the modification like this is better >> slp_en = !!(data & ACPI_GED_SLP_EN); > > then one has to got and check what ACPI_GED_SLP_EN is > and why it's that specific value. > while keeping it as is would be consistent with slp_typ > line right above it. > But it's stylistic, I don't really care wrt it. > >> >>> >>>> + slp_en = !!(data & ACPI_GED_SLP_EN); >>>> + if (slp_en && slp_typ == ACPI_GED_SLP_TYPx_S5) { >>>> qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); >>>> } >>>> return; >>>> diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c >>>> index 279da6b4aa..1e424076d2 100644 >>>> --- a/hw/i386/acpi-microvm.c >>>> +++ b/hw/i386/acpi-microvm.c >>>> @@ -131,7 +131,7 @@ build_dsdt_microvm(GArray *table_data, BIOSLinker *linker, >>>> /* ACPI 5.0: Table 7-209 System State Package */ >>>> scope = aml_scope("\\"); >>>> pkg = aml_package(4); >>>> - aml_append(pkg, aml_int(ACPI_GED_SLP_TYP_S5)); >>>> + aml_append(pkg, aml_int(ACPI_GED_SLP_TYPx_S5)); >>> >>> what's the point of renaming this? >> ACPI spec set name with SLP_TYPx. I am ok with both, it seems less >> modification is better. > > I'd avoid renaming, if one need to reference spec we usually add > a comment about field/value that points to earliest spec where it > was introduced and chapter in it. Also for fields we also add > a comment with _verbatim_ field name from spec, so that one > can copy/past/search it in spec when needed. Got it, thanks for your detailed explanation. Will refresh it in the next patch. Regards Bibo Mao > > >> >> Regards >> Bibo Mao >>> >>>> aml_append(pkg, aml_int(0)); /* ignored */ >>>> aml_append(pkg, aml_int(0)); /* reserved */ >>>> aml_append(pkg, aml_int(0)); /* reserved */ >>>> diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c >>>> index 2638f87434..974519a347 100644 >>>> --- a/hw/loongarch/acpi-build.c >>>> +++ b/hw/loongarch/acpi-build.c >>>> @@ -418,7 +418,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, MachineState *machine) >>>> /* System State Package */ >>>> scope = aml_scope("\\"); >>>> pkg = aml_package(4); >>>> - aml_append(pkg, aml_int(ACPI_GED_SLP_TYP_S5)); >>>> + aml_append(pkg, aml_int(ACPI_GED_SLP_TYPx_S5)); >>>> aml_append(pkg, aml_int(0)); /* ignored */ >>>> aml_append(pkg, aml_int(0)); /* reserved */ >>>> aml_append(pkg, aml_int(0)); /* reserved */ >>>> diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h >>>> index 40af3550b5..41741e94ea 100644 >>>> --- a/include/hw/acpi/generic_event_device.h >>>> +++ b/include/hw/acpi/generic_event_device.h >>>> @@ -81,8 +81,13 @@ OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED) >>>> /* ACPI_GED_REG_RESET value for reset*/ >>>> #define ACPI_GED_RESET_VALUE 0x42 >>>> >>>> -/* ACPI_GED_REG_SLEEP_CTL.SLP_TYP value for S5 (aka poweroff) */ >>>> -#define ACPI_GED_SLP_TYP_S5 0x05 >>>> +/* [ACPI 5.0+ FADT] Sleep Control Register */ >>>> +/* 3-bit field defines the type of hardware sleep state */ >>>> +#define ACPI_GED_SLP_TYPx_POS 0x2 >>>> +#define ACPI_GED_SLP_TYPx_MASK (0x07 << ACPI_GED_SLP_TYPx_POS) >>>> +#define ACPI_GED_SLP_TYPx_S5 0x05 /* System \_S5 State (Soft Off) */ >>>> +/* Write-only, Set this bit causes system to enter SLP_TYPx sleeping state */ >>>> +#define ACPI_GED_SLP_EN 0x20 >>>> >>>> #define GED_DEVICE "GED" >>>> #define AML_GED_EVT_REG "EREG" >>> >>
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c index 15b4c3ebbf..94992e6119 100644 --- a/hw/acpi/generic_event_device.c +++ b/hw/acpi/generic_event_device.c @@ -201,9 +201,9 @@ static void ged_regs_write(void *opaque, hwaddr addr, uint64_t data, switch (addr) { case ACPI_GED_REG_SLEEP_CTL: - slp_typ = (data >> 2) & 0x07; - slp_en = (data >> 5) & 0x01; - if (slp_en && slp_typ == 5) { + slp_typ = (data & ACPI_GED_SLP_TYPx_MASK) >> ACPI_GED_SLP_TYPx_POS; + slp_en = !!(data & ACPI_GED_SLP_EN); + if (slp_en && slp_typ == ACPI_GED_SLP_TYPx_S5) { qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); } return; diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c index 279da6b4aa..1e424076d2 100644 --- a/hw/i386/acpi-microvm.c +++ b/hw/i386/acpi-microvm.c @@ -131,7 +131,7 @@ build_dsdt_microvm(GArray *table_data, BIOSLinker *linker, /* ACPI 5.0: Table 7-209 System State Package */ scope = aml_scope("\\"); pkg = aml_package(4); - aml_append(pkg, aml_int(ACPI_GED_SLP_TYP_S5)); + aml_append(pkg, aml_int(ACPI_GED_SLP_TYPx_S5)); aml_append(pkg, aml_int(0)); /* ignored */ aml_append(pkg, aml_int(0)); /* reserved */ aml_append(pkg, aml_int(0)); /* reserved */ diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c index 2638f87434..974519a347 100644 --- a/hw/loongarch/acpi-build.c +++ b/hw/loongarch/acpi-build.c @@ -418,7 +418,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, MachineState *machine) /* System State Package */ scope = aml_scope("\\"); pkg = aml_package(4); - aml_append(pkg, aml_int(ACPI_GED_SLP_TYP_S5)); + aml_append(pkg, aml_int(ACPI_GED_SLP_TYPx_S5)); aml_append(pkg, aml_int(0)); /* ignored */ aml_append(pkg, aml_int(0)); /* reserved */ aml_append(pkg, aml_int(0)); /* reserved */ diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h index 40af3550b5..41741e94ea 100644 --- a/include/hw/acpi/generic_event_device.h +++ b/include/hw/acpi/generic_event_device.h @@ -81,8 +81,13 @@ OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED) /* ACPI_GED_REG_RESET value for reset*/ #define ACPI_GED_RESET_VALUE 0x42 -/* ACPI_GED_REG_SLEEP_CTL.SLP_TYP value for S5 (aka poweroff) */ -#define ACPI_GED_SLP_TYP_S5 0x05 +/* [ACPI 5.0+ FADT] Sleep Control Register */ +/* 3-bit field defines the type of hardware sleep state */ +#define ACPI_GED_SLP_TYPx_POS 0x2 +#define ACPI_GED_SLP_TYPx_MASK (0x07 << ACPI_GED_SLP_TYPx_POS) +#define ACPI_GED_SLP_TYPx_S5 0x05 /* System \_S5 State (Soft Off) */ +/* Write-only, Set this bit causes system to enter SLP_TYPx sleeping state */ +#define ACPI_GED_SLP_EN 0x20 #define GED_DEVICE "GED" #define AML_GED_EVT_REG "EREG"
Macro definition is added for acpi sleep control register, so that ged emulation driver can use this, also it can be used in FDT table if ged is exposed with FDT table. Signed-off-by: Bibo Mao <maobibo@loongson.cn> --- hw/acpi/generic_event_device.c | 6 +++--- hw/i386/acpi-microvm.c | 2 +- hw/loongarch/acpi-build.c | 2 +- include/hw/acpi/generic_event_device.h | 9 +++++++-- 4 files changed, 12 insertions(+), 7 deletions(-)