2022-05-26 18:33:38

by Sunil V L

[permalink] [raw]
Subject: [PATCH V2 0/5] Support for 64bit hartid on RV64 platforms

The hartid can be a 64bit value on RV64 platforms. This series updates
the code so that 64bit hartid can be supported on RV64 platforms.

The series has been tested on both RV32 and RV64 qemu platforms.

Changes since V1:
1) Updated RB tag for PATCH 1 and PATCH3
2) Type Casting NR_CPUS before comparing with hartid in PATCH 2
3) Changed commit message of PATCH 2 to provide details about
the bug it is fixing.
4) Updated PATCH 5 for unaligned 64bit read


Sunil V L (5):
riscv: cpu_ops_sbi: Support for 64bit hartid
riscv: spinwait: Fix hartid variable type
riscv: smp: Support for 64bit hartid
riscv: cpu: Support for 64bit hartid
riscv/efi_stub: Support for 64bit boot-hartid

arch/riscv/include/asm/processor.h | 4 ++--
arch/riscv/include/asm/smp.h | 4 ++--
arch/riscv/kernel/cpu.c | 26 +++++++++++++----------
arch/riscv/kernel/cpu_ops_sbi.c | 4 ++--
arch/riscv/kernel/cpu_ops_spinwait.c | 4 ++--
arch/riscv/kernel/cpufeature.c | 6 ++++--
arch/riscv/kernel/smp.c | 4 ++--
arch/riscv/kernel/smpboot.c | 9 ++++----
drivers/clocksource/timer-riscv.c | 15 +++++++------
drivers/firmware/efi/libstub/riscv-stub.c | 13 +++++++++---
drivers/irqchip/irq-riscv-intc.c | 7 +++---
drivers/irqchip/irq-sifive-plic.c | 7 +++---
12 files changed, 60 insertions(+), 43 deletions(-)

--
2.25.1



2022-05-26 22:12:06

by Sunil V L

[permalink] [raw]
Subject: [PATCH V2 4/5] riscv: cpu: Support for 64bit hartid

Adds support for 64bit hartid in riscv_of_processor_hartid()
- Separate return value and status code.
- Make hartid variable type as unsigned long.
- Update the callers.

Signed-off-by: Sunil V L <[email protected]>
---
arch/riscv/include/asm/processor.h | 4 ++--
arch/riscv/kernel/cpu.c | 26 +++++++++++++++-----------
arch/riscv/kernel/cpufeature.c | 6 ++++--
arch/riscv/kernel/smpboot.c | 9 +++++----
drivers/clocksource/timer-riscv.c | 15 ++++++++-------
drivers/irqchip/irq-riscv-intc.c | 7 ++++---
drivers/irqchip/irq-sifive-plic.c | 7 ++++---
7 files changed, 42 insertions(+), 32 deletions(-)

diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
index 0749924d9e55..99fae9398506 100644
--- a/arch/riscv/include/asm/processor.h
+++ b/arch/riscv/include/asm/processor.h
@@ -75,8 +75,8 @@ static inline void wait_for_interrupt(void)
}

struct device_node;
-int riscv_of_processor_hartid(struct device_node *node);
-int riscv_of_parent_hartid(struct device_node *node);
+int riscv_of_processor_hartid(struct device_node *node, unsigned long *hartid);
+int riscv_of_parent_hartid(struct device_node *node, unsigned long *hartid);

extern void riscv_fill_hwcap(void);
extern int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index ccb617791e56..477a33b34c95 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -14,37 +14,36 @@
* Returns the hart ID of the given device tree node, or -ENODEV if the node
* isn't an enabled and valid RISC-V hart node.
*/
-int riscv_of_processor_hartid(struct device_node *node)
+int riscv_of_processor_hartid(struct device_node *node, unsigned long *hart)
{
const char *isa;
- u32 hart;

if (!of_device_is_compatible(node, "riscv")) {
pr_warn("Found incompatible CPU\n");
return -ENODEV;
}

- hart = of_get_cpu_hwid(node, 0);
- if (hart == ~0U) {
+ *hart = (unsigned long) of_get_cpu_hwid(node, 0);
+ if (*hart == ~0UL) {
pr_warn("Found CPU without hart ID\n");
return -ENODEV;
}

if (!of_device_is_available(node)) {
- pr_info("CPU with hartid=%d is not available\n", hart);
+ pr_info("CPU with hartid=%lu is not available\n", *hart);
return -ENODEV;
}

if (of_property_read_string(node, "riscv,isa", &isa)) {
- pr_warn("CPU with hartid=%d has no \"riscv,isa\" property\n", hart);
+ pr_warn("CPU with hartid=%lu has no \"riscv,isa\" property\n", *hart);
return -ENODEV;
}
if (isa[0] != 'r' || isa[1] != 'v') {
- pr_warn("CPU with hartid=%d has an invalid ISA of \"%s\"\n", hart, isa);
+ pr_warn("CPU with hartid=%lu has an invalid ISA of \"%s\"\n", *hart, isa);
return -ENODEV;
}

- return hart;
+ return 0;
}

/*
@@ -53,11 +52,16 @@ int riscv_of_processor_hartid(struct device_node *node)
* To achieve this, we walk up the DT tree until we find an active
* RISC-V core (HART) node and extract the cpuid from it.
*/
-int riscv_of_parent_hartid(struct device_node *node)
+int riscv_of_parent_hartid(struct device_node *node, unsigned long *hartid)
{
+ int rc;
+
for (; node; node = node->parent) {
- if (of_device_is_compatible(node, "riscv"))
- return riscv_of_processor_hartid(node);
+ if (of_device_is_compatible(node, "riscv")) {
+ rc = riscv_of_processor_hartid(node, hartid);
+ if (!rc)
+ return 0;
+ }
}

return -1;
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 1b2d42d7f589..49c05bd9352d 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -67,8 +67,9 @@ void __init riscv_fill_hwcap(void)
struct device_node *node;
const char *isa;
char print_str[NUM_ALPHA_EXTS + 1];
- int i, j;
+ int i, j, rc;
static unsigned long isa2hwcap[256] = {0};
+ unsigned long hartid;

isa2hwcap['i'] = isa2hwcap['I'] = COMPAT_HWCAP_ISA_I;
isa2hwcap['m'] = isa2hwcap['M'] = COMPAT_HWCAP_ISA_M;
@@ -86,7 +87,8 @@ void __init riscv_fill_hwcap(void)
DECLARE_BITMAP(this_isa, RISCV_ISA_EXT_MAX);
const char *temp;

- if (riscv_of_processor_hartid(node) < 0)
+ rc = riscv_of_processor_hartid(node, &hartid);
+ if (rc < 0)
continue;

if (of_property_read_string(node, "riscv,isa", &isa)) {
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 622f226454d5..4336610a19ee 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -76,15 +76,16 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
void __init setup_smp(void)
{
struct device_node *dn;
- int hart;
+ unsigned long hart;
bool found_boot_cpu = false;
int cpuid = 1;
+ int rc;

cpu_set_ops(0);

for_each_of_cpu_node(dn) {
- hart = riscv_of_processor_hartid(dn);
- if (hart < 0)
+ rc = riscv_of_processor_hartid(dn, &hart);
+ if (rc < 0)
continue;

if (hart == cpuid_to_hartid_map(0)) {
@@ -94,7 +95,7 @@ void __init setup_smp(void)
continue;
}
if (cpuid >= NR_CPUS) {
- pr_warn("Invalid cpuid [%d] for hartid [%d]\n",
+ pr_warn("Invalid cpuid [%d] for hartid [%lu]\n",
cpuid, hart);
continue;
}
diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
index 1767f8bf2013..55142c27f0bc 100644
--- a/drivers/clocksource/timer-riscv.c
+++ b/drivers/clocksource/timer-riscv.c
@@ -101,20 +101,21 @@ static irqreturn_t riscv_timer_interrupt(int irq, void *dev_id)

static int __init riscv_timer_init_dt(struct device_node *n)
{
- int cpuid, hartid, error;
+ int cpuid, error;
+ unsigned long hartid;
struct device_node *child;
struct irq_domain *domain;

- hartid = riscv_of_processor_hartid(n);
- if (hartid < 0) {
- pr_warn("Not valid hartid for node [%pOF] error = [%d]\n",
+ error = riscv_of_processor_hartid(n, &hartid);
+ if (error < 0) {
+ pr_warn("Not valid hartid for node [%pOF] error = [%lu]\n",
n, hartid);
- return hartid;
+ return error;
}

cpuid = riscv_hartid_to_cpuid(hartid);
if (cpuid < 0) {
- pr_warn("Invalid cpuid for hartid [%d]\n", hartid);
+ pr_warn("Invalid cpuid for hartid [%lu]\n", hartid);
return cpuid;
}

@@ -140,7 +141,7 @@ static int __init riscv_timer_init_dt(struct device_node *n)
return -ENODEV;
}

- pr_info("%s: Registering clocksource cpuid [%d] hartid [%d]\n",
+ pr_info("%s: Registering clocksource cpuid [%d] hartid [%lu]\n",
__func__, cpuid, hartid);
error = clocksource_register_hz(&riscv_clocksource, riscv_timebase);
if (error) {
diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
index b65bd8878d4f..499e5f81b3fe 100644
--- a/drivers/irqchip/irq-riscv-intc.c
+++ b/drivers/irqchip/irq-riscv-intc.c
@@ -95,10 +95,11 @@ static const struct irq_domain_ops riscv_intc_domain_ops = {
static int __init riscv_intc_init(struct device_node *node,
struct device_node *parent)
{
- int rc, hartid;
+ int rc;
+ unsigned long hartid;

- hartid = riscv_of_parent_hartid(node);
- if (hartid < 0) {
+ rc = riscv_of_parent_hartid(node, &hartid);
+ if (rc < 0) {
pr_warn("unable to find hart id for %pOF\n", node);
return 0;
}
diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
index bb87e4c3b88e..4710d9741f36 100644
--- a/drivers/irqchip/irq-sifive-plic.c
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -317,7 +317,8 @@ static int __init plic_init(struct device_node *node,
for (i = 0; i < nr_contexts; i++) {
struct of_phandle_args parent;
irq_hw_number_t hwirq;
- int cpu, hartid;
+ int cpu;
+ unsigned long hartid;

if (of_irq_parse_one(node, i, &parent)) {
pr_err("failed to parse parent for context %d.\n", i);
@@ -341,8 +342,8 @@ static int __init plic_init(struct device_node *node,
continue;
}

- hartid = riscv_of_parent_hartid(parent.np);
- if (hartid < 0) {
+ error = riscv_of_parent_hartid(parent.np, &hartid);
+ if (error < 0) {
pr_warn("failed to parse hart ID for context %d.\n", i);
continue;
}
--
2.25.1


2022-05-26 22:15:04

by Sunil V L

[permalink] [raw]
Subject: [PATCH V2 1/5] riscv: cpu_ops_sbi: Support for 64bit hartid

The hartid can be a 64bit value on RV64 platforms. This patch modifies
the hartid variable type to unsigned long so that it can hold 64bit
value on RV64 platforms.

Signed-off-by: Sunil V L <[email protected]>
Reviewed-by: Heinrich Schuchardt <[email protected]>
---
arch/riscv/kernel/cpu_ops_sbi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/kernel/cpu_ops_sbi.c b/arch/riscv/kernel/cpu_ops_sbi.c
index 4f5a6f84e2a4..efa0f0816634 100644
--- a/arch/riscv/kernel/cpu_ops_sbi.c
+++ b/arch/riscv/kernel/cpu_ops_sbi.c
@@ -65,7 +65,7 @@ static int sbi_hsm_hart_get_status(unsigned long hartid)
static int sbi_cpu_start(unsigned int cpuid, struct task_struct *tidle)
{
unsigned long boot_addr = __pa_symbol(secondary_start_sbi);
- int hartid = cpuid_to_hartid_map(cpuid);
+ unsigned long hartid = cpuid_to_hartid_map(cpuid);
unsigned long hsm_data;
struct sbi_hart_boot_data *bdata = &per_cpu(boot_data, cpuid);

@@ -107,7 +107,7 @@ static void sbi_cpu_stop(void)
static int sbi_cpu_is_stopped(unsigned int cpuid)
{
int rc;
- int hartid = cpuid_to_hartid_map(cpuid);
+ unsigned long hartid = cpuid_to_hartid_map(cpuid);

rc = sbi_hsm_hart_get_status(hartid);

--
2.25.1


2022-05-27 04:02:16

by Sunil V L

[permalink] [raw]
Subject: [PATCH V2 5/5] riscv/efi_stub: Support for 64bit boot-hartid

The boot-hartid can be a 64bit value on RV64 platforms. Currently,
the "boot-hartid" in DT is assumed to be 32bit only. This patch
detects the size of the "boot-hartid" and uses 32bit or 64bit
FDT reads appropriately.

Signed-off-by: Sunil V L <[email protected]>
---
drivers/firmware/efi/libstub/riscv-stub.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/efi/libstub/riscv-stub.c b/drivers/firmware/efi/libstub/riscv-stub.c
index 9e85e58d1f27..b450ebf95977 100644
--- a/drivers/firmware/efi/libstub/riscv-stub.c
+++ b/drivers/firmware/efi/libstub/riscv-stub.c
@@ -8,6 +8,7 @@

#include <asm/efi.h>
#include <asm/sections.h>
+#include <asm/unaligned.h>

#include "efistub.h"

@@ -29,7 +30,7 @@ static int get_boot_hartid_from_fdt(void)
{
const void *fdt;
int chosen_node, len;
- const fdt32_t *prop;
+ const void *prop;

fdt = get_efi_config_table(DEVICE_TREE_GUID);
if (!fdt)
@@ -40,10 +41,16 @@ static int get_boot_hartid_from_fdt(void)
return -EINVAL;

prop = fdt_getprop((void *)fdt, chosen_node, "boot-hartid", &len);
- if (!prop || len != sizeof(u32))
+ if (!prop)
+ return -EINVAL;
+
+ if (len == sizeof(u32))
+ hartid = (unsigned long) fdt32_to_cpu(*(fdt32_t *)prop);
+ else if (len == sizeof(u64))
+ hartid = (unsigned long) fdt64_to_cpu(__get_unaligned_t(fdt64_t, prop));
+ else
return -EINVAL;

- hartid = fdt32_to_cpu(*prop);
return 0;
}

--
2.25.1


2022-05-27 08:19:34

by Atish Patra

[permalink] [raw]
Subject: Re: [PATCH V2 4/5] riscv: cpu: Support for 64bit hartid

On Thu, May 26, 2022 at 3:12 AM Sunil V L <[email protected]> wrote:
>
> Adds support for 64bit hartid in riscv_of_processor_hartid()

The commit text is a bit misleading as you are adding support for XLEN
hartid. For RV32, it is still 32bit.
This applies to the entire series.

> - Separate return value and status code.
> - Make hartid variable type as unsigned long.
> - Update the callers.
>
> Signed-off-by: Sunil V L <[email protected]>
> ---
> arch/riscv/include/asm/processor.h | 4 ++--
> arch/riscv/kernel/cpu.c | 26 +++++++++++++++-----------
> arch/riscv/kernel/cpufeature.c | 6 ++++--
> arch/riscv/kernel/smpboot.c | 9 +++++----
> drivers/clocksource/timer-riscv.c | 15 ++++++++-------
> drivers/irqchip/irq-riscv-intc.c | 7 ++++---
> drivers/irqchip/irq-sifive-plic.c | 7 ++++---
> 7 files changed, 42 insertions(+), 32 deletions(-)
>
> diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
> index 0749924d9e55..99fae9398506 100644
> --- a/arch/riscv/include/asm/processor.h
> +++ b/arch/riscv/include/asm/processor.h
> @@ -75,8 +75,8 @@ static inline void wait_for_interrupt(void)
> }
>
> struct device_node;
> -int riscv_of_processor_hartid(struct device_node *node);
> -int riscv_of_parent_hartid(struct device_node *node);
> +int riscv_of_processor_hartid(struct device_node *node, unsigned long *hartid);
> +int riscv_of_parent_hartid(struct device_node *node, unsigned long *hartid);
>
> extern void riscv_fill_hwcap(void);
> extern int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index ccb617791e56..477a33b34c95 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -14,37 +14,36 @@
> * Returns the hart ID of the given device tree node, or -ENODEV if the node
> * isn't an enabled and valid RISC-V hart node.
> */
> -int riscv_of_processor_hartid(struct device_node *node)
> +int riscv_of_processor_hartid(struct device_node *node, unsigned long *hart)
> {
> const char *isa;
> - u32 hart;
>
> if (!of_device_is_compatible(node, "riscv")) {
> pr_warn("Found incompatible CPU\n");
> return -ENODEV;
> }
>
> - hart = of_get_cpu_hwid(node, 0);
> - if (hart == ~0U) {
> + *hart = (unsigned long) of_get_cpu_hwid(node, 0);
> + if (*hart == ~0UL) {
> pr_warn("Found CPU without hart ID\n");
> return -ENODEV;
> }
>
> if (!of_device_is_available(node)) {
> - pr_info("CPU with hartid=%d is not available\n", hart);
> + pr_info("CPU with hartid=%lu is not available\n", *hart);
> return -ENODEV;
> }
>
> if (of_property_read_string(node, "riscv,isa", &isa)) {
> - pr_warn("CPU with hartid=%d has no \"riscv,isa\" property\n", hart);
> + pr_warn("CPU with hartid=%lu has no \"riscv,isa\" property\n", *hart);
> return -ENODEV;
> }
> if (isa[0] != 'r' || isa[1] != 'v') {
> - pr_warn("CPU with hartid=%d has an invalid ISA of \"%s\"\n", hart, isa);
> + pr_warn("CPU with hartid=%lu has an invalid ISA of \"%s\"\n", *hart, isa);
> return -ENODEV;
> }
>
> - return hart;
> + return 0;
> }
>
> /*
> @@ -53,11 +52,16 @@ int riscv_of_processor_hartid(struct device_node *node)
> * To achieve this, we walk up the DT tree until we find an active
> * RISC-V core (HART) node and extract the cpuid from it.
> */
> -int riscv_of_parent_hartid(struct device_node *node)
> +int riscv_of_parent_hartid(struct device_node *node, unsigned long *hartid)
> {
> + int rc;
> +
> for (; node; node = node->parent) {
> - if (of_device_is_compatible(node, "riscv"))
> - return riscv_of_processor_hartid(node);
> + if (of_device_is_compatible(node, "riscv")) {
> + rc = riscv_of_processor_hartid(node, hartid);
> + if (!rc)
> + return 0;
> + }
> }
>
> return -1;
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 1b2d42d7f589..49c05bd9352d 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -67,8 +67,9 @@ void __init riscv_fill_hwcap(void)
> struct device_node *node;
> const char *isa;
> char print_str[NUM_ALPHA_EXTS + 1];
> - int i, j;
> + int i, j, rc;
> static unsigned long isa2hwcap[256] = {0};
> + unsigned long hartid;
>
> isa2hwcap['i'] = isa2hwcap['I'] = COMPAT_HWCAP_ISA_I;
> isa2hwcap['m'] = isa2hwcap['M'] = COMPAT_HWCAP_ISA_M;
> @@ -86,7 +87,8 @@ void __init riscv_fill_hwcap(void)
> DECLARE_BITMAP(this_isa, RISCV_ISA_EXT_MAX);
> const char *temp;
>
> - if (riscv_of_processor_hartid(node) < 0)
> + rc = riscv_of_processor_hartid(node, &hartid);
> + if (rc < 0)
> continue;
>
> if (of_property_read_string(node, "riscv,isa", &isa)) {
> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> index 622f226454d5..4336610a19ee 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -76,15 +76,16 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
> void __init setup_smp(void)
> {
> struct device_node *dn;
> - int hart;
> + unsigned long hart;
> bool found_boot_cpu = false;
> int cpuid = 1;
> + int rc;
>
> cpu_set_ops(0);
>
> for_each_of_cpu_node(dn) {
> - hart = riscv_of_processor_hartid(dn);
> - if (hart < 0)
> + rc = riscv_of_processor_hartid(dn, &hart);
> + if (rc < 0)
> continue;
>
> if (hart == cpuid_to_hartid_map(0)) {
> @@ -94,7 +95,7 @@ void __init setup_smp(void)
> continue;
> }
> if (cpuid >= NR_CPUS) {
> - pr_warn("Invalid cpuid [%d] for hartid [%d]\n",
> + pr_warn("Invalid cpuid [%d] for hartid [%lu]\n",
> cpuid, hart);
> continue;
> }
> diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
> index 1767f8bf2013..55142c27f0bc 100644
> --- a/drivers/clocksource/timer-riscv.c
> +++ b/drivers/clocksource/timer-riscv.c
> @@ -101,20 +101,21 @@ static irqreturn_t riscv_timer_interrupt(int irq, void *dev_id)
>
> static int __init riscv_timer_init_dt(struct device_node *n)
> {
> - int cpuid, hartid, error;
> + int cpuid, error;
> + unsigned long hartid;
> struct device_node *child;
> struct irq_domain *domain;
>
> - hartid = riscv_of_processor_hartid(n);
> - if (hartid < 0) {
> - pr_warn("Not valid hartid for node [%pOF] error = [%d]\n",
> + error = riscv_of_processor_hartid(n, &hartid);
> + if (error < 0) {
> + pr_warn("Not valid hartid for node [%pOF] error = [%lu]\n",
> n, hartid);
> - return hartid;
> + return error;
> }
>
> cpuid = riscv_hartid_to_cpuid(hartid);
> if (cpuid < 0) {
> - pr_warn("Invalid cpuid for hartid [%d]\n", hartid);
> + pr_warn("Invalid cpuid for hartid [%lu]\n", hartid);
> return cpuid;
> }
>
> @@ -140,7 +141,7 @@ static int __init riscv_timer_init_dt(struct device_node *n)
> return -ENODEV;
> }
>
> - pr_info("%s: Registering clocksource cpuid [%d] hartid [%d]\n",
> + pr_info("%s: Registering clocksource cpuid [%d] hartid [%lu]\n",
> __func__, cpuid, hartid);
> error = clocksource_register_hz(&riscv_clocksource, riscv_timebase);
> if (error) {
> diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
> index b65bd8878d4f..499e5f81b3fe 100644
> --- a/drivers/irqchip/irq-riscv-intc.c
> +++ b/drivers/irqchip/irq-riscv-intc.c
> @@ -95,10 +95,11 @@ static const struct irq_domain_ops riscv_intc_domain_ops = {
> static int __init riscv_intc_init(struct device_node *node,
> struct device_node *parent)
> {
> - int rc, hartid;
> + int rc;
> + unsigned long hartid;
>
> - hartid = riscv_of_parent_hartid(node);
> - if (hartid < 0) {
> + rc = riscv_of_parent_hartid(node, &hartid);
> + if (rc < 0) {
> pr_warn("unable to find hart id for %pOF\n", node);
> return 0;
> }
> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> index bb87e4c3b88e..4710d9741f36 100644
> --- a/drivers/irqchip/irq-sifive-plic.c
> +++ b/drivers/irqchip/irq-sifive-plic.c
> @@ -317,7 +317,8 @@ static int __init plic_init(struct device_node *node,
> for (i = 0; i < nr_contexts; i++) {
> struct of_phandle_args parent;
> irq_hw_number_t hwirq;
> - int cpu, hartid;
> + int cpu;
> + unsigned long hartid;
>
> if (of_irq_parse_one(node, i, &parent)) {
> pr_err("failed to parse parent for context %d.\n", i);
> @@ -341,8 +342,8 @@ static int __init plic_init(struct device_node *node,
> continue;
> }
>
> - hartid = riscv_of_parent_hartid(parent.np);
> - if (hartid < 0) {
> + error = riscv_of_parent_hartid(parent.np, &hartid);
> + if (error < 0) {
> pr_warn("failed to parse hart ID for context %d.\n", i);
> continue;
> }
> --
> 2.25.1
>

Otherwise, it looks good.

Reviewed-by: Atish Patra <[email protected]>

--
Regards,
Atish

2022-05-27 08:56:10

by Sunil V L

[permalink] [raw]
Subject: [PATCH V2 3/5] riscv: smp: Support for 64bit hartid

The hartid can be a 64bit value on RV64 platforms. This patch
modifies the hartid parameter in riscv_hartid_to_cpuid() as
unsigned long so that it can hold 64bit value on RV64 platforms.

Signed-off-by: Sunil V L <[email protected]>
Reviewed-by: Heinrich Schuchardt <[email protected]>
---
arch/riscv/include/asm/smp.h | 4 ++--
arch/riscv/kernel/smp.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
index 23170c933d73..d3443be7eedc 100644
--- a/arch/riscv/include/asm/smp.h
+++ b/arch/riscv/include/asm/smp.h
@@ -42,7 +42,7 @@ void arch_send_call_function_ipi_mask(struct cpumask *mask);
/* Hook for the generic smp_call_function_single() routine. */
void arch_send_call_function_single_ipi(int cpu);

-int riscv_hartid_to_cpuid(int hartid);
+int riscv_hartid_to_cpuid(unsigned long hartid);

/* Set custom IPI operations */
void riscv_set_ipi_ops(const struct riscv_ipi_ops *ops);
@@ -70,7 +70,7 @@ static inline void show_ipi_stats(struct seq_file *p, int prec)
{
}

-static inline int riscv_hartid_to_cpuid(int hartid)
+static inline int riscv_hartid_to_cpuid(unsigned long hartid)
{
if (hartid == boot_cpu_hartid)
return 0;
diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
index b5d30ea92292..018e7dc45df6 100644
--- a/arch/riscv/kernel/smp.c
+++ b/arch/riscv/kernel/smp.c
@@ -47,7 +47,7 @@ static struct {
unsigned long bits ____cacheline_aligned;
} ipi_data[NR_CPUS] __cacheline_aligned;

-int riscv_hartid_to_cpuid(int hartid)
+int riscv_hartid_to_cpuid(unsigned long hartid)
{
int i;

@@ -55,7 +55,7 @@ int riscv_hartid_to_cpuid(int hartid)
if (cpuid_to_hartid_map(i) == hartid)
return i;

- pr_err("Couldn't find cpu id for hartid [%d]\n", hartid);
+ pr_err("Couldn't find cpu id for hartid [%lu]\n", hartid);
return -ENOENT;
}

--
2.25.1


2022-05-28 10:49:19

by Sunil V L

[permalink] [raw]
Subject: Re: [PATCH V2 4/5] riscv: cpu: Support for 64bit hartid

On Thu, May 26, 2022 at 04:10:51PM -0700, Atish Patra wrote:
> On Thu, May 26, 2022 at 3:12 AM Sunil V L <[email protected]> wrote:
> >
> > Adds support for 64bit hartid in riscv_of_processor_hartid()
>
> The commit text is a bit misleading as you are adding support for XLEN
> hartid. For RV32, it is still 32bit.
> This applies to the entire series.

Thanks Atish. I somehow missed mentioning RV64 in this patch. Will
update and send.

Thanks
Sunil
>
> > - Separate return value and status code.
> > - Make hartid variable type as unsigned long.
> > - Update the callers.
> >
> > Signed-off-by: Sunil V L <[email protected]>
> > ---
> > arch/riscv/include/asm/processor.h | 4 ++--
> > arch/riscv/kernel/cpu.c | 26 +++++++++++++++-----------
> > arch/riscv/kernel/cpufeature.c | 6 ++++--
> > arch/riscv/kernel/smpboot.c | 9 +++++----
> > drivers/clocksource/timer-riscv.c | 15 ++++++++-------
> > drivers/irqchip/irq-riscv-intc.c | 7 ++++---
> > drivers/irqchip/irq-sifive-plic.c | 7 ++++---
> > 7 files changed, 42 insertions(+), 32 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
> > index 0749924d9e55..99fae9398506 100644
> > --- a/arch/riscv/include/asm/processor.h
> > +++ b/arch/riscv/include/asm/processor.h
> > @@ -75,8 +75,8 @@ static inline void wait_for_interrupt(void)
> > }
> >
> > struct device_node;
> > -int riscv_of_processor_hartid(struct device_node *node);
> > -int riscv_of_parent_hartid(struct device_node *node);
> > +int riscv_of_processor_hartid(struct device_node *node, unsigned long *hartid);
> > +int riscv_of_parent_hartid(struct device_node *node, unsigned long *hartid);
> >
> > extern void riscv_fill_hwcap(void);
> > extern int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > index ccb617791e56..477a33b34c95 100644
> > --- a/arch/riscv/kernel/cpu.c
> > +++ b/arch/riscv/kernel/cpu.c
> > @@ -14,37 +14,36 @@
> > * Returns the hart ID of the given device tree node, or -ENODEV if the node
> > * isn't an enabled and valid RISC-V hart node.
> > */
> > -int riscv_of_processor_hartid(struct device_node *node)
> > +int riscv_of_processor_hartid(struct device_node *node, unsigned long *hart)
> > {
> > const char *isa;
> > - u32 hart;
> >
> > if (!of_device_is_compatible(node, "riscv")) {
> > pr_warn("Found incompatible CPU\n");
> > return -ENODEV;
> > }
> >
> > - hart = of_get_cpu_hwid(node, 0);
> > - if (hart == ~0U) {
> > + *hart = (unsigned long) of_get_cpu_hwid(node, 0);
> > + if (*hart == ~0UL) {
> > pr_warn("Found CPU without hart ID\n");
> > return -ENODEV;
> > }
> >
> > if (!of_device_is_available(node)) {
> > - pr_info("CPU with hartid=%d is not available\n", hart);
> > + pr_info("CPU with hartid=%lu is not available\n", *hart);
> > return -ENODEV;
> > }
> >
> > if (of_property_read_string(node, "riscv,isa", &isa)) {
> > - pr_warn("CPU with hartid=%d has no \"riscv,isa\" property\n", hart);
> > + pr_warn("CPU with hartid=%lu has no \"riscv,isa\" property\n", *hart);
> > return -ENODEV;
> > }
> > if (isa[0] != 'r' || isa[1] != 'v') {
> > - pr_warn("CPU with hartid=%d has an invalid ISA of \"%s\"\n", hart, isa);
> > + pr_warn("CPU with hartid=%lu has an invalid ISA of \"%s\"\n", *hart, isa);
> > return -ENODEV;
> > }
> >
> > - return hart;
> > + return 0;
> > }
> >
> > /*
> > @@ -53,11 +52,16 @@ int riscv_of_processor_hartid(struct device_node *node)
> > * To achieve this, we walk up the DT tree until we find an active
> > * RISC-V core (HART) node and extract the cpuid from it.
> > */
> > -int riscv_of_parent_hartid(struct device_node *node)
> > +int riscv_of_parent_hartid(struct device_node *node, unsigned long *hartid)
> > {
> > + int rc;
> > +
> > for (; node; node = node->parent) {
> > - if (of_device_is_compatible(node, "riscv"))
> > - return riscv_of_processor_hartid(node);
> > + if (of_device_is_compatible(node, "riscv")) {
> > + rc = riscv_of_processor_hartid(node, hartid);
> > + if (!rc)
> > + return 0;
> > + }
> > }
> >
> > return -1;
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index 1b2d42d7f589..49c05bd9352d 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -67,8 +67,9 @@ void __init riscv_fill_hwcap(void)
> > struct device_node *node;
> > const char *isa;
> > char print_str[NUM_ALPHA_EXTS + 1];
> > - int i, j;
> > + int i, j, rc;
> > static unsigned long isa2hwcap[256] = {0};
> > + unsigned long hartid;
> >
> > isa2hwcap['i'] = isa2hwcap['I'] = COMPAT_HWCAP_ISA_I;
> > isa2hwcap['m'] = isa2hwcap['M'] = COMPAT_HWCAP_ISA_M;
> > @@ -86,7 +87,8 @@ void __init riscv_fill_hwcap(void)
> > DECLARE_BITMAP(this_isa, RISCV_ISA_EXT_MAX);
> > const char *temp;
> >
> > - if (riscv_of_processor_hartid(node) < 0)
> > + rc = riscv_of_processor_hartid(node, &hartid);
> > + if (rc < 0)
> > continue;
> >
> > if (of_property_read_string(node, "riscv,isa", &isa)) {
> > diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> > index 622f226454d5..4336610a19ee 100644
> > --- a/arch/riscv/kernel/smpboot.c
> > +++ b/arch/riscv/kernel/smpboot.c
> > @@ -76,15 +76,16 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
> > void __init setup_smp(void)
> > {
> > struct device_node *dn;
> > - int hart;
> > + unsigned long hart;
> > bool found_boot_cpu = false;
> > int cpuid = 1;
> > + int rc;
> >
> > cpu_set_ops(0);
> >
> > for_each_of_cpu_node(dn) {
> > - hart = riscv_of_processor_hartid(dn);
> > - if (hart < 0)
> > + rc = riscv_of_processor_hartid(dn, &hart);
> > + if (rc < 0)
> > continue;
> >
> > if (hart == cpuid_to_hartid_map(0)) {
> > @@ -94,7 +95,7 @@ void __init setup_smp(void)
> > continue;
> > }
> > if (cpuid >= NR_CPUS) {
> > - pr_warn("Invalid cpuid [%d] for hartid [%d]\n",
> > + pr_warn("Invalid cpuid [%d] for hartid [%lu]\n",
> > cpuid, hart);
> > continue;
> > }
> > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
> > index 1767f8bf2013..55142c27f0bc 100644
> > --- a/drivers/clocksource/timer-riscv.c
> > +++ b/drivers/clocksource/timer-riscv.c
> > @@ -101,20 +101,21 @@ static irqreturn_t riscv_timer_interrupt(int irq, void *dev_id)
> >
> > static int __init riscv_timer_init_dt(struct device_node *n)
> > {
> > - int cpuid, hartid, error;
> > + int cpuid, error;
> > + unsigned long hartid;
> > struct device_node *child;
> > struct irq_domain *domain;
> >
> > - hartid = riscv_of_processor_hartid(n);
> > - if (hartid < 0) {
> > - pr_warn("Not valid hartid for node [%pOF] error = [%d]\n",
> > + error = riscv_of_processor_hartid(n, &hartid);
> > + if (error < 0) {
> > + pr_warn("Not valid hartid for node [%pOF] error = [%lu]\n",
> > n, hartid);
> > - return hartid;
> > + return error;
> > }
> >
> > cpuid = riscv_hartid_to_cpuid(hartid);
> > if (cpuid < 0) {
> > - pr_warn("Invalid cpuid for hartid [%d]\n", hartid);
> > + pr_warn("Invalid cpuid for hartid [%lu]\n", hartid);
> > return cpuid;
> > }
> >
> > @@ -140,7 +141,7 @@ static int __init riscv_timer_init_dt(struct device_node *n)
> > return -ENODEV;
> > }
> >
> > - pr_info("%s: Registering clocksource cpuid [%d] hartid [%d]\n",
> > + pr_info("%s: Registering clocksource cpuid [%d] hartid [%lu]\n",
> > __func__, cpuid, hartid);
> > error = clocksource_register_hz(&riscv_clocksource, riscv_timebase);
> > if (error) {
> > diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
> > index b65bd8878d4f..499e5f81b3fe 100644
> > --- a/drivers/irqchip/irq-riscv-intc.c
> > +++ b/drivers/irqchip/irq-riscv-intc.c
> > @@ -95,10 +95,11 @@ static const struct irq_domain_ops riscv_intc_domain_ops = {
> > static int __init riscv_intc_init(struct device_node *node,
> > struct device_node *parent)
> > {
> > - int rc, hartid;
> > + int rc;
> > + unsigned long hartid;
> >
> > - hartid = riscv_of_parent_hartid(node);
> > - if (hartid < 0) {
> > + rc = riscv_of_parent_hartid(node, &hartid);
> > + if (rc < 0) {
> > pr_warn("unable to find hart id for %pOF\n", node);
> > return 0;
> > }
> > diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> > index bb87e4c3b88e..4710d9741f36 100644
> > --- a/drivers/irqchip/irq-sifive-plic.c
> > +++ b/drivers/irqchip/irq-sifive-plic.c
> > @@ -317,7 +317,8 @@ static int __init plic_init(struct device_node *node,
> > for (i = 0; i < nr_contexts; i++) {
> > struct of_phandle_args parent;
> > irq_hw_number_t hwirq;
> > - int cpu, hartid;
> > + int cpu;
> > + unsigned long hartid;
> >
> > if (of_irq_parse_one(node, i, &parent)) {
> > pr_err("failed to parse parent for context %d.\n", i);
> > @@ -341,8 +342,8 @@ static int __init plic_init(struct device_node *node,
> > continue;
> > }
> >
> > - hartid = riscv_of_parent_hartid(parent.np);
> > - if (hartid < 0) {
> > + error = riscv_of_parent_hartid(parent.np, &hartid);
> > + if (error < 0) {
> > pr_warn("failed to parse hart ID for context %d.\n", i);
> > continue;
> > }
> > --
> > 2.25.1
> >
>
> Otherwise, it looks good.
>
> Reviewed-by: Atish Patra <[email protected]>
>
> --
> Regards,
> Atish

2022-05-28 20:31:18

by Sunil V L

[permalink] [raw]
Subject: [PATCH V2 2/5] riscv: spinwait: Fix hartid variable type

The hartid variable is of type int but compared with
ULONG_MAX(INVALID_HARTID). This issue is fixed by changing
the hartid variable type to unsigned long.

Fixes: c78f94f35cf6 ("RISC-V: Use __cpu_up_stack/task_pointer only for spinwait method")
Cc: [email protected]

Signed-off-by: Sunil V L <[email protected]>
---
arch/riscv/kernel/cpu_ops_spinwait.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/kernel/cpu_ops_spinwait.c b/arch/riscv/kernel/cpu_ops_spinwait.c
index 346847f6c41c..3ade9152a3c7 100644
--- a/arch/riscv/kernel/cpu_ops_spinwait.c
+++ b/arch/riscv/kernel/cpu_ops_spinwait.c
@@ -18,7 +18,7 @@ void *__cpu_spinwait_task_pointer[NR_CPUS] __section(".data");
static void cpu_update_secondary_bootdata(unsigned int cpuid,
struct task_struct *tidle)
{
- int hartid = cpuid_to_hartid_map(cpuid);
+ unsigned long hartid = cpuid_to_hartid_map(cpuid);

/*
* The hartid must be less than NR_CPUS to avoid out-of-bound access
@@ -27,7 +27,7 @@ static void cpu_update_secondary_bootdata(unsigned int cpuid,
* spinwait booting is not the recommended approach for any platforms
* booting Linux in S-mode and can be disabled in the future.
*/
- if (hartid == INVALID_HARTID || hartid >= NR_CPUS)
+ if (hartid == INVALID_HARTID || hartid >= (unsigned long) NR_CPUS)
return;

/* Make sure tidle is updated */
--
2.25.1


2022-05-28 20:37:28

by Atish Patra

[permalink] [raw]
Subject: Re: [PATCH V2 1/5] riscv: cpu_ops_sbi: Support for 64bit hartid

On Thu, May 26, 2022 at 3:11 AM Sunil V L <[email protected]> wrote:
>
> The hartid can be a 64bit value on RV64 platforms. This patch modifies
> the hartid variable type to unsigned long so that it can hold 64bit
> value on RV64 platforms.
>
> Signed-off-by: Sunil V L <[email protected]>
> Reviewed-by: Heinrich Schuchardt <[email protected]>
> ---
> arch/riscv/kernel/cpu_ops_sbi.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/kernel/cpu_ops_sbi.c b/arch/riscv/kernel/cpu_ops_sbi.c
> index 4f5a6f84e2a4..efa0f0816634 100644
> --- a/arch/riscv/kernel/cpu_ops_sbi.c
> +++ b/arch/riscv/kernel/cpu_ops_sbi.c
> @@ -65,7 +65,7 @@ static int sbi_hsm_hart_get_status(unsigned long hartid)
> static int sbi_cpu_start(unsigned int cpuid, struct task_struct *tidle)
> {
> unsigned long boot_addr = __pa_symbol(secondary_start_sbi);
> - int hartid = cpuid_to_hartid_map(cpuid);
> + unsigned long hartid = cpuid_to_hartid_map(cpuid);
> unsigned long hsm_data;
> struct sbi_hart_boot_data *bdata = &per_cpu(boot_data, cpuid);
>
> @@ -107,7 +107,7 @@ static void sbi_cpu_stop(void)
> static int sbi_cpu_is_stopped(unsigned int cpuid)
> {
> int rc;
> - int hartid = cpuid_to_hartid_map(cpuid);
> + unsigned long hartid = cpuid_to_hartid_map(cpuid);
>
> rc = sbi_hsm_hart_get_status(hartid);
>
> --
> 2.25.1
>


Reviewed-by: Atish Patra <[email protected]>
--
Regards,
Atish

2022-05-28 20:48:11

by Atish Patra

[permalink] [raw]
Subject: Re: [PATCH V2 3/5] riscv: smp: Support for 64bit hartid

On Thu, May 26, 2022 at 3:12 AM Sunil V L <[email protected]> wrote:
>
> The hartid can be a 64bit value on RV64 platforms. This patch
> modifies the hartid parameter in riscv_hartid_to_cpuid() as
> unsigned long so that it can hold 64bit value on RV64 platforms.
>
> Signed-off-by: Sunil V L <[email protected]>
> Reviewed-by: Heinrich Schuchardt <[email protected]>
> ---
> arch/riscv/include/asm/smp.h | 4 ++--
> arch/riscv/kernel/smp.c | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
> index 23170c933d73..d3443be7eedc 100644
> --- a/arch/riscv/include/asm/smp.h
> +++ b/arch/riscv/include/asm/smp.h
> @@ -42,7 +42,7 @@ void arch_send_call_function_ipi_mask(struct cpumask *mask);
> /* Hook for the generic smp_call_function_single() routine. */
> void arch_send_call_function_single_ipi(int cpu);
>
> -int riscv_hartid_to_cpuid(int hartid);
> +int riscv_hartid_to_cpuid(unsigned long hartid);
>
> /* Set custom IPI operations */
> void riscv_set_ipi_ops(const struct riscv_ipi_ops *ops);
> @@ -70,7 +70,7 @@ static inline void show_ipi_stats(struct seq_file *p, int prec)
> {
> }
>
> -static inline int riscv_hartid_to_cpuid(int hartid)
> +static inline int riscv_hartid_to_cpuid(unsigned long hartid)
> {
> if (hartid == boot_cpu_hartid)
> return 0;
> diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
> index b5d30ea92292..018e7dc45df6 100644
> --- a/arch/riscv/kernel/smp.c
> +++ b/arch/riscv/kernel/smp.c
> @@ -47,7 +47,7 @@ static struct {
> unsigned long bits ____cacheline_aligned;
> } ipi_data[NR_CPUS] __cacheline_aligned;
>
> -int riscv_hartid_to_cpuid(int hartid)
> +int riscv_hartid_to_cpuid(unsigned long hartid)
> {
> int i;
>
> @@ -55,7 +55,7 @@ int riscv_hartid_to_cpuid(int hartid)
> if (cpuid_to_hartid_map(i) == hartid)
> return i;
>
> - pr_err("Couldn't find cpu id for hartid [%d]\n", hartid);
> + pr_err("Couldn't find cpu id for hartid [%lu]\n", hartid);
> return -ENOENT;
> }
>
> --
> 2.25.1
>


Reviewed-by: Atish Patra <[email protected]>
--
Regards,
Atish