| Message ID | 20230216182043.1946553-9-sunilvl@ventanamicro.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add basic ACPI support for RISC-V | expand |
On Thu, Feb 16, 2023 at 11:50:30PM +0530, Sunil V L wrote: > RHCT is a new table defined for RISC-V to communicate the > features of the CPU to the OS. Create a new architecture folder > in drivers/acpi and add RHCT parsing code. > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > arch/riscv/include/asm/acpi.h | 9 ++++ > drivers/acpi/Makefile | 2 + > drivers/acpi/riscv/Makefile | 2 + > drivers/acpi/riscv/rhct.c | 92 +++++++++++++++++++++++++++++++++++ > 4 files changed, 105 insertions(+) > create mode 100644 drivers/acpi/riscv/Makefile > create mode 100644 drivers/acpi/riscv/rhct.c > > diff --git a/arch/riscv/include/asm/acpi.h b/arch/riscv/include/asm/acpi.h > index 4a3622b38159..7bc49f65c86b 100644 > --- a/arch/riscv/include/asm/acpi.h > +++ b/arch/riscv/include/asm/acpi.h > @@ -58,6 +58,15 @@ static inline bool acpi_has_cpu_in_madt(void) > > static inline void arch_fix_phys_package_id(int num, u32 slot) { } > > +int acpi_get_riscv_isa(struct acpi_table_header *table, > + unsigned int cpu, const char **isa); > +#else > +static inline int acpi_get_riscv_isa(struct acpi_table_header *table, > + unsigned int cpu, const char **isa) > +{ > + return -EINVAL; > +} > + > #endif /* CONFIG_ACPI */ > > #endif /*_ASM_ACPI_H*/ > diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile > index feb36c0b9446..3fc5a0d54f6e 100644 > --- a/drivers/acpi/Makefile > +++ b/drivers/acpi/Makefile > @@ -131,3 +131,5 @@ obj-y += dptf/ > obj-$(CONFIG_ARM64) += arm64/ > > obj-$(CONFIG_ACPI_VIOT) += viot.o > + > +obj-$(CONFIG_RISCV) += riscv/ > diff --git a/drivers/acpi/riscv/Makefile b/drivers/acpi/riscv/Makefile > new file mode 100644 > index 000000000000..8b3b126e0b94 > --- /dev/null > +++ b/drivers/acpi/riscv/Makefile > @@ -0,0 +1,2 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +obj-y += rhct.o > diff --git a/drivers/acpi/riscv/rhct.c b/drivers/acpi/riscv/rhct.c > new file mode 100644 > index 000000000000..5bafc236d627 > --- /dev/null > +++ b/drivers/acpi/riscv/rhct.c > @@ -0,0 +1,92 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (C) 2022-2023, Ventana Micro Systems Inc > + * Author: Sunil V L <sunilvl@ventanamicro.com> > + * > + */ > + > +#define pr_fmt(fmt) "ACPI: RHCT: " fmt > + > +#include <linux/acpi.h> > + > +static void acpi_rhct_warn_missing(void) > +{ > + pr_warn_once("No RHCT table found\n"); > +} > + > +static struct acpi_table_header *acpi_get_rhct(void) > +{ > + static struct acpi_table_header *rhct; > + acpi_status status; > + > + /* > + * RHCT will be used at runtime on every CPU, so we > + * don't need to call acpi_put_table() to release the table mapping. > + */ > + if (!rhct) { > + status = acpi_get_table(ACPI_SIG_RHCT, 0, &rhct); > + if (ACPI_FAILURE(status)) > + acpi_rhct_warn_missing(); Probably don't need the wrapper function for this one callsite. Also, returning NULL here, rather than relying on acpi_get_table() to set rhct to NULL would be a bit more robust. > + } > + > + return rhct; > +} > + > +/* > + * During early boot, the caller should call acpi_get_table() and pass its pointer to > + * these functions(and free up later). At run time, since this table can be used > + * multiple times, pass NULL so that the table remains in memory ...multiple times, NULL may be passed in order to use the cached table. > + */ > +int acpi_get_riscv_isa(struct acpi_table_header *table, unsigned int acpi_cpu_id, const char **isa) > +{ > + struct acpi_rhct_node_header *node, *ref_node, *end; > + struct acpi_table_rhct *rhct; > + struct acpi_rhct_hart_info *hart_info; > + struct acpi_rhct_isa_string *isa_node; > + u32 *hart_info_node_offset; > + int i, j; > + u32 size_hdr = sizeof(struct acpi_rhct_node_header); > + u32 size_hartinfo = sizeof(struct acpi_rhct_hart_info); > + > + if (acpi_disabled) { > + pr_debug("%s: acpi is disabled\n", __func__); > + return -1; This seems like something that should never happen and easy to catch and fix with a BUG_ON. Is there any chance that BUG'ing here would be a bad idea? > + } > + > + if (!table) { > + rhct = (struct acpi_table_rhct *)acpi_get_rhct(); > + if (!rhct) > + return -ENOENT; > + } else { > + rhct = (struct acpi_table_rhct *)table; > + } > + > + node = ACPI_ADD_PTR(struct acpi_rhct_node_header, rhct, rhct->node_offset); > + end = ACPI_ADD_PTR(struct acpi_rhct_node_header, rhct, rhct->header.length); > + > + for (i = 0; i < rhct->node_count; i++) { > + if (node >= end) > + break; for (node = ACPI_ADD_PTR(struct acpi_rhct_node_header, rhct, rhct->node_offset); node < end; node = ACPI_ADD_PTR(struct acpi_rhct_node_header, node node->length)) > + switch (node->type) { > + case ACPI_RHCT_NODE_TYPE_HART_INFO: if (node->type == ACPI_RHCT_NODE_TYPE_HART_INFO) > + hart_info = ACPI_ADD_PTR(struct acpi_rhct_hart_info, node, size_hdr); > + hart_info_node_offset = ACPI_ADD_PTR(u32, hart_info, size_hartinfo); > + if (acpi_cpu_id != hart_info->uid) > + break; With the above suggested changes, this 'break' becomes 'continue'. > + for (j = 0; j < hart_info->num_offsets; j++) { > + ref_node = ACPI_ADD_PTR(struct acpi_rhct_node_header, > + rhct, hart_info_node_offset[j]); > + if (ref_node->type == ACPI_RHCT_NODE_TYPE_ISA_STRING) { > + isa_node = ACPI_ADD_PTR(struct acpi_rhct_isa_string, > + ref_node, size_hdr); > + *isa = isa_node->isa; > + return 0; > + } > + } > + break; > + } > + node = ACPI_ADD_PTR(struct acpi_rhct_node_header, node, node->length); > + } > + > + return -1; > +} > -- > 2.34.1 > Other than the nits, Reviewed-by: Andrew Jones <ajones@ventanamicro.com> Thanks, drew
On Mon, Feb 20, 2023 at 05:36:48PM +0100, Andrew Jones wrote: > On Thu, Feb 16, 2023 at 11:50:30PM +0530, Sunil V L wrote: > > RHCT is a new table defined for RISC-V to communicate the > > features of the CPU to the OS. Create a new architecture folder > > in drivers/acpi and add RHCT parsing code. > > > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > --- > > arch/riscv/include/asm/acpi.h | 9 ++++ > > drivers/acpi/Makefile | 2 + > > drivers/acpi/riscv/Makefile | 2 + > > drivers/acpi/riscv/rhct.c | 92 +++++++++++++++++++++++++++++++++++ > > 4 files changed, 105 insertions(+) > > create mode 100644 drivers/acpi/riscv/Makefile > > create mode 100644 drivers/acpi/riscv/rhct.c > > > > diff --git a/arch/riscv/include/asm/acpi.h b/arch/riscv/include/asm/acpi.h > > index 4a3622b38159..7bc49f65c86b 100644 > > --- a/arch/riscv/include/asm/acpi.h > > +++ b/arch/riscv/include/asm/acpi.h > > @@ -58,6 +58,15 @@ static inline bool acpi_has_cpu_in_madt(void) > > > > static inline void arch_fix_phys_package_id(int num, u32 slot) { } > > > > +int acpi_get_riscv_isa(struct acpi_table_header *table, > > + unsigned int cpu, const char **isa); > > +#else > > +static inline int acpi_get_riscv_isa(struct acpi_table_header *table, > > + unsigned int cpu, const char **isa) > > +{ > > + return -EINVAL; > > +} > > + > > #endif /* CONFIG_ACPI */ > > > > #endif /*_ASM_ACPI_H*/ > > diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile > > index feb36c0b9446..3fc5a0d54f6e 100644 > > --- a/drivers/acpi/Makefile > > +++ b/drivers/acpi/Makefile > > @@ -131,3 +131,5 @@ obj-y += dptf/ > > obj-$(CONFIG_ARM64) += arm64/ > > > > obj-$(CONFIG_ACPI_VIOT) += viot.o > > + > > +obj-$(CONFIG_RISCV) += riscv/ > > diff --git a/drivers/acpi/riscv/Makefile b/drivers/acpi/riscv/Makefile > > new file mode 100644 > > index 000000000000..8b3b126e0b94 > > --- /dev/null > > +++ b/drivers/acpi/riscv/Makefile > > @@ -0,0 +1,2 @@ > > +# SPDX-License-Identifier: GPL-2.0-only > > +obj-y += rhct.o > > diff --git a/drivers/acpi/riscv/rhct.c b/drivers/acpi/riscv/rhct.c > > new file mode 100644 > > index 000000000000..5bafc236d627 > > --- /dev/null > > +++ b/drivers/acpi/riscv/rhct.c > > @@ -0,0 +1,92 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Copyright (C) 2022-2023, Ventana Micro Systems Inc > > + * Author: Sunil V L <sunilvl@ventanamicro.com> > > + * > > + */ > > + > > +#define pr_fmt(fmt) "ACPI: RHCT: " fmt > > + > > +#include <linux/acpi.h> > > + > > +static void acpi_rhct_warn_missing(void) > > +{ > > + pr_warn_once("No RHCT table found\n"); > > +} > > + > > +static struct acpi_table_header *acpi_get_rhct(void) > > +{ > > + static struct acpi_table_header *rhct; > > + acpi_status status; > > + > > + /* > > + * RHCT will be used at runtime on every CPU, so we > > + * don't need to call acpi_put_table() to release the table mapping. > > + */ > > + if (!rhct) { > > + status = acpi_get_table(ACPI_SIG_RHCT, 0, &rhct); > > + if (ACPI_FAILURE(status)) > > + acpi_rhct_warn_missing(); > > Probably don't need the wrapper function for this one callsite. Also, > returning NULL here, rather than relying on acpi_get_table() to set > rhct to NULL would be a bit more robust. > Sure. Will update. > > + } > > + > > + return rhct; > > +} > > + > > +/* > > + * During early boot, the caller should call acpi_get_table() and pass its pointer to > > + * these functions(and free up later). At run time, since this table can be used > > + * multiple times, pass NULL so that the table remains in memory > > ...multiple times, NULL may be passed in order to use the cached table. > Okay. > > + */ > > +int acpi_get_riscv_isa(struct acpi_table_header *table, unsigned int acpi_cpu_id, const char **isa) > > +{ > > + struct acpi_rhct_node_header *node, *ref_node, *end; > > + struct acpi_table_rhct *rhct; > > + struct acpi_rhct_hart_info *hart_info; > > + struct acpi_rhct_isa_string *isa_node; > > + u32 *hart_info_node_offset; > > + int i, j; > > + u32 size_hdr = sizeof(struct acpi_rhct_node_header); > > + u32 size_hartinfo = sizeof(struct acpi_rhct_hart_info); > > + > > + if (acpi_disabled) { > > + pr_debug("%s: acpi is disabled\n", __func__); > > + return -1; > > This seems like something that should never happen and easy to catch > and fix with a BUG_ON. Is there any chance that BUG'ing here would > be a bad idea? > Yes, we can use BUG_ON. Will update. > > + } > > + > > + if (!table) { > > + rhct = (struct acpi_table_rhct *)acpi_get_rhct(); > > + if (!rhct) > > + return -ENOENT; > > + } else { > > + rhct = (struct acpi_table_rhct *)table; > > + } > > + > > + node = ACPI_ADD_PTR(struct acpi_rhct_node_header, rhct, rhct->node_offset); > > + end = ACPI_ADD_PTR(struct acpi_rhct_node_header, rhct, rhct->header.length); > > + > > + for (i = 0; i < rhct->node_count; i++) { > > + if (node >= end) > > + break; > > for (node = ACPI_ADD_PTR(struct acpi_rhct_node_header, rhct, rhct->node_offset); > node < end; > node = ACPI_ADD_PTR(struct acpi_rhct_node_header, node node->length)) > > > + switch (node->type) { > > + case ACPI_RHCT_NODE_TYPE_HART_INFO: > > if (node->type == ACPI_RHCT_NODE_TYPE_HART_INFO) > > > + hart_info = ACPI_ADD_PTR(struct acpi_rhct_hart_info, node, size_hdr); > > + hart_info_node_offset = ACPI_ADD_PTR(u32, hart_info, size_hartinfo); > > + if (acpi_cpu_id != hart_info->uid) > > + break; > > With the above suggested changes, this 'break' becomes 'continue'. > Okay. > > + for (j = 0; j < hart_info->num_offsets; j++) { > > + ref_node = ACPI_ADD_PTR(struct acpi_rhct_node_header, > > + rhct, hart_info_node_offset[j]); > > + if (ref_node->type == ACPI_RHCT_NODE_TYPE_ISA_STRING) { > > + isa_node = ACPI_ADD_PTR(struct acpi_rhct_isa_string, > > + ref_node, size_hdr); > > + *isa = isa_node->isa; > > + return 0; > > + } > > + } > > + break; > > + } > > + node = ACPI_ADD_PTR(struct acpi_rhct_node_header, node, node->length); > > + } > > + > > + return -1; > > +} > > -- > > 2.34.1 > > > > Other than the nits, > > Reviewed-by: Andrew Jones <ajones@ventanamicro.com> > Thanks! Sunil
diff --git a/arch/riscv/include/asm/acpi.h b/arch/riscv/include/asm/acpi.h index 4a3622b38159..7bc49f65c86b 100644 --- a/arch/riscv/include/asm/acpi.h +++ b/arch/riscv/include/asm/acpi.h @@ -58,6 +58,15 @@ static inline bool acpi_has_cpu_in_madt(void) static inline void arch_fix_phys_package_id(int num, u32 slot) { } +int acpi_get_riscv_isa(struct acpi_table_header *table, + unsigned int cpu, const char **isa); +#else +static inline int acpi_get_riscv_isa(struct acpi_table_header *table, + unsigned int cpu, const char **isa) +{ + return -EINVAL; +} + #endif /* CONFIG_ACPI */ #endif /*_ASM_ACPI_H*/ diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index feb36c0b9446..3fc5a0d54f6e 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -131,3 +131,5 @@ obj-y += dptf/ obj-$(CONFIG_ARM64) += arm64/ obj-$(CONFIG_ACPI_VIOT) += viot.o + +obj-$(CONFIG_RISCV) += riscv/ diff --git a/drivers/acpi/riscv/Makefile b/drivers/acpi/riscv/Makefile new file mode 100644 index 000000000000..8b3b126e0b94 --- /dev/null +++ b/drivers/acpi/riscv/Makefile @@ -0,0 +1,2 @@ +# SPDX-License-Identifier: GPL-2.0-only +obj-y += rhct.o diff --git a/drivers/acpi/riscv/rhct.c b/drivers/acpi/riscv/rhct.c new file mode 100644 index 000000000000..5bafc236d627 --- /dev/null +++ b/drivers/acpi/riscv/rhct.c @@ -0,0 +1,92 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2022-2023, Ventana Micro Systems Inc + * Author: Sunil V L <sunilvl@ventanamicro.com> + * + */ + +#define pr_fmt(fmt) "ACPI: RHCT: " fmt + +#include <linux/acpi.h> + +static void acpi_rhct_warn_missing(void) +{ + pr_warn_once("No RHCT table found\n"); +} + +static struct acpi_table_header *acpi_get_rhct(void) +{ + static struct acpi_table_header *rhct; + acpi_status status; + + /* + * RHCT will be used at runtime on every CPU, so we + * don't need to call acpi_put_table() to release the table mapping. + */ + if (!rhct) { + status = acpi_get_table(ACPI_SIG_RHCT, 0, &rhct); + if (ACPI_FAILURE(status)) + acpi_rhct_warn_missing(); + } + + return rhct; +} + +/* + * During early boot, the caller should call acpi_get_table() and pass its pointer to + * these functions(and free up later). At run time, since this table can be used + * multiple times, pass NULL so that the table remains in memory + */ +int acpi_get_riscv_isa(struct acpi_table_header *table, unsigned int acpi_cpu_id, const char **isa) +{ + struct acpi_rhct_node_header *node, *ref_node, *end; + struct acpi_table_rhct *rhct; + struct acpi_rhct_hart_info *hart_info; + struct acpi_rhct_isa_string *isa_node; + u32 *hart_info_node_offset; + int i, j; + u32 size_hdr = sizeof(struct acpi_rhct_node_header); + u32 size_hartinfo = sizeof(struct acpi_rhct_hart_info); + + if (acpi_disabled) { + pr_debug("%s: acpi is disabled\n", __func__); + return -1; + } + + if (!table) { + rhct = (struct acpi_table_rhct *)acpi_get_rhct(); + if (!rhct) + return -ENOENT; + } else { + rhct = (struct acpi_table_rhct *)table; + } + + node = ACPI_ADD_PTR(struct acpi_rhct_node_header, rhct, rhct->node_offset); + end = ACPI_ADD_PTR(struct acpi_rhct_node_header, rhct, rhct->header.length); + + for (i = 0; i < rhct->node_count; i++) { + if (node >= end) + break; + switch (node->type) { + case ACPI_RHCT_NODE_TYPE_HART_INFO: + hart_info = ACPI_ADD_PTR(struct acpi_rhct_hart_info, node, size_hdr); + hart_info_node_offset = ACPI_ADD_PTR(u32, hart_info, size_hartinfo); + if (acpi_cpu_id != hart_info->uid) + break; + for (j = 0; j < hart_info->num_offsets; j++) { + ref_node = ACPI_ADD_PTR(struct acpi_rhct_node_header, + rhct, hart_info_node_offset[j]); + if (ref_node->type == ACPI_RHCT_NODE_TYPE_ISA_STRING) { + isa_node = ACPI_ADD_PTR(struct acpi_rhct_isa_string, + ref_node, size_hdr); + *isa = isa_node->isa; + return 0; + } + } + break; + } + node = ACPI_ADD_PTR(struct acpi_rhct_node_header, node, node->length); + } + + return -1; +}