| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20231017-b394a120094eeee31693e9a7@orel>
Date: Tue, 17 Oct 2023 10:37:41 +0200
From: Andrew Jones <ajones@...tanamicro.com>
To: Sunil V L <sunilvl@...tanamicro.com>
Cc: linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-acpi@...r.kernel.org,
Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>,
"Rafael J . Wysocki" <rafael@...nel.org>,
Len Brown <lenb@...nel.org>,
Conor Dooley <conor.dooley@...rochip.com>,
Anup Patel <apatel@...tanamicro.com>,
Ard Biesheuvel <ardb@...nel.org>,
Atish Kumar Patra <atishp@...osinc.com>
Subject: Re: [PATCH v3 -next 2/3] RISC-V: ACPI: RHCT: Add function to get CBO
block sizes
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@...tanamicro.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@...tanamicro.com>
Thanks,
drew
Powered by blists - more mailing lists