lkml.org 
[lkml]   [2018]   [Aug]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC PATCH 2/5] RISC-V: Use Linux logical cpu number instead of hartid
    From
    Date
    On 8/15/18 9:24 PM, Anup Patel wrote:
    > On Thu, Aug 16, 2018 at 5:26 AM, Atish Patra <atish.patra@wdc.com> wrote:
    >> Setup the cpu_logical_map during boot. Moreover, every SBI call
    >> and PLIC context are based on the physical hartid. Use the logical
    >> cpu to hartid mapping to pass correct hartid to respective functions.
    >>
    >> Signed-off-by: Atish Patra <atish.patra@wdc.com>
    >> ---
    >> arch/riscv/include/asm/tlbflush.h | 17 +++++++++++++----
    >> arch/riscv/kernel/cpu.c | 4 +++-
    >> arch/riscv/kernel/setup.c | 10 ++++++++++
    >> arch/riscv/kernel/smp.c | 24 +++++++++++++++---------
    >> arch/riscv/kernel/smpboot.c | 30 ++++++++++++++++++------------
    >> drivers/clocksource/riscv_timer.c | 12 ++++++++----
    >> drivers/irqchip/irq-sifive-plic.c | 11 +++++++----
    >> 7 files changed, 74 insertions(+), 34 deletions(-)
    >>
    >> diff --git a/arch/riscv/include/asm/tlbflush.h b/arch/riscv/include/asm/tlbflush.h
    >> index 85c2d8ba..ecfd9b0e 100644
    >> --- a/arch/riscv/include/asm/tlbflush.h
    >> +++ b/arch/riscv/include/asm/tlbflush.h
    >> @@ -16,6 +16,7 @@
    >> #define _ASM_RISCV_TLBFLUSH_H
    >>
    >> #include <linux/mm_types.h>
    >> +#include <asm/smp.h>
    >>
    >> /*
    >> * Flush entire local TLB. 'sfence.vma' implicitly fences with the instruction
    >> @@ -49,13 +50,21 @@ static inline void flush_tlb_range(struct vm_area_struct *vma,
    >>
    >> #include <asm/sbi.h>
    >>
    >> -#define flush_tlb_all() sbi_remote_sfence_vma(NULL, 0, -1)
    >> +static inline void remote_sfence_vma(struct cpumask *cmask, unsigned long start,
    >> + unsigned long size)
    >> +{
    >> + struct cpumask hmask;
    >> +
    >> + cpuid_to_hartid_mask(cmask, &hmask);
    >> + sbi_remote_sfence_vma(hmask.bits, start, size);
    >> +}
    >> +
    >> +#define flush_tlb_all() remote_sfence_vma(NULL, 0, -1)
    >> #define flush_tlb_page(vma, addr) flush_tlb_range(vma, addr, 0)
    >> #define flush_tlb_range(vma, start, end) \
    >> - sbi_remote_sfence_vma(mm_cpumask((vma)->vm_mm)->bits, \
    >> - start, (end) - (start))
    >> + remote_sfence_vma(mm_cpumask((vma)->vm_mm), start, (end) - (start))
    >> #define flush_tlb_mm(mm) \
    >> - sbi_remote_sfence_vma(mm_cpumask(mm)->bits, 0, -1)
    >> + remote_sfence_vma(mm_cpumask(mm), 0, -1)
    >>
    >> #endif /* CONFIG_SMP */
    >>
    >> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
    >> index ca6c81e5..f8a18ace 100644
    >> --- a/arch/riscv/kernel/cpu.c
    >> +++ b/arch/riscv/kernel/cpu.c
    >> @@ -14,6 +14,7 @@
    >> #include <linux/init.h>
    >> #include <linux/seq_file.h>
    >> #include <linux/of.h>
    >> +#include <asm/smp.h>
    >>
    >> /* Return -1 if not a valid hart */
    >> int riscv_of_processor_hart(struct device_node *node)
    >> @@ -79,7 +80,8 @@ static void c_stop(struct seq_file *m, void *v)
    >> static int c_show(struct seq_file *m, void *v)
    >> {
    >> unsigned long hart_id = (unsigned long)v - 1;
    >> - struct device_node *node = of_get_cpu_node(hart_id, NULL);
    >> + struct device_node *node = of_get_cpu_node(cpu_logical_map(hart_id),
    >> + NULL);
    >
    > The hart_id is misleading name here. It should be cpu_id. Please replace
    > all instances of hart_id with cpu_id and where hard ID is to be displayed
    > use cpu_logical_map(cpu_id).
    >
    Correct. Thanks for catching it. I will fix it in v2.


    >> const char *compat, *isa, *mmu;
    >>
    >> seq_printf(m, "hart\t: %lu\n", hart_id);
    >> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
    >> index e21ed481..97b586f8 100644
    >> --- a/arch/riscv/kernel/setup.c
    >> +++ b/arch/riscv/kernel/setup.c
    >> @@ -84,6 +84,16 @@ atomic_t hart_lottery;
    >>
    >> u64 __cpu_logical_map[NR_CPUS];
    >>
    >> +void __init smp_setup_processor_id(void)
    >> +{
    >> + int cpu = smp_processor_id();
    >> +
    >> + cpu_logical_map(0) = cpu;
    >
    > I think this should be:
    > cpu_logical_map(cpu) = hart_id;
    >
    > Here hart_id for boot CPU will be value of a0 register passed at boot-time.
    >
    I will change the variable name to hart_id. The assembly code in head.S
    have already stored hart id in thread info structure. So
    smp_processor_id() and hart id would be the same.


    Regards,
    Atish
    >> +
    >> + /* Change the boot cpu ID in thread_info */
    >> + current->thread_info.cpu = 0;
    >> +}
    >> +
    >> #ifdef CONFIG_BLK_DEV_INITRD
    >> static void __init setup_initrd(void)
    >> {
    >> diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
    >> index d55379ee..4ab70480 100644
    >> --- a/arch/riscv/kernel/smp.c
    >> +++ b/arch/riscv/kernel/smp.c
    >> @@ -97,14 +97,18 @@ void riscv_software_interrupt(void)
    >> static void
    >> send_ipi_message(const struct cpumask *to_whom, enum ipi_message_type operation)
    >> {
    >> - int i;
    >> + int cpuid, hartid;
    >> + struct cpumask hartid_mask;
    >>
    >> + cpumask_clear(&hartid_mask);
    >> mb();
    >> - for_each_cpu(i, to_whom)
    >> - set_bit(operation, &ipi_data[i].bits);
    >> -
    >> + for_each_cpu(cpuid, to_whom) {
    >> + set_bit(operation, &ipi_data[cpuid].bits);
    >> + hartid = cpu_logical_map(cpuid);
    >> + cpumask_set_cpu(hartid, &hartid_mask);
    >> + }
    >> mb();
    >> - sbi_send_ipi(cpumask_bits(to_whom));
    >> + sbi_send_ipi(cpumask_bits(&hartid_mask));
    >> }
    >>
    >> void arch_send_call_function_ipi_mask(struct cpumask *mask)
    >> @@ -146,7 +150,7 @@ void smp_send_reschedule(int cpu)
    >> void flush_icache_mm(struct mm_struct *mm, bool local)
    >> {
    >> unsigned int cpu;
    >> - cpumask_t others, *mask;
    >> + cpumask_t others, hmask, *mask;
    >>
    >> preempt_disable();
    >>
    >> @@ -164,9 +168,11 @@ void flush_icache_mm(struct mm_struct *mm, bool local)
    >> */
    >> cpumask_andnot(&others, mm_cpumask(mm), cpumask_of(cpu));
    >> local |= cpumask_empty(&others);
    >> - if (mm != current->active_mm || !local)
    >> - sbi_remote_fence_i(others.bits);
    >> - else {
    >> + if (mm != current->active_mm || !local) {
    >> + cpumask_clear(&hmask);
    >> + cpuid_to_hartid_mask(&others, &hmask);
    >> + sbi_remote_fence_i(hmask.bits);
    >> + } else {
    >> /*
    >> * It's assumed that at least one strongly ordered operation is
    >> * performed on this hart between setting a hart's cpumask bit
    >> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
    >> index 56abab6a..6ab2bb1b 100644
    >> --- a/arch/riscv/kernel/smpboot.c
    >> +++ b/arch/riscv/kernel/smpboot.c
    >> @@ -50,27 +50,33 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
    >> void __init setup_smp(void)
    >> {
    >> struct device_node *dn = NULL;
    >> - int hart, im_okay_therefore_i_am = 0;
    >> + int hart, found_boot_cpu = 0;
    >> + int cpuid = 1;
    >>
    >> while ((dn = of_find_node_by_type(dn, "cpu"))) {
    >> hart = riscv_of_processor_hart(dn);
    >> - if (hart >= 0) {
    >> - set_cpu_possible(hart, true);
    >> - set_cpu_present(hart, true);
    >> - if (hart == smp_processor_id()) {
    >> - BUG_ON(im_okay_therefore_i_am);
    >> - im_okay_therefore_i_am = 1;
    >> - }
    >> +
    >> + if (hart < 0)
    >> + continue;
    >> + if (hart == cpu_logical_map(0)) {
    >> + BUG_ON(found_boot_cpu);
    >> + found_boot_cpu = 1;
    >> + continue;
    >> }
    >> +
    >> + cpu_logical_map(cpuid) = hart;
    >> + set_cpu_possible(cpuid, true);
    >> + set_cpu_present(cpuid, true);
    >> + cpuid++;
    >> }
    >>
    >> - BUG_ON(!im_okay_therefore_i_am);
    >> + BUG_ON(!found_boot_cpu);
    >> }
    >>
    >> int __cpu_up(unsigned int cpu, struct task_struct *tidle)
    >> {
    >> + int hartid = cpu_logical_map(cpu);
    >> tidle->thread_info.cpu = cpu;
    >> -
    >> /*
    >> * On RISC-V systems, all harts boot on their own accord. Our _start
    >> * selects the first hart to boot the kernel and causes the remainder
    >> @@ -79,8 +85,8 @@ int __cpu_up(unsigned int cpu, struct task_struct *tidle)
    >> * the spinning harts that they can continue the boot process.
    >> */
    >> smp_mb();
    >> - __cpu_up_stack_pointer[cpu] = task_stack_page(tidle) + THREAD_SIZE;
    >> - __cpu_up_task_pointer[cpu] = tidle;
    >> + __cpu_up_stack_pointer[hartid] = task_stack_page(tidle) + THREAD_SIZE;
    >> + __cpu_up_task_pointer[hartid] = tidle;
    >>
    >> while (!cpu_online(cpu))
    >> cpu_relax();
    >> diff --git a/drivers/clocksource/riscv_timer.c b/drivers/clocksource/riscv_timer.c
    >> index 4e8b347e..f1f205e5 100644
    >> --- a/drivers/clocksource/riscv_timer.c
    >> +++ b/drivers/clocksource/riscv_timer.c
    >> @@ -8,6 +8,7 @@
    >> #include <linux/cpu.h>
    >> #include <linux/delay.h>
    >> #include <linux/irq.h>
    >> +#include <asm/smp.h>
    >> #include <asm/sbi.h>
    >>
    >> /*
    >> @@ -84,13 +85,16 @@ void riscv_timer_interrupt(void)
    >>
    >> static int __init riscv_timer_init_dt(struct device_node *n)
    >> {
    >> - int cpu_id = riscv_of_processor_hart(n), error;
    >> + int cpuid, hartid, error;
    >> struct clocksource *cs;
    >>
    >> - if (cpu_id != smp_processor_id())
    >> + hartid = riscv_of_processor_hart(n);
    >> + cpuid = riscv_hartid_to_cpuid(hartid);
    >> +
    >> + if (cpuid != smp_processor_id())
    >> return 0;
    >>
    >> - cs = per_cpu_ptr(&riscv_clocksource, cpu_id);
    >> + cs = per_cpu_ptr(&riscv_clocksource, cpuid);
    >> clocksource_register_hz(cs, riscv_timebase);
    >>
    >> error = cpuhp_setup_state(CPUHP_AP_RISCV_TIMER_STARTING,
    >> @@ -98,7 +102,7 @@ static int __init riscv_timer_init_dt(struct device_node *n)
    >> riscv_timer_starting_cpu, riscv_timer_dying_cpu);
    >> if (error)
    >> pr_err("RISCV timer register failed [%d] for cpu = [%d]\n",
    >> - error, cpu_id);
    >> + error, cpuid);
    >> return error;
    >> }
    >>
    >> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
    >> index 298685e5..2eb2e78c 100644
    >> --- a/drivers/irqchip/irq-sifive-plic.c
    >> +++ b/drivers/irqchip/irq-sifive-plic.c
    >> @@ -15,6 +15,7 @@
    >> #include <linux/of_irq.h>
    >> #include <linux/platform_device.h>
    >> #include <linux/spinlock.h>
    >> +#include <asm/smp.h>
    >>
    >> /*
    >> * This driver implements a version of the RISC-V PLIC with the actual layout
    >> @@ -93,10 +94,11 @@ static inline void plic_toggle(int ctxid, int hwirq, int enable)
    >> static inline void plic_irq_toggle(struct irq_data *d, int enable)
    >> {
    >> int cpu;
    >> + struct plic_handler *handler;
    >>
    >> writel(enable, plic_regs + PRIORITY_BASE + d->hwirq * PRIORITY_PER_ID);
    >> for_each_cpu(cpu, irq_data_get_affinity_mask(d)) {
    >> - struct plic_handler *handler = per_cpu_ptr(&plic_handlers, cpu);
    >> + handler = per_cpu_ptr(&plic_handlers, cpu);
    >>
    >> if (handler->present)
    >> plic_toggle(handler->ctxid, d->hwirq, enable);
    >> @@ -217,7 +219,7 @@ static int __init plic_init(struct device_node *node,
    >> struct of_phandle_args parent;
    >> struct plic_handler *handler;
    >> irq_hw_number_t hwirq;
    >> - int cpu;
    >> + int cpu, hartid;
    >>
    >> if (of_irq_parse_one(node, i, &parent)) {
    >> pr_err("failed to parse parent for context %d.\n", i);
    >> @@ -228,12 +230,13 @@ static int __init plic_init(struct device_node *node,
    >> if (parent.args[0] == -1)
    >> continue;
    >>
    >> - cpu = plic_find_hart_id(parent.np);
    >> - if (cpu < 0) {
    >> + hartid = plic_find_hart_id(parent.np);
    >> + if (hartid < 0) {
    >> pr_warn("failed to parse hart ID for context %d.\n", i);
    >> continue;
    >> }
    >>
    >> + cpu = riscv_hartid_to_cpuid(hartid);
    >> handler = per_cpu_ptr(&plic_handlers, cpu);
    >> handler->present = true;
    >> handler->ctxid = i;
    >> --
    >> 2.7.4
    >>
    >
    > Regards,
    > Anup
    >

    \
     
     \ /
      Last update: 2018-08-16 07:23    [W:5.381 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site