2024-01-18 06:29:51

by Sunil V L

[permalink] [raw]
Subject: [PATCH v3 -next 0/3] RISC-V: ACPI: Add LPI support

This series adds support for Low Power Idle (LPI) on ACPI based
platforms.

LPI is described in the ACPI spec [1]. RISC-V FFH spec required to
enable this is available at [2].

[1] - https://uefi.org/specs/ACPI/6.5/08_Processor_Configuration_and_Control.html#lpi-low-power-idle-states
[2] - https://github.com/riscv-non-isa/riscv-acpi-ffh/releases/download/v/riscv-ffh.pdf

Changes since v2:
1) Added "riscv_" prefix for functions made non static (Feedback from Drew)
2) Added RB tags from Drew.

Changes since v1:
1) Reordered the commits such that the patch which enables
ACPI_PROCESSOR is the last one in the series.
2) Used GENMASK and other changes to address Drew's comments.
3) Moved common functions required by both DT and ACPI based cpuidle
drivers from the DT driver to common arch/riscv/suspend.c.
4) ACPI cpuidle driver is added under drivers/acpi/riscv
5) Rebased to latest for-next branch of linux-riscv.


Sunil V L (3):
cpuidle: RISC-V: Move few functions to arch/riscv
ACPI: RISC-V: Add LPI driver
ACPI: Enable ACPI_PROCESSOR for RISC-V

arch/riscv/include/asm/suspend.h | 3 ++
arch/riscv/kernel/suspend.c | 49 +++++++++++++++++
drivers/acpi/Kconfig | 2 +-
drivers/acpi/riscv/Makefile | 3 +-
drivers/acpi/riscv/cpuidle.c | 81 +++++++++++++++++++++++++++++
drivers/cpuidle/cpuidle-riscv-sbi.c | 49 ++---------------
6 files changed, 141 insertions(+), 46 deletions(-)
create mode 100644 drivers/acpi/riscv/cpuidle.c

--
2.34.1



2024-01-18 06:30:08

by Sunil V L

[permalink] [raw]
Subject: [PATCH v3 -next 1/3] cpuidle: RISC-V: Move few functions to arch/riscv

To support ACPI Low Power Idle (LPI), few functions are required which
are currently static functions in the DT based cpuidle driver. Hence,
move them under arch/riscv so that ACPI driver also can use them. Since
they are no longer static functions, append "riscv_" prefix to the
function name.

Signed-off-by: Sunil V L <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
---
arch/riscv/include/asm/suspend.h | 3 ++
arch/riscv/kernel/suspend.c | 49 +++++++++++++++++++++++++++++
drivers/cpuidle/cpuidle-riscv-sbi.c | 49 +++--------------------------
3 files changed, 57 insertions(+), 44 deletions(-)

diff --git a/arch/riscv/include/asm/suspend.h b/arch/riscv/include/asm/suspend.h
index 02f87867389a..076f8a9437cf 100644
--- a/arch/riscv/include/asm/suspend.h
+++ b/arch/riscv/include/asm/suspend.h
@@ -55,4 +55,7 @@ int hibernate_resume_nonboot_cpu_disable(void);
asmlinkage void hibernate_restore_image(unsigned long resume_satp, unsigned long satp_temp,
unsigned long cpu_resume);
asmlinkage int hibernate_core_restore_code(void);
+bool riscv_sbi_hsm_is_supported(void);
+bool riscv_sbi_suspend_state_is_valid(u32 state);
+int riscv_sbi_hart_suspend(u32 state);
#endif
diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c
index 239509367e42..b20f2cb5879f 100644
--- a/arch/riscv/kernel/suspend.c
+++ b/arch/riscv/kernel/suspend.c
@@ -128,4 +128,53 @@ static int __init sbi_system_suspend_init(void)
}

arch_initcall(sbi_system_suspend_init);
+
+static int sbi_suspend_finisher(unsigned long suspend_type,
+ unsigned long resume_addr,
+ unsigned long opaque)
+{
+ struct sbiret ret;
+
+ ret = sbi_ecall(SBI_EXT_HSM, SBI_EXT_HSM_HART_SUSPEND,
+ suspend_type, resume_addr, opaque, 0, 0, 0);
+
+ return (ret.error) ? sbi_err_map_linux_errno(ret.error) : 0;
+}
+
+int riscv_sbi_hart_suspend(u32 state)
+{
+ if (state & SBI_HSM_SUSP_NON_RET_BIT)
+ return cpu_suspend(state, sbi_suspend_finisher);
+ else
+ return sbi_suspend_finisher(state, 0, 0);
+}
+
+bool riscv_sbi_suspend_state_is_valid(u32 state)
+{
+ if (state > SBI_HSM_SUSPEND_RET_DEFAULT &&
+ state < SBI_HSM_SUSPEND_RET_PLATFORM)
+ return false;
+
+ if (state > SBI_HSM_SUSPEND_NON_RET_DEFAULT &&
+ state < SBI_HSM_SUSPEND_NON_RET_PLATFORM)
+ return false;
+
+ return true;
+}
+
+bool riscv_sbi_hsm_is_supported(void)
+{
+ /*
+ * The SBI HSM suspend function is only available when:
+ * 1) SBI version is 0.3 or higher
+ * 2) SBI HSM extension is available
+ */
+ if (sbi_spec_version < sbi_mk_version(0, 3) ||
+ !sbi_probe_extension(SBI_EXT_HSM)) {
+ pr_info("HSM suspend not available\n");
+ return false;
+ }
+
+ return true;
+}
#endif /* CONFIG_RISCV_SBI */
diff --git a/drivers/cpuidle/cpuidle-riscv-sbi.c b/drivers/cpuidle/cpuidle-riscv-sbi.c
index e8094fc92491..a6e123dfe394 100644
--- a/drivers/cpuidle/cpuidle-riscv-sbi.c
+++ b/drivers/cpuidle/cpuidle-riscv-sbi.c
@@ -73,26 +73,6 @@ static inline bool sbi_is_domain_state_available(void)
return data->available;
}

-static int sbi_suspend_finisher(unsigned long suspend_type,
- unsigned long resume_addr,
- unsigned long opaque)
-{
- struct sbiret ret;
-
- ret = sbi_ecall(SBI_EXT_HSM, SBI_EXT_HSM_HART_SUSPEND,
- suspend_type, resume_addr, opaque, 0, 0, 0);
-
- return (ret.error) ? sbi_err_map_linux_errno(ret.error) : 0;
-}
-
-static int sbi_suspend(u32 state)
-{
- if (state & SBI_HSM_SUSP_NON_RET_BIT)
- return cpu_suspend(state, sbi_suspend_finisher);
- else
- return sbi_suspend_finisher(state, 0, 0);
-}
-
static __cpuidle int sbi_cpuidle_enter_state(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int idx)
{
@@ -100,9 +80,9 @@ static __cpuidle int sbi_cpuidle_enter_state(struct cpuidle_device *dev,
u32 state = states[idx];

if (state & SBI_HSM_SUSP_NON_RET_BIT)
- return CPU_PM_CPU_IDLE_ENTER_PARAM(sbi_suspend, idx, state);
+ return CPU_PM_CPU_IDLE_ENTER_PARAM(riscv_sbi_hart_suspend, idx, state);
else
- return CPU_PM_CPU_IDLE_ENTER_RETENTION_PARAM(sbi_suspend,
+ return CPU_PM_CPU_IDLE_ENTER_RETENTION_PARAM(riscv_sbi_hart_suspend,
idx, state);
}

@@ -133,7 +113,7 @@ static __cpuidle int __sbi_enter_domain_idle_state(struct cpuidle_device *dev,
else
state = states[idx];

- ret = sbi_suspend(state) ? -1 : idx;
+ ret = riscv_sbi_hart_suspend(state) ? -1 : idx;

ct_cpuidle_exit();

@@ -206,17 +186,6 @@ static const struct of_device_id sbi_cpuidle_state_match[] = {
{ },
};

-static bool sbi_suspend_state_is_valid(u32 state)
-{
- if (state > SBI_HSM_SUSPEND_RET_DEFAULT &&
- state < SBI_HSM_SUSPEND_RET_PLATFORM)
- return false;
- if (state > SBI_HSM_SUSPEND_NON_RET_DEFAULT &&
- state < SBI_HSM_SUSPEND_NON_RET_PLATFORM)
- return false;
- return true;
-}
-
static int sbi_dt_parse_state_node(struct device_node *np, u32 *state)
{
int err = of_property_read_u32(np, "riscv,sbi-suspend-param", state);
@@ -226,7 +195,7 @@ static int sbi_dt_parse_state_node(struct device_node *np, u32 *state)
return err;
}

- if (!sbi_suspend_state_is_valid(*state)) {
+ if (!riscv_sbi_suspend_state_is_valid(*state)) {
pr_warn("Invalid SBI suspend state %#x\n", *state);
return -EINVAL;
}
@@ -607,16 +576,8 @@ static int __init sbi_cpuidle_init(void)
int ret;
struct platform_device *pdev;

- /*
- * The SBI HSM suspend function is only available when:
- * 1) SBI version is 0.3 or higher
- * 2) SBI HSM extension is available
- */
- if ((sbi_spec_version < sbi_mk_version(0, 3)) ||
- !sbi_probe_extension(SBI_EXT_HSM)) {
- pr_info("HSM suspend not available\n");
+ if (!riscv_sbi_hsm_is_supported())
return 0;
- }

ret = platform_driver_register(&sbi_cpuidle_driver);
if (ret)
--
2.34.1


2024-01-18 06:30:25

by Sunil V L

[permalink] [raw]
Subject: [PATCH v3 -next 2/3] ACPI: RISC-V: Add LPI driver

Enable Low Power Idle (LPI) based cpuidle driver for RISC-V platforms.
It depends on SBI HSM calls for idle state transitions.

Signed-off-by: Sunil V L <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
---
drivers/acpi/riscv/Makefile | 3 +-
drivers/acpi/riscv/cpuidle.c | 81 ++++++++++++++++++++++++++++++++++++
2 files changed, 83 insertions(+), 1 deletion(-)
create mode 100644 drivers/acpi/riscv/cpuidle.c

diff --git a/drivers/acpi/riscv/Makefile b/drivers/acpi/riscv/Makefile
index 8b3b126e0b94..7309d92dd477 100644
--- a/drivers/acpi/riscv/Makefile
+++ b/drivers/acpi/riscv/Makefile
@@ -1,2 +1,3 @@
# SPDX-License-Identifier: GPL-2.0-only
-obj-y += rhct.o
+obj-y += rhct.o
+obj-$(CONFIG_ACPI_PROCESSOR_IDLE) += cpuidle.o
diff --git a/drivers/acpi/riscv/cpuidle.c b/drivers/acpi/riscv/cpuidle.c
new file mode 100644
index 000000000000..624f9bbdb58c
--- /dev/null
+++ b/drivers/acpi/riscv/cpuidle.c
@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2024, Ventana Micro Systems Inc
+ * Author: Sunil V L <[email protected]>
+ *
+ */
+
+#include <linux/acpi.h>
+#include <acpi/processor.h>
+#include <linux/cpu_pm.h>
+#include <linux/cpuidle.h>
+#include <linux/suspend.h>
+#include <asm/cpuidle.h>
+#include <asm/sbi.h>
+#include <asm/suspend.h>
+
+#define RISCV_FFH_LPI_TYPE_MASK GENMASK_ULL(63, 60)
+#define RISCV_FFH_LPI_RSVD_MASK GENMASK_ULL(59, 32)
+
+#define RISCV_FFH_LPI_TYPE_SBI BIT_ULL(60)
+
+static int acpi_cpu_init_idle(unsigned int cpu)
+{
+ int i;
+ struct acpi_lpi_state *lpi;
+ struct acpi_processor *pr = per_cpu(processors, cpu);
+
+ if (unlikely(!pr || !pr->flags.has_lpi))
+ return -EINVAL;
+
+ if (!riscv_sbi_hsm_is_supported())
+ return -ENODEV;
+
+ if (pr->power.count <= 1)
+ return -ENODEV;
+
+ for (i = 1; i < pr->power.count; i++) {
+ u32 state;
+
+ lpi = &pr->power.lpi_states[i];
+
+ /*
+ * Validate Entry Method as per FFH spec.
+ * bits[63:60] should be 0x1
+ * bits[59:32] should be 0x0
+ * bits[31:0] represent a SBI power_state
+ */
+ if (((lpi->address & RISCV_FFH_LPI_TYPE_MASK) != RISCV_FFH_LPI_TYPE_SBI) ||
+ (lpi->address & RISCV_FFH_LPI_RSVD_MASK)) {
+ pr_warn("Invalid LPI entry method %#llx\n", lpi->address);
+ return -EINVAL;
+ }
+
+ state = lpi->address;
+ if (!riscv_sbi_suspend_state_is_valid(state)) {
+ pr_warn("Invalid SBI power state %#x\n", state);
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
+int acpi_processor_ffh_lpi_probe(unsigned int cpu)
+{
+ return acpi_cpu_init_idle(cpu);
+}
+
+int acpi_processor_ffh_lpi_enter(struct acpi_lpi_state *lpi)
+{
+ u32 state = lpi->address;
+
+ if (state & SBI_HSM_SUSP_NON_RET_BIT)
+ return CPU_PM_CPU_IDLE_ENTER_PARAM(riscv_sbi_hart_suspend,
+ lpi->index,
+ state);
+ else
+ return CPU_PM_CPU_IDLE_ENTER_RETENTION_PARAM(riscv_sbi_hart_suspend,
+ lpi->index,
+ state);
+}
--
2.34.1


2024-01-18 06:30:41

by Sunil V L

[permalink] [raw]
Subject: [PATCH v3 -next 3/3] ACPI: Enable ACPI_PROCESSOR for RISC-V

The ACPI processor driver is not currently enabled for RISC-V.
This is required to enable CPU related functionalities like
LPI and CPPC. Hence, enable ACPI_PROCESSOR for RISC-V.

Signed-off-by: Sunil V L <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
---
drivers/acpi/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index f819e760ff19..9a920752171c 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -282,7 +282,7 @@ config ACPI_CPPC_LIB

config ACPI_PROCESSOR
tristate "Processor"
- depends on X86 || ARM64 || LOONGARCH
+ depends on X86 || ARM64 || LOONGARCH || RISCV
select ACPI_PROCESSOR_IDLE
select ACPI_CPU_FREQ_PSS if X86 || LOONGARCH
select THERMAL
--
2.34.1


2024-02-15 04:58:51

by Sunil V L

[permalink] [raw]
Subject: Re: [PATCH v3 -next 0/3] RISC-V: ACPI: Add LPI support

Hi Rafel,

On Thu, Jan 18, 2024 at 11:59:27AM +0530, Sunil V L wrote:
> This series adds support for Low Power Idle (LPI) on ACPI based
> platforms.
>
> LPI is described in the ACPI spec [1]. RISC-V FFH spec required to
> enable this is available at [2].
>
> [1] - https://uefi.org/specs/ACPI/6.5/08_Processor_Configuration_and_Control.html#lpi-low-power-idle-states
> [2] - https://github.com/riscv-non-isa/riscv-acpi-ffh/releases/download/v/riscv-ffh.pdf
>
Could you please take a look at this series also and provide ACK if
looks fine?

Thanks,
Sunil
> Changes since v2:
> 1) Added "riscv_" prefix for functions made non static (Feedback from Drew)
> 2) Added RB tags from Drew.
>
> Changes since v1:
> 1) Reordered the commits such that the patch which enables
> ACPI_PROCESSOR is the last one in the series.
> 2) Used GENMASK and other changes to address Drew's comments.
> 3) Moved common functions required by both DT and ACPI based cpuidle
> drivers from the DT driver to common arch/riscv/suspend.c.
> 4) ACPI cpuidle driver is added under drivers/acpi/riscv
> 5) Rebased to latest for-next branch of linux-riscv.
>
>
> Sunil V L (3):
> cpuidle: RISC-V: Move few functions to arch/riscv
> ACPI: RISC-V: Add LPI driver
> ACPI: Enable ACPI_PROCESSOR for RISC-V
>
> arch/riscv/include/asm/suspend.h | 3 ++
> arch/riscv/kernel/suspend.c | 49 +++++++++++++++++
> drivers/acpi/Kconfig | 2 +-
> drivers/acpi/riscv/Makefile | 3 +-
> drivers/acpi/riscv/cpuidle.c | 81 +++++++++++++++++++++++++++++
> drivers/cpuidle/cpuidle-riscv-sbi.c | 49 ++---------------
> 6 files changed, 141 insertions(+), 46 deletions(-)
> create mode 100644 drivers/acpi/riscv/cpuidle.c
>
> --
> 2.34.1
>

2024-03-15 05:26:51

by Drew Fustini

[permalink] [raw]
Subject: Re: [PATCH v3 -next 0/3] RISC-V: ACPI: Add LPI support

On Thu, Jan 18, 2024 at 11:59:27AM +0530, Sunil V L wrote:
> This series adds support for Low Power Idle (LPI) on ACPI based
> platforms.
>
> LPI is described in the ACPI spec [1]. RISC-V FFH spec required to
> enable this is available at [2].

I'm interested in trying out this series. Might you be able to provide
some guidance on how to setup a test environment?

Are there specific branches of qemu and edk2 that I should use?

thanks,
drew

2024-03-15 05:53:45

by Sunil V L

[permalink] [raw]
Subject: Re: [PATCH v3 -next 0/3] RISC-V: ACPI: Add LPI support

Hi Drew,

On Thu, Mar 14, 2024 at 07:59:46PM -0700, Drew Fustini wrote:
> On Thu, Jan 18, 2024 at 11:59:27AM +0530, Sunil V L wrote:
> > This series adds support for Low Power Idle (LPI) on ACPI based
> > platforms.
> >
> > LPI is described in the ACPI spec [1]. RISC-V FFH spec required to
> > enable this is available at [2].
>
> I'm interested in trying out this series. Might you be able to provide
> some guidance on how to setup a test environment?
>
> Are there specific branches of qemu and edk2 that I should use?
>
1) You need LPI objects in the platform. I have added dummy objects for
testing this for qemu virt machine. Please use below branch.

https://github.com/vlsunil/qemu/tree/lpi_exp

Since interrupt controllers are not merged yet in linux, we need to boot
without any IO devices and use only polling based console and ram disk.
Above qemu branch disables IO devices as well.

2) Enable below config options while building linux kernel.
RISCV_SBI_V01
HVC_RISCV_SBI

3) Use upstream EDK2 (RiscVVirt)

4) Boot:
qemu-system-riscv64 \
-M virt,pflash0=pflash0,pflash1=pflash1 \
-m 2G -smp 8 \
-serial mon:stdio \
-blockdev node-name=pflash0,driver=file,read-only=on,filename=RISCV_VIRT_CODE.fd \
-blockdev node-name=pflash1,driver=file,filename=RISCV_VIRT_VARS.fd \
-kernel arch/riscv/boot/Image \
-initrd buildroot/output/images/rootfs.cpio \
-append "root=/dev/ram ro console=hvc earlycon=sbi"

Feel free to ping me if you have any difficulties.

Thanks!
Sunil

2024-03-15 12:32:18

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v3 -next 0/3] RISC-V: ACPI: Add LPI support

On Thu, Feb 15, 2024 at 5:37 AM Sunil V L <[email protected]> wrote:
>
> Hi Rafel,
>
> On Thu, Jan 18, 2024 at 11:59:27AM +0530, Sunil V L wrote:
> > This series adds support for Low Power Idle (LPI) on ACPI based
> > platforms.
> >
> > LPI is described in the ACPI spec [1]. RISC-V FFH spec required to
> > enable this is available at [2].
> >
> > [1] - https://uefi.org/specs/ACPI/6.5/08_Processor_Configuration_and_Control.html#lpi-low-power-idle-states
> > [2] - https://github.com/riscv-non-isa/riscv-acpi-ffh/releases/download/v/riscv-ffh.pdf
> >
> Could you please take a look at this series also and provide ACK if
> looks fine?

I cannot really comment on RISC-V-specific changes.

As for the ACPI Kconfig change, please feel free to add

Acked-by: Rafael J. Wysocki <[email protected]>

to that patch.

2024-03-18 08:43:10

by Sunil V L

[permalink] [raw]
Subject: Re: [PATCH v3 -next 2/3] ACPI: RISC-V: Add LPI driver

On Sun, Mar 17, 2024 at 10:33:15PM -0700, Drew Fustini wrote:
> On Thu, Jan 18, 2024 at 11:59:29AM +0530, Sunil V L wrote:
> > Enable Low Power Idle (LPI) based cpuidle driver for RISC-V platforms.
> > It depends on SBI HSM calls for idle state transitions.
> >
> > Signed-off-by: Sunil V L <[email protected]>
> > Reviewed-by: Andrew Jones <[email protected]>
> > ---
> > drivers/acpi/riscv/Makefile | 3 +-
> > drivers/acpi/riscv/cpuidle.c | 81 ++++++++++++++++++++++++++++++++++++
> > 2 files changed, 83 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/acpi/riscv/cpuidle.c
> >
> > diff --git a/drivers/acpi/riscv/Makefile b/drivers/acpi/riscv/Makefile
> > index 8b3b126e0b94..7309d92dd477 100644
> > --- a/drivers/acpi/riscv/Makefile
> > +++ b/drivers/acpi/riscv/Makefile
> > @@ -1,2 +1,3 @@
> > # SPDX-License-Identifier: GPL-2.0-only
> > -obj-y += rhct.o
> > +obj-y += rhct.o
> > +obj-$(CONFIG_ACPI_PROCESSOR_IDLE) += cpuidle.o
> > diff --git a/drivers/acpi/riscv/cpuidle.c b/drivers/acpi/riscv/cpuidle.c
> > new file mode 100644
> > index 000000000000..624f9bbdb58c
> > --- /dev/null
> > +++ b/drivers/acpi/riscv/cpuidle.c
> > @@ -0,0 +1,81 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2024, Ventana Micro Systems Inc
> > + * Author: Sunil V L <[email protected]>
> > + *
> > + */
> > +
> > +#include <linux/acpi.h>
> > +#include <acpi/processor.h>
> > +#include <linux/cpu_pm.h>
> > +#include <linux/cpuidle.h>
> > +#include <linux/suspend.h>
> > +#include <asm/cpuidle.h>
> > +#include <asm/sbi.h>
> > +#include <asm/suspend.h>
> > +
> > +#define RISCV_FFH_LPI_TYPE_MASK GENMASK_ULL(63, 60)
> > +#define RISCV_FFH_LPI_RSVD_MASK GENMASK_ULL(59, 32)
> > +
> > +#define RISCV_FFH_LPI_TYPE_SBI BIT_ULL(60)
> > +
> > +static int acpi_cpu_init_idle(unsigned int cpu)
> > +{
> > + int i;
> > + struct acpi_lpi_state *lpi;
> > + struct acpi_processor *pr = per_cpu(processors, cpu);
> > +
> > + if (unlikely(!pr || !pr->flags.has_lpi))
> > + return -EINVAL;
> > +
> > + if (!riscv_sbi_hsm_is_supported())
> > + return -ENODEV;
> > +
> > + if (pr->power.count <= 1)
> > + return -ENODEV;
> > +
> > + for (i = 1; i < pr->power.count; i++) {
> > + u32 state;
> > +
> > + lpi = &pr->power.lpi_states[i];
> > +
> > + /*
> > + * Validate Entry Method as per FFH spec.
> > + * bits[63:60] should be 0x1
> > + * bits[59:32] should be 0x0
> > + * bits[31:0] represent a SBI power_state
> > + */
> > + if (((lpi->address & RISCV_FFH_LPI_TYPE_MASK) != RISCV_FFH_LPI_TYPE_SBI) ||
> > + (lpi->address & RISCV_FFH_LPI_RSVD_MASK)) {
> > + pr_warn("Invalid LPI entry method %#llx\n", lpi->address);
> > + return -EINVAL;
> > + }
> > +
> > + state = lpi->address;
>
> It seems that acpi_lpi_state.address is u64, so shouldn't state be u64
> instead of u32?
>
SBI suspend state is only 32 bits represented by lower 32 bits of
lpi->address.

Thanks,
Sunil

2024-03-18 09:12:02

by Drew Fustini

[permalink] [raw]
Subject: Re: [PATCH v3 -next 2/3] ACPI: RISC-V: Add LPI driver

On Thu, Jan 18, 2024 at 11:59:29AM +0530, Sunil V L wrote:
> Enable Low Power Idle (LPI) based cpuidle driver for RISC-V platforms.
> It depends on SBI HSM calls for idle state transitions.
>
> Signed-off-by: Sunil V L <[email protected]>
> Reviewed-by: Andrew Jones <[email protected]>
> ---
> drivers/acpi/riscv/Makefile | 3 +-
> drivers/acpi/riscv/cpuidle.c | 81 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 83 insertions(+), 1 deletion(-)
> create mode 100644 drivers/acpi/riscv/cpuidle.c
>
> diff --git a/drivers/acpi/riscv/Makefile b/drivers/acpi/riscv/Makefile
> index 8b3b126e0b94..7309d92dd477 100644
> --- a/drivers/acpi/riscv/Makefile
> +++ b/drivers/acpi/riscv/Makefile
> @@ -1,2 +1,3 @@
> # SPDX-License-Identifier: GPL-2.0-only
> -obj-y += rhct.o
> +obj-y += rhct.o
> +obj-$(CONFIG_ACPI_PROCESSOR_IDLE) += cpuidle.o
> diff --git a/drivers/acpi/riscv/cpuidle.c b/drivers/acpi/riscv/cpuidle.c
> new file mode 100644
> index 000000000000..624f9bbdb58c
> --- /dev/null
> +++ b/drivers/acpi/riscv/cpuidle.c
> @@ -0,0 +1,81 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2024, Ventana Micro Systems Inc
> + * Author: Sunil V L <[email protected]>
> + *
> + */
> +
> +#include <linux/acpi.h>
> +#include <acpi/processor.h>
> +#include <linux/cpu_pm.h>
> +#include <linux/cpuidle.h>
> +#include <linux/suspend.h>
> +#include <asm/cpuidle.h>
> +#include <asm/sbi.h>
> +#include <asm/suspend.h>
> +
> +#define RISCV_FFH_LPI_TYPE_MASK GENMASK_ULL(63, 60)
> +#define RISCV_FFH_LPI_RSVD_MASK GENMASK_ULL(59, 32)
> +
> +#define RISCV_FFH_LPI_TYPE_SBI BIT_ULL(60)
> +
> +static int acpi_cpu_init_idle(unsigned int cpu)
> +{
> + int i;
> + struct acpi_lpi_state *lpi;
> + struct acpi_processor *pr = per_cpu(processors, cpu);
> +
> + if (unlikely(!pr || !pr->flags.has_lpi))
> + return -EINVAL;
> +
> + if (!riscv_sbi_hsm_is_supported())
> + return -ENODEV;
> +
> + if (pr->power.count <= 1)
> + return -ENODEV;
> +
> + for (i = 1; i < pr->power.count; i++) {
> + u32 state;
> +
> + lpi = &pr->power.lpi_states[i];
> +
> + /*
> + * Validate Entry Method as per FFH spec.
> + * bits[63:60] should be 0x1
> + * bits[59:32] should be 0x0
> + * bits[31:0] represent a SBI power_state
> + */
> + if (((lpi->address & RISCV_FFH_LPI_TYPE_MASK) != RISCV_FFH_LPI_TYPE_SBI) ||
> + (lpi->address & RISCV_FFH_LPI_RSVD_MASK)) {
> + pr_warn("Invalid LPI entry method %#llx\n", lpi->address);
> + return -EINVAL;
> + }
> +
> + state = lpi->address;

It seems that acpi_lpi_state.address is u64, so shouldn't state be u64
instead of u32?

thanks,
drew

Subject: Re: [PATCH v3 -next 0/3] RISC-V: ACPI: Add LPI support

Hello:

This series was applied to riscv/linux.git (for-next)
by Palmer Dabbelt <[email protected]>:

On Thu, 18 Jan 2024 11:59:27 +0530 you wrote:
> This series adds support for Low Power Idle (LPI) on ACPI based
> platforms.
>
> LPI is described in the ACPI spec [1]. RISC-V FFH spec required to
> enable this is available at [2].
>
> [1] - https://uefi.org/specs/ACPI/6.5/08_Processor_Configuration_and_Control.html#lpi-low-power-idle-states
> [2] - https://github.com/riscv-non-isa/riscv-acpi-ffh/releases/download/v/riscv-ffh.pdf
>
> [...]

Here is the summary with links:
- [v3,-next,1/3] cpuidle: RISC-V: Move few functions to arch/riscv
https://git.kernel.org/riscv/c/6649182a383c
- [v3,-next,2/3] ACPI: RISC-V: Add LPI driver
https://git.kernel.org/riscv/c/4877fc92142f
- [v3,-next,3/3] ACPI: Enable ACPI_PROCESSOR for RISC-V
https://git.kernel.org/riscv/c/359df7c5be4b

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html