[PATCH 0/5] Support for 64bit hartid on RV64 platforms

[PATCH 0/5] Support for 64bit hartid on RV64 platforms

[Sunil V L]

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.

Sunil V L (5):
  riscv: cpu_ops_sbi: Support for 64bit hartid
  riscv: cpu_ops_spinwait: Support for 64bit hartid
  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      |  2 +-
 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 | 12 ++++++++---
 drivers/irqchip/irq-riscv-intc.c          |  7 +++---
 drivers/irqchip/irq-sifive-plic.c         |  7 +++---
 12 files changed, 58 insertions(+), 42 deletions(-)

-- 
2.25.1

[PATCH 1/5] riscv: cpu_ops_sbi: Support for 64bit hartid

[Sunil V L]

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 <sunilvl@ventanamicro.com>
---
 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

[PATCH 2/5] riscv: cpu_ops_spinwait: Support for 64bit hartid

[Sunil V L]

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 <sunilvl@ventanamicro.com>
---
 arch/riscv/kernel/cpu_ops_spinwait.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/cpu_ops_spinwait.c b/arch/riscv/kernel/cpu_ops_spinwait.c
index 346847f6c41c..51ac07514a62 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
-- 
2.25.1

[PATCH 3/5] riscv: smp: Support for 64bit hartid

[Sunil V L]

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 <sunilvl@ventanamicro.com>
---
 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

[PATCH 4/5] riscv: cpu: Support for 64bit hartid

[Sunil V L]

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 <sunilvl@ventanamicro.com>
---
 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..c49ed1eac011 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 == ~0U) {
 		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

[PATCH 5/5] riscv/efi_stub: Support for 64bit boot-hartid

[Sunil V L]

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 <sunilvl@ventanamicro.com>
---
 drivers/firmware/efi/libstub/riscv-stub.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/efi/libstub/riscv-stub.c b/drivers/firmware/efi/libstub/riscv-stub.c
index 9e85e58d1f27..d748533f1329 100644
--- a/drivers/firmware/efi/libstub/riscv-stub.c
+++ b/drivers/firmware/efi/libstub/riscv-stub.c
@@ -29,7 +29,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 +40,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(*(fdt64_t *)prop);
+	else
 		return -EINVAL;
 
-	hartid = fdt32_to_cpu(*prop);
 	return 0;
 }
 
-- 
2.25.1

Re: [PATCH 1/5] riscv: cpu_ops_sbi: Support for 64bit hartid

[Heinrich Schuchardt]

On 5/25/22 17:11, Sunil V L 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 <sunilvl@ventanamicro.com>

Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

> ---
>   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);
>   

Re: [PATCH 2/5] riscv: cpu_ops_spinwait: Support for 64bit hartid

[Heinrich Schuchardt]

On 5/25/22 17:11, Sunil V L 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 <sunilvl@ventanamicro.com>
> ---
>   arch/riscv/kernel/cpu_ops_spinwait.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/kernel/cpu_ops_spinwait.c b/arch/riscv/kernel/cpu_ops_spinwait.c
> index 346847f6c41c..51ac07514a62 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

This line follows:

if (hartid == INVALID_HARTID || hartid >= NR_CPUS)

INVALID_HARTID is defined as ULONG_MAX. Please, mention that you are 
fixing a bug:

Fixes: c78f94f35cf648 ("RISC-V: Use __cpu_up_stack/task_pointer only for 
spinwait method")

NR_CPUS alias CONFIG_NR_CPUS is an int. You should convert it to 
unsigned before comparing it to hartid to avoid build warnings.

Best regards

Heinrich

Re: [PATCH 5/5] riscv/efi_stub: Support for 64bit boot-hartid

[Ard Biesheuvel]

On Wed, 25 May 2022 at 17:11, Sunil V L <sunilvl@ventanamicro.com> wrote:
>
> 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 <sunilvl@ventanamicro.com>
> ---
>  drivers/firmware/efi/libstub/riscv-stub.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/riscv-stub.c b/drivers/firmware/efi/libstub/riscv-stub.c
> index 9e85e58d1f27..d748533f1329 100644
> --- a/drivers/firmware/efi/libstub/riscv-stub.c
> +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> @@ -29,7 +29,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 +40,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(*(fdt64_t *)prop);

Does RISC-V care about alignment? A 64-bit quantity is not guaranteed
to appear 64-bit aligned in the DT, and the cast violates C alignment
rules, so this should probably used get_unaligned_be64() or something
like that.


> +       else
>                 return -EINVAL;
>
> -       hartid = fdt32_to_cpu(*prop);
>         return 0;
>  }
>
> --
> 2.25.1
>

Re: [PATCH 3/5] riscv: smp: Support for 64bit hartid

[Heinrich Schuchardt]

On 5/25/22 17:11, Sunil V L 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 <sunilvl@ventanamicro.com>

Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

> ---
>   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;
>   }
>   

Re: [PATCH 5/5] riscv/efi_stub: Support for 64bit boot-hartid

[Heinrich Schuchardt]

On 5/25/22 17:48, Ard Biesheuvel wrote:
> On Wed, 25 May 2022 at 17:11, Sunil V L <sunilvl@ventanamicro.com> wrote:
>>
>> 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 <sunilvl@ventanamicro.com>
>> ---
>>   drivers/firmware/efi/libstub/riscv-stub.c | 12 +++++++++---
>>   1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/firmware/efi/libstub/riscv-stub.c b/drivers/firmware/efi/libstub/riscv-stub.c
>> index 9e85e58d1f27..d748533f1329 100644
>> --- a/drivers/firmware/efi/libstub/riscv-stub.c
>> +++ b/drivers/firmware/efi/libstub/riscv-stub.c
>> @@ -29,7 +29,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 +40,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(*(fdt64_t *)prop);
> 
> Does RISC-V care about alignment? A 64-bit quantity is not guaranteed
> to appear 64-bit aligned in the DT, and the cast violates C alignment
> rules, so this should probably used get_unaligned_be64() or something
> like that.

When running in S-mode the SBI handles unaligned access but this has a 
performance penalty.

We could use fdt64_to_cpu(__get_unaligned_t(fdt64_t, prop)) here.

Best regards

Heinrich

> 
> 
>> +       else
>>                  return -EINVAL;
>>
>> -       hartid = fdt32_to_cpu(*prop);
>>          return 0;
>>   }
>>
>> --
>> 2.25.1
>>

Re: [PATCH 5/5] riscv/efi_stub: Support for 64bit boot-hartid

[Atish Patra]

On Wed, May 25, 2022 at 9:09 AM Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> On 5/25/22 17:48, Ard Biesheuvel wrote:
> > On Wed, 25 May 2022 at 17:11, Sunil V L <sunilvl@ventanamicro.com> wrote:
> >>
> >> 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 <sunilvl@ventanamicro.com>
> >> ---
> >>   drivers/firmware/efi/libstub/riscv-stub.c | 12 +++++++++---
> >>   1 file changed, 9 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/firmware/efi/libstub/riscv-stub.c b/drivers/firmware/efi/libstub/riscv-stub.c
> >> index 9e85e58d1f27..d748533f1329 100644
> >> --- a/drivers/firmware/efi/libstub/riscv-stub.c
> >> +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> >> @@ -29,7 +29,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 +40,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(*(fdt64_t *)prop);
> >
> > Does RISC-V care about alignment? A 64-bit quantity is not guaranteed
> > to appear 64-bit aligned in the DT, and the cast violates C alignment
> > rules, so this should probably used get_unaligned_be64() or something
> > like that.
>
> When running in S-mode the SBI handles unaligned access but this has a
> performance penalty.
>
> We could use fdt64_to_cpu(__get_unaligned_t(fdt64_t, prop)) here.
>

It is better to avoid unaligned access in the kernel. There are some
plans to disable
misaligned load/store emulation in the firmware if user space requests
it via prctl.

We need another SBI extension to do that. The idea is to keep it
enabled by default in the firmware but
userspace should have an option to disable it via prctl. If we make
sure that the kernel doesn't invoke any
unaligned access, this feature can be implemented easily.

> Best regards
>
> Heinrich
>
> >
> >
> >> +       else
> >>                  return -EINVAL;
> >>
> >> -       hartid = fdt32_to_cpu(*prop);
> >>          return 0;
> >>   }
> >>
> >> --
> >> 2.25.1
> >>
>


-- 
Regards,
Atish

Re: [PATCH 5/5] riscv/efi_stub: Support for 64bit boot-hartid

[Jessica Clarke]

On 26 May 2022, at 00:11, Atish Patra <atishp@atishpatra.org> wrote:
> 
> On Wed, May 25, 2022 at 9:09 AM Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>> 
>> On 5/25/22 17:48, Ard Biesheuvel wrote:
>>> On Wed, 25 May 2022 at 17:11, Sunil V L <sunilvl@ventanamicro.com> wrote:
>>>> 
>>>> 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 <sunilvl@ventanamicro.com>
>>>> ---
>>>> drivers/firmware/efi/libstub/riscv-stub.c | 12 +++++++++---
>>>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>>> 
>>>> diff --git a/drivers/firmware/efi/libstub/riscv-stub.c b/drivers/firmware/efi/libstub/riscv-stub.c
>>>> index 9e85e58d1f27..d748533f1329 100644
>>>> --- a/drivers/firmware/efi/libstub/riscv-stub.c
>>>> +++ b/drivers/firmware/efi/libstub/riscv-stub.c
>>>> @@ -29,7 +29,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 +40,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(*(fdt64_t *)prop);
>>> 
>>> Does RISC-V care about alignment? A 64-bit quantity is not guaranteed
>>> to appear 64-bit aligned in the DT, and the cast violates C alignment
>>> rules, so this should probably used get_unaligned_be64() or something
>>> like that.
>> 
>> When running in S-mode the SBI handles unaligned access but this has a
>> performance penalty.
>> 
>> We could use fdt64_to_cpu(__get_unaligned_t(fdt64_t, prop)) here.
>> 
> 
> It is better to avoid unaligned access in the kernel. There are some
> plans to disable
> misaligned load/store emulation in the firmware if user space requests
> it via prctl.

Why?

Jess

> We need another SBI extension to do that. The idea is to keep it
> enabled by default in the firmware but
> userspace should have an option to disable it via prctl. If we make
> sure that the kernel doesn't invoke any
> unaligned access, this feature can be implemented easily.
> 
>> Best regards
>> 
>> Heinrich
>> 
>>> 
>>> 
>>>> + else
>>>> return -EINVAL;
>>>> 
>>>> - hartid = fdt32_to_cpu(*prop);
>>>> return 0;
>>>> }
>>>> 
>>>> --
>>>> 2.25.1
>>>> 
>> 
> 
> 
> -- 
> Regards,
> Atish
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 5/5] riscv/efi_stub: Support for 64bit boot-hartid

[Atish Patra]

On Wed, May 25, 2022 at 4:36 PM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>
> On 26 May 2022, at 00:11, Atish Patra <atishp@atishpatra.org> wrote:
> >
> > On Wed, May 25, 2022 at 9:09 AM Heinrich Schuchardt
> > <heinrich.schuchardt@canonical.com> wrote:
> >>
> >> On 5/25/22 17:48, Ard Biesheuvel wrote:
> >>> On Wed, 25 May 2022 at 17:11, Sunil V L <sunilvl@ventanamicro.com> wrote:
> >>>>
> >>>> 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 <sunilvl@ventanamicro.com>
> >>>> ---
> >>>> drivers/firmware/efi/libstub/riscv-stub.c | 12 +++++++++---
> >>>> 1 file changed, 9 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/drivers/firmware/efi/libstub/riscv-stub.c b/drivers/firmware/efi/libstub/riscv-stub.c
> >>>> index 9e85e58d1f27..d748533f1329 100644
> >>>> --- a/drivers/firmware/efi/libstub/riscv-stub.c
> >>>> +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> >>>> @@ -29,7 +29,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 +40,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(*(fdt64_t *)prop);
> >>>
> >>> Does RISC-V care about alignment? A 64-bit quantity is not guaranteed
> >>> to appear 64-bit aligned in the DT, and the cast violates C alignment
> >>> rules, so this should probably used get_unaligned_be64() or something
> >>> like that.
> >>
> >> When running in S-mode the SBI handles unaligned access but this has a
> >> performance penalty.
> >>
> >> We could use fdt64_to_cpu(__get_unaligned_t(fdt64_t, prop)) here.
> >>
> >
> > It is better to avoid unaligned access in the kernel. There are some
> > plans to disable
> > misaligned load/store emulation in the firmware if user space requests
> > it via prctl.
>
> Why?
>
To support prctl call with PR_SET_UNALIGN

> Jess
>
> > We need another SBI extension to do that. The idea is to keep it
> > enabled by default in the firmware but
> > userspace should have an option to disable it via prctl. If we make
> > sure that the kernel doesn't invoke any
> > unaligned access, this feature can be implemented easily.
> >
> >> Best regards
> >>
> >> Heinrich
> >>
> >>>
> >>>
> >>>> + else
> >>>> return -EINVAL;
> >>>>
> >>>> - hartid = fdt32_to_cpu(*prop);
> >>>> return 0;
> >>>> }
> >>>>
> >>>> --
> >>>> 2.25.1
> >>>>
> >>
> >
> >
> > --
> > Regards,
> > Atish
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
>


-- 
Regards,
Atish

Re: [PATCH 5/5] riscv/efi_stub: Support for 64bit boot-hartid

[Jessica Clarke]

On 26 May 2022, at 00:49, Atish Patra <atishp@atishpatra.org> wrote:
> 
> On Wed, May 25, 2022 at 4:36 PM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>> 
>> On 26 May 2022, at 00:11, Atish Patra <atishp@atishpatra.org> wrote:
>>> 
>>> On Wed, May 25, 2022 at 9:09 AM Heinrich Schuchardt
>>> <heinrich.schuchardt@canonical.com> wrote:
>>>> 
>>>> On 5/25/22 17:48, Ard Biesheuvel wrote:
>>>>> On Wed, 25 May 2022 at 17:11, Sunil V L <sunilvl@ventanamicro.com> wrote:
>>>>>> 
>>>>>> 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 <sunilvl@ventanamicro.com>
>>>>>> ---
>>>>>> drivers/firmware/efi/libstub/riscv-stub.c | 12 +++++++++---
>>>>>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>>>>> 
>>>>>> diff --git a/drivers/firmware/efi/libstub/riscv-stub.c b/drivers/firmware/efi/libstub/riscv-stub.c
>>>>>> index 9e85e58d1f27..d748533f1329 100644
>>>>>> --- a/drivers/firmware/efi/libstub/riscv-stub.c
>>>>>> +++ b/drivers/firmware/efi/libstub/riscv-stub.c
>>>>>> @@ -29,7 +29,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 +40,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(*(fdt64_t *)prop);
>>>>> 
>>>>> Does RISC-V care about alignment? A 64-bit quantity is not guaranteed
>>>>> to appear 64-bit aligned in the DT, and the cast violates C alignment
>>>>> rules, so this should probably used get_unaligned_be64() or something
>>>>> like that.
>>>> 
>>>> When running in S-mode the SBI handles unaligned access but this has a
>>>> performance penalty.
>>>> 
>>>> We could use fdt64_to_cpu(__get_unaligned_t(fdt64_t, prop)) here.
>>>> 
>>> 
>>> It is better to avoid unaligned access in the kernel. There are some
>>> plans to disable
>>> misaligned load/store emulation in the firmware if user space requests
>>> it via prctl.
>> 
>> Why?
>> 
> To support prctl call with PR_SET_UNALIGN

Is that needed? It’s almost entirely unused as far as I can tell, with
all but one use turning unaligned fixups *on*, and the other use being
IA-64-specific. What is the actual use case other than seeing a thing
that exists on some architectures and wanting to have it do something
on RISC-V?

Jess

Re: [PATCH 5/5] riscv/efi_stub: Support for 64bit boot-hartid

[Sunil V L]

On Wed, May 25, 2022 at 06:09:05PM +0200, Heinrich Schuchardt wrote:
> On 5/25/22 17:48, Ard Biesheuvel wrote:
> > On Wed, 25 May 2022 at 17:11, Sunil V L <sunilvl@ventanamicro.com> wrote:
> > > 
> > > 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 <sunilvl@ventanamicro.com>
> > > ---
> > >   drivers/firmware/efi/libstub/riscv-stub.c | 12 +++++++++---
> > >   1 file changed, 9 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/firmware/efi/libstub/riscv-stub.c b/drivers/firmware/efi/libstub/riscv-stub.c
> > > index 9e85e58d1f27..d748533f1329 100644
> > > --- a/drivers/firmware/efi/libstub/riscv-stub.c
> > > +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> > > @@ -29,7 +29,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 +40,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(*(fdt64_t *)prop);
> > 
> > Does RISC-V care about alignment? A 64-bit quantity is not guaranteed
> > to appear 64-bit aligned in the DT, and the cast violates C alignment
> > rules, so this should probably used get_unaligned_be64() or something
> > like that.
> 
> When running in S-mode the SBI handles unaligned access but this has a
> performance penalty.
> 
> We could use fdt64_to_cpu(__get_unaligned_t(fdt64_t, prop)) here.

Thank you very much for the feedback. Have updated as per your
suggestion and sent V2.

Thanks
Sunil

> 
> Best regards
> 
> Heinrich
> 
> > 
> > 
> > > +       else
> > >                  return -EINVAL;
> > > 
> > > -       hartid = fdt32_to_cpu(*prop);
> > >          return 0;
> > >   }
> > > 
> > > --
> > > 2.25.1
> > > 
> 

Re: [PATCH 2/5] riscv: cpu_ops_spinwait: Support for 64bit hartid

[Sunil V L]

On Wed, May 25, 2022 at 05:27:51PM +0200, Heinrich Schuchardt wrote:
> On 5/25/22 17:11, Sunil V L 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 <sunilvl@ventanamicro.com>
> > ---
> >   arch/riscv/kernel/cpu_ops_spinwait.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/riscv/kernel/cpu_ops_spinwait.c b/arch/riscv/kernel/cpu_ops_spinwait.c
> > index 346847f6c41c..51ac07514a62 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
> 
> This line follows:
> 
> if (hartid == INVALID_HARTID || hartid >= NR_CPUS)
> 
> INVALID_HARTID is defined as ULONG_MAX. Please, mention that you are fixing
> a bug:
> 
> Fixes: c78f94f35cf648 ("RISC-V: Use __cpu_up_stack/task_pointer only for
> spinwait method")
> 
> NR_CPUS alias CONFIG_NR_CPUS is an int. You should convert it to unsigned
> before comparing it to hartid to avoid build warnings.

Thank you for the feedback. Have modified the patch and commit message
as per your suggestion in V2 version. Please check.

Thanks
Sunil
> 
> Best regards
> 
> Heinrich
>