| Message ID | 20231016164958.1191529-3-sunilvl@ventanamicro.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | RISC-V: ACPI improvements | expand |
On Mon, Oct 16, 2023 at 10:19:57PM +0530, Sunil V L wrote: > Cache Block Operation (CBO) related block size in ACPI is provided by RHCT. > Add support to read the CMO node in RHCT to get this information. > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> > --- > arch/riscv/include/asm/acpi.h | 6 +++ > drivers/acpi/riscv/rhct.c | 93 +++++++++++++++++++++++++++++++++++ > 2 files changed, 99 insertions(+) > > diff --git a/arch/riscv/include/asm/acpi.h b/arch/riscv/include/asm/acpi.h > index d5604d2073bc..7dad0cf9d701 100644 > --- a/arch/riscv/include/asm/acpi.h > +++ b/arch/riscv/include/asm/acpi.h > @@ -66,6 +66,8 @@ int acpi_get_riscv_isa(struct acpi_table_header *table, > unsigned int cpu, const char **isa); > > static inline int acpi_numa_get_nid(unsigned int cpu) { return NUMA_NO_NODE; } > +void acpi_get_cbo_block_size(struct acpi_table_header *table, u32 *cbom_size, > + u32 *cboz_size, u32 *cbop_size); > #else > static inline void acpi_init_rintc_map(void) { } > static inline struct acpi_madt_rintc *acpi_cpu_get_madt_rintc(int cpu) > @@ -79,6 +81,10 @@ static inline int acpi_get_riscv_isa(struct acpi_table_header *table, > return -EINVAL; > } > > +static inline void acpi_get_cbo_block_size(struct acpi_table_header *table, > + u32 *cbom_size, u32 *cboz_size, > + u32 *cbop_size) { } > + > #endif /* CONFIG_ACPI */ > > #endif /*_ASM_ACPI_H*/ > diff --git a/drivers/acpi/riscv/rhct.c b/drivers/acpi/riscv/rhct.c > index b280b3e9c7d9..105f1aaa3fac 100644 > --- a/drivers/acpi/riscv/rhct.c > +++ b/drivers/acpi/riscv/rhct.c > @@ -8,6 +8,7 @@ > #define pr_fmt(fmt) "ACPI: RHCT: " fmt > > #include <linux/acpi.h> > +#include <linux/bits.h> > > static struct acpi_table_header *acpi_get_rhct(void) > { > @@ -81,3 +82,95 @@ int acpi_get_riscv_isa(struct acpi_table_header *table, unsigned int cpu, const > > return -1; > } > + > +static void acpi_parse_hart_info_cmo_node(struct acpi_table_rhct *rhct, > + struct acpi_rhct_hart_info *hart_info, > + u32 *cbom_size, u32 *cboz_size, u32 *cbop_size) > +{ > + u32 size_hartinfo = sizeof(struct acpi_rhct_hart_info); > + u32 size_hdr = sizeof(struct acpi_rhct_node_header); > + struct acpi_rhct_node_header *ref_node; > + struct acpi_rhct_cmo_node *cmo_node; > + u32 *hart_info_node_offset; > + > + hart_info_node_offset = ACPI_ADD_PTR(u32, hart_info, size_hartinfo); > + for (int i = 0; i < hart_info->num_offsets; i++) { > + ref_node = ACPI_ADD_PTR(struct acpi_rhct_node_header, > + rhct, hart_info_node_offset[i]); > + if (ref_node->type == ACPI_RHCT_NODE_TYPE_CMO) { > + cmo_node = ACPI_ADD_PTR(struct acpi_rhct_cmo_node, > + ref_node, size_hdr); > + if (cbom_size && cmo_node->cbom_size <= 30) { > + if (!*cbom_size) { > + *cbom_size = BIT(cmo_node->cbom_size); > + } else if (*cbom_size != > + BIT(cmo_node->cbom_size)) { No need to break the if line, we can go to 100 chars. And then, since both the if and else if arms only have single statements, all the {} can be dropped too. Same comment for cboz and cbop. > + pr_warn("CBOM size is not the same across harts\n"); > + } > + } > + > + if (cboz_size && cmo_node->cboz_size <= 30) { > + if (!*cboz_size) { > + *cboz_size = BIT(cmo_node->cboz_size); > + } else if (*cboz_size != > + BIT(cmo_node->cboz_size)) { > + pr_warn("CBOZ size is not the same across harts\n"); > + } > + } > + > + if (cbop_size && cmo_node->cbop_size <= 30) { > + if (!*cbop_size) { > + *cbop_size = BIT(cmo_node->cbop_size); > + } else if (*cbop_size != > + BIT(cmo_node->cbop_size)) { > + pr_warn("CBOP size is not the same across harts\n"); > + } > + } > + } > + } > +} > + > +/* > + * 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 ^ add a space here > + * multiple times, pass NULL so that the table remains in memory > + */ > +void acpi_get_cbo_block_size(struct acpi_table_header *table, u32 *cbom_size, > + u32 *cboz_size, u32 *cbop_size) > +{ > + u32 size_hdr = sizeof(struct acpi_rhct_node_header); > + struct acpi_rhct_node_header *node, *end; > + struct acpi_rhct_hart_info *hart_info; > + struct acpi_table_rhct *rhct; > + > + if (acpi_disabled) > + return; > + > + if (table) { > + rhct = (struct acpi_table_rhct *)table; > + } else { > + rhct = (struct acpi_table_rhct *)acpi_get_rhct(); Not an issue of this patch, but it seems like acpi_get_rhct() should return a struct acpi_table_rhct pointer instead of a struct acpi_table_header pointer since it's specifically returning an RHCT. > + if (!rhct) > + return; > + } > + > + if (cbom_size) > + *cbom_size = 0; > + > + if (cboz_size) > + *cboz_size = 0; > + > + if (cbop_size) > + *cbop_size = 0; > + > + end = ACPI_ADD_PTR(struct acpi_rhct_node_header, rhct, rhct->header.length); > + 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)) { > + if (node->type == ACPI_RHCT_NODE_TYPE_HART_INFO) { > + hart_info = ACPI_ADD_PTR(struct acpi_rhct_hart_info, node, size_hdr); > + acpi_parse_hart_info_cmo_node(rhct, hart_info, cbom_size, > + cboz_size, cbop_size); > + } > + } > +} > -- > 2.39.2 > Other than the nits Reviewed-by: Andrew Jones <ajones@ventanamicro.com> Thanks, drew
On Tue, Oct 17, 2023 at 10:37:41AM +0200, Andrew Jones wrote: > On Mon, Oct 16, 2023 at 10:19:57PM +0530, Sunil V L wrote: > > Cache Block Operation (CBO) related block size in ACPI is provided by RHCT. > > Add support to read the CMO node in RHCT to get this information. > > > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> > > --- > > arch/riscv/include/asm/acpi.h | 6 +++ > > drivers/acpi/riscv/rhct.c | 93 +++++++++++++++++++++++++++++++++++ > > 2 files changed, 99 insertions(+) > > > > diff --git a/arch/riscv/include/asm/acpi.h b/arch/riscv/include/asm/acpi.h > > index d5604d2073bc..7dad0cf9d701 100644 > > --- a/arch/riscv/include/asm/acpi.h > > +++ b/arch/riscv/include/asm/acpi.h > > @@ -66,6 +66,8 @@ int acpi_get_riscv_isa(struct acpi_table_header *table, > > unsigned int cpu, const char **isa); > > > > static inline int acpi_numa_get_nid(unsigned int cpu) { return NUMA_NO_NODE; } > > +void acpi_get_cbo_block_size(struct acpi_table_header *table, u32 *cbom_size, > > + u32 *cboz_size, u32 *cbop_size); > > #else > > static inline void acpi_init_rintc_map(void) { } > > static inline struct acpi_madt_rintc *acpi_cpu_get_madt_rintc(int cpu) > > @@ -79,6 +81,10 @@ static inline int acpi_get_riscv_isa(struct acpi_table_header *table, > > return -EINVAL; > > } > > > > +static inline void acpi_get_cbo_block_size(struct acpi_table_header *table, > > + u32 *cbom_size, u32 *cboz_size, > > + u32 *cbop_size) { } > > + > > #endif /* CONFIG_ACPI */ > > > > #endif /*_ASM_ACPI_H*/ > > diff --git a/drivers/acpi/riscv/rhct.c b/drivers/acpi/riscv/rhct.c > > index b280b3e9c7d9..105f1aaa3fac 100644 > > --- a/drivers/acpi/riscv/rhct.c > > +++ b/drivers/acpi/riscv/rhct.c > > @@ -8,6 +8,7 @@ > > #define pr_fmt(fmt) "ACPI: RHCT: " fmt > > > > #include <linux/acpi.h> > > +#include <linux/bits.h> > > > > static struct acpi_table_header *acpi_get_rhct(void) > > { > > @@ -81,3 +82,95 @@ int acpi_get_riscv_isa(struct acpi_table_header *table, unsigned int cpu, const > > > > return -1; > > } > > + > > +static void acpi_parse_hart_info_cmo_node(struct acpi_table_rhct *rhct, > > + struct acpi_rhct_hart_info *hart_info, > > + u32 *cbom_size, u32 *cboz_size, u32 *cbop_size) > > +{ > > + u32 size_hartinfo = sizeof(struct acpi_rhct_hart_info); > > + u32 size_hdr = sizeof(struct acpi_rhct_node_header); > > + struct acpi_rhct_node_header *ref_node; > > + struct acpi_rhct_cmo_node *cmo_node; > > + u32 *hart_info_node_offset; > > + > > + hart_info_node_offset = ACPI_ADD_PTR(u32, hart_info, size_hartinfo); > > + for (int i = 0; i < hart_info->num_offsets; i++) { > > + ref_node = ACPI_ADD_PTR(struct acpi_rhct_node_header, > > + rhct, hart_info_node_offset[i]); > > + if (ref_node->type == ACPI_RHCT_NODE_TYPE_CMO) { > > + cmo_node = ACPI_ADD_PTR(struct acpi_rhct_cmo_node, > > + ref_node, size_hdr); > > + if (cbom_size && cmo_node->cbom_size <= 30) { > > + if (!*cbom_size) { > > + *cbom_size = BIT(cmo_node->cbom_size); > > + } else if (*cbom_size != > > + BIT(cmo_node->cbom_size)) { > > No need to break the if line, we can go to 100 chars. And then, since both > the if and else if arms only have single statements, all the {} can be > dropped too. Same comment for cboz and cbop. > Yeah, it is a side effect of working on repos with different coding standards. It is interesting that checkpatch didn't recommend to remove the braces. Let me fix it in next revision. Thanks!. > > + pr_warn("CBOM size is not the same across harts\n"); > > + } > > + } > > + > > + if (cboz_size && cmo_node->cboz_size <= 30) { > > + if (!*cboz_size) { > > + *cboz_size = BIT(cmo_node->cboz_size); > > + } else if (*cboz_size != > > + BIT(cmo_node->cboz_size)) { > > + pr_warn("CBOZ size is not the same across harts\n"); > > + } > > + } > > + > > + if (cbop_size && cmo_node->cbop_size <= 30) { > > + if (!*cbop_size) { > > + *cbop_size = BIT(cmo_node->cbop_size); > > + } else if (*cbop_size != > > + BIT(cmo_node->cbop_size)) { > > + pr_warn("CBOP size is not the same across harts\n"); > > + } > > + } > > + } > > + } > > +} > > + > > +/* > > + * 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 > ^ add a space here > > > + * multiple times, pass NULL so that the table remains in memory > > + */ > > +void acpi_get_cbo_block_size(struct acpi_table_header *table, u32 *cbom_size, > > + u32 *cboz_size, u32 *cbop_size) > > +{ > > + u32 size_hdr = sizeof(struct acpi_rhct_node_header); > > + struct acpi_rhct_node_header *node, *end; > > + struct acpi_rhct_hart_info *hart_info; > > + struct acpi_table_rhct *rhct; > > + > > + if (acpi_disabled) > > + return; > > + > > + if (table) { > > + rhct = (struct acpi_table_rhct *)table; > > + } else { > > + rhct = (struct acpi_table_rhct *)acpi_get_rhct(); > > Not an issue of this patch, but it seems like acpi_get_rhct() should > return a struct acpi_table_rhct pointer instead of a struct > acpi_table_header pointer since it's specifically returning an RHCT. > Makes sense. Let me add a patch to improve this. > > + if (!rhct) > > + return; > > + } > > + > > + if (cbom_size) > > + *cbom_size = 0; > > + > > + if (cboz_size) > > + *cboz_size = 0; > > + > > + if (cbop_size) > > + *cbop_size = 0; > > + > > + end = ACPI_ADD_PTR(struct acpi_rhct_node_header, rhct, rhct->header.length); > > + 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)) { > > + if (node->type == ACPI_RHCT_NODE_TYPE_HART_INFO) { > > + hart_info = ACPI_ADD_PTR(struct acpi_rhct_hart_info, node, size_hdr); > > + acpi_parse_hart_info_cmo_node(rhct, hart_info, cbom_size, > > + cboz_size, cbop_size); > > + } > > + } > > +} > > -- > > 2.39.2 > > > > Other than the nits > > Reviewed-by: Andrew Jones <ajones@ventanamicro.com> > Thanks a lot for the review! Sunil
On Tue, Oct 17, 2023 at 09:09:23PM +0530, Sunil V L wrote: > On Tue, Oct 17, 2023 at 10:37:41AM +0200, Andrew Jones wrote: > > On Mon, Oct 16, 2023 at 10:19:57PM +0530, Sunil V L wrote: > > > Cache Block Operation (CBO) related block size in ACPI is provided by RHCT. > > > Add support to read the CMO node in RHCT to get this information. > > > > > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> > > > --- > > > arch/riscv/include/asm/acpi.h | 6 +++ > > > drivers/acpi/riscv/rhct.c | 93 +++++++++++++++++++++++++++++++++++ > > > 2 files changed, 99 insertions(+) > > > > > > diff --git a/arch/riscv/include/asm/acpi.h b/arch/riscv/include/asm/acpi.h > > > index d5604d2073bc..7dad0cf9d701 100644 > > > --- a/arch/riscv/include/asm/acpi.h > > > +++ b/arch/riscv/include/asm/acpi.h > > > @@ -66,6 +66,8 @@ int acpi_get_riscv_isa(struct acpi_table_header *table, > > > unsigned int cpu, const char **isa); > > > > > > static inline int acpi_numa_get_nid(unsigned int cpu) { return NUMA_NO_NODE; } > > > +void acpi_get_cbo_block_size(struct acpi_table_header *table, u32 *cbom_size, > > > + u32 *cboz_size, u32 *cbop_size); > > > #else > > > static inline void acpi_init_rintc_map(void) { } > > > static inline struct acpi_madt_rintc *acpi_cpu_get_madt_rintc(int cpu) > > > @@ -79,6 +81,10 @@ static inline int acpi_get_riscv_isa(struct acpi_table_header *table, > > > return -EINVAL; > > > } > > > > > > +static inline void acpi_get_cbo_block_size(struct acpi_table_header *table, > > > + u32 *cbom_size, u32 *cboz_size, > > > + u32 *cbop_size) { } > > > + > > > #endif /* CONFIG_ACPI */ > > > > > > #endif /*_ASM_ACPI_H*/ > > > diff --git a/drivers/acpi/riscv/rhct.c b/drivers/acpi/riscv/rhct.c > > > index b280b3e9c7d9..105f1aaa3fac 100644 > > > --- a/drivers/acpi/riscv/rhct.c > > > +++ b/drivers/acpi/riscv/rhct.c > > > @@ -8,6 +8,7 @@ > > > #define pr_fmt(fmt) "ACPI: RHCT: " fmt > > > > > > #include <linux/acpi.h> > > > +#include <linux/bits.h> > > > > > > static struct acpi_table_header *acpi_get_rhct(void) > > > { > > > @@ -81,3 +82,95 @@ int acpi_get_riscv_isa(struct acpi_table_header *table, unsigned int cpu, const > > > > > > return -1; > > > } > > > + > > > +static void acpi_parse_hart_info_cmo_node(struct acpi_table_rhct *rhct, > > > + struct acpi_rhct_hart_info *hart_info, > > > + u32 *cbom_size, u32 *cboz_size, u32 *cbop_size) > > > +{ > > > + u32 size_hartinfo = sizeof(struct acpi_rhct_hart_info); > > > + u32 size_hdr = sizeof(struct acpi_rhct_node_header); > > > + struct acpi_rhct_node_header *ref_node; > > > + struct acpi_rhct_cmo_node *cmo_node; > > > + u32 *hart_info_node_offset; > > > + > > > + hart_info_node_offset = ACPI_ADD_PTR(u32, hart_info, size_hartinfo); > > > + for (int i = 0; i < hart_info->num_offsets; i++) { > > > + ref_node = ACPI_ADD_PTR(struct acpi_rhct_node_header, > > > + rhct, hart_info_node_offset[i]); > > > + if (ref_node->type == ACPI_RHCT_NODE_TYPE_CMO) { > > > + cmo_node = ACPI_ADD_PTR(struct acpi_rhct_cmo_node, > > > + ref_node, size_hdr); > > > + if (cbom_size && cmo_node->cbom_size <= 30) { > > > + if (!*cbom_size) { > > > + *cbom_size = BIT(cmo_node->cbom_size); > > > + } else if (*cbom_size != > > > + BIT(cmo_node->cbom_size)) { > > > > No need to break the if line, we can go to 100 chars. And then, since both > > the if and else if arms only have single statements, all the {} can be > > dropped too. Same comment for cboz and cbop. > > > Yeah, it is a side effect of working on repos with different coding > standards. It is interesting that checkpatch didn't recommend to remove > the braces. Let me fix it in next revision. Thanks!. I think checkpatch is OK with single lines that get broken into multiple lines having braces around them. In fact, it may even be preferred. My suggestion is to not break the line though, so checkpatch would likely complain after turning these lines into true single lines. Thanks, drew
diff --git a/arch/riscv/include/asm/acpi.h b/arch/riscv/include/asm/acpi.h index d5604d2073bc..7dad0cf9d701 100644 --- a/arch/riscv/include/asm/acpi.h +++ b/arch/riscv/include/asm/acpi.h @@ -66,6 +66,8 @@ int acpi_get_riscv_isa(struct acpi_table_header *table, unsigned int cpu, const char **isa); static inline int acpi_numa_get_nid(unsigned int cpu) { return NUMA_NO_NODE; } +void acpi_get_cbo_block_size(struct acpi_table_header *table, u32 *cbom_size, + u32 *cboz_size, u32 *cbop_size); #else static inline void acpi_init_rintc_map(void) { } static inline struct acpi_madt_rintc *acpi_cpu_get_madt_rintc(int cpu) @@ -79,6 +81,10 @@ static inline int acpi_get_riscv_isa(struct acpi_table_header *table, return -EINVAL; } +static inline void acpi_get_cbo_block_size(struct acpi_table_header *table, + u32 *cbom_size, u32 *cboz_size, + u32 *cbop_size) { } + #endif /* CONFIG_ACPI */ #endif /*_ASM_ACPI_H*/ diff --git a/drivers/acpi/riscv/rhct.c b/drivers/acpi/riscv/rhct.c index b280b3e9c7d9..105f1aaa3fac 100644 --- a/drivers/acpi/riscv/rhct.c +++ b/drivers/acpi/riscv/rhct.c @@ -8,6 +8,7 @@ #define pr_fmt(fmt) "ACPI: RHCT: " fmt #include <linux/acpi.h> +#include <linux/bits.h> static struct acpi_table_header *acpi_get_rhct(void) { @@ -81,3 +82,95 @@ int acpi_get_riscv_isa(struct acpi_table_header *table, unsigned int cpu, const return -1; } + +static void acpi_parse_hart_info_cmo_node(struct acpi_table_rhct *rhct, + struct acpi_rhct_hart_info *hart_info, + u32 *cbom_size, u32 *cboz_size, u32 *cbop_size) +{ + u32 size_hartinfo = sizeof(struct acpi_rhct_hart_info); + u32 size_hdr = sizeof(struct acpi_rhct_node_header); + struct acpi_rhct_node_header *ref_node; + struct acpi_rhct_cmo_node *cmo_node; + u32 *hart_info_node_offset; + + hart_info_node_offset = ACPI_ADD_PTR(u32, hart_info, size_hartinfo); + for (int i = 0; i < hart_info->num_offsets; i++) { + ref_node = ACPI_ADD_PTR(struct acpi_rhct_node_header, + rhct, hart_info_node_offset[i]); + if (ref_node->type == ACPI_RHCT_NODE_TYPE_CMO) { + cmo_node = ACPI_ADD_PTR(struct acpi_rhct_cmo_node, + ref_node, size_hdr); + if (cbom_size && cmo_node->cbom_size <= 30) { + if (!*cbom_size) { + *cbom_size = BIT(cmo_node->cbom_size); + } else if (*cbom_size != + BIT(cmo_node->cbom_size)) { + pr_warn("CBOM size is not the same across harts\n"); + } + } + + if (cboz_size && cmo_node->cboz_size <= 30) { + if (!*cboz_size) { + *cboz_size = BIT(cmo_node->cboz_size); + } else if (*cboz_size != + BIT(cmo_node->cboz_size)) { + pr_warn("CBOZ size is not the same across harts\n"); + } + } + + if (cbop_size && cmo_node->cbop_size <= 30) { + if (!*cbop_size) { + *cbop_size = BIT(cmo_node->cbop_size); + } else if (*cbop_size != + BIT(cmo_node->cbop_size)) { + pr_warn("CBOP size is not the same across harts\n"); + } + } + } + } +} + +/* + * 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 + */ +void acpi_get_cbo_block_size(struct acpi_table_header *table, u32 *cbom_size, + u32 *cboz_size, u32 *cbop_size) +{ + u32 size_hdr = sizeof(struct acpi_rhct_node_header); + struct acpi_rhct_node_header *node, *end; + struct acpi_rhct_hart_info *hart_info; + struct acpi_table_rhct *rhct; + + if (acpi_disabled) + return; + + if (table) { + rhct = (struct acpi_table_rhct *)table; + } else { + rhct = (struct acpi_table_rhct *)acpi_get_rhct(); + if (!rhct) + return; + } + + if (cbom_size) + *cbom_size = 0; + + if (cboz_size) + *cboz_size = 0; + + if (cbop_size) + *cbop_size = 0; + + end = ACPI_ADD_PTR(struct acpi_rhct_node_header, rhct, rhct->header.length); + 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)) { + if (node->type == ACPI_RHCT_NODE_TYPE_HART_INFO) { + hart_info = ACPI_ADD_PTR(struct acpi_rhct_hart_info, node, size_hdr); + acpi_parse_hart_info_cmo_node(rhct, hart_info, cbom_size, + cboz_size, cbop_size); + } + } +}
Cache Block Operation (CBO) related block size in ACPI is provided by RHCT. Add support to read the CMO node in RHCT to get this information. Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> --- arch/riscv/include/asm/acpi.h | 6 +++ drivers/acpi/riscv/rhct.c | 93 +++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+)