[PATCH 6/8] RISC-V: Use mmgrab()

[Palmer Dabbelt]

f1f1007644ff ("mm: add new mmgrab() helper") added a helper that we
missed out on.

Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/kernel/smpboot.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index e1f6a5ad0416..5437a04babcd 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -79,8 +80,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;
+   WRITE_ONCE(__cpu_up_stack_pointer[cpu], task_stack_page(tidle) + 
THREAD_SIZE);
+   WRITE_ONCE(__cpu_up_task_pointer[cpu], tidle);
 
while (!cpu_online(cpu))
cpu_relax();
@@ -100,7 +101,7 @@ asmlinkage void __init smp_callin(void)
struct mm_struct *mm = _mm;
 
/* All kernel threads share the same mm context.  */
-   atomic_inc(>mm_count);
+   mmgrab(mm);
current->active_mm = mm;
 
trap_init();
-- 
2.16.4

[PATCH 7/8] RISC-V: Comment on the TLB flush in smp_callin()

[Palmer Dabbelt]

This isn't readily apparent from reading the code.

Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/kernel/smpboot.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 5437a04babcd..953bc540207d 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -107,6 +107,8 @@ asmlinkage void __init smp_callin(void)
trap_init();
notify_cpu_starting(smp_processor_id());
set_cpu_online(smp_processor_id(), 1);
+   /* Remote TLB flushes are ignored while the CPU is offline, so emit a 
local
+* TLB flush right now just in case. */
local_flush_tlb_all();
local_irq_enable();
preempt_disable();
-- 
2.16.4

[PATCH 4/8] RISC-V: Filter ISA and MMU values in cpuinfo

[Palmer Dabbelt]

We shouldn't be directly passing device tree values to userspace, both
because there could be mistakes in device trees and because the kernel
doesn't support arbitrary ISAs.

Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/kernel/cpu.c | 62 +++--
 1 file changed, 55 insertions(+), 7 deletions(-)

diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index 19e98c1710dd..a18b4e3962a1 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -58,6 +58,57 @@ int riscv_of_processor_hartid(struct device_node *node)
 
 #ifdef CONFIG_PROC_FS
 
+static void print_isa(struct seq_file *f, const char *orig_isa)
+{
+static const char *ext = "mafdc";
+const char *isa = orig_isa;
+const char *e;
+
+/* Linux doesn't support rv32e or rv128i, and we only support booting
+ * kernels on harts with the same ISA that the kernel is compiled for. */
+#if defined(CONFIG_32BIT)
+if (strncmp(isa, "rv32i", 5) != 0)
+return;
+#elif defined(CONFIG_64BIT)
+if (strncmp(isa, "rv64i", 5) != 0)
+return;
+#endif
+
+/* Print the base ISA, as we already know it's legal. */
+seq_printf(f, "isa\t: ");
+seq_write(f, isa, 5);
+isa += 5;
+
+/* Check the rest of the ISA string for valid extensions, printing those we
+ * find.  RISC-V ISA strings define an order, so we only print the
+ * extension bits when they're in order. */
+for (e = ext; *e != '\0'; ++e) {
+if (isa[0] == e[0]) {
+seq_write(f, isa, 1);
+isa++;
+}
+}
+
+/* If we were given an unsupported ISA in the device tree then print a bit
+ * of info describing what went wrong. */
+if (isa[0] != '\0')
+pr_info("unsupported ISA \"%s\" in device tree", orig_isa);
+}
+
+static void print_mmu(struct seq_file *f, const char *mmu_type)
+{
+#if defined(CONFIG_32BIT)
+if (strcmp(mmu_type, "riscv,sv32") != 0)
+return;
+#elif defined(CONFIG_64BIT)
+if ((strcmp(mmu_type, "riscv,sv39") != 0)
+&& (strcmp(mmu_type, "riscv,sv48") != 0))
+return;
+#endif
+
+seq_printf(f, "mmu\t: %s\n", mmu_type+6);
+}
+
 static void *c_start(struct seq_file *m, loff_t *pos)
 {
*pos = cpumask_next(*pos - 1, cpu_online_mask);
@@ -83,13 +134,10 @@ static int c_show(struct seq_file *m, void *v)
const char *compat, *isa, *mmu;
 
seq_printf(m, "hart\t: %lu\n", hart_id);
-   if (!of_property_read_string(node, "riscv,isa", )
-   && isa[0] == 'r'
-   && isa[1] == 'v')
-   seq_printf(m, "isa\t: %s\n", isa);
-   if (!of_property_read_string(node, "mmu-type", )
-   && !strncmp(mmu, "riscv,", 6))
-   seq_printf(m, "mmu\t: %s\n", mmu+6);
+   if (!of_property_read_string(node, "riscv,isa", ))
+print_isa(m, isa);
+   if (!of_property_read_string(node, "mmu-type", ))
+print_mmu(m, mmu);
if (!of_property_read_string(node, "compatible", )
&& strcmp(compat, "riscv"))
seq_printf(m, "uarch\t: %s\n", compat);
-- 
2.16.4

[PATCH 6/8] RISC-V: Use mmgrab()

[Palmer Dabbelt]

f1f1007644ff ("mm: add new mmgrab() helper") added a helper that we
missed out on.

Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/kernel/smpboot.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index e1f6a5ad0416..5437a04babcd 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -79,8 +80,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;
+   WRITE_ONCE(__cpu_up_stack_pointer[cpu], task_stack_page(tidle) + 
THREAD_SIZE);
+   WRITE_ONCE(__cpu_up_task_pointer[cpu], tidle);
 
while (!cpu_online(cpu))
cpu_relax();
@@ -100,7 +101,7 @@ asmlinkage void __init smp_callin(void)
struct mm_struct *mm = _mm;
 
/* All kernel threads share the same mm context.  */
-   atomic_inc(>mm_count);
+   mmgrab(mm);
current->active_mm = mm;
 
trap_init();
-- 
2.16.4

[PATCH 7/8] RISC-V: Comment on the TLB flush in smp_callin()

[Palmer Dabbelt]

This isn't readily apparent from reading the code.

Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/kernel/smpboot.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 5437a04babcd..953bc540207d 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -107,6 +107,8 @@ asmlinkage void __init smp_callin(void)
trap_init();
notify_cpu_starting(smp_processor_id());
set_cpu_online(smp_processor_id(), 1);
+   /* Remote TLB flushes are ignored while the CPU is offline, so emit a 
local
+* TLB flush right now just in case. */
local_flush_tlb_all();
local_irq_enable();
preempt_disable();
-- 
2.16.4

[PATCH 5/8] RISC-V: Rename im_okay_therefore_i_am to found_boot_cpu

[Palmer Dabbelt]

The old name was a bit odd.

Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/kernel/smpboot.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 5f29f8562cf6..e1f6a5ad0416 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -50,7 +50,7 @@ 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;
 
while ((dn = of_find_node_by_type(dn, "cpu"))) {
hart = riscv_of_processor_hartid(dn);
@@ -58,13 +58,13 @@ void __init setup_smp(void)
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;
+   BUG_ON(found_boot_cpu);
+   found_boot_cpu = 1;
}
}
}
 
-   BUG_ON(!im_okay_therefore_i_am);
+   BUG_ON(!found_boot_cpu);
 }
 
 int __cpu_up(unsigned int cpu, struct task_struct *tidle)
-- 
2.16.4

[PATCH 2/8] RISC-V: Don't set cacheinfo.{physical_line_partition,attributes}

[Palmer Dabbelt]

These are just hard coded in the RISC-V port, which doesn't make any
sense.  We should probably be setting these from device tree entries
when they exist, but for now I think it's saner to just leave them all
as their default values.

Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/kernel/cacheinfo.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/arch/riscv/kernel/cacheinfo.c b/arch/riscv/kernel/cacheinfo.c
index 0bc86e5f8f3f..cb35ffd8ec6b 100644
--- a/arch/riscv/kernel/cacheinfo.c
+++ b/arch/riscv/kernel/cacheinfo.c
@@ -22,13 +22,6 @@ static void ci_leaf_init(struct cacheinfo *this_leaf,
 {
this_leaf->level = level;
this_leaf->type = type;
-   /* not a sector cache */
-   this_leaf->physical_line_partition = 1;
-   /* TODO: Add to DTS */
-   this_leaf->attributes =
-   CACHE_WRITE_BACK
-   | CACHE_READ_ALLOCATE
-   | CACHE_WRITE_ALLOCATE;
 }
 
 static int __init_cache_level(unsigned int cpu)
-- 
2.16.4

[PATCH 3/8] RISC-V: Rename riscv_of_processor_hart to riscv_of_processor_hartid

[Palmer Dabbelt]

It's a bit confusing exactly what this function does: it actually
returns the hartid of an OF processor node, failing with -1 on invalid
nodes.  I've changed the name to _hartid() in order to make that a bit
more clear, as well as adding a comment.

Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/include/asm/processor.h | 4 +++-
 arch/riscv/kernel/cpu.c| 2 +-
 arch/riscv/kernel/smpboot.c| 2 +-
 drivers/clocksource/riscv_timer.c  | 2 +-
 drivers/irqchip/irq-sifive-plic.c  | 2 +-
 5 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/riscv/include/asm/processor.h 
b/arch/riscv/include/asm/processor.h
index 3fe4af8147d2..9d32670d84a4 100644
--- a/arch/riscv/include/asm/processor.h
+++ b/arch/riscv/include/asm/processor.h
@@ -88,7 +88,9 @@ static inline void wait_for_interrupt(void)
 }
 
 struct device_node;
-extern int riscv_of_processor_hart(struct device_node *node);
+/* Returns the hart ID of the given device tree node, or -1 if the device tree
+ * node isn't a RISC-V hart. */
+extern int riscv_of_processor_hartid(struct device_node *node);
 
 extern void riscv_fill_hwcap(void);
 
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index ca6c81e54e37..19e98c1710dd 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -16,7 +16,7 @@
 #include 
 
 /* Return -1 if not a valid hart */
-int riscv_of_processor_hart(struct device_node *node)
+int riscv_of_processor_hartid(struct device_node *node)
 {
const char *isa, *status;
u32 hart;
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 56abab6a9812..5f29f8562cf6 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -53,7 +53,7 @@ void __init setup_smp(void)
int hart, im_okay_therefore_i_am = 0;
 
while ((dn = of_find_node_by_type(dn, "cpu"))) {
-   hart = riscv_of_processor_hart(dn);
+   hart = riscv_of_processor_hartid(dn);
if (hart >= 0) {
set_cpu_possible(hart, true);
set_cpu_present(hart, true);
diff --git a/drivers/clocksource/riscv_timer.c 
b/drivers/clocksource/riscv_timer.c
index 4e8b347e43e2..ad7453fc3129 100644
--- a/drivers/clocksource/riscv_timer.c
+++ b/drivers/clocksource/riscv_timer.c
@@ -84,7 +84,7 @@ 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 cpu_id = riscv_of_processor_hartid(n), error;
struct clocksource *cs;
 
if (cpu_id != smp_processor_id())
diff --git a/drivers/irqchip/irq-sifive-plic.c 
b/drivers/irqchip/irq-sifive-plic.c
index 532e9d68c704..c55eaa31cde2 100644
--- a/drivers/irqchip/irq-sifive-plic.c
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -176,7 +176,7 @@ static int plic_find_hart_id(struct device_node *node)
 {
for (; node; node = node->parent) {
if (of_device_is_compatible(node, "riscv"))
-   return riscv_of_processor_hart(node);
+   return riscv_of_processor_hartid(node);
}
 
return -1;
-- 
2.16.4

[PATCH 3/8] RISC-V: Rename riscv_of_processor_hart to riscv_of_processor_hartid

[Palmer Dabbelt]

It's a bit confusing exactly what this function does: it actually
returns the hartid of an OF processor node, failing with -1 on invalid
nodes.  I've changed the name to _hartid() in order to make that a bit
more clear, as well as adding a comment.

Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/include/asm/processor.h | 4 +++-
 arch/riscv/kernel/cpu.c| 2 +-
 arch/riscv/kernel/smpboot.c| 2 +-
 drivers/clocksource/riscv_timer.c  | 2 +-
 drivers/irqchip/irq-sifive-plic.c  | 2 +-
 5 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/riscv/include/asm/processor.h 
b/arch/riscv/include/asm/processor.h
index 3fe4af8147d2..9d32670d84a4 100644
--- a/arch/riscv/include/asm/processor.h
+++ b/arch/riscv/include/asm/processor.h
@@ -88,7 +88,9 @@ static inline void wait_for_interrupt(void)
 }
 
 struct device_node;
-extern int riscv_of_processor_hart(struct device_node *node);
+/* Returns the hart ID of the given device tree node, or -1 if the device tree
+ * node isn't a RISC-V hart. */
+extern int riscv_of_processor_hartid(struct device_node *node);
 
 extern void riscv_fill_hwcap(void);
 
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index ca6c81e54e37..19e98c1710dd 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -16,7 +16,7 @@
 #include 
 
 /* Return -1 if not a valid hart */
-int riscv_of_processor_hart(struct device_node *node)
+int riscv_of_processor_hartid(struct device_node *node)
 {
const char *isa, *status;
u32 hart;
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 56abab6a9812..5f29f8562cf6 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -53,7 +53,7 @@ void __init setup_smp(void)
int hart, im_okay_therefore_i_am = 0;
 
while ((dn = of_find_node_by_type(dn, "cpu"))) {
-   hart = riscv_of_processor_hart(dn);
+   hart = riscv_of_processor_hartid(dn);
if (hart >= 0) {
set_cpu_possible(hart, true);
set_cpu_present(hart, true);
diff --git a/drivers/clocksource/riscv_timer.c 
b/drivers/clocksource/riscv_timer.c
index 4e8b347e43e2..ad7453fc3129 100644
--- a/drivers/clocksource/riscv_timer.c
+++ b/drivers/clocksource/riscv_timer.c
@@ -84,7 +84,7 @@ 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 cpu_id = riscv_of_processor_hartid(n), error;
struct clocksource *cs;
 
if (cpu_id != smp_processor_id())
diff --git a/drivers/irqchip/irq-sifive-plic.c 
b/drivers/irqchip/irq-sifive-plic.c
index 532e9d68c704..c55eaa31cde2 100644
--- a/drivers/irqchip/irq-sifive-plic.c
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -176,7 +176,7 @@ static int plic_find_hart_id(struct device_node *node)
 {
for (; node; node = node->parent) {
if (of_device_is_compatible(node, "riscv"))
-   return riscv_of_processor_hart(node);
+   return riscv_of_processor_hartid(node);
}
 
return -1;
-- 
2.16.4

[PATCH 5/8] RISC-V: Rename im_okay_therefore_i_am to found_boot_cpu

[Palmer Dabbelt]

The old name was a bit odd.

Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/kernel/smpboot.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 5f29f8562cf6..e1f6a5ad0416 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -50,7 +50,7 @@ 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;
 
while ((dn = of_find_node_by_type(dn, "cpu"))) {
hart = riscv_of_processor_hartid(dn);
@@ -58,13 +58,13 @@ void __init setup_smp(void)
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;
+   BUG_ON(found_boot_cpu);
+   found_boot_cpu = 1;
}
}
}
 
-   BUG_ON(!im_okay_therefore_i_am);
+   BUG_ON(!found_boot_cpu);
 }
 
 int __cpu_up(unsigned int cpu, struct task_struct *tidle)
-- 
2.16.4

[PATCH 2/8] RISC-V: Don't set cacheinfo.{physical_line_partition,attributes}

[Palmer Dabbelt]

These are just hard coded in the RISC-V port, which doesn't make any
sense.  We should probably be setting these from device tree entries
when they exist, but for now I think it's saner to just leave them all
as their default values.

Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/kernel/cacheinfo.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/arch/riscv/kernel/cacheinfo.c b/arch/riscv/kernel/cacheinfo.c
index 0bc86e5f8f3f..cb35ffd8ec6b 100644
--- a/arch/riscv/kernel/cacheinfo.c
+++ b/arch/riscv/kernel/cacheinfo.c
@@ -22,13 +22,6 @@ static void ci_leaf_init(struct cacheinfo *this_leaf,
 {
this_leaf->level = level;
this_leaf->type = type;
-   /* not a sector cache */
-   this_leaf->physical_line_partition = 1;
-   /* TODO: Add to DTS */
-   this_leaf->attributes =
-   CACHE_WRITE_BACK
-   | CACHE_READ_ALLOCATE
-   | CACHE_WRITE_ALLOCATE;
 }
 
 static int __init_cache_level(unsigned int cpu)
-- 
2.16.4

[PATCH 8/8] RISC-V: Disable preemption before enabling interrupts when booting secondary harts

[Palmer Dabbelt]

I'm not sure, but I think this was a bug: if the scheduler fired right
here then I believe it would blow up.

Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/kernel/smpboot.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 953bc540207d..45515cc70181 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -110,7 +110,9 @@ asmlinkage void __init smp_callin(void)
/* Remote TLB flushes are ignored while the CPU is offline, so emit a 
local
 * TLB flush right now just in case. */
local_flush_tlb_all();
-   local_irq_enable();
+   /* Disable preemption before enabling interrupts, so we don't try to
+* schedule a CPU that hasn't actually started yet. */
preempt_disable();
+   local_irq_enable();
cpu_startup_entry(CPUHP_AP_ONLINE_IDLE);
 }
-- 
2.16.4

[PATCH 8/8] RISC-V: Disable preemption before enabling interrupts when booting secondary harts

[Palmer Dabbelt]

I'm not sure, but I think this was a bug: if the scheduler fired right
here then I believe it would blow up.

Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/kernel/smpboot.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 953bc540207d..45515cc70181 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -110,7 +110,9 @@ asmlinkage void __init smp_callin(void)
/* Remote TLB flushes are ignored while the CPU is offline, so emit a 
local
 * TLB flush right now just in case. */
local_flush_tlb_all();
-   local_irq_enable();
+   /* Disable preemption before enabling interrupts, so we don't try to
+* schedule a CPU that hasn't actually started yet. */
preempt_disable();
+   local_irq_enable();
cpu_startup_entry(CPUHP_AP_ONLINE_IDLE);
 }
-- 
2.16.4

[PATCH 1/8] RISC-V: Provide a cleaner raw_smp_processor_id()

[Palmer Dabbelt]

I'm not sure how I managed to miss this the first time, but this is much
better.

Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/include/asm/smp.h | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
index 36016845461d..27fd085188df 100644
--- a/arch/riscv/include/asm/smp.h
+++ b/arch/riscv/include/asm/smp.h
@@ -14,11 +14,6 @@
 #ifndef _ASM_RISCV_SMP_H
 #define _ASM_RISCV_SMP_H
 
-/* This both needs asm-offsets.h and is used when generating it. */
-#ifndef GENERATING_ASM_OFFSETS
-#include 
-#endif
-
 #include 
 #include 
 
@@ -33,13 +28,12 @@ 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);
 
-/*
- * This is particularly ugly: it appears we can't actually get the definition
- * of task_struct here, but we need access to the CPU this task is running on.
- * Instead of using C we're using asm-offsets.h to get the current processor
- * ID.
- */
-#define raw_smp_processor_id() (*((int*)((char*)get_current() + TASK_TI_CPU)))
+/* Obtains the hart ID of the currently executing task.  This relies on
+ * THREAD_INFO_IN_TASK, but we define that unconditionally. */
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+#define get_ti()((struct thread_info *)get_current())
+#define raw_smp_processor_id()  (get_ti()->cpu)
+#endif
 
 #endif /* CONFIG_SMP */
 
-- 
2.16.4

[PATCH 1/8] RISC-V: Provide a cleaner raw_smp_processor_id()

[Palmer Dabbelt]

I'm not sure how I managed to miss this the first time, but this is much
better.

Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/include/asm/smp.h | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
index 36016845461d..27fd085188df 100644
--- a/arch/riscv/include/asm/smp.h
+++ b/arch/riscv/include/asm/smp.h
@@ -14,11 +14,6 @@
 #ifndef _ASM_RISCV_SMP_H
 #define _ASM_RISCV_SMP_H
 
-/* This both needs asm-offsets.h and is used when generating it. */
-#ifndef GENERATING_ASM_OFFSETS
-#include 
-#endif
-
 #include 
 #include 
 
@@ -33,13 +28,12 @@ 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);
 
-/*
- * This is particularly ugly: it appears we can't actually get the definition
- * of task_struct here, but we need access to the CPU this task is running on.
- * Instead of using C we're using asm-offsets.h to get the current processor
- * ID.
- */
-#define raw_smp_processor_id() (*((int*)((char*)get_current() + TASK_TI_CPU)))
+/* Obtains the hart ID of the currently executing task.  This relies on
+ * THREAD_INFO_IN_TASK, but we define that unconditionally. */
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+#define get_ti()((struct thread_info *)get_current())
+#define raw_smp_processor_id()  (get_ti()->cpu)
+#endif
 
 #endif /* CONFIG_SMP */
 
-- 
2.16.4

[PATCH 0/8] RISC-V: Assorted Cleanups

[Palmer Dabbelt]

I finally got around to answering a very old email in my inbox that was
a response to our original patch set.  These are meant to cause little
to no functional changes to the port, but I've given them very little
testing so I wouldn't be surprised if I've managed to screw something
up here.

I'll be sure to give these some testing before putting them on for-next,
but I figured it'd be better to send them out now rather than later.

[PATCH 0/8] RISC-V: Assorted Cleanups

[Palmer Dabbelt]

I finally got around to answering a very old email in my inbox that was
a response to our original patch set.  These are meant to cause little
to no functional changes to the port, but I've given them very little
testing so I wouldn't be surprised if I've managed to screw something
up here.

I'll be sure to give these some testing before putting them on for-next,
but I figured it'd be better to send them out now rather than later.

Re: [RFC PATCH 2/2] mm: mmu_notifier fix for tlb_end_vma (build failures)

[Palmer Dabbelt]

On Fri, 24 Aug 2018 06:50:48 PDT (-0700), Will Deacon wrote:

On Fri, Aug 24, 2018 at 02:34:27PM +0100, Will Deacon wrote:

On Fri, Aug 24, 2018 at 06:24:19AM -0700, Guenter Roeck wrote:
> On Fri, Aug 24, 2018 at 02:10:27PM +0100, Will Deacon wrote:
> > On Fri, Aug 24, 2018 at 06:07:22AM -0700, Guenter Roeck wrote:
> > > On Thu, Aug 23, 2018 at 06:47:09PM +1000, Nicholas Piggin wrote:
> > > > The generic tlb_end_vma does not call invalidate_range mmu notifier,
> > > > and it resets resets the mmu_gather range, which means the notifier
> > > > won't be called on part of the range in case of an unmap that spans
> > > > multiple vmas.
> > > >
> > > > ARM64 seems to be the only arch I could see that has notifiers and
> > > > uses the generic tlb_end_vma. I have not actually tested it.
> > > >
> > > > Signed-off-by: Nicholas Piggin 
> > > > Acked-by: Will Deacon 
> > >
> > > This patch breaks riscv builds in mainline.
> >
> > Looks very similar to the breakage we hit on arm64. diff below should fix
> > it.
> >
>
> Unfortunately it doesn't.
>
> In file included from ./arch/riscv/include/asm/pgtable.h:26:0,
>  from ./include/linux/memremap.h:7,
>  from ./include/linux/mm.h:27,
>  from arch/riscv/mm/fault.c:23:
> ./arch/riscv/include/asm/tlb.h: In function ‘tlb_flush’:
> ./arch/riscv/include/asm/tlb.h:19:18: error: dereferencing pointer to 
incomplete type ‘struct mmu_gather’
>   flush_tlb_mm(tlb->mm);
>   ^

Sorry, I was a bit quick of the mark there. You'll need a forward
declaration for the paramater type. Here it is with a commit message,
although still untested because I haven't got round to setting up a riscv
toolchain yet.


FWIW, Arnd built them last time he updated the cross tools so you should be 
able to get GCC 8.1.0 for RISC-V from there.  I use this make.cross script that 
I stole from the Intel 0-day robot


   
https://github.com/palmer-dabbelt/home/blob/master/.local/src/local-scripts/make.cross.bash

If I'm reading it correctly the tools come from here

   
http://cdn.kernel.org/pub/tools/crosstool/files/bin/x86_64/8.1.0/x86_64-gcc-8.1.0-nolibc-riscv64-linux.tar.gz

I use the make.cross script as it makes it super easy to test my 
across-the-tree patches on other people's ports.




Will

--->8

From adb9be33d68320edcda80d540a97a647792894d2 Mon Sep 17 00:00:00 2001
From: Will Deacon 
Date: Fri, 24 Aug 2018 14:33:48 +0100
Subject: [PATCH] riscv: tlb: Provide definition of tlb_flush() before
 including tlb.h

As of commit fd1102f0aade ("mm: mmu_notifier fix for tlb_end_vma"),
asm-generic/tlb.h now calls tlb_flush() from a static inline function,
so we need to make sure that it's declared before #including the
asm-generic header in the arch header.

Since tlb_flush() is a one-liner for riscv, we can define it before
including asm-generic/tlb.h as long as we provide a forward declaration
of struct mmu_gather.

Reported-by: Guenter Roeck 
Signed-off-by: Will Deacon 
---
 arch/riscv/include/asm/tlb.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/tlb.h b/arch/riscv/include/asm/tlb.h
index c229509288ea..a3d1380ad970 100644
--- a/arch/riscv/include/asm/tlb.h
+++ b/arch/riscv/include/asm/tlb.h
@@ -14,11 +14,13 @@
 #ifndef _ASM_RISCV_TLB_H
 #define _ASM_RISCV_TLB_H

-#include 
+struct mmu_gather;

 static inline void tlb_flush(struct mmu_gather *tlb)
 {
flush_tlb_mm(tlb->mm);


Bah, didn't spot the dereference so this won't work either. You basically
just need to copy what I did for arm64 in d475fac95779.

Will

Re: [RFC PATCH 2/2] mm: mmu_notifier fix for tlb_end_vma (build failures)

[Palmer Dabbelt]

On Fri, 24 Aug 2018 06:50:48 PDT (-0700), Will Deacon wrote:

On Fri, Aug 24, 2018 at 02:34:27PM +0100, Will Deacon wrote:

On Fri, Aug 24, 2018 at 06:24:19AM -0700, Guenter Roeck wrote:
> On Fri, Aug 24, 2018 at 02:10:27PM +0100, Will Deacon wrote:
> > On Fri, Aug 24, 2018 at 06:07:22AM -0700, Guenter Roeck wrote:
> > > On Thu, Aug 23, 2018 at 06:47:09PM +1000, Nicholas Piggin wrote:
> > > > The generic tlb_end_vma does not call invalidate_range mmu notifier,
> > > > and it resets resets the mmu_gather range, which means the notifier
> > > > won't be called on part of the range in case of an unmap that spans
> > > > multiple vmas.
> > > >
> > > > ARM64 seems to be the only arch I could see that has notifiers and
> > > > uses the generic tlb_end_vma. I have not actually tested it.
> > > >
> > > > Signed-off-by: Nicholas Piggin 
> > > > Acked-by: Will Deacon 
> > >
> > > This patch breaks riscv builds in mainline.
> >
> > Looks very similar to the breakage we hit on arm64. diff below should fix
> > it.
> >
>
> Unfortunately it doesn't.
>
> In file included from ./arch/riscv/include/asm/pgtable.h:26:0,
>  from ./include/linux/memremap.h:7,
>  from ./include/linux/mm.h:27,
>  from arch/riscv/mm/fault.c:23:
> ./arch/riscv/include/asm/tlb.h: In function ‘tlb_flush’:
> ./arch/riscv/include/asm/tlb.h:19:18: error: dereferencing pointer to 
incomplete type ‘struct mmu_gather’
>   flush_tlb_mm(tlb->mm);
>   ^

Sorry, I was a bit quick of the mark there. You'll need a forward
declaration for the paramater type. Here it is with a commit message,
although still untested because I haven't got round to setting up a riscv
toolchain yet.


FWIW, Arnd built them last time he updated the cross tools so you should be 
able to get GCC 8.1.0 for RISC-V from there.  I use this make.cross script that 
I stole from the Intel 0-day robot


   
https://github.com/palmer-dabbelt/home/blob/master/.local/src/local-scripts/make.cross.bash

If I'm reading it correctly the tools come from here

   
http://cdn.kernel.org/pub/tools/crosstool/files/bin/x86_64/8.1.0/x86_64-gcc-8.1.0-nolibc-riscv64-linux.tar.gz

I use the make.cross script as it makes it super easy to test my 
across-the-tree patches on other people's ports.




Will

--->8

From adb9be33d68320edcda80d540a97a647792894d2 Mon Sep 17 00:00:00 2001
From: Will Deacon 
Date: Fri, 24 Aug 2018 14:33:48 +0100
Subject: [PATCH] riscv: tlb: Provide definition of tlb_flush() before
 including tlb.h

As of commit fd1102f0aade ("mm: mmu_notifier fix for tlb_end_vma"),
asm-generic/tlb.h now calls tlb_flush() from a static inline function,
so we need to make sure that it's declared before #including the
asm-generic header in the arch header.

Since tlb_flush() is a one-liner for riscv, we can define it before
including asm-generic/tlb.h as long as we provide a forward declaration
of struct mmu_gather.

Reported-by: Guenter Roeck 
Signed-off-by: Will Deacon 
---
 arch/riscv/include/asm/tlb.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/tlb.h b/arch/riscv/include/asm/tlb.h
index c229509288ea..a3d1380ad970 100644
--- a/arch/riscv/include/asm/tlb.h
+++ b/arch/riscv/include/asm/tlb.h
@@ -14,11 +14,13 @@
 #ifndef _ASM_RISCV_TLB_H
 #define _ASM_RISCV_TLB_H

-#include 
+struct mmu_gather;

 static inline void tlb_flush(struct mmu_gather *tlb)
 {
flush_tlb_mm(tlb->mm);


Bah, didn't spot the dereference so this won't work either. You basically
just need to copy what I did for arm64 in d475fac95779.

Will

Re: [PATCH v2] kbuild: rename LDFLAGS to KBUILD_LDFLAGS

[Palmer Dabbelt]

On Wed, 22 Aug 2018 06:43:25 PDT (-0700), yamada.masah...@socionext.com wrote:

Commit a0f97e06a43c ("kbuild: enable 'make CFLAGS=...' to add
additional options to CC") renamed CFLAGS to KBUILD_CFLAGS.

Commit 222d394d30e7 ("kbuild: enable 'make AFLAGS=...' to add
additional options to AS") renamed AFLAGS to KBUILD_AFLAGS.

Commit 06c5040cdb13 ("kbuild: enable 'make CPPFLAGS=...' to add
additional options to CPP") renamed CPPFLAGS to KBUILD_CPPFLAGS.

For some reason, LDFLAGS was not renamed.

Using a well-known variable like LDFLAGS may result in accidental
override of the variable.

Kbuild generally uses KBUILD_ prefixed variables for the internally
appended options, so here is one more conversion to sanitize the
naming convention.

I did not touch Makefiles under tools/ since the tools build system
is a different world.

Signed-off-by: Masahiro Yamada 
Acked-by: Kirill A. Shutemov 
---

Changes in v2:
  - Rebased on Linus's tree

 Makefile   | 6 +++---
 arch/arc/Makefile  | 2 +-
 arch/arm/Makefile  | 4 ++--
 arch/arm64/Makefile| 4 ++--
 arch/c6x/Makefile  | 3 +--
 arch/h8300/Makefile| 2 +-
 arch/hexagon/Makefile  | 4 +---
 arch/m68k/Makefile | 2 +-
 arch/microblaze/Makefile   | 4 ++--
 arch/mips/Makefile | 2 +-
 arch/mips/boot/compressed/Makefile | 2 +-
 arch/mips/lasat/image/Makefile | 2 +-
 arch/nds32/Makefile| 4 ++--
 arch/powerpc/Makefile  | 6 +++---
 arch/riscv/Makefile| 4 ++--
 arch/s390/Makefile | 2 +-
 arch/sh/Makefile   | 4 ++--
 arch/sparc/Makefile| 4 ++--
 arch/um/Makefile   | 2 +-
 arch/x86/Makefile  | 4 ++--
 arch/x86/Makefile.um   | 4 ++--
 arch/x86/boot/compressed/Makefile  | 6 +++---
 arch/xtensa/Makefile   | 2 +-
 arch/xtensa/boot/boot-elf/Makefile | 2 +-
 scripts/Kbuild.include | 4 ++--
 scripts/Makefile.build | 4 ++--
 scripts/Makefile.lib   | 2 +-
 scripts/Makefile.modpost   | 2 +-
 28 files changed, 45 insertions(+), 48 deletions(-)


[...]


diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 9ddd88b..61ec424 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -29,7 +29,7 @@ ifeq ($(CONFIG_ARCH_RV64I),y)
KBUILD_CFLAGS   += $(call cc-ifversion, -ge, 0500, 
-DCONFIG_ARCH_SUPPORTS_INT128)

KBUILD_MARCH = rv64im
-   LDFLAGS += -melf64lriscv
+   KBUILD_LDFLAGS += -melf64lriscv
 else
BITS := 32
UTS_MACHINE := riscv32
@@ -37,7 +37,7 @@ else
KBUILD_CFLAGS += -mabi=ilp32
KBUILD_AFLAGS += -mabi=ilp32
KBUILD_MARCH = rv32im
-   LDFLAGS += -melf32lriscv
+   KBUILD_LDFLAGS += -melf32lriscv
 endif

 KBUILD_CFLAGS += -Wall


Ah, thanks -- I'd noticed this when we were messing around in here recently and 
assumed there was some reason for the non-orthogonality.  As far as the RISC-V 
stuff goes, feel free to add a 


Reviewed-by: Palmer Dabbelt 

I can deal with the inevitable merge conflicts on our end, as they'll be 
trivial :).

Re: [PATCH v2] kbuild: rename LDFLAGS to KBUILD_LDFLAGS

[Palmer Dabbelt]

On Wed, 22 Aug 2018 06:43:25 PDT (-0700), yamada.masah...@socionext.com wrote:

Commit a0f97e06a43c ("kbuild: enable 'make CFLAGS=...' to add
additional options to CC") renamed CFLAGS to KBUILD_CFLAGS.

Commit 222d394d30e7 ("kbuild: enable 'make AFLAGS=...' to add
additional options to AS") renamed AFLAGS to KBUILD_AFLAGS.

Commit 06c5040cdb13 ("kbuild: enable 'make CPPFLAGS=...' to add
additional options to CPP") renamed CPPFLAGS to KBUILD_CPPFLAGS.

For some reason, LDFLAGS was not renamed.

Using a well-known variable like LDFLAGS may result in accidental
override of the variable.

Kbuild generally uses KBUILD_ prefixed variables for the internally
appended options, so here is one more conversion to sanitize the
naming convention.

I did not touch Makefiles under tools/ since the tools build system
is a different world.

Signed-off-by: Masahiro Yamada 
Acked-by: Kirill A. Shutemov 
---

Changes in v2:
  - Rebased on Linus's tree

 Makefile   | 6 +++---
 arch/arc/Makefile  | 2 +-
 arch/arm/Makefile  | 4 ++--
 arch/arm64/Makefile| 4 ++--
 arch/c6x/Makefile  | 3 +--
 arch/h8300/Makefile| 2 +-
 arch/hexagon/Makefile  | 4 +---
 arch/m68k/Makefile | 2 +-
 arch/microblaze/Makefile   | 4 ++--
 arch/mips/Makefile | 2 +-
 arch/mips/boot/compressed/Makefile | 2 +-
 arch/mips/lasat/image/Makefile | 2 +-
 arch/nds32/Makefile| 4 ++--
 arch/powerpc/Makefile  | 6 +++---
 arch/riscv/Makefile| 4 ++--
 arch/s390/Makefile | 2 +-
 arch/sh/Makefile   | 4 ++--
 arch/sparc/Makefile| 4 ++--
 arch/um/Makefile   | 2 +-
 arch/x86/Makefile  | 4 ++--
 arch/x86/Makefile.um   | 4 ++--
 arch/x86/boot/compressed/Makefile  | 6 +++---
 arch/xtensa/Makefile   | 2 +-
 arch/xtensa/boot/boot-elf/Makefile | 2 +-
 scripts/Kbuild.include | 4 ++--
 scripts/Makefile.build | 4 ++--
 scripts/Makefile.lib   | 2 +-
 scripts/Makefile.modpost   | 2 +-
 28 files changed, 45 insertions(+), 48 deletions(-)


[...]


diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 9ddd88b..61ec424 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -29,7 +29,7 @@ ifeq ($(CONFIG_ARCH_RV64I),y)
KBUILD_CFLAGS   += $(call cc-ifversion, -ge, 0500, 
-DCONFIG_ARCH_SUPPORTS_INT128)

KBUILD_MARCH = rv64im
-   LDFLAGS += -melf64lriscv
+   KBUILD_LDFLAGS += -melf64lriscv
 else
BITS := 32
UTS_MACHINE := riscv32
@@ -37,7 +37,7 @@ else
KBUILD_CFLAGS += -mabi=ilp32
KBUILD_AFLAGS += -mabi=ilp32
KBUILD_MARCH = rv32im
-   LDFLAGS += -melf32lriscv
+   KBUILD_LDFLAGS += -melf32lriscv
 endif

 KBUILD_CFLAGS += -Wall


Ah, thanks -- I'd noticed this when we were messing around in here recently and 
assumed there was some reason for the non-orthogonality.  As far as the RISC-V 
stuff goes, feel free to add a 


Reviewed-by: Palmer Dabbelt 

I can deal with the inevitable merge conflicts on our end, as they'll be 
trivial :).

Re: [RFC PATCH 3/5] RISC-V: Add cpu_operatios structure

[Palmer Dabbelt]

On Tue, 21 Aug 2018 23:03:53 PDT (-0700), Christoph Hellwig wrote:

On Tue, Aug 21, 2018 at 10:34:38PM +0530, Anup Patel wrote:

The cpu_operations is certainly required because SOC vendors will add
vendor-specific mechanism to selectively bringing-up CPUs/HARTs instead
of all CPUs entering Linux kernel simultaneously. In fact, we might also end-up
having CPU ON/OFF operations in SBI.


Your forgot an essential part in your analysis:  Right now we only have
one single way to deal with cpu on/offlining, and that is the dummy WFI
kind.  Once other ways show up we can build proper infrastructure, but
until then this is just a white elephant as we have no idea how these
abstractions will look like.

And my hope is that we'll just see new SBI calls, in which case we'll
just need SBI and dummy version and can avoid all the indirect calls.


Yes, the goal here is to define one good interface via the SBI.  We've got a 
session at Plumbers this year and one of the blocks is to sit down and talk 
about this API, hopefully we can make some headway while there.

Re: [RFC PATCH 3/5] RISC-V: Add cpu_operatios structure

[Palmer Dabbelt]

On Tue, 21 Aug 2018 23:03:53 PDT (-0700), Christoph Hellwig wrote:

On Tue, Aug 21, 2018 at 10:34:38PM +0530, Anup Patel wrote:

The cpu_operations is certainly required because SOC vendors will add
vendor-specific mechanism to selectively bringing-up CPUs/HARTs instead
of all CPUs entering Linux kernel simultaneously. In fact, we might also end-up
having CPU ON/OFF operations in SBI.


Your forgot an essential part in your analysis:  Right now we only have
one single way to deal with cpu on/offlining, and that is the dummy WFI
kind.  Once other ways show up we can build proper infrastructure, but
until then this is just a white elephant as we have no idea how these
abstractions will look like.

And my hope is that we'll just see new SBI calls, in which case we'll
just need SBI and dummy version and can avoid all the indirect calls.


Yes, the goal here is to define one good interface via the SBI.  We've got a 
session at Plumbers this year and one of the blocks is to sit down and talk 
about this API, hopefully we can make some headway while there.

Re: [GIT PULL] RISC-V Updates for the 4.19 Merge Window

[Palmer Dabbelt]

On Tue, 21 Aug 2018 12:45:50 PDT (-0700), mer...@debian.org wrote:

On Tue, Aug 21, 2018 at 11:31:48AM -0700, Palmer Dabbelt wrote:

On Sat, 18 Aug 2018 06:37:59 PDT (-0700), li...@roeck-us.net wrote:

[...]

> Do you have vmlinux embedded in bbl ?
>
> With separate bbl and vmlinux, and the following qemu command line
> (with qemu 3.0)
>
> qemu-system-riscv64 -M virt -m 512M -no-reboot \
>-bios bbl -kernel vmlinux \
>-netdev user,id=net0 -device virtio-net-device,netdev=net0 \
>-device virtio-blk-device,drive=d0 \
>-drive file=rootfs.ext2,if=none,id=d0,format=raw \
>-append 'root=/dev/vda rw console=ttyS0,115200' \
>-nographic -monitor none
>
> all I get is
>
> rom: requested regions overlap (rom mrom.reset. free=0x0001cbe8, 
addr=0x1000)
>
> However, the she system boots fine with the same qemu command line if I use 
qemu
> built from https://github.com/riscv/riscv-qemu.git, branch qemu-for-upstream.

Yes, I have a vmlinux built into my BBL.  I didn't actually look closely at
the command line I was copying and see that vmlinux in there, my guess would
be that it's getting ignored.  I don't remember if upstream BBL actually
works with the split bbl/vmlinux setup, I've kind of stopped paying
attention to BBL as I'm just waiting for someone to tell me instructions as
to how to use a real bootloader... :)


JFTR, upstream bbl supports the split bbl/vmlinux setup, it's just
upstream qemu that lacks the support for now (qemu-riscv has it).


Thanks!

Re: [GIT PULL] RISC-V Updates for the 4.19 Merge Window

[Palmer Dabbelt]

On Tue, 21 Aug 2018 12:45:50 PDT (-0700), mer...@debian.org wrote:

On Tue, Aug 21, 2018 at 11:31:48AM -0700, Palmer Dabbelt wrote:

On Sat, 18 Aug 2018 06:37:59 PDT (-0700), li...@roeck-us.net wrote:

[...]

> Do you have vmlinux embedded in bbl ?
>
> With separate bbl and vmlinux, and the following qemu command line
> (with qemu 3.0)
>
> qemu-system-riscv64 -M virt -m 512M -no-reboot \
>-bios bbl -kernel vmlinux \
>-netdev user,id=net0 -device virtio-net-device,netdev=net0 \
>-device virtio-blk-device,drive=d0 \
>-drive file=rootfs.ext2,if=none,id=d0,format=raw \
>-append 'root=/dev/vda rw console=ttyS0,115200' \
>-nographic -monitor none
>
> all I get is
>
> rom: requested regions overlap (rom mrom.reset. free=0x0001cbe8, 
addr=0x1000)
>
> However, the she system boots fine with the same qemu command line if I use 
qemu
> built from https://github.com/riscv/riscv-qemu.git, branch qemu-for-upstream.

Yes, I have a vmlinux built into my BBL.  I didn't actually look closely at
the command line I was copying and see that vmlinux in there, my guess would
be that it's getting ignored.  I don't remember if upstream BBL actually
works with the split bbl/vmlinux setup, I've kind of stopped paying
attention to BBL as I'm just waiting for someone to tell me instructions as
to how to use a real bootloader... :)


JFTR, upstream bbl supports the split bbl/vmlinux setup, it's just
upstream qemu that lacks the support for now (qemu-riscv has it).


Thanks!

Re: [PATCH] microblaze/PCI: Remove stale pcibios_align_resource() comment

[Palmer Dabbelt]

On Mon, 20 Aug 2018 23:24:53 PDT (-0700), mon...@monstr.eu wrote:

On 20.8.2018 11:47, Lorenzo Pieralisi wrote:

commit 01cf9d524ff0 ("microblaze/PCI: Support generic Xilinx AXI PCIe Host
Bridge IP driver")

and

commit ecf677c8dcaa ("PCI: Add a generic weak pcibios_align_resource()")

first patched then removed pcibios_align_resource() from the microblaze
architecture code but failed to remove the comment that was added to
it.

Remove it since it has now become stale and it is quite confusing.

Signed-off-by: Lorenzo Pieralisi 
Cc: Palmer Dabbelt 
Cc: Bjorn Helgaas 
Cc: Bharat Kumar Gogada 
Cc: Michal Simek 
---
 arch/microblaze/pci/pci-common.c | 13 -
 1 file changed, 13 deletions(-)

diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index f34346d56095..2ffd171af8b6 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -597,19 +597,6 @@ static void pcibios_fixup_resources(struct pci_dev *dev)
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pcibios_fixup_resources);

-/*
- * We need to avoid collisions with `mirrored' VGA ports
- * and other strange ISA hardware, so we always want the
- * addresses to be allocated in the 0x000-0x0ff region
- * modulo 0x400.
- *
- * Why? Because some silly external IO cards only decode
- * the low 10 bits of the IO address. The 0x00-0xff region
- * is reserved for motherboard devices that decode all 16
- * bits, so it's ok to allocate at, say, 0x2800-0x28ff,
- * but we want to try to avoid allocating at 0x2900-0x2bff
- * which might have be mirrored at 0x0100-0x03ff..
- */
 int pcibios_add_device(struct pci_dev *dev)
 {
dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);



Applied.


Sorry, I missed this.  Thanks for the fix!

Re: [PATCH] microblaze/PCI: Remove stale pcibios_align_resource() comment

[Palmer Dabbelt]

On Mon, 20 Aug 2018 23:24:53 PDT (-0700), mon...@monstr.eu wrote:

On 20.8.2018 11:47, Lorenzo Pieralisi wrote:

commit 01cf9d524ff0 ("microblaze/PCI: Support generic Xilinx AXI PCIe Host
Bridge IP driver")

and

commit ecf677c8dcaa ("PCI: Add a generic weak pcibios_align_resource()")

first patched then removed pcibios_align_resource() from the microblaze
architecture code but failed to remove the comment that was added to
it.

Remove it since it has now become stale and it is quite confusing.

Signed-off-by: Lorenzo Pieralisi 
Cc: Palmer Dabbelt 
Cc: Bjorn Helgaas 
Cc: Bharat Kumar Gogada 
Cc: Michal Simek 
---
 arch/microblaze/pci/pci-common.c | 13 -
 1 file changed, 13 deletions(-)

diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index f34346d56095..2ffd171af8b6 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -597,19 +597,6 @@ static void pcibios_fixup_resources(struct pci_dev *dev)
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pcibios_fixup_resources);

-/*
- * We need to avoid collisions with `mirrored' VGA ports
- * and other strange ISA hardware, so we always want the
- * addresses to be allocated in the 0x000-0x0ff region
- * modulo 0x400.
- *
- * Why? Because some silly external IO cards only decode
- * the low 10 bits of the IO address. The 0x00-0xff region
- * is reserved for motherboard devices that decode all 16
- * bits, so it's ok to allocate at, say, 0x2800-0x28ff,
- * but we want to try to avoid allocating at 0x2900-0x2bff
- * which might have be mirrored at 0x0100-0x03ff..
- */
 int pcibios_add_device(struct pci_dev *dev)
 {
dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);



Applied.


Sorry, I missed this.  Thanks for the fix!

Re: [PATCH v4 3/5] Cleanup ISA string setting

[Palmer Dabbelt]

On Mon, 20 Aug 2018 19:47:28 PDT (-0700), alan...@andestech.com wrote:

On Mon, Aug 20, 2018 at 03:22:55PM -0700, Palmer Dabbelt wrote:

On Tue, 07 Aug 2018 20:24:43 PDT (-0700), alan...@andestech.com wrote:
>Just a side note: (Assume that atomic and compressed is on)
>
>Before this patch, assembler was always given the riscv64imafdc
>MARCH string because there are fld/fsd's in entry.S; compiler was
>always given riscv64imac because kernel doesn't need floating point
>code generation.  After this, the MARCH string in AFLAGS and CFLAGS
>become the same.
>
>Signed-off-by: Alan Kao 
>Cc: Greentime Hu 
>Cc: Vincent Chen 
>Cc: Zong Li 
>Cc: Nick Hu 
>Reviewed-by: Christoph Hellwig 
>---
> arch/riscv/Makefile | 19 ---
> 1 file changed, 8 insertions(+), 11 deletions(-)
>
>diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
>index 6d4a5f6c3f4f..e0fe6790624f 100644
>--- a/arch/riscv/Makefile
>+++ b/arch/riscv/Makefile
>@@ -26,7 +26,6 @@ ifeq ($(CONFIG_ARCH_RV64I),y)
>
>KBUILD_CFLAGS += -mabi=lp64
>KBUILD_AFLAGS += -mabi=lp64
>-   KBUILD_MARCH = rv64im
>LDFLAGS += -melf64lriscv
> else
>BITS := 32
>@@ -34,22 +33,20 @@ else
>
>KBUILD_CFLAGS += -mabi=ilp32
>KBUILD_AFLAGS += -mabi=ilp32
>-   KBUILD_MARCH = rv32im
>LDFLAGS += -melf32lriscv
> endif
>
> KBUILD_CFLAGS += -Wall
>
>-ifeq ($(CONFIG_RISCV_ISA_A),y)
>-   KBUILD_ARCH_A = a
>-endif
>-ifeq ($(CONFIG_RISCV_ISA_C),y)
>-   KBUILD_ARCH_C = c
>-endif
>-
>-KBUILD_AFLAGS += -march=$(KBUILD_MARCH)$(KBUILD_ARCH_A)fd$(KBUILD_ARCH_C)
>+# ISA string setting
>+riscv-march-$(CONFIG_ARCH_RV32I)   := rv32im
>+riscv-march-$(CONFIG_ARCH_RV64I)   := rv64im
>+riscv-march-$(CONFIG_RISCV_ISA_A)  := $(riscv-march-y)a
>+riscv-march-y  := $(riscv-march-y)fd
>+riscv-march-$(CONFIG_RISCV_ISA_C)  := $(riscv-march-y)c
>+KBUILD_CFLAGS += -march=$(riscv-march-y)
>+KBUILD_AFLAGS += -march=$(riscv-march-y)
>
>-KBUILD_CFLAGS += -march=$(KBUILD_MARCH)$(KBUILD_ARCH_A)$(KBUILD_ARCH_C)

We used to have "fd" disabled in KBUILD_CFLAGS because we wanted to ensure
that any use of floating-point types within the kernel would trigger the
inclusion of soft-float libraries rather than emitting hardware
floating-point instructions.  The worry here is that we'd end up corrupting
the user's floating-point state with the kernel accesses because of the lazy
save/restore stuff going on.


Thanks for pointing that out.

Just as you said, the use of KBUILD_CFLAGS=*fd* is based on the assumption that
not a single floating-point instruction involves in the kernel, and that might
be wrong.


I remember at some point it was illegal to use floating-point within the
kernel, but for some reason I thought that had changed.  I do see a handful
of references to "float" and "double" in the kernel source, and most of
references to kernel_fpu_begin() appear to be in SSE-specific stuff.  My one
worry are the usages in drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c, as
I can't quickly demonstrate they won't happen.


Empirically, this CFLAGS with *fd* did not cause any trouble to me, but of
course this is not a good statement to support this patch.

Meanwhile, I find a flaw in "[PATCH 4/5] Allow to Disable FPU Support."
The purpose of this "[PATCH 3/5] Cleanup ISA String" was to make CONFIG_FPU
a switch for the appearance of "fd" in both KBUILD_CFLAGS and KBUILD_ASFLAGS,
but somehow the modification was forgotten.



If we do this I think we should at least ensure kernel_fpu_{begin,end}() do
the right thing for RISC-V.  I'd still feel safer telling the C compiler to
disallow floating-point, though, as I'm a bit paranoid that GCC might go and
emit a floating-point instruction even when it's not explicitly asked for.
I just asked Jim, who actually understands GCC, and he said that it'll spill
to floating-point registers if the cost function determines it's cheaper
than the stack.  While he thinks that's unlikely, I don't really want to
rely a GCC cost function for the correctness of our kernel port.


The purpose of this whole patchset was to remove all floating-point-related
logic in kernel space (while remainging FPU systems work as usual), so
implementing the two APIs would be out of scope here.

I suggest that, some people have to provide these APIs if they do need hardware
floating-point calculation in kernel space (kernel or module) in the future.
It seems that we don't need those for the kernel and any already supported
hardware drivers at least for now.  Please correct me if my understanding is
out-of-date.



I'd like to avoid having "-march=*fd*" in CFLAGS, unless someone can
convince me it's safe and that I'm just being too paranoid :).


I will send a new version of the patchset, which will make sure that CFLAGS has
no *fd* (3/5) and the CONFIG_FPU did remove *fd* from ASFLAGS (4/5).


Thanks!





> KBUILD_CFLAGS += -mno-save-restore
> KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)


Thanks!

Alan

Re: [PATCH v4 3/5] Cleanup ISA string setting

[Palmer Dabbelt]

On Mon, 20 Aug 2018 19:47:28 PDT (-0700), alan...@andestech.com wrote:

On Mon, Aug 20, 2018 at 03:22:55PM -0700, Palmer Dabbelt wrote:

On Tue, 07 Aug 2018 20:24:43 PDT (-0700), alan...@andestech.com wrote:
>Just a side note: (Assume that atomic and compressed is on)
>
>Before this patch, assembler was always given the riscv64imafdc
>MARCH string because there are fld/fsd's in entry.S; compiler was
>always given riscv64imac because kernel doesn't need floating point
>code generation.  After this, the MARCH string in AFLAGS and CFLAGS
>become the same.
>
>Signed-off-by: Alan Kao 
>Cc: Greentime Hu 
>Cc: Vincent Chen 
>Cc: Zong Li 
>Cc: Nick Hu 
>Reviewed-by: Christoph Hellwig 
>---
> arch/riscv/Makefile | 19 ---
> 1 file changed, 8 insertions(+), 11 deletions(-)
>
>diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
>index 6d4a5f6c3f4f..e0fe6790624f 100644
>--- a/arch/riscv/Makefile
>+++ b/arch/riscv/Makefile
>@@ -26,7 +26,6 @@ ifeq ($(CONFIG_ARCH_RV64I),y)
>
>KBUILD_CFLAGS += -mabi=lp64
>KBUILD_AFLAGS += -mabi=lp64
>-   KBUILD_MARCH = rv64im
>LDFLAGS += -melf64lriscv
> else
>BITS := 32
>@@ -34,22 +33,20 @@ else
>
>KBUILD_CFLAGS += -mabi=ilp32
>KBUILD_AFLAGS += -mabi=ilp32
>-   KBUILD_MARCH = rv32im
>LDFLAGS += -melf32lriscv
> endif
>
> KBUILD_CFLAGS += -Wall
>
>-ifeq ($(CONFIG_RISCV_ISA_A),y)
>-   KBUILD_ARCH_A = a
>-endif
>-ifeq ($(CONFIG_RISCV_ISA_C),y)
>-   KBUILD_ARCH_C = c
>-endif
>-
>-KBUILD_AFLAGS += -march=$(KBUILD_MARCH)$(KBUILD_ARCH_A)fd$(KBUILD_ARCH_C)
>+# ISA string setting
>+riscv-march-$(CONFIG_ARCH_RV32I)   := rv32im
>+riscv-march-$(CONFIG_ARCH_RV64I)   := rv64im
>+riscv-march-$(CONFIG_RISCV_ISA_A)  := $(riscv-march-y)a
>+riscv-march-y  := $(riscv-march-y)fd
>+riscv-march-$(CONFIG_RISCV_ISA_C)  := $(riscv-march-y)c
>+KBUILD_CFLAGS += -march=$(riscv-march-y)
>+KBUILD_AFLAGS += -march=$(riscv-march-y)
>
>-KBUILD_CFLAGS += -march=$(KBUILD_MARCH)$(KBUILD_ARCH_A)$(KBUILD_ARCH_C)

We used to have "fd" disabled in KBUILD_CFLAGS because we wanted to ensure
that any use of floating-point types within the kernel would trigger the
inclusion of soft-float libraries rather than emitting hardware
floating-point instructions.  The worry here is that we'd end up corrupting
the user's floating-point state with the kernel accesses because of the lazy
save/restore stuff going on.


Thanks for pointing that out.

Just as you said, the use of KBUILD_CFLAGS=*fd* is based on the assumption that
not a single floating-point instruction involves in the kernel, and that might
be wrong.


I remember at some point it was illegal to use floating-point within the
kernel, but for some reason I thought that had changed.  I do see a handful
of references to "float" and "double" in the kernel source, and most of
references to kernel_fpu_begin() appear to be in SSE-specific stuff.  My one
worry are the usages in drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c, as
I can't quickly demonstrate they won't happen.


Empirically, this CFLAGS with *fd* did not cause any trouble to me, but of
course this is not a good statement to support this patch.

Meanwhile, I find a flaw in "[PATCH 4/5] Allow to Disable FPU Support."
The purpose of this "[PATCH 3/5] Cleanup ISA String" was to make CONFIG_FPU
a switch for the appearance of "fd" in both KBUILD_CFLAGS and KBUILD_ASFLAGS,
but somehow the modification was forgotten.



If we do this I think we should at least ensure kernel_fpu_{begin,end}() do
the right thing for RISC-V.  I'd still feel safer telling the C compiler to
disallow floating-point, though, as I'm a bit paranoid that GCC might go and
emit a floating-point instruction even when it's not explicitly asked for.
I just asked Jim, who actually understands GCC, and he said that it'll spill
to floating-point registers if the cost function determines it's cheaper
than the stack.  While he thinks that's unlikely, I don't really want to
rely a GCC cost function for the correctness of our kernel port.


The purpose of this whole patchset was to remove all floating-point-related
logic in kernel space (while remainging FPU systems work as usual), so
implementing the two APIs would be out of scope here.

I suggest that, some people have to provide these APIs if they do need hardware
floating-point calculation in kernel space (kernel or module) in the future.
It seems that we don't need those for the kernel and any already supported
hardware drivers at least for now.  Please correct me if my understanding is
out-of-date.



I'd like to avoid having "-march=*fd*" in CFLAGS, unless someone can
convince me it's safe and that I'm just being too paranoid :).


I will send a new version of the patchset, which will make sure that CFLAGS has
no *fd* (3/5) and the CONFIG_FPU did remove *fd* from ASFLAGS (4/5).


Thanks!





> KBUILD_CFLAGS += -mno-save-restore
> KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)


Thanks!

Alan

Re: [GIT PULL] RISC-V Updates for the 4.19 Merge Window

[Palmer Dabbelt]

On Sat, 18 Aug 2018 11:15:18 PDT (-0700), Linus Torvalds wrote:

On Fri, Aug 17, 2018 at 1:28 PM Palmer Dabbelt  wrote:


I remember having sent this on Wednesday, but for some reason I don't see it in
your tree or my outbox so I might be crazy.


You might indeed have been having hallucinations. I don't see any
other pull request from you in my mailbox than this one.

Google does find a posting from you saying

 "Below is the pull request I plan to submit on Wednesday morning"

on the RISC-V development google group list, so I think you just
remembered your _plan_, not your actual email ...


That does sound like something I would do...  It's odd because I remember 
specifically being excited that I finally got the arguments to "git send-pull" 
correct, but I guess that excitement was pre-mature :)



Anyway, I can confirm that this new pull request is now in my queue
even if I don't see any earlier ones.


Thanks!

I'm planning on submitting another PR tomorrow (which has been baking since 
yesterday like it's supposed to), so fingers crossed I'm slightly less crazy 
this week.

Re: [GIT PULL] RISC-V Updates for the 4.19 Merge Window

[Palmer Dabbelt]

On Sat, 18 Aug 2018 06:37:59 PDT (-0700), li...@roeck-us.net wrote:

Hi Palmer,

On Fri, Aug 17, 2018 at 01:28:11PM -0700, Palmer Dabbelt wrote:
[ ... ]



This tag boots a Fedora root filesystem on QEMU's master branch for me,
and before this morning's rebase (from 4.18-rc8 to 4.18) it booted on
the HiFive Unleashed.



Do you have vmlinux embedded in bbl ?

With separate bbl and vmlinux, and the following qemu command line
(with qemu 3.0)

qemu-system-riscv64 -M virt -m 512M -no-reboot \
-bios bbl -kernel vmlinux \
-netdev user,id=net0 -device virtio-net-device,netdev=net0 \
-device virtio-blk-device,drive=d0 \
-drive file=rootfs.ext2,if=none,id=d0,format=raw \
-append 'root=/dev/vda rw console=ttyS0,115200' \
-nographic -monitor none

all I get is

rom: requested regions overlap (rom mrom.reset. free=0x0001cbe8, 
addr=0x1000)

However, the she system boots fine with the same qemu command line if I use qemu
built from https://github.com/riscv/riscv-qemu.git, branch qemu-for-upstream.


Yes, I have a vmlinux built into my BBL.  I didn't actually look closely at the 
command line I was copying and see that vmlinux in there, my guess would be 
that it's getting ignored.  I don't remember if upstream BBL actually works 
with the split bbl/vmlinux setup, I've kind of stopped paying attention to BBL 
as I'm just waiting for someone to tell me instructions as to how to use a real 
bootloader... :)


I'm building master from QEMU as of a few weeks ago

   * f7502360397d - (HEAD -> master, tag: v3.0.0-rc3, origin/master, origin/HEAD) 
Update version for v3.0.0-rc3 release (3 weeks ago) 
   *   b89041647422 - Merge remote-tracking branch 
'remotes/armbru/tags/pull-monitor-2018-07-31' into staging (3 weeks ago) 
   |\
   | * 9a1054061c62 - monitor: temporary fix for dead-lock on event recursion (3 
weeks ago) 
   |/
   *   42e76456cf68 - Merge remote-tracking branch 
'remotes/vivier2/tags/linux-user-for-3.0-pull-request' into staging (3 weeks ago) 

   |\
   | * 5d9f3ea08172 - linux-user: ppc64: don't use volatile register during 
safe_syscall (3 weeks ago) 
   | * 28cbb997d66e - tests: add check_invalid_maps to test-mmap (3 weeks ago) 

   | * 38138fab9358 - linux-user/mmap.c: handle invalid len maps correctly (3 weeks 
ago) 
   * |   45a505d0a4b3 - Merge remote-tracking branch 
'remotes/bonzini/tags/for-upstream' into staging (3 weeks ago) 

in general once we get a port upstream I jump over to using master from 
upstream as that way we'll find any bugs quickly.  Michael Clark has a pretty 
big QEMU patch queue, but nothing appears to be critical for Linux boot.



Excellent - once this series hits mainline, I'll add riscv to my
boot tests.


Thanks!  I'm super excited to get CI stuff up and running as now that we can 
boot I'm worried people will notice when I screw something up :)

Re: [GIT PULL] RISC-V Updates for the 4.19 Merge Window

[Palmer Dabbelt]

On Sat, 18 Aug 2018 11:15:18 PDT (-0700), Linus Torvalds wrote:

On Fri, Aug 17, 2018 at 1:28 PM Palmer Dabbelt  wrote:


I remember having sent this on Wednesday, but for some reason I don't see it in
your tree or my outbox so I might be crazy.


You might indeed have been having hallucinations. I don't see any
other pull request from you in my mailbox than this one.

Google does find a posting from you saying

 "Below is the pull request I plan to submit on Wednesday morning"

on the RISC-V development google group list, so I think you just
remembered your _plan_, not your actual email ...


That does sound like something I would do...  It's odd because I remember 
specifically being excited that I finally got the arguments to "git send-pull" 
correct, but I guess that excitement was pre-mature :)



Anyway, I can confirm that this new pull request is now in my queue
even if I don't see any earlier ones.


Thanks!

I'm planning on submitting another PR tomorrow (which has been baking since 
yesterday like it's supposed to), so fingers crossed I'm slightly less crazy 
this week.

Re: [GIT PULL] RISC-V Updates for the 4.19 Merge Window

[Palmer Dabbelt]

On Sat, 18 Aug 2018 06:37:59 PDT (-0700), li...@roeck-us.net wrote:

Hi Palmer,

On Fri, Aug 17, 2018 at 01:28:11PM -0700, Palmer Dabbelt wrote:
[ ... ]



This tag boots a Fedora root filesystem on QEMU's master branch for me,
and before this morning's rebase (from 4.18-rc8 to 4.18) it booted on
the HiFive Unleashed.



Do you have vmlinux embedded in bbl ?

With separate bbl and vmlinux, and the following qemu command line
(with qemu 3.0)

qemu-system-riscv64 -M virt -m 512M -no-reboot \
-bios bbl -kernel vmlinux \
-netdev user,id=net0 -device virtio-net-device,netdev=net0 \
-device virtio-blk-device,drive=d0 \
-drive file=rootfs.ext2,if=none,id=d0,format=raw \
-append 'root=/dev/vda rw console=ttyS0,115200' \
-nographic -monitor none

all I get is

rom: requested regions overlap (rom mrom.reset. free=0x0001cbe8, 
addr=0x1000)

However, the she system boots fine with the same qemu command line if I use qemu
built from https://github.com/riscv/riscv-qemu.git, branch qemu-for-upstream.


Yes, I have a vmlinux built into my BBL.  I didn't actually look closely at the 
command line I was copying and see that vmlinux in there, my guess would be 
that it's getting ignored.  I don't remember if upstream BBL actually works 
with the split bbl/vmlinux setup, I've kind of stopped paying attention to BBL 
as I'm just waiting for someone to tell me instructions as to how to use a real 
bootloader... :)


I'm building master from QEMU as of a few weeks ago

   * f7502360397d - (HEAD -> master, tag: v3.0.0-rc3, origin/master, origin/HEAD) 
Update version for v3.0.0-rc3 release (3 weeks ago) 
   *   b89041647422 - Merge remote-tracking branch 
'remotes/armbru/tags/pull-monitor-2018-07-31' into staging (3 weeks ago) 
   |\
   | * 9a1054061c62 - monitor: temporary fix for dead-lock on event recursion (3 
weeks ago) 
   |/
   *   42e76456cf68 - Merge remote-tracking branch 
'remotes/vivier2/tags/linux-user-for-3.0-pull-request' into staging (3 weeks ago) 

   |\
   | * 5d9f3ea08172 - linux-user: ppc64: don't use volatile register during 
safe_syscall (3 weeks ago) 
   | * 28cbb997d66e - tests: add check_invalid_maps to test-mmap (3 weeks ago) 

   | * 38138fab9358 - linux-user/mmap.c: handle invalid len maps correctly (3 weeks 
ago) 
   * |   45a505d0a4b3 - Merge remote-tracking branch 
'remotes/bonzini/tags/for-upstream' into staging (3 weeks ago) 

in general once we get a port upstream I jump over to using master from 
upstream as that way we'll find any bugs quickly.  Michael Clark has a pretty 
big QEMU patch queue, but nothing appears to be critical for Linux boot.



Excellent - once this series hits mainline, I'll add riscv to my
boot tests.


Thanks!  I'm super excited to get CI stuff up and running as now that we can 
boot I'm worried people will notice when I screw something up :)

Re: [PATCH v3 1/2] RISC-V: Define sys_riscv_flush_icache when SMP=n

[Palmer Dabbelt]

On Tue, 14 Aug 2018 06:39:23 PDT (-0700), Christoph Hellwig wrote:

 SYSCALL_DEFINE3(riscv_flush_icache, uintptr_t, start, uintptr_t, end,
uintptr_t, flags)
 {
+#ifdef CONFIG_SMP
struct mm_struct *mm = current->mm;
bool local = (flags & SYS_RISCV_FLUSH_ICACHE_LOCAL) != 0;
+#endif

/* Check the reserved flags. */
if (unlikely(flags & ~SYS_RISCV_FLUSH_ICACHE_ALL))
return -EINVAL;

+   /*
+* Without CONFIG_SMP flush_icache_mm is a just a flush_icache_all(),
+* which generates unused variable warnings all over this function.
+*/
+#ifdef CONFIG_SMP
flush_icache_mm(mm, local);
+#else
+   flush_icache_all();
+#endif


Eeek.

Something like an unconditional:

flush_icache_mm(current->mm, flags & SYS_RISCV_FLUSH_ICACHE_LOCAL);

should solve those issues.

Also in the longer run we should turn the !SMP flush_icache_mm stub
into an inline function to solve this problem for all potential
callers.  Excepte that flush_icache_mm happens to be a RISC-V specific
API without any other callers.  So for now I think the above is what
I'd do, but this area has a lot of room for cleanup.


Thanks, that's a lot cleaner.  I missed this for the PR, but I'll submit a 
cleanup patch after RC1.

Re: [PATCH v3 1/2] RISC-V: Define sys_riscv_flush_icache when SMP=n

[Palmer Dabbelt]

On Tue, 14 Aug 2018 06:39:23 PDT (-0700), Christoph Hellwig wrote:

 SYSCALL_DEFINE3(riscv_flush_icache, uintptr_t, start, uintptr_t, end,
uintptr_t, flags)
 {
+#ifdef CONFIG_SMP
struct mm_struct *mm = current->mm;
bool local = (flags & SYS_RISCV_FLUSH_ICACHE_LOCAL) != 0;
+#endif

/* Check the reserved flags. */
if (unlikely(flags & ~SYS_RISCV_FLUSH_ICACHE_ALL))
return -EINVAL;

+   /*
+* Without CONFIG_SMP flush_icache_mm is a just a flush_icache_all(),
+* which generates unused variable warnings all over this function.
+*/
+#ifdef CONFIG_SMP
flush_icache_mm(mm, local);
+#else
+   flush_icache_all();
+#endif


Eeek.

Something like an unconditional:

flush_icache_mm(current->mm, flags & SYS_RISCV_FLUSH_ICACHE_LOCAL);

should solve those issues.

Also in the longer run we should turn the !SMP flush_icache_mm stub
into an inline function to solve this problem for all potential
callers.  Excepte that flush_icache_mm happens to be a RISC-V specific
API without any other callers.  So for now I think the above is what
I'd do, but this area has a lot of room for cleanup.


Thanks, that's a lot cleaner.  I missed this for the PR, but I'll submit a 
cleanup patch after RC1.

Re: [PATCH v3 2/2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h

[Palmer Dabbelt]

On Tue, 14 Aug 2018 06:40:27 PDT (-0700), Christoph Hellwig wrote:

index 818655b0d535..690beb002d1d 100644
--- a/arch/riscv/include/uapi/asm/syscalls.h
+++ b/arch/riscv/include/uapi/asm/syscalls.h
@@ -1,10 +1,13 @@
-/* SPDX-License-Identifier: GPL-2.0 */
+// SPDX-License-Identifier: GPL-2.0


/* ... */  is the right style SPDX tag for headers, so please keep it
as-is.


Thanks, I didn't miss this one so I managed to fix it before the PR!

Re: [PATCH v3 2/2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h

[Palmer Dabbelt]

On Tue, 14 Aug 2018 06:40:27 PDT (-0700), Christoph Hellwig wrote:

index 818655b0d535..690beb002d1d 100644
--- a/arch/riscv/include/uapi/asm/syscalls.h
+++ b/arch/riscv/include/uapi/asm/syscalls.h
@@ -1,10 +1,13 @@
-/* SPDX-License-Identifier: GPL-2.0 */
+// SPDX-License-Identifier: GPL-2.0


/* ... */  is the right style SPDX tag for headers, so please keep it
as-is.


Thanks, I didn't miss this one so I managed to fix it before the PR!

[PATCH] dt-bindings: riscv,cpu-intc: Cleanups from a missed review

[Palmer Dabbelt]

I managed to miss one of Rob's code reviews on the mailing list
<http://lists.infradead.org/pipermail/linux-riscv/2018-August/001139.html>.
The patch has already been merged, so I'm submitting a fixup.

Sorry!

Fixes: b67bc7cb4088 ("dt-bindings: interrupt-controller: RISC-V local interrupt 
controller")
Cc: Rob Herring 
Cc: Christoph Hellwig 
Cc: Karsten Merker 
Signed-off-by: Palmer Dabbelt 
---
 .../bindings/interrupt-controller/riscv,cpu-intc.txt   | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git 
a/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt 
b/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt
index b0a8af51c388..265b223cd978 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt
@@ -11,7 +11,7 @@ The RISC-V supervisor ISA manual specifies three interrupt 
sources that are
 attached to every HLIC: software interrupts, the timer interrupt, and external
 interrupts.  Software interrupts are used to send IPIs between cores.  The
 timer interrupt comes from an architecturally mandated real-time timer that is
-controller via Supervisor Binary Interface (SBI) calls and CSR reads.  External
+controlled via Supervisor Binary Interface (SBI) calls and CSR reads.  External
 interrupts connect all other device interrupts to the HLIC, which are routed
 via the platform-level interrupt controller (PLIC).
 
@@ -25,7 +25,15 @@ in the system.
 
 Required properties:
 - compatible : "riscv,cpu-intc"
-- #interrupt-cells : should be <1>
+- #interrupt-cells : should be <1>.  The interrupt sources are defined by the
+  RISC-V supervisor ISA manual, with only the following three interrupts being
+  defined for supervisor mode:
+- Source 1 is the supervisor software interrupt, which can be sent by an 
SBI
+  call and is reserved for use by software.
+- Source 5 is the supervisor timer interrupt, which can be configured by
+  SBI calls and implements a one-shot timer.
+- Source 9 is the supervisor external interrupt, which chains to all other
+  device interrupts.
 - interrupt-controller : Identifies the node as an interrupt controller
 
 Furthermore, this interrupt-controller MUST be embedded inside the cpu
@@ -38,7 +46,7 @@ An example device tree entry for a HLIC is show below.
...
cpu1-intc: interrupt-controller {
#interrupt-cells = <1>;
-   compatible = "riscv,cpu-intc", 
"sifive,fu540-c000-cpu-intc";
+   compatible = "sifive,fu540-c000-cpu-intc", 
"riscv,cpu-intc";
interrupt-controller;
};
};
-- 
2.16.4

[PATCH] dt-bindings: riscv,cpu-intc: Cleanups from a missed review

[Palmer Dabbelt]

I managed to miss one of Rob's code reviews on the mailing list
<http://lists.infradead.org/pipermail/linux-riscv/2018-August/001139.html>.
The patch has already been merged, so I'm submitting a fixup.

Sorry!

Fixes: b67bc7cb4088 ("dt-bindings: interrupt-controller: RISC-V local interrupt 
controller")
Cc: Rob Herring 
Cc: Christoph Hellwig 
Cc: Karsten Merker 
Signed-off-by: Palmer Dabbelt 
---
 .../bindings/interrupt-controller/riscv,cpu-intc.txt   | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git 
a/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt 
b/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt
index b0a8af51c388..265b223cd978 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt
@@ -11,7 +11,7 @@ The RISC-V supervisor ISA manual specifies three interrupt 
sources that are
 attached to every HLIC: software interrupts, the timer interrupt, and external
 interrupts.  Software interrupts are used to send IPIs between cores.  The
 timer interrupt comes from an architecturally mandated real-time timer that is
-controller via Supervisor Binary Interface (SBI) calls and CSR reads.  External
+controlled via Supervisor Binary Interface (SBI) calls and CSR reads.  External
 interrupts connect all other device interrupts to the HLIC, which are routed
 via the platform-level interrupt controller (PLIC).
 
@@ -25,7 +25,15 @@ in the system.
 
 Required properties:
 - compatible : "riscv,cpu-intc"
-- #interrupt-cells : should be <1>
+- #interrupt-cells : should be <1>.  The interrupt sources are defined by the
+  RISC-V supervisor ISA manual, with only the following three interrupts being
+  defined for supervisor mode:
+- Source 1 is the supervisor software interrupt, which can be sent by an 
SBI
+  call and is reserved for use by software.
+- Source 5 is the supervisor timer interrupt, which can be configured by
+  SBI calls and implements a one-shot timer.
+- Source 9 is the supervisor external interrupt, which chains to all other
+  device interrupts.
 - interrupt-controller : Identifies the node as an interrupt controller
 
 Furthermore, this interrupt-controller MUST be embedded inside the cpu
@@ -38,7 +46,7 @@ An example device tree entry for a HLIC is show below.
...
cpu1-intc: interrupt-controller {
#interrupt-cells = <1>;
-   compatible = "riscv,cpu-intc", 
"sifive,fu540-c000-cpu-intc";
+   compatible = "sifive,fu540-c000-cpu-intc", 
"riscv,cpu-intc";
interrupt-controller;
};
};
-- 
2.16.4

Re: [PATCH] riscv: Drop setup_initrd

[Palmer Dabbelt]

On Thu, 09 Aug 2018 21:11:40 PDT (-0700), li...@roeck-us.net wrote:

setup_initrd() does not appear to serve a practical purpose other than
preventing qemu boots with "-initrd" parameter, so let's drop it.

Signed-off-by: Guenter Roeck 
---
 arch/riscv/kernel/setup.c | 39 ---
 1 file changed, 39 deletions(-)


I missed this for the merge window but I think it's suitable for an RC so I'll 
target it for next week.  It's at kernel.org/palmer/linux.git/fix-initrd and 
I've added it to for-next.


Thanks!

Re: [PATCH] riscv: Drop setup_initrd

[Palmer Dabbelt]

On Thu, 09 Aug 2018 21:11:40 PDT (-0700), li...@roeck-us.net wrote:

setup_initrd() does not appear to serve a practical purpose other than
preventing qemu boots with "-initrd" parameter, so let's drop it.

Signed-off-by: Guenter Roeck 
---
 arch/riscv/kernel/setup.c | 39 ---
 1 file changed, 39 deletions(-)


I missed this for the merge window but I think it's suitable for an RC so I'll 
target it for next week.  It's at kernel.org/palmer/linux.git/fix-initrd and 
I've added it to for-next.


Thanks!

Re: [PATCH v4 0/5] riscv: Add support to no-FPU systems

[Palmer Dabbelt]

On Tue, 07 Aug 2018 20:24:40 PDT (-0700), alan...@andestech.com wrote:

This patchset adds an option, CONFIG_FPU, to enable/disable floating-
point procedures.

Kernel's new behavior will be as follows:

* with CONFIG_FPU=y
  All FPU codes are reserved.  If no FPU is found during booting, a
  global flag will be set, and those functions will be bypassed with
  condition check to that flag.

* with CONFIG_FPU=n
  No floating-point instructions in kernel and all related settings
  are excluded.

Changes in v4:
 - Append a new patch to detect existence of FPU and followups.
 - Add SPDX header to newly created fpu.S.
 - Fix a build error, sorry for that.
 - Fix wording, style, etc.

Changes in v3:
 - Refactor the whole patch into independent ones.

Changes in v2:
 - Various code cleanups and style fixes.

Alan Kao (5):
  Extract FPU context operations from entry.S
  Refactor FPU code in signal setup/return procedures
  Cleanup ISA string setting
  Allow to disable FPU support
  Auto-detect whether a FPU exists

 arch/riscv/Kconfig |   9 +++
 arch/riscv/Makefile|  19 +++---
 arch/riscv/include/asm/hwcap.h |   3 +
 arch/riscv/include/asm/switch_to.h |  21 ++
 arch/riscv/kernel/Makefile |   1 +
 arch/riscv/kernel/cpufeature.c |  11 +++
 arch/riscv/kernel/entry.S  |  87 ---
 arch/riscv/kernel/fpu.S| 106 +
 arch/riscv/kernel/process.c|   4 +-
 arch/riscv/kernel/signal.c |  79 +
 10 files changed, 214 insertions(+), 126 deletions(-)
 create mode 100644 arch/riscv/kernel/fpu.S


Aside from the CFLAGS issue this looks good.  I've queued this up in 
kernel.org/palmer/linux.git/next-nofpu, but I'm not going to put this on 
for-next until after the merge window closes.


Thanks!

Re: [PATCH v4 0/5] riscv: Add support to no-FPU systems

[Palmer Dabbelt]

On Tue, 07 Aug 2018 20:24:40 PDT (-0700), alan...@andestech.com wrote:

This patchset adds an option, CONFIG_FPU, to enable/disable floating-
point procedures.

Kernel's new behavior will be as follows:

* with CONFIG_FPU=y
  All FPU codes are reserved.  If no FPU is found during booting, a
  global flag will be set, and those functions will be bypassed with
  condition check to that flag.

* with CONFIG_FPU=n
  No floating-point instructions in kernel and all related settings
  are excluded.

Changes in v4:
 - Append a new patch to detect existence of FPU and followups.
 - Add SPDX header to newly created fpu.S.
 - Fix a build error, sorry for that.
 - Fix wording, style, etc.

Changes in v3:
 - Refactor the whole patch into independent ones.

Changes in v2:
 - Various code cleanups and style fixes.

Alan Kao (5):
  Extract FPU context operations from entry.S
  Refactor FPU code in signal setup/return procedures
  Cleanup ISA string setting
  Allow to disable FPU support
  Auto-detect whether a FPU exists

 arch/riscv/Kconfig |   9 +++
 arch/riscv/Makefile|  19 +++---
 arch/riscv/include/asm/hwcap.h |   3 +
 arch/riscv/include/asm/switch_to.h |  21 ++
 arch/riscv/kernel/Makefile |   1 +
 arch/riscv/kernel/cpufeature.c |  11 +++
 arch/riscv/kernel/entry.S  |  87 ---
 arch/riscv/kernel/fpu.S| 106 +
 arch/riscv/kernel/process.c|   4 +-
 arch/riscv/kernel/signal.c |  79 +
 10 files changed, 214 insertions(+), 126 deletions(-)
 create mode 100644 arch/riscv/kernel/fpu.S


Aside from the CFLAGS issue this looks good.  I've queued this up in 
kernel.org/palmer/linux.git/next-nofpu, but I'm not going to put this on 
for-next until after the merge window closes.


Thanks!

Re: [PATCH v4 3/5] Cleanup ISA string setting

[Palmer Dabbelt]

On Tue, 07 Aug 2018 20:24:43 PDT (-0700), alan...@andestech.com wrote:

Just a side note: (Assume that atomic and compressed is on)

Before this patch, assembler was always given the riscv64imafdc
MARCH string because there are fld/fsd's in entry.S; compiler was
always given riscv64imac because kernel doesn't need floating point
code generation.  After this, the MARCH string in AFLAGS and CFLAGS
become the same.

Signed-off-by: Alan Kao 
Cc: Greentime Hu 
Cc: Vincent Chen 
Cc: Zong Li 
Cc: Nick Hu 
Reviewed-by: Christoph Hellwig 
---
 arch/riscv/Makefile | 19 ---
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 6d4a5f6c3f4f..e0fe6790624f 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -26,7 +26,6 @@ ifeq ($(CONFIG_ARCH_RV64I),y)

KBUILD_CFLAGS += -mabi=lp64
KBUILD_AFLAGS += -mabi=lp64
-   KBUILD_MARCH = rv64im
LDFLAGS += -melf64lriscv
 else
BITS := 32
@@ -34,22 +33,20 @@ else

KBUILD_CFLAGS += -mabi=ilp32
KBUILD_AFLAGS += -mabi=ilp32
-   KBUILD_MARCH = rv32im
LDFLAGS += -melf32lriscv
 endif

 KBUILD_CFLAGS += -Wall

-ifeq ($(CONFIG_RISCV_ISA_A),y)
-   KBUILD_ARCH_A = a
-endif
-ifeq ($(CONFIG_RISCV_ISA_C),y)
-   KBUILD_ARCH_C = c
-endif
-
-KBUILD_AFLAGS += -march=$(KBUILD_MARCH)$(KBUILD_ARCH_A)fd$(KBUILD_ARCH_C)
+# ISA string setting
+riscv-march-$(CONFIG_ARCH_RV32I)   := rv32im
+riscv-march-$(CONFIG_ARCH_RV64I)   := rv64im
+riscv-march-$(CONFIG_RISCV_ISA_A)  := $(riscv-march-y)a
+riscv-march-y  := $(riscv-march-y)fd
+riscv-march-$(CONFIG_RISCV_ISA_C)  := $(riscv-march-y)c
+KBUILD_CFLAGS += -march=$(riscv-march-y)
+KBUILD_AFLAGS += -march=$(riscv-march-y)

-KBUILD_CFLAGS += -march=$(KBUILD_MARCH)$(KBUILD_ARCH_A)$(KBUILD_ARCH_C)


We used to have "fd" disabled in KBUILD_CFLAGS because we wanted to ensure that 
any use of floating-point types within the kernel would trigger the inclusion 
of soft-float libraries rather than emitting hardware floating-point 
instructions.  The worry here is that we'd end up corrupting the user's 
floating-point state with the kernel accesses because of the lazy save/restore 
stuff going on.


I remember at some point it was illegal to use floating-point within the 
kernel, but for some reason I thought that had changed.  I do see a handful of 
references to "float" and "double" in the kernel source, and most of references 
to kernel_fpu_begin() appear to be in SSE-specific stuff.  My one worry are the 
usages in drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c, as I can't quickly 
demonstrate they won't happen.


If we do this I think we should at least ensure kernel_fpu_{begin,end}() do the 
right thing for RISC-V.  I'd still feel safer telling the C compiler to 
disallow floating-point, though, as I'm a bit paranoid that GCC might go and 
emit a floating-point instruction even when it's not explicitly asked for.  I 
just asked Jim, who actually understands GCC, and he said that it'll spill to 
floating-point registers if the cost function determines it's cheaper than the 
stack.  While he thinks that's unlikely, I don't really want to rely a GCC cost 
function for the correctness of our kernel port.


I'd like to avoid having "-march=*fd*" in CFLAGS, unless someone can convince 
me it's safe and that I'm just being too paranoid :).



 KBUILD_CFLAGS += -mno-save-restore
 KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)

Re: [PATCH v4 3/5] Cleanup ISA string setting

[Palmer Dabbelt]

On Tue, 07 Aug 2018 20:24:43 PDT (-0700), alan...@andestech.com wrote:

Just a side note: (Assume that atomic and compressed is on)

Before this patch, assembler was always given the riscv64imafdc
MARCH string because there are fld/fsd's in entry.S; compiler was
always given riscv64imac because kernel doesn't need floating point
code generation.  After this, the MARCH string in AFLAGS and CFLAGS
become the same.

Signed-off-by: Alan Kao 
Cc: Greentime Hu 
Cc: Vincent Chen 
Cc: Zong Li 
Cc: Nick Hu 
Reviewed-by: Christoph Hellwig 
---
 arch/riscv/Makefile | 19 ---
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 6d4a5f6c3f4f..e0fe6790624f 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -26,7 +26,6 @@ ifeq ($(CONFIG_ARCH_RV64I),y)

KBUILD_CFLAGS += -mabi=lp64
KBUILD_AFLAGS += -mabi=lp64
-   KBUILD_MARCH = rv64im
LDFLAGS += -melf64lriscv
 else
BITS := 32
@@ -34,22 +33,20 @@ else

KBUILD_CFLAGS += -mabi=ilp32
KBUILD_AFLAGS += -mabi=ilp32
-   KBUILD_MARCH = rv32im
LDFLAGS += -melf32lriscv
 endif

 KBUILD_CFLAGS += -Wall

-ifeq ($(CONFIG_RISCV_ISA_A),y)
-   KBUILD_ARCH_A = a
-endif
-ifeq ($(CONFIG_RISCV_ISA_C),y)
-   KBUILD_ARCH_C = c
-endif
-
-KBUILD_AFLAGS += -march=$(KBUILD_MARCH)$(KBUILD_ARCH_A)fd$(KBUILD_ARCH_C)
+# ISA string setting
+riscv-march-$(CONFIG_ARCH_RV32I)   := rv32im
+riscv-march-$(CONFIG_ARCH_RV64I)   := rv64im
+riscv-march-$(CONFIG_RISCV_ISA_A)  := $(riscv-march-y)a
+riscv-march-y  := $(riscv-march-y)fd
+riscv-march-$(CONFIG_RISCV_ISA_C)  := $(riscv-march-y)c
+KBUILD_CFLAGS += -march=$(riscv-march-y)
+KBUILD_AFLAGS += -march=$(riscv-march-y)

-KBUILD_CFLAGS += -march=$(KBUILD_MARCH)$(KBUILD_ARCH_A)$(KBUILD_ARCH_C)


We used to have "fd" disabled in KBUILD_CFLAGS because we wanted to ensure that 
any use of floating-point types within the kernel would trigger the inclusion 
of soft-float libraries rather than emitting hardware floating-point 
instructions.  The worry here is that we'd end up corrupting the user's 
floating-point state with the kernel accesses because of the lazy save/restore 
stuff going on.


I remember at some point it was illegal to use floating-point within the 
kernel, but for some reason I thought that had changed.  I do see a handful of 
references to "float" and "double" in the kernel source, and most of references 
to kernel_fpu_begin() appear to be in SSE-specific stuff.  My one worry are the 
usages in drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c, as I can't quickly 
demonstrate they won't happen.


If we do this I think we should at least ensure kernel_fpu_{begin,end}() do the 
right thing for RISC-V.  I'd still feel safer telling the C compiler to 
disallow floating-point, though, as I'm a bit paranoid that GCC might go and 
emit a floating-point instruction even when it's not explicitly asked for.  I 
just asked Jim, who actually understands GCC, and he said that it'll spill to 
floating-point registers if the cost function determines it's cheaper than the 
stack.  While he thinks that's unlikely, I don't really want to rely a GCC cost 
function for the correctness of our kernel port.


I'd like to avoid having "-march=*fd*" in CFLAGS, unless someone can convince 
me it's safe and that I'm just being too paranoid :).



 KBUILD_CFLAGS += -mno-save-restore
 KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)

[GIT PULL] RISC-V Updates for the 4.19 Merge Window

[Palmer Dabbelt]

I remember having sent this on Wednesday, but for some reason I don't see it in
your tree or my outbox so I might be crazy.  I was planning submitting some
more patches next week anyway, so while I'm OK just rolling these up as well
it'd be slightly easier if we can get these into -rc1 so we can test them.

Sorry!




The following changes since commit 94710cac0ef4ee177a63b5227664b38c95bbf703:

  Linux 4.18 (2018-08-12 13:41:04 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/palmer/riscv-linux.git 
tags/riscv-for-linus-4.19-mw0

for you to fetch changes up to 627672cf431b0379c07cc8d146f907cda6797222:

  dt-bindings: interrupt-controller: SiFive Plaform Level Interrupt Controller 
(2018-08-13 09:39:11 -0700)


RISC-V Updates for the 4.19 Merge Window

This tag contains some major improvements to the RISC-V port, including
the necessary interrupt controller and timer support to actually make it
to userspace.  Support for three devices has been added:

* Support for the ISA-mandated timers on RISC-V systems.
* Support for the ISA-mandated first-level interrupt controller on
  RISC-V systems, which is handled as part of our core arch code because
  it's very small and tightly tied to the ISA.
* Support for SiFive's platform-level interrupt controller, which talks
  to the actual devices.

In addition to these new devices, there are a handful of cleanups all
over the RISC-V tree:

* Build fixes for various configurations
* A fix to the vDSO build's makefile so it respects CFLAGS.
* The addition of __lshrti3, a libgcc derived function necessary for
  some 32-bit configurations.
* !SMP && PERF_EVENTS
* Cleanups to the arch code to remove the remnants of old versions of
  the drivers that were just properly submitted.
* Some dead code from the timer driver, most of which wasn't ever
  even compiled.
* Cleanups of some interrupt #defines, which are now local to the
  interrupt handling code.
* Fixes to ptrace(), which while not being sufficient to fully make GDB
  work are at least sufficient to get simple GDB tasks to work.
* Early printk support via RISC-V's architecturally mandated SBI console
  device.
* A fix to our early debug trap handler to ensure it's always aligned.

These patches have all been through a fairly extensive review process,
but as this enables a whole pile of functionality (ie, userspace) I'm
confident we'll need to submit a few more patches.  The only concrete
issues I know about are the sys_riscv_flush_icache patches, but as I
managed to screw those up on Friday I figured it'd be best to let them
bake another week.

This tag boots a Fedora root filesystem on QEMU's master branch for me,
and before this morning's rebase (from 4.18-rc8 to 4.18) it booted on
the HiFive Unleashed.

Thanks to Christoph Hellwig and the other guys at WD for getting the new
drivers in shape!


Alex Guo (1):
  RISC-V: implement __lshrti3.

Atish Patra (1):
  RISC-V: Fix !CONFIG_SMP compilation error

Christoph Hellwig (6):
  RISC-V: remove timer leftovers
  RISC-V: simplify software interrupt / IPI code
  RISC-V: remove INTERRUPT_CAUSE_* defines from asm/irq.h
  RISC-V: add a definition for the SIE SEIE bit
  RISC-V: implement low-level interrupt handling
  irqchip: add a SiFive PLIC driver

Jim Wilson (1):
  RISC-V: Don't increment sepc after breakpoint.

Palmer Dabbelt (5):
  RISC-V: Use KBUILD_CFLAGS instead of KCFLAGS when building the vDSO
  RISC-V: Add early printk support via the SBI console
  clocksource: new RISC-V SBI timer driver
  dt-bindings: interrupt-controller: RISC-V local interrupt controller
  dt-bindings: interrupt-controller: SiFive Plaform Level Interrupt 
Controller

Zong Li (1):
  RISC-V: Add the directive for alignment of stvec's value

 .../interrupt-controller/riscv,cpu-intc.txt|  44 
 .../interrupt-controller/sifive,plic-1.0.0.txt |  58 +
 arch/riscv/Makefile|   3 +
 arch/riscv/configs/defconfig   |   1 +
 arch/riscv/include/asm/csr.h   |   1 +
 arch/riscv/include/asm/irq.h   |   5 +-
 arch/riscv/include/asm/perf_event.h|   1 +
 arch/riscv/include/asm/smp.h   |   6 -
 arch/riscv/kernel/entry.S  |   4 +-
 arch/riscv/kernel/head.S   |   2 +
 arch/riscv/kernel/irq.c|  55 -
 arch/riscv/kernel/perf_event.c |   1 -
 arch/riscv/kernel/setup.c  |  27 +++
 arch/riscv/kernel/smp.c|   6 +-
 arch/riscv/kernel/smpboot.c|   1 -
 arch/riscv/kernel/time.c   |  30 +--
 arch/riscv/kernel

[GIT PULL] RISC-V Updates for the 4.19 Merge Window

[Palmer Dabbelt]

I remember having sent this on Wednesday, but for some reason I don't see it in
your tree or my outbox so I might be crazy.  I was planning submitting some
more patches next week anyway, so while I'm OK just rolling these up as well
it'd be slightly easier if we can get these into -rc1 so we can test them.

Sorry!




The following changes since commit 94710cac0ef4ee177a63b5227664b38c95bbf703:

  Linux 4.18 (2018-08-12 13:41:04 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/palmer/riscv-linux.git 
tags/riscv-for-linus-4.19-mw0

for you to fetch changes up to 627672cf431b0379c07cc8d146f907cda6797222:

  dt-bindings: interrupt-controller: SiFive Plaform Level Interrupt Controller 
(2018-08-13 09:39:11 -0700)


RISC-V Updates for the 4.19 Merge Window

This tag contains some major improvements to the RISC-V port, including
the necessary interrupt controller and timer support to actually make it
to userspace.  Support for three devices has been added:

* Support for the ISA-mandated timers on RISC-V systems.
* Support for the ISA-mandated first-level interrupt controller on
  RISC-V systems, which is handled as part of our core arch code because
  it's very small and tightly tied to the ISA.
* Support for SiFive's platform-level interrupt controller, which talks
  to the actual devices.

In addition to these new devices, there are a handful of cleanups all
over the RISC-V tree:

* Build fixes for various configurations
* A fix to the vDSO build's makefile so it respects CFLAGS.
* The addition of __lshrti3, a libgcc derived function necessary for
  some 32-bit configurations.
* !SMP && PERF_EVENTS
* Cleanups to the arch code to remove the remnants of old versions of
  the drivers that were just properly submitted.
* Some dead code from the timer driver, most of which wasn't ever
  even compiled.
* Cleanups of some interrupt #defines, which are now local to the
  interrupt handling code.
* Fixes to ptrace(), which while not being sufficient to fully make GDB
  work are at least sufficient to get simple GDB tasks to work.
* Early printk support via RISC-V's architecturally mandated SBI console
  device.
* A fix to our early debug trap handler to ensure it's always aligned.

These patches have all been through a fairly extensive review process,
but as this enables a whole pile of functionality (ie, userspace) I'm
confident we'll need to submit a few more patches.  The only concrete
issues I know about are the sys_riscv_flush_icache patches, but as I
managed to screw those up on Friday I figured it'd be best to let them
bake another week.

This tag boots a Fedora root filesystem on QEMU's master branch for me,
and before this morning's rebase (from 4.18-rc8 to 4.18) it booted on
the HiFive Unleashed.

Thanks to Christoph Hellwig and the other guys at WD for getting the new
drivers in shape!


Alex Guo (1):
  RISC-V: implement __lshrti3.

Atish Patra (1):
  RISC-V: Fix !CONFIG_SMP compilation error

Christoph Hellwig (6):
  RISC-V: remove timer leftovers
  RISC-V: simplify software interrupt / IPI code
  RISC-V: remove INTERRUPT_CAUSE_* defines from asm/irq.h
  RISC-V: add a definition for the SIE SEIE bit
  RISC-V: implement low-level interrupt handling
  irqchip: add a SiFive PLIC driver

Jim Wilson (1):
  RISC-V: Don't increment sepc after breakpoint.

Palmer Dabbelt (5):
  RISC-V: Use KBUILD_CFLAGS instead of KCFLAGS when building the vDSO
  RISC-V: Add early printk support via the SBI console
  clocksource: new RISC-V SBI timer driver
  dt-bindings: interrupt-controller: RISC-V local interrupt controller
  dt-bindings: interrupt-controller: SiFive Plaform Level Interrupt 
Controller

Zong Li (1):
  RISC-V: Add the directive for alignment of stvec's value

 .../interrupt-controller/riscv,cpu-intc.txt|  44 
 .../interrupt-controller/sifive,plic-1.0.0.txt |  58 +
 arch/riscv/Makefile|   3 +
 arch/riscv/configs/defconfig   |   1 +
 arch/riscv/include/asm/csr.h   |   1 +
 arch/riscv/include/asm/irq.h   |   5 +-
 arch/riscv/include/asm/perf_event.h|   1 +
 arch/riscv/include/asm/smp.h   |   6 -
 arch/riscv/kernel/entry.S  |   4 +-
 arch/riscv/kernel/head.S   |   2 +
 arch/riscv/kernel/irq.c|  55 -
 arch/riscv/kernel/perf_event.c |   1 -
 arch/riscv/kernel/setup.c  |  27 +++
 arch/riscv/kernel/smp.c|   6 +-
 arch/riscv/kernel/smpboot.c|   1 -
 arch/riscv/kernel/time.c   |  30 +--
 arch/riscv/kernel

Re: [RFC] RISC-V: Fix !CONFIG_SMP compilation error

[Palmer Dabbelt]

On Fri, 10 Aug 2018 11:31:08 PDT (-0700), atish.pa...@wdc.com wrote:

On 8/6/18 4:17 PM, Atish Patra wrote:

Enabling both CONFIG_PERF_EVENTS without !CONFIG_SMP
generates following compilation error.

arch/riscv/include/asm/perf_event.h:80:2: error: expected
specifier-qualifier-list before 'irqreturn_t'

   irqreturn_t (*handle_irq)(int irq_num, void *dev);
   ^~~

Include interrupt.h in proper place to avoid compilation
error.

Signed-off-by: Atish Patra 
---
  arch/riscv/include/asm/perf_event.h | 1 +
  arch/riscv/kernel/perf_event.c  | 1 -
  2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/perf_event.h 
b/arch/riscv/include/asm/perf_event.h
index 0e638a0c..aefbfaa6 100644
--- a/arch/riscv/include/asm/perf_event.h
+++ b/arch/riscv/include/asm/perf_event.h
@@ -10,6 +10,7 @@

  #include 
  #include 
+#include 

  #define RISCV_BASE_COUNTERS   2

diff --git a/arch/riscv/kernel/perf_event.c b/arch/riscv/kernel/perf_event.c
index b0e10c4e..a243fae1 100644
--- a/arch/riscv/kernel/perf_event.c
+++ b/arch/riscv/kernel/perf_event.c
@@ -27,7 +27,6 @@
  #include 
  #include 
  #include 
-#include 
  #include 
  #include 
  #include 


ping ?


Sorry about this, it got dropped.  This looks fine and fixes the bug, so I've 
gone and put it on for-next.


Thanks!

Re: [PATCH v3 1/2] RISC-V: Define sys_riscv_flush_icache when SMP=n

[Palmer Dabbelt]

On Fri, 10 Aug 2018 11:47:15 PDT (-0700), li...@roeck-us.net wrote:

On Fri, Aug 10, 2018 at 11:27:37AM -0700, Palmer Dabbelt wrote:

On Fri, 10 Aug 2018 01:38:04 PDT (-0700), Christoph Hellwig wrote:
>On Thu, Aug 09, 2018 at 03:19:51PM -0700, Palmer Dabbelt wrote:
>>This would be necessary to make non-SMP builds work, but there is
>>another error in the implementation of our syscall linkage that actually
>>just causes sys_riscv_flush_icache to never build.  I've build tested
>>this on allnoconfig and allnoconfig+SMP=y, as well as defconfig like
>>normal.
>
>Would't it make sense to use COND_SYSCALL to stub out the syscall
>for !SMP builds?

I'm not sure.  We can implement the syscall fine in !SMP, it's just that the
vDSO is expected to always eat these calls because in non-SMP mode you can
do a global fence.i by just doing a local fence.i (there's only one hart).

The original rationale behind not having the syscall in non-SMP mode was to
limit the user ABI, but on looking again that seems like it's just a bit of
extra complexity that doesn't help anything.  It's already been demonstrated


Doesn't this mean that some userspace code will only run if the kernel was
compiled for SMP ? I always thought that was unacceptable.


Well, the officially sanctioned way to obtain this functionality is via a vDSO 
call.  On non-SMP systems it will never make the system call.  As a result we 
thought we'd keep it out of the ABI, but after looking again it seems yucky to 
do so.  Here's the vDSO entry, for reference:


   ENTRY(__vdso_flush_icache)
   .cfi_startproc
   #ifdef CONFIG_SMP
   li a7, __NR_riscv_flush_icache
   ecall
   #else
   fence.i
   li a0, 0
   #endif
   ret
   .cfi_endproc
   ENDPROC(__vdso_flush_icache)

Note that glibc has a fallback to make the system call if it can't find the 
vDSO entry, but then doesn't have a secondary fallback to emit a local fence.i 
if the system call doesn't exist.  It seems easier to fix the kernel to always 
provide the syscall and just call it a bug.

Re: [RFC] RISC-V: Fix !CONFIG_SMP compilation error

[Palmer Dabbelt]

On Fri, 10 Aug 2018 11:31:08 PDT (-0700), atish.pa...@wdc.com wrote:

On 8/6/18 4:17 PM, Atish Patra wrote:

Enabling both CONFIG_PERF_EVENTS without !CONFIG_SMP
generates following compilation error.

arch/riscv/include/asm/perf_event.h:80:2: error: expected
specifier-qualifier-list before 'irqreturn_t'

   irqreturn_t (*handle_irq)(int irq_num, void *dev);
   ^~~

Include interrupt.h in proper place to avoid compilation
error.

Signed-off-by: Atish Patra 
---
  arch/riscv/include/asm/perf_event.h | 1 +
  arch/riscv/kernel/perf_event.c  | 1 -
  2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/perf_event.h 
b/arch/riscv/include/asm/perf_event.h
index 0e638a0c..aefbfaa6 100644
--- a/arch/riscv/include/asm/perf_event.h
+++ b/arch/riscv/include/asm/perf_event.h
@@ -10,6 +10,7 @@

  #include 
  #include 
+#include 

  #define RISCV_BASE_COUNTERS   2

diff --git a/arch/riscv/kernel/perf_event.c b/arch/riscv/kernel/perf_event.c
index b0e10c4e..a243fae1 100644
--- a/arch/riscv/kernel/perf_event.c
+++ b/arch/riscv/kernel/perf_event.c
@@ -27,7 +27,6 @@
  #include 
  #include 
  #include 
-#include 
  #include 
  #include 
  #include 


ping ?


Sorry about this, it got dropped.  This looks fine and fixes the bug, so I've 
gone and put it on for-next.


Thanks!

Re: [PATCH v3 1/2] RISC-V: Define sys_riscv_flush_icache when SMP=n

[Palmer Dabbelt]

On Fri, 10 Aug 2018 11:47:15 PDT (-0700), li...@roeck-us.net wrote:

On Fri, Aug 10, 2018 at 11:27:37AM -0700, Palmer Dabbelt wrote:

On Fri, 10 Aug 2018 01:38:04 PDT (-0700), Christoph Hellwig wrote:
>On Thu, Aug 09, 2018 at 03:19:51PM -0700, Palmer Dabbelt wrote:
>>This would be necessary to make non-SMP builds work, but there is
>>another error in the implementation of our syscall linkage that actually
>>just causes sys_riscv_flush_icache to never build.  I've build tested
>>this on allnoconfig and allnoconfig+SMP=y, as well as defconfig like
>>normal.
>
>Would't it make sense to use COND_SYSCALL to stub out the syscall
>for !SMP builds?

I'm not sure.  We can implement the syscall fine in !SMP, it's just that the
vDSO is expected to always eat these calls because in non-SMP mode you can
do a global fence.i by just doing a local fence.i (there's only one hart).

The original rationale behind not having the syscall in non-SMP mode was to
limit the user ABI, but on looking again that seems like it's just a bit of
extra complexity that doesn't help anything.  It's already been demonstrated


Doesn't this mean that some userspace code will only run if the kernel was
compiled for SMP ? I always thought that was unacceptable.


Well, the officially sanctioned way to obtain this functionality is via a vDSO 
call.  On non-SMP systems it will never make the system call.  As a result we 
thought we'd keep it out of the ABI, but after looking again it seems yucky to 
do so.  Here's the vDSO entry, for reference:


   ENTRY(__vdso_flush_icache)
   .cfi_startproc
   #ifdef CONFIG_SMP
   li a7, __NR_riscv_flush_icache
   ecall
   #else
   fence.i
   li a0, 0
   #endif
   ret
   .cfi_endproc
   ENDPROC(__vdso_flush_icache)

Note that glibc has a fallback to make the system call if it can't find the 
vDSO entry, but then doesn't have a secondary fallback to emit a local fence.i 
if the system call doesn't exist.  It seems easier to fix the kernel to always 
provide the syscall and just call it a bug.

Re: [PATCH 03/11] dt-bindings: interrupt-controller: RISC-V PLIC documentation

[Palmer Dabbelt]

On Fri, 10 Aug 2018 09:57:03 PDT (-0700), robh...@kernel.org wrote:

On Thu, Aug 9, 2018 at 12:29 AM Palmer Dabbelt  wrote:


On Wed, 08 Aug 2018 16:32:07 PDT (-0700), robh...@kernel.org wrote:
> On Wed, Aug 8, 2018 at 1:38 PM Palmer Dabbelt  wrote:
>>
>> On Wed, 08 Aug 2018 07:16:14 PDT (-0700), robh...@kernel.org wrote:
>> > On Tue, Aug 7, 2018 at 8:17 PM Palmer Dabbelt  wrote:
>> >>
>> >> On Mon, 06 Aug 2018 13:59:48 PDT (-0700), robh...@kernel.org wrote:
>> >> > On Thu, Aug 2, 2018 at 4:08 PM Atish Patra  wrote:
>> >> >>
>> >> >> On 8/2/18 4:50 AM, Christoph Hellwig wrote:
>> >> >> > From: Palmer Dabbelt 
>> >> >> >
>> >> >> > This patch adds documentation for the platform-level interrupt
>> >> >> > controller (PLIC) found in all RISC-V systems.  This interrupt
>> >> >> > controller routes interrupts from all the devices in the system to 
each
>> >> >> > hart-local interrupt controller.
>> >> >> >
>> >> >> > Note: the DTS bindings for the PLIC aren't set in stone yet, as we 
might
>> >> >> > want to change how we're specifying holes in the hart list.
>> >> >> >
>> >> >> > Signed-off-by: Palmer Dabbelt 
>> >> >> > [hch: various fixes and updates]
>> >> >> > Signed-off-by: Christoph Hellwig 
>> >> >> > ---
>> >> >> >   .../interrupt-controller/sifive,plic0.txt | 57 
+++
>> >> >> >   1 file changed, 57 insertions(+)
>> >> >> >   create mode 100644 
Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
>> >> >> >
>> >> >> > diff --git 
a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt 
b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
>> >> >> > new file mode 100644
>> >> >> > index ..c756cd208a93
>> >> >> > --- /dev/null
>> >> >> > +++ 
b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
>> >> >> > @@ -0,0 +1,57 @@
>> >> >> > +SiFive Platform-Level Interrupt Controller (PLIC)
>> >> >> > +-
>> >> >> > +
>> >> >> > +SiFive SOCs include an implementation of the Platform-Level 
Interrupt Controller
>> >> >> > +(PLIC) high-level specification in the RISC-V Privileged 
Architecture
>> >> >> > +specification.  The PLIC connects all external interrupts in the 
system to all
>> >> >> > +hart contexts in the system, via the external interrupt source in 
each hart.
>> >> >> > +
>> >> >> > +A hart context is a privilege mode in a hardware execution thread.  
For example,
>> >> >> > +in an 4 core system with 2-way SMT, you have 8 harts and probably 
at least two
>> >> >> > +privilege modes per hart; machine mode and supervisor mode.
>> >> >> > +
>> >> >> > +Each interrupt can be enabled on per-context basis. Any context can 
claim
>> >> >> > +a pending enabled interrupt and then release it once it has been 
handled.
>> >> >> > +
>> >> >> > +Each interrupt has a configurable priority. Higher priority 
interrupts are
>> >> >> > +serviced first. Each context can specify a priority threshold. 
Interrupts
>> >> >> > +with priority below this threshold will not cause the PLIC to raise 
its
>> >> >> > +interrupt line leading to the context.
>> >> >> > +
>> >> >> > +While the PLIC supports both edge-triggered and level-triggered 
interrupts,
>> >> >> > +interrupt handlers are oblivious to this distinction and therefore 
it is not
>> >> >> > +specified in the PLIC device-tree binding.
>> >> >> > +
>> >> >> > +While the RISC-V ISA doesn't specify a memory layout for the PLIC, 
the
>> >> >> > +"sifive,plic0" device is a concrete implementation of the PLIC that 
contains a
>> >> >> > +specific memory layout, which is documented in chapter 8 of the 
SiFive U5
>> >> >> > +Coreplex Series Manual 
<https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>.
>> >> >> &

Re: [PATCH 03/11] dt-bindings: interrupt-controller: RISC-V PLIC documentation

[Palmer Dabbelt]

On Fri, 10 Aug 2018 09:57:03 PDT (-0700), robh...@kernel.org wrote:

On Thu, Aug 9, 2018 at 12:29 AM Palmer Dabbelt  wrote:


On Wed, 08 Aug 2018 16:32:07 PDT (-0700), robh...@kernel.org wrote:
> On Wed, Aug 8, 2018 at 1:38 PM Palmer Dabbelt  wrote:
>>
>> On Wed, 08 Aug 2018 07:16:14 PDT (-0700), robh...@kernel.org wrote:
>> > On Tue, Aug 7, 2018 at 8:17 PM Palmer Dabbelt  wrote:
>> >>
>> >> On Mon, 06 Aug 2018 13:59:48 PDT (-0700), robh...@kernel.org wrote:
>> >> > On Thu, Aug 2, 2018 at 4:08 PM Atish Patra  wrote:
>> >> >>
>> >> >> On 8/2/18 4:50 AM, Christoph Hellwig wrote:
>> >> >> > From: Palmer Dabbelt 
>> >> >> >
>> >> >> > This patch adds documentation for the platform-level interrupt
>> >> >> > controller (PLIC) found in all RISC-V systems.  This interrupt
>> >> >> > controller routes interrupts from all the devices in the system to 
each
>> >> >> > hart-local interrupt controller.
>> >> >> >
>> >> >> > Note: the DTS bindings for the PLIC aren't set in stone yet, as we 
might
>> >> >> > want to change how we're specifying holes in the hart list.
>> >> >> >
>> >> >> > Signed-off-by: Palmer Dabbelt 
>> >> >> > [hch: various fixes and updates]
>> >> >> > Signed-off-by: Christoph Hellwig 
>> >> >> > ---
>> >> >> >   .../interrupt-controller/sifive,plic0.txt | 57 
+++
>> >> >> >   1 file changed, 57 insertions(+)
>> >> >> >   create mode 100644 
Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
>> >> >> >
>> >> >> > diff --git 
a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt 
b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
>> >> >> > new file mode 100644
>> >> >> > index ..c756cd208a93
>> >> >> > --- /dev/null
>> >> >> > +++ 
b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
>> >> >> > @@ -0,0 +1,57 @@
>> >> >> > +SiFive Platform-Level Interrupt Controller (PLIC)
>> >> >> > +-
>> >> >> > +
>> >> >> > +SiFive SOCs include an implementation of the Platform-Level 
Interrupt Controller
>> >> >> > +(PLIC) high-level specification in the RISC-V Privileged 
Architecture
>> >> >> > +specification.  The PLIC connects all external interrupts in the 
system to all
>> >> >> > +hart contexts in the system, via the external interrupt source in 
each hart.
>> >> >> > +
>> >> >> > +A hart context is a privilege mode in a hardware execution thread.  
For example,
>> >> >> > +in an 4 core system with 2-way SMT, you have 8 harts and probably 
at least two
>> >> >> > +privilege modes per hart; machine mode and supervisor mode.
>> >> >> > +
>> >> >> > +Each interrupt can be enabled on per-context basis. Any context can 
claim
>> >> >> > +a pending enabled interrupt and then release it once it has been 
handled.
>> >> >> > +
>> >> >> > +Each interrupt has a configurable priority. Higher priority 
interrupts are
>> >> >> > +serviced first. Each context can specify a priority threshold. 
Interrupts
>> >> >> > +with priority below this threshold will not cause the PLIC to raise 
its
>> >> >> > +interrupt line leading to the context.
>> >> >> > +
>> >> >> > +While the PLIC supports both edge-triggered and level-triggered 
interrupts,
>> >> >> > +interrupt handlers are oblivious to this distinction and therefore 
it is not
>> >> >> > +specified in the PLIC device-tree binding.
>> >> >> > +
>> >> >> > +While the RISC-V ISA doesn't specify a memory layout for the PLIC, 
the
>> >> >> > +"sifive,plic0" device is a concrete implementation of the PLIC that 
contains a
>> >> >> > +specific memory layout, which is documented in chapter 8 of the 
SiFive U5
>> >> >> > +Coreplex Series Manual 
<https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>.
>> >> >> &

Re: [PATCH v3 1/2] RISC-V: Define sys_riscv_flush_icache when SMP=n

[Palmer Dabbelt]

On Fri, 10 Aug 2018 01:38:04 PDT (-0700), Christoph Hellwig wrote:

On Thu, Aug 09, 2018 at 03:19:51PM -0700, Palmer Dabbelt wrote:

This would be necessary to make non-SMP builds work, but there is
another error in the implementation of our syscall linkage that actually
just causes sys_riscv_flush_icache to never build.  I've build tested
this on allnoconfig and allnoconfig+SMP=y, as well as defconfig like
normal.


Would't it make sense to use COND_SYSCALL to stub out the syscall
for !SMP builds?


I'm not sure.  We can implement the syscall fine in !SMP, it's just that the 
vDSO is expected to always eat these calls because in non-SMP mode you can do a 
global fence.i by just doing a local fence.i (there's only one hart).


The original rationale behind not having the syscall in non-SMP mode was to 
limit the user ABI, but on looking again that seems like it's just a bit of 
extra complexity that doesn't help anything.  It's already been demonstrated 
that nothing is checking the error because it's been silently slipping past 
userspace for six months, so the extra complexity seems like it'll just cause 
someone else to have to chase the bug in the future.


But I'm really OK either way.  Is there a precedent for what to do here?

Re: [PATCH v3 1/2] RISC-V: Define sys_riscv_flush_icache when SMP=n

[Palmer Dabbelt]

On Fri, 10 Aug 2018 01:38:04 PDT (-0700), Christoph Hellwig wrote:

On Thu, Aug 09, 2018 at 03:19:51PM -0700, Palmer Dabbelt wrote:

This would be necessary to make non-SMP builds work, but there is
another error in the implementation of our syscall linkage that actually
just causes sys_riscv_flush_icache to never build.  I've build tested
this on allnoconfig and allnoconfig+SMP=y, as well as defconfig like
normal.


Would't it make sense to use COND_SYSCALL to stub out the syscall
for !SMP builds?


I'm not sure.  We can implement the syscall fine in !SMP, it's just that the 
vDSO is expected to always eat these calls because in non-SMP mode you can do a 
global fence.i by just doing a local fence.i (there's only one hart).


The original rationale behind not having the syscall in non-SMP mode was to 
limit the user ABI, but on looking again that seems like it's just a bit of 
extra complexity that doesn't help anything.  It's already been demonstrated 
that nothing is checking the error because it's been silently slipping past 
userspace for six months, so the extra complexity seems like it'll just cause 
someone else to have to chase the bug in the future.


But I'm really OK either way.  Is there a precedent for what to do here?

Re: [PATCH v2 2/2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h

[Palmer Dabbelt]

On Thu, 09 Aug 2018 19:40:55 PDT (-0700), li...@roeck-us.net wrote:

On 08/09/2018 06:03 PM, Palmer Dabbelt wrote:

On Thu, 09 Aug 2018 14:24:22 PDT (-0700), li...@roeck-us.net wrote:

On Thu, Aug 09, 2018 at 01:25:24PM -0700, Palmer Dabbelt wrote:

This file is expected to be included multiple times in the same file in
order to allow the __SYSCALL macro to generate system call tables.  With
a global include guard we end up missing __NR_riscv_flush_icache in the
syscall table, which results in icache flushes that escape the vDSO call
to not actually do anything.

The fix is to move to per-#define include guards, which allows the
system call tables to actually be populated.  Thanks to Macrus Comstedt
for finding and fixing the bug!

I also went ahead and fixed the SPDX header to use a //-style comment,
which I've been told is the canonical way to do it.

Cc: Marcus Comstedt 
Signed-off-by: Palmer Dabbelt 


[Compile-]Tested-by: Guenter Roeck 

on top of linux-next after reverting the version of the patch there.

I also tried to run the resulting image (defconfig) with qemu (built
from https://github.com/riscv/riscv-qemu.git), but that still doesn't
work. I assume there are still some patches missing ?


Do you have the PLIC patches?  They'll be necessary to make this all work, and 
there's a v4 out now that when combined with for-next should get you to 
userspace.

    https://lore.kernel.org/lkml/20180809075602.989-1-...@lst.de/T/#u


Yes, after merging that branch on top of linux-next I can boot into Linux.
If I add my "riscv: Drop setup_initrd" patch as well, I can boot using
initrd, otherwise I have to use virtio-blk-device.


Awesome!  If you patch isn't on for-next then I must have missed it, do you 
mind sending me a pointer?  I can't find any references in my email.



Also, what is your methodology?  I follow

    https://wiki.qemu.org/Documentation/Platforms/RISCV

and could could natively compile and run hello world with an earlier version of 
Christoph's patch set, which is really only cosmetically different than the v4. 
I use qemu's master branch as well, which when I tried was exactly 3.0.0-rc3.



That doesn't work for me, possibly because I don't specify a bbl
image with -kernel but vmlinux (using -bios for the bbl image).
I use branch qemu-for-upstream of https://github.com/riscv/riscv-qemu.git.

Guenter

Re: [PATCH v2 2/2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h

[Palmer Dabbelt]

On Thu, 09 Aug 2018 19:40:55 PDT (-0700), li...@roeck-us.net wrote:

On 08/09/2018 06:03 PM, Palmer Dabbelt wrote:

On Thu, 09 Aug 2018 14:24:22 PDT (-0700), li...@roeck-us.net wrote:

On Thu, Aug 09, 2018 at 01:25:24PM -0700, Palmer Dabbelt wrote:

This file is expected to be included multiple times in the same file in
order to allow the __SYSCALL macro to generate system call tables.  With
a global include guard we end up missing __NR_riscv_flush_icache in the
syscall table, which results in icache flushes that escape the vDSO call
to not actually do anything.

The fix is to move to per-#define include guards, which allows the
system call tables to actually be populated.  Thanks to Macrus Comstedt
for finding and fixing the bug!

I also went ahead and fixed the SPDX header to use a //-style comment,
which I've been told is the canonical way to do it.

Cc: Marcus Comstedt 
Signed-off-by: Palmer Dabbelt 


[Compile-]Tested-by: Guenter Roeck 

on top of linux-next after reverting the version of the patch there.

I also tried to run the resulting image (defconfig) with qemu (built
from https://github.com/riscv/riscv-qemu.git), but that still doesn't
work. I assume there are still some patches missing ?


Do you have the PLIC patches?  They'll be necessary to make this all work, and 
there's a v4 out now that when combined with for-next should get you to 
userspace.

    https://lore.kernel.org/lkml/20180809075602.989-1-...@lst.de/T/#u


Yes, after merging that branch on top of linux-next I can boot into Linux.
If I add my "riscv: Drop setup_initrd" patch as well, I can boot using
initrd, otherwise I have to use virtio-blk-device.


Awesome!  If you patch isn't on for-next then I must have missed it, do you 
mind sending me a pointer?  I can't find any references in my email.



Also, what is your methodology?  I follow

    https://wiki.qemu.org/Documentation/Platforms/RISCV

and could could natively compile and run hello world with an earlier version of 
Christoph's patch set, which is really only cosmetically different than the v4. 
I use qemu's master branch as well, which when I tried was exactly 3.0.0-rc3.



That doesn't work for me, possibly because I don't specify a bbl
image with -kernel but vmlinux (using -bios for the bbl image).
I use branch qemu-for-upstream of https://github.com/riscv/riscv-qemu.git.

Guenter

Re: FW: [PATCH v2] RISC-V: Add the directive for alignment of stvec's value

[Palmer Dabbelt]

On Thu, 09 Aug 2018 17:37:39 PDT (-0700), zong...@gmail.com wrote:

Subject: [PATCH v2] RISC-V: Add the directive for alignment of stvec's value

The stvec's value must be 4 byte alignment by specification definition.
These directives avoid to stvec be set the non-alignment value.

Signed-off-by: Zong Li 
---
 arch/riscv/kernel/head.S | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S index 
3b6293f..11066d5 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -94,6 +94,7 @@ relocate:
or a0, a0, a1
sfence.vma
csrw sptbr, a0
+.align 2
 1:
/* Set trap vector to spin forever to help debug */
la a0, .Lsecondary_park
@@ -143,6 +144,7 @@ relocate:
tail smp_callin
 #endif

+.align 2
 .Lsecondary_park:
/* We lack SMP support or have too many harts, so park this hart */
wfi


Thanks, this got lost in the shuffle somewhere.  It's in for-next now.

Re: FW: [PATCH v2] RISC-V: Add the directive for alignment of stvec's value

[Palmer Dabbelt]

On Thu, 09 Aug 2018 17:37:39 PDT (-0700), zong...@gmail.com wrote:

Subject: [PATCH v2] RISC-V: Add the directive for alignment of stvec's value

The stvec's value must be 4 byte alignment by specification definition.
These directives avoid to stvec be set the non-alignment value.

Signed-off-by: Zong Li 
---
 arch/riscv/kernel/head.S | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S index 
3b6293f..11066d5 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -94,6 +94,7 @@ relocate:
or a0, a0, a1
sfence.vma
csrw sptbr, a0
+.align 2
 1:
/* Set trap vector to spin forever to help debug */
la a0, .Lsecondary_park
@@ -143,6 +144,7 @@ relocate:
tail smp_callin
 #endif

+.align 2
 .Lsecondary_park:
/* We lack SMP support or have too many harts, so park this hart */
wfi


Thanks, this got lost in the shuffle somewhere.  It's in for-next now.

Re: [PATCH v2 2/2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h

[Palmer Dabbelt]

On Thu, 09 Aug 2018 14:24:22 PDT (-0700), li...@roeck-us.net wrote:

On Thu, Aug 09, 2018 at 01:25:24PM -0700, Palmer Dabbelt wrote:

This file is expected to be included multiple times in the same file in
order to allow the __SYSCALL macro to generate system call tables.  With
a global include guard we end up missing __NR_riscv_flush_icache in the
syscall table, which results in icache flushes that escape the vDSO call
to not actually do anything.

The fix is to move to per-#define include guards, which allows the
system call tables to actually be populated.  Thanks to Macrus Comstedt
for finding and fixing the bug!

I also went ahead and fixed the SPDX header to use a //-style comment,
which I've been told is the canonical way to do it.

Cc: Marcus Comstedt 
Signed-off-by: Palmer Dabbelt 


[Compile-]Tested-by: Guenter Roeck 

on top of linux-next after reverting the version of the patch there.

I also tried to run the resulting image (defconfig) with qemu (built
from https://github.com/riscv/riscv-qemu.git), but that still doesn't
work. I assume there are still some patches missing ?


Do you have the PLIC patches?  They'll be necessary to make this all work, and 
there's a v4 out now that when combined with for-next should get you to 
userspace.


   https://lore.kernel.org/lkml/20180809075602.989-1-...@lst.de/T/#u

Also, what is your methodology?  I follow

   https://wiki.qemu.org/Documentation/Platforms/RISCV

and could could natively compile and run hello world with an earlier version of 
Christoph's patch set, which is really only cosmetically different than the v4.  
I use qemu's master branch as well, which when I tried was exactly 3.0.0-rc3.

Re: [PATCH v2 2/2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h

[Palmer Dabbelt]

On Thu, 09 Aug 2018 14:24:22 PDT (-0700), li...@roeck-us.net wrote:

On Thu, Aug 09, 2018 at 01:25:24PM -0700, Palmer Dabbelt wrote:

This file is expected to be included multiple times in the same file in
order to allow the __SYSCALL macro to generate system call tables.  With
a global include guard we end up missing __NR_riscv_flush_icache in the
syscall table, which results in icache flushes that escape the vDSO call
to not actually do anything.

The fix is to move to per-#define include guards, which allows the
system call tables to actually be populated.  Thanks to Macrus Comstedt
for finding and fixing the bug!

I also went ahead and fixed the SPDX header to use a //-style comment,
which I've been told is the canonical way to do it.

Cc: Marcus Comstedt 
Signed-off-by: Palmer Dabbelt 


[Compile-]Tested-by: Guenter Roeck 

on top of linux-next after reverting the version of the patch there.

I also tried to run the resulting image (defconfig) with qemu (built
from https://github.com/riscv/riscv-qemu.git), but that still doesn't
work. I assume there are still some patches missing ?


Do you have the PLIC patches?  They'll be necessary to make this all work, and 
there's a v4 out now that when combined with for-next should get you to 
userspace.


   https://lore.kernel.org/lkml/20180809075602.989-1-...@lst.de/T/#u

Also, what is your methodology?  I follow

   https://wiki.qemu.org/Documentation/Platforms/RISCV

and could could natively compile and run hello world with an earlier version of 
Christoph's patch set, which is really only cosmetically different than the v4.  
I use qemu's master branch as well, which when I tried was exactly 3.0.0-rc3.

[PATCH v3 1/2] RISC-V: Define sys_riscv_flush_icache when SMP=n

[Palmer Dabbelt]

This would be necessary to make non-SMP builds work, but there is
another error in the implementation of our syscall linkage that actually
just causes sys_riscv_flush_icache to never build.  I've build tested
this on allnoconfig and allnoconfig+SMP=y, as well as defconfig like
normal.

CC: Christoph Hellwig 
CC: Guenter Roeck 
In-Reply-To: <20180809055830.ga17...@infradead.org>
In-Reply-To: <20180809132612.ga31...@roeck-us.net>
Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/include/asm/vdso.h |  2 --
 arch/riscv/kernel/sys_riscv.c | 12 ++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/include/asm/vdso.h b/arch/riscv/include/asm/vdso.h
index 541544d64c33..ec6180a4b55d 100644
--- a/arch/riscv/include/asm/vdso.h
+++ b/arch/riscv/include/asm/vdso.h
@@ -38,8 +38,6 @@ struct vdso_data {
(void __user *)((unsigned long)(base) + __vdso_##name); 
\
 })
 
-#ifdef CONFIG_SMP
 asmlinkage long sys_riscv_flush_icache(uintptr_t, uintptr_t, uintptr_t);
-#endif
 
 #endif /* _ASM_RISCV_VDSO_H */
diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c
index f7181ed8aafc..568026ccf6e8 100644
--- a/arch/riscv/kernel/sys_riscv.c
+++ b/arch/riscv/kernel/sys_riscv.c
@@ -48,7 +48,6 @@ SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, 
len,
 }
 #endif /* !CONFIG_64BIT */
 
-#ifdef CONFIG_SMP
 /*
  * Allows the instruction cache to be flushed from userspace.  Despite RISC-V
  * having a direct 'fence.i' instruction available to userspace (which we
@@ -66,15 +65,24 @@ SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, 
len,
 SYSCALL_DEFINE3(riscv_flush_icache, uintptr_t, start, uintptr_t, end,
uintptr_t, flags)
 {
+#ifdef CONFIG_SMP
struct mm_struct *mm = current->mm;
bool local = (flags & SYS_RISCV_FLUSH_ICACHE_LOCAL) != 0;
+#endif
 
/* Check the reserved flags. */
if (unlikely(flags & ~SYS_RISCV_FLUSH_ICACHE_ALL))
return -EINVAL;
 
+   /*
+* Without CONFIG_SMP flush_icache_mm is a just a flush_icache_all(),
+* which generates unused variable warnings all over this function.
+*/
+#ifdef CONFIG_SMP
flush_icache_mm(mm, local);
+#else
+   flush_icache_all();
+#endif
 
return 0;
 }
-#endif
-- 
2.16.4

[PATCH v3 1/2] RISC-V: Define sys_riscv_flush_icache when SMP=n

[Palmer Dabbelt]

This would be necessary to make non-SMP builds work, but there is
another error in the implementation of our syscall linkage that actually
just causes sys_riscv_flush_icache to never build.  I've build tested
this on allnoconfig and allnoconfig+SMP=y, as well as defconfig like
normal.

CC: Christoph Hellwig 
CC: Guenter Roeck 
In-Reply-To: <20180809055830.ga17...@infradead.org>
In-Reply-To: <20180809132612.ga31...@roeck-us.net>
Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/include/asm/vdso.h |  2 --
 arch/riscv/kernel/sys_riscv.c | 12 ++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/include/asm/vdso.h b/arch/riscv/include/asm/vdso.h
index 541544d64c33..ec6180a4b55d 100644
--- a/arch/riscv/include/asm/vdso.h
+++ b/arch/riscv/include/asm/vdso.h
@@ -38,8 +38,6 @@ struct vdso_data {
(void __user *)((unsigned long)(base) + __vdso_##name); 
\
 })
 
-#ifdef CONFIG_SMP
 asmlinkage long sys_riscv_flush_icache(uintptr_t, uintptr_t, uintptr_t);
-#endif
 
 #endif /* _ASM_RISCV_VDSO_H */
diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c
index f7181ed8aafc..568026ccf6e8 100644
--- a/arch/riscv/kernel/sys_riscv.c
+++ b/arch/riscv/kernel/sys_riscv.c
@@ -48,7 +48,6 @@ SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, 
len,
 }
 #endif /* !CONFIG_64BIT */
 
-#ifdef CONFIG_SMP
 /*
  * Allows the instruction cache to be flushed from userspace.  Despite RISC-V
  * having a direct 'fence.i' instruction available to userspace (which we
@@ -66,15 +65,24 @@ SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, 
len,
 SYSCALL_DEFINE3(riscv_flush_icache, uintptr_t, start, uintptr_t, end,
uintptr_t, flags)
 {
+#ifdef CONFIG_SMP
struct mm_struct *mm = current->mm;
bool local = (flags & SYS_RISCV_FLUSH_ICACHE_LOCAL) != 0;
+#endif
 
/* Check the reserved flags. */
if (unlikely(flags & ~SYS_RISCV_FLUSH_ICACHE_ALL))
return -EINVAL;
 
+   /*
+* Without CONFIG_SMP flush_icache_mm is a just a flush_icache_all(),
+* which generates unused variable warnings all over this function.
+*/
+#ifdef CONFIG_SMP
flush_icache_mm(mm, local);
+#else
+   flush_icache_all();
+#endif
 
return 0;
 }
-#endif
-- 
2.16.4

[PATCH v3 2/2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h

[Palmer Dabbelt]

This file is expected to be included multiple times in the same file in
order to allow the __SYSCALL macro to generate system call tables.  With
a global include guard we end up missing __NR_riscv_flush_icache in the
syscall table, which results in icache flushes that escape the vDSO call
to not actually do anything.

The fix is to move to per-#define include guards, which allows the
system call tables to actually be populated.  Thanks to Macrus Comstedt
for finding and fixing the bug!

I also went ahead and fixed the SPDX header to use a //-style comment,
which I've been told is the canonical way to do it.

Cc: Marcus Comstedt 
Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/include/asm/unistd.h|  5 +
 arch/riscv/include/uapi/asm/syscalls.h | 15 +--
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/include/asm/unistd.h b/arch/riscv/include/asm/unistd.h
index 080fb28061de..0caea01d5cca 100644
--- a/arch/riscv/include/asm/unistd.h
+++ b/arch/riscv/include/asm/unistd.h
@@ -11,6 +11,11 @@
  *   GNU General Public License for more details.
  */
 
+/*
+ * There is explicitly no include guard here because this file is expected to
+ * be included multiple times.  See uapi/asm/syscalls.h for more info.
+ */
+
 #define __ARCH_WANT_SYS_CLONE
 #include 
 #include 
diff --git a/arch/riscv/include/uapi/asm/syscalls.h 
b/arch/riscv/include/uapi/asm/syscalls.h
index 818655b0d535..690beb002d1d 100644
--- a/arch/riscv/include/uapi/asm/syscalls.h
+++ b/arch/riscv/include/uapi/asm/syscalls.h
@@ -1,10 +1,13 @@
-/* SPDX-License-Identifier: GPL-2.0 */
+// SPDX-License-Identifier: GPL-2.0
 /*
- * Copyright (C) 2017 SiFive
+ * Copyright (C) 2017-2018 SiFive
  */
 
-#ifndef _ASM__UAPI__SYSCALLS_H
-#define _ASM__UAPI__SYSCALLS_H
+/*
+ * There is explicitly no include guard here because this file is expected to
+ * be included multiple times in order to define the syscall macros via
+ * __SYSCALL.
+ */
 
 /*
  * Allows the instruction cache to be flushed from userspace.  Despite RISC-V
@@ -20,7 +23,7 @@
  * caller.  We don't currently do anything with the address range, that's just
  * in there for forwards compatibility.
  */
+#ifndef __NR_riscv_flush_icache
 #define __NR_riscv_flush_icache (__NR_arch_specific_syscall + 15)
-__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache)
-
 #endif
+__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache)
-- 
2.16.4

[PATCH v3 2/2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h

[Palmer Dabbelt]

This file is expected to be included multiple times in the same file in
order to allow the __SYSCALL macro to generate system call tables.  With
a global include guard we end up missing __NR_riscv_flush_icache in the
syscall table, which results in icache flushes that escape the vDSO call
to not actually do anything.

The fix is to move to per-#define include guards, which allows the
system call tables to actually be populated.  Thanks to Macrus Comstedt
for finding and fixing the bug!

I also went ahead and fixed the SPDX header to use a //-style comment,
which I've been told is the canonical way to do it.

Cc: Marcus Comstedt 
Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/include/asm/unistd.h|  5 +
 arch/riscv/include/uapi/asm/syscalls.h | 15 +--
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/include/asm/unistd.h b/arch/riscv/include/asm/unistd.h
index 080fb28061de..0caea01d5cca 100644
--- a/arch/riscv/include/asm/unistd.h
+++ b/arch/riscv/include/asm/unistd.h
@@ -11,6 +11,11 @@
  *   GNU General Public License for more details.
  */
 
+/*
+ * There is explicitly no include guard here because this file is expected to
+ * be included multiple times.  See uapi/asm/syscalls.h for more info.
+ */
+
 #define __ARCH_WANT_SYS_CLONE
 #include 
 #include 
diff --git a/arch/riscv/include/uapi/asm/syscalls.h 
b/arch/riscv/include/uapi/asm/syscalls.h
index 818655b0d535..690beb002d1d 100644
--- a/arch/riscv/include/uapi/asm/syscalls.h
+++ b/arch/riscv/include/uapi/asm/syscalls.h
@@ -1,10 +1,13 @@
-/* SPDX-License-Identifier: GPL-2.0 */
+// SPDX-License-Identifier: GPL-2.0
 /*
- * Copyright (C) 2017 SiFive
+ * Copyright (C) 2017-2018 SiFive
  */
 
-#ifndef _ASM__UAPI__SYSCALLS_H
-#define _ASM__UAPI__SYSCALLS_H
+/*
+ * There is explicitly no include guard here because this file is expected to
+ * be included multiple times in order to define the syscall macros via
+ * __SYSCALL.
+ */
 
 /*
  * Allows the instruction cache to be flushed from userspace.  Despite RISC-V
@@ -20,7 +23,7 @@
  * caller.  We don't currently do anything with the address range, that's just
  * in there for forwards compatibility.
  */
+#ifndef __NR_riscv_flush_icache
 #define __NR_riscv_flush_icache (__NR_arch_specific_syscall + 15)
-__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache)
-
 #endif
+__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache)
-- 
2.16.4

RISC-V: Don't use a global include guard for uapi/asm/syscalls.

[Palmer Dabbelt]

It turns out that we weren't actually hooking sys_riscv_flush_icache
into the syscall table, which results in any flush_icache() call that
escapes the vDSO to silently do nothing.

Changes since v2:

* sys_riscv_flush_icache actually flushes the icache when SMP=n.  Thanks
  to Andrew for pointing out the I screwed it up!

Changes since v1:

* sys_riscv_flush_icache is now defined even when SMP=n, which allows
  this patch set to build against SMP=n and SMP=y.

RISC-V: Don't use a global include guard for uapi/asm/syscalls.

[Palmer Dabbelt]

It turns out that we weren't actually hooking sys_riscv_flush_icache
into the syscall table, which results in any flush_icache() call that
escapes the vDSO to silently do nothing.

Changes since v2:

* sys_riscv_flush_icache actually flushes the icache when SMP=n.  Thanks
  to Andrew for pointing out the I screwed it up!

Changes since v1:

* sys_riscv_flush_icache is now defined even when SMP=n, which allows
  this patch set to build against SMP=n and SMP=y.

Re: [PATCH v2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h

[Palmer Dabbelt]

On Wed, 08 Aug 2018 22:58:30 PDT (-0700), Christoph Hellwig wrote:

This actually seems to break the compilation for me in for-next:

hch@carbon:~/work/linux$ make ARCH=riscv
  CALLscripts/checksyscalls.sh
:1335:2: warning: #warning syscall rseq not implemented [-Wcpp]
  CHK include/generated/compile.h
  CC  arch/riscv/kernel/syscall_table.o
./arch/riscv/include/uapi/asm/syscalls.h:29:36: error: 'sys_riscv_flush_icache' 
undeclared here (not in a function); did you mean '__NR_riscv_flush_icache'?
 __SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache)
^
arch/riscv/kernel/syscall_table.c:21:37: note: in definition of macro 
'__SYSCALL'
 #define __SYSCALL(nr, call) [nr] = (call),
 ^~~~
make[1]: *** [scripts/Makefile.build:318: arch/riscv/kernel/syscall_table.o] 
Error 1
make: *** [Makefile:1029: arch/riscv/kernel] Error 2

Reverting it for now, but I guess this might hit others too..


Thanks.  I dropped this from for-next and just sent out a replacement.

Re: [PATCH v2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h

[Palmer Dabbelt]

On Wed, 08 Aug 2018 22:58:30 PDT (-0700), Christoph Hellwig wrote:

This actually seems to break the compilation for me in for-next:

hch@carbon:~/work/linux$ make ARCH=riscv
  CALLscripts/checksyscalls.sh
:1335:2: warning: #warning syscall rseq not implemented [-Wcpp]
  CHK include/generated/compile.h
  CC  arch/riscv/kernel/syscall_table.o
./arch/riscv/include/uapi/asm/syscalls.h:29:36: error: 'sys_riscv_flush_icache' 
undeclared here (not in a function); did you mean '__NR_riscv_flush_icache'?
 __SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache)
^
arch/riscv/kernel/syscall_table.c:21:37: note: in definition of macro 
'__SYSCALL'
 #define __SYSCALL(nr, call) [nr] = (call),
 ^~~~
make[1]: *** [scripts/Makefile.build:318: arch/riscv/kernel/syscall_table.o] 
Error 1
make: *** [Makefile:1029: arch/riscv/kernel] Error 2

Reverting it for now, but I guess this might hit others too..


Thanks.  I dropped this from for-next and just sent out a replacement.

Re: [PATCH] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h

[Palmer Dabbelt]

On Thu, 09 Aug 2018 06:26:12 PDT (-0700), li...@roeck-us.net wrote:

On Fri, Aug 03, 2018 at 12:53:44PM -0700, Palmer Dabbelt wrote:

This file is expected to be included multiple times in the same file in
order to allow the __SYSCALL macro to generate system call tables.  With
a global include guard we end up missing __NR_riscv_flush_icache in the
syscall table, which results in icache flushes that escape the vDSO call
to not actually do anything.

The fix is to move to per-#define include guards, which allows the
system call tables to actually be populated.  Thanks to Macrus Comstedt
for finding and fixing the bug!

I also went ahead and fixed the SPDX header to use a //-style comment,
which I've been told is the canonical way to do it.

Cc: Marcus Comstedt 
Signed-off-by: Palmer Dabbelt 


Fails to build riscv:allnoconfig.

CC  arch/riscv/kernel/syscall_table.o
./arch/riscv/include/uapi/asm/syscalls.h:29:36: error:
‘sys_riscv_flush_icache’ undeclared here (not in a function); did you mean 
‘__NR_riscv_flush_icache’?


Thanks.  I added you to another patch set.

Re: [PATCH] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h

[Palmer Dabbelt]

On Thu, 09 Aug 2018 06:26:12 PDT (-0700), li...@roeck-us.net wrote:

On Fri, Aug 03, 2018 at 12:53:44PM -0700, Palmer Dabbelt wrote:

This file is expected to be included multiple times in the same file in
order to allow the __SYSCALL macro to generate system call tables.  With
a global include guard we end up missing __NR_riscv_flush_icache in the
syscall table, which results in icache flushes that escape the vDSO call
to not actually do anything.

The fix is to move to per-#define include guards, which allows the
system call tables to actually be populated.  Thanks to Macrus Comstedt
for finding and fixing the bug!

I also went ahead and fixed the SPDX header to use a //-style comment,
which I've been told is the canonical way to do it.

Cc: Marcus Comstedt 
Signed-off-by: Palmer Dabbelt 


Fails to build riscv:allnoconfig.

CC  arch/riscv/kernel/syscall_table.o
./arch/riscv/include/uapi/asm/syscalls.h:29:36: error:
‘sys_riscv_flush_icache’ undeclared here (not in a function); did you mean 
‘__NR_riscv_flush_icache’?


Thanks.  I added you to another patch set.

[PATCH v2 0/2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.

[Palmer Dabbelt]

It turns out that we weren't actually hooking sys_riscv_flush_icache
into the syscall table, which results in any flush_icache() call that
escapes the vDSO to silently do nothing.

Changes since v1:

* sys_riscv_flush_icache is now defined even when SMP=n, which allows
  this patch set to build against SMP=n and SMP=y.

[PATCH v2 1/2] RISC-V: Define sys_riscv_flush_icache when SMP=n

[Palmer Dabbelt]

This would be necessary to make non-SMP builds work, but there is
another error in the implementation of our syscall linkage that actually
just causes sys_riscv_flush_icache to never build.  I've build tested
this on allnoconfig and allnoconfig+SMP=y, as well as defconfig like
normal.

CC: Christoph Hellwig 
CC: Guenter Roeck 
In-Reply-To: <20180809055830.ga17...@infradead.org>
In-Reply-To: <20180809132612.ga31...@roeck-us.net>
Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/include/asm/vdso.h |  2 --
 arch/riscv/kernel/sys_riscv.c | 10 --
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/include/asm/vdso.h b/arch/riscv/include/asm/vdso.h
index 541544d64c33..ec6180a4b55d 100644
--- a/arch/riscv/include/asm/vdso.h
+++ b/arch/riscv/include/asm/vdso.h
@@ -38,8 +38,6 @@ struct vdso_data {
(void __user *)((unsigned long)(base) + __vdso_##name); 
\
 })
 
-#ifdef CONFIG_SMP
 asmlinkage long sys_riscv_flush_icache(uintptr_t, uintptr_t, uintptr_t);
-#endif
 
 #endif /* _ASM_RISCV_VDSO_H */
diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c
index f7181ed8aafc..180da8d4e14a 100644
--- a/arch/riscv/kernel/sys_riscv.c
+++ b/arch/riscv/kernel/sys_riscv.c
@@ -48,7 +48,6 @@ SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, 
len,
 }
 #endif /* !CONFIG_64BIT */
 
-#ifdef CONFIG_SMP
 /*
  * Allows the instruction cache to be flushed from userspace.  Despite RISC-V
  * having a direct 'fence.i' instruction available to userspace (which we
@@ -66,15 +65,22 @@ SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, 
len,
 SYSCALL_DEFINE3(riscv_flush_icache, uintptr_t, start, uintptr_t, end,
uintptr_t, flags)
 {
+#ifdef CONFIG_SMP
struct mm_struct *mm = current->mm;
bool local = (flags & SYS_RISCV_FLUSH_ICACHE_LOCAL) != 0;
+#endif
 
/* Check the reserved flags. */
if (unlikely(flags & ~SYS_RISCV_FLUSH_ICACHE_ALL))
return -EINVAL;
 
+   /*
+* Without CONFIG_SMP flush_icache_mm is a NOP, which generates unused
+* variable warnings all over this function.
+*/
+#ifdef CONFIG_SMP
flush_icache_mm(mm, local);
+#endif
 
return 0;
 }
-#endif
-- 
2.16.4

[PATCH v2 0/2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.

[Palmer Dabbelt]

It turns out that we weren't actually hooking sys_riscv_flush_icache
into the syscall table, which results in any flush_icache() call that
escapes the vDSO to silently do nothing.

Changes since v1:

* sys_riscv_flush_icache is now defined even when SMP=n, which allows
  this patch set to build against SMP=n and SMP=y.

[PATCH v2 1/2] RISC-V: Define sys_riscv_flush_icache when SMP=n

[Palmer Dabbelt]

This would be necessary to make non-SMP builds work, but there is
another error in the implementation of our syscall linkage that actually
just causes sys_riscv_flush_icache to never build.  I've build tested
this on allnoconfig and allnoconfig+SMP=y, as well as defconfig like
normal.

CC: Christoph Hellwig 
CC: Guenter Roeck 
In-Reply-To: <20180809055830.ga17...@infradead.org>
In-Reply-To: <20180809132612.ga31...@roeck-us.net>
Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/include/asm/vdso.h |  2 --
 arch/riscv/kernel/sys_riscv.c | 10 --
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/include/asm/vdso.h b/arch/riscv/include/asm/vdso.h
index 541544d64c33..ec6180a4b55d 100644
--- a/arch/riscv/include/asm/vdso.h
+++ b/arch/riscv/include/asm/vdso.h
@@ -38,8 +38,6 @@ struct vdso_data {
(void __user *)((unsigned long)(base) + __vdso_##name); 
\
 })
 
-#ifdef CONFIG_SMP
 asmlinkage long sys_riscv_flush_icache(uintptr_t, uintptr_t, uintptr_t);
-#endif
 
 #endif /* _ASM_RISCV_VDSO_H */
diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c
index f7181ed8aafc..180da8d4e14a 100644
--- a/arch/riscv/kernel/sys_riscv.c
+++ b/arch/riscv/kernel/sys_riscv.c
@@ -48,7 +48,6 @@ SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, 
len,
 }
 #endif /* !CONFIG_64BIT */
 
-#ifdef CONFIG_SMP
 /*
  * Allows the instruction cache to be flushed from userspace.  Despite RISC-V
  * having a direct 'fence.i' instruction available to userspace (which we
@@ -66,15 +65,22 @@ SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, 
len,
 SYSCALL_DEFINE3(riscv_flush_icache, uintptr_t, start, uintptr_t, end,
uintptr_t, flags)
 {
+#ifdef CONFIG_SMP
struct mm_struct *mm = current->mm;
bool local = (flags & SYS_RISCV_FLUSH_ICACHE_LOCAL) != 0;
+#endif
 
/* Check the reserved flags. */
if (unlikely(flags & ~SYS_RISCV_FLUSH_ICACHE_ALL))
return -EINVAL;
 
+   /*
+* Without CONFIG_SMP flush_icache_mm is a NOP, which generates unused
+* variable warnings all over this function.
+*/
+#ifdef CONFIG_SMP
flush_icache_mm(mm, local);
+#endif
 
return 0;
 }
-#endif
-- 
2.16.4

[PATCH v2 2/2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h

[Palmer Dabbelt]

This file is expected to be included multiple times in the same file in
order to allow the __SYSCALL macro to generate system call tables.  With
a global include guard we end up missing __NR_riscv_flush_icache in the
syscall table, which results in icache flushes that escape the vDSO call
to not actually do anything.

The fix is to move to per-#define include guards, which allows the
system call tables to actually be populated.  Thanks to Macrus Comstedt
for finding and fixing the bug!

I also went ahead and fixed the SPDX header to use a //-style comment,
which I've been told is the canonical way to do it.

Cc: Marcus Comstedt 
Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/include/asm/unistd.h|  5 +
 arch/riscv/include/uapi/asm/syscalls.h | 15 +--
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/include/asm/unistd.h b/arch/riscv/include/asm/unistd.h
index 080fb28061de..0caea01d5cca 100644
--- a/arch/riscv/include/asm/unistd.h
+++ b/arch/riscv/include/asm/unistd.h
@@ -11,6 +11,11 @@
  *   GNU General Public License for more details.
  */
 
+/*
+ * There is explicitly no include guard here because this file is expected to
+ * be included multiple times.  See uapi/asm/syscalls.h for more info.
+ */
+
 #define __ARCH_WANT_SYS_CLONE
 #include 
 #include 
diff --git a/arch/riscv/include/uapi/asm/syscalls.h 
b/arch/riscv/include/uapi/asm/syscalls.h
index 818655b0d535..690beb002d1d 100644
--- a/arch/riscv/include/uapi/asm/syscalls.h
+++ b/arch/riscv/include/uapi/asm/syscalls.h
@@ -1,10 +1,13 @@
-/* SPDX-License-Identifier: GPL-2.0 */
+// SPDX-License-Identifier: GPL-2.0
 /*
- * Copyright (C) 2017 SiFive
+ * Copyright (C) 2017-2018 SiFive
  */
 
-#ifndef _ASM__UAPI__SYSCALLS_H
-#define _ASM__UAPI__SYSCALLS_H
+/*
+ * There is explicitly no include guard here because this file is expected to
+ * be included multiple times in order to define the syscall macros via
+ * __SYSCALL.
+ */
 
 /*
  * Allows the instruction cache to be flushed from userspace.  Despite RISC-V
@@ -20,7 +23,7 @@
  * caller.  We don't currently do anything with the address range, that's just
  * in there for forwards compatibility.
  */
+#ifndef __NR_riscv_flush_icache
 #define __NR_riscv_flush_icache (__NR_arch_specific_syscall + 15)
-__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache)
-
 #endif
+__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache)
-- 
2.16.4

[PATCH v2 2/2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h

[Palmer Dabbelt]

This file is expected to be included multiple times in the same file in
order to allow the __SYSCALL macro to generate system call tables.  With
a global include guard we end up missing __NR_riscv_flush_icache in the
syscall table, which results in icache flushes that escape the vDSO call
to not actually do anything.

The fix is to move to per-#define include guards, which allows the
system call tables to actually be populated.  Thanks to Macrus Comstedt
for finding and fixing the bug!

I also went ahead and fixed the SPDX header to use a //-style comment,
which I've been told is the canonical way to do it.

Cc: Marcus Comstedt 
Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/include/asm/unistd.h|  5 +
 arch/riscv/include/uapi/asm/syscalls.h | 15 +--
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/include/asm/unistd.h b/arch/riscv/include/asm/unistd.h
index 080fb28061de..0caea01d5cca 100644
--- a/arch/riscv/include/asm/unistd.h
+++ b/arch/riscv/include/asm/unistd.h
@@ -11,6 +11,11 @@
  *   GNU General Public License for more details.
  */
 
+/*
+ * There is explicitly no include guard here because this file is expected to
+ * be included multiple times.  See uapi/asm/syscalls.h for more info.
+ */
+
 #define __ARCH_WANT_SYS_CLONE
 #include 
 #include 
diff --git a/arch/riscv/include/uapi/asm/syscalls.h 
b/arch/riscv/include/uapi/asm/syscalls.h
index 818655b0d535..690beb002d1d 100644
--- a/arch/riscv/include/uapi/asm/syscalls.h
+++ b/arch/riscv/include/uapi/asm/syscalls.h
@@ -1,10 +1,13 @@
-/* SPDX-License-Identifier: GPL-2.0 */
+// SPDX-License-Identifier: GPL-2.0
 /*
- * Copyright (C) 2017 SiFive
+ * Copyright (C) 2017-2018 SiFive
  */
 
-#ifndef _ASM__UAPI__SYSCALLS_H
-#define _ASM__UAPI__SYSCALLS_H
+/*
+ * There is explicitly no include guard here because this file is expected to
+ * be included multiple times in order to define the syscall macros via
+ * __SYSCALL.
+ */
 
 /*
  * Allows the instruction cache to be flushed from userspace.  Despite RISC-V
@@ -20,7 +23,7 @@
  * caller.  We don't currently do anything with the address range, that's just
  * in there for forwards compatibility.
  */
+#ifndef __NR_riscv_flush_icache
 #define __NR_riscv_flush_icache (__NR_arch_specific_syscall + 15)
-__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache)
-
 #endif
+__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache)
-- 
2.16.4

Re: [PATCH 03/11] dt-bindings: interrupt-controller: RISC-V PLIC documentation

[Palmer Dabbelt]

On Wed, 08 Aug 2018 16:32:07 PDT (-0700), robh...@kernel.org wrote:

On Wed, Aug 8, 2018 at 1:38 PM Palmer Dabbelt  wrote:


On Wed, 08 Aug 2018 07:16:14 PDT (-0700), robh...@kernel.org wrote:
> On Tue, Aug 7, 2018 at 8:17 PM Palmer Dabbelt  wrote:
>>
>> On Mon, 06 Aug 2018 13:59:48 PDT (-0700), robh...@kernel.org wrote:
>> > On Thu, Aug 2, 2018 at 4:08 PM Atish Patra  wrote:
>> >>
>> >> On 8/2/18 4:50 AM, Christoph Hellwig wrote:
>> >> > From: Palmer Dabbelt 
>> >> >
>> >> > This patch adds documentation for the platform-level interrupt
>> >> > controller (PLIC) found in all RISC-V systems.  This interrupt
>> >> > controller routes interrupts from all the devices in the system to each
>> >> > hart-local interrupt controller.
>> >> >
>> >> > Note: the DTS bindings for the PLIC aren't set in stone yet, as we might
>> >> > want to change how we're specifying holes in the hart list.
>> >> >
>> >> > Signed-off-by: Palmer Dabbelt 
>> >> > [hch: various fixes and updates]
>> >> > Signed-off-by: Christoph Hellwig 
>> >> > ---
>> >> >   .../interrupt-controller/sifive,plic0.txt | 57 +++
>> >> >   1 file changed, 57 insertions(+)
>> >> >   create mode 100644 
Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
>> >> >
>> >> > diff --git 
a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt 
b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
>> >> > new file mode 100644
>> >> > index ..c756cd208a93
>> >> > --- /dev/null
>> >> > +++ 
b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
>> >> > @@ -0,0 +1,57 @@
>> >> > +SiFive Platform-Level Interrupt Controller (PLIC)
>> >> > +-
>> >> > +
>> >> > +SiFive SOCs include an implementation of the Platform-Level Interrupt 
Controller
>> >> > +(PLIC) high-level specification in the RISC-V Privileged Architecture
>> >> > +specification.  The PLIC connects all external interrupts in the 
system to all
>> >> > +hart contexts in the system, via the external interrupt source in each 
hart.
>> >> > +
>> >> > +A hart context is a privilege mode in a hardware execution thread.  
For example,
>> >> > +in an 4 core system with 2-way SMT, you have 8 harts and probably at 
least two
>> >> > +privilege modes per hart; machine mode and supervisor mode.
>> >> > +
>> >> > +Each interrupt can be enabled on per-context basis. Any context can 
claim
>> >> > +a pending enabled interrupt and then release it once it has been 
handled.
>> >> > +
>> >> > +Each interrupt has a configurable priority. Higher priority interrupts 
are
>> >> > +serviced first. Each context can specify a priority threshold. 
Interrupts
>> >> > +with priority below this threshold will not cause the PLIC to raise its
>> >> > +interrupt line leading to the context.
>> >> > +
>> >> > +While the PLIC supports both edge-triggered and level-triggered 
interrupts,
>> >> > +interrupt handlers are oblivious to this distinction and therefore it 
is not
>> >> > +specified in the PLIC device-tree binding.
>> >> > +
>> >> > +While the RISC-V ISA doesn't specify a memory layout for the PLIC, the
>> >> > +"sifive,plic0" device is a concrete implementation of the PLIC that 
contains a
>> >> > +specific memory layout, which is documented in chapter 8 of the SiFive 
U5
>> >> > +Coreplex Series Manual 
<https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>.
>> >> > +
>> >> > +Required properties:
>> >> > +- compatible : "sifive,plic0"
>>
>> I think there was a thread bouncing around somewhere where decided to pick 
the
>> official name of the compatible string to be "sifive,plic-1.0".  The idea 
here
>> is that the PLIC is compatible across all of SiFive's current 
implementations,
>> but there's some limitations in the memory map that will probably cause us to
>> spin a version 2 at some point so we want major version number.  The minor
>> number is just in case we find some errata in the PLIC.
>
> Is 1.0 an actual version number corre

Re: [PATCH 03/11] dt-bindings: interrupt-controller: RISC-V PLIC documentation

[Palmer Dabbelt]

On Wed, 08 Aug 2018 16:32:07 PDT (-0700), robh...@kernel.org wrote:

On Wed, Aug 8, 2018 at 1:38 PM Palmer Dabbelt  wrote:


On Wed, 08 Aug 2018 07:16:14 PDT (-0700), robh...@kernel.org wrote:
> On Tue, Aug 7, 2018 at 8:17 PM Palmer Dabbelt  wrote:
>>
>> On Mon, 06 Aug 2018 13:59:48 PDT (-0700), robh...@kernel.org wrote:
>> > On Thu, Aug 2, 2018 at 4:08 PM Atish Patra  wrote:
>> >>
>> >> On 8/2/18 4:50 AM, Christoph Hellwig wrote:
>> >> > From: Palmer Dabbelt 
>> >> >
>> >> > This patch adds documentation for the platform-level interrupt
>> >> > controller (PLIC) found in all RISC-V systems.  This interrupt
>> >> > controller routes interrupts from all the devices in the system to each
>> >> > hart-local interrupt controller.
>> >> >
>> >> > Note: the DTS bindings for the PLIC aren't set in stone yet, as we might
>> >> > want to change how we're specifying holes in the hart list.
>> >> >
>> >> > Signed-off-by: Palmer Dabbelt 
>> >> > [hch: various fixes and updates]
>> >> > Signed-off-by: Christoph Hellwig 
>> >> > ---
>> >> >   .../interrupt-controller/sifive,plic0.txt | 57 +++
>> >> >   1 file changed, 57 insertions(+)
>> >> >   create mode 100644 
Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
>> >> >
>> >> > diff --git 
a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt 
b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
>> >> > new file mode 100644
>> >> > index ..c756cd208a93
>> >> > --- /dev/null
>> >> > +++ 
b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
>> >> > @@ -0,0 +1,57 @@
>> >> > +SiFive Platform-Level Interrupt Controller (PLIC)
>> >> > +-
>> >> > +
>> >> > +SiFive SOCs include an implementation of the Platform-Level Interrupt 
Controller
>> >> > +(PLIC) high-level specification in the RISC-V Privileged Architecture
>> >> > +specification.  The PLIC connects all external interrupts in the 
system to all
>> >> > +hart contexts in the system, via the external interrupt source in each 
hart.
>> >> > +
>> >> > +A hart context is a privilege mode in a hardware execution thread.  
For example,
>> >> > +in an 4 core system with 2-way SMT, you have 8 harts and probably at 
least two
>> >> > +privilege modes per hart; machine mode and supervisor mode.
>> >> > +
>> >> > +Each interrupt can be enabled on per-context basis. Any context can 
claim
>> >> > +a pending enabled interrupt and then release it once it has been 
handled.
>> >> > +
>> >> > +Each interrupt has a configurable priority. Higher priority interrupts 
are
>> >> > +serviced first. Each context can specify a priority threshold. 
Interrupts
>> >> > +with priority below this threshold will not cause the PLIC to raise its
>> >> > +interrupt line leading to the context.
>> >> > +
>> >> > +While the PLIC supports both edge-triggered and level-triggered 
interrupts,
>> >> > +interrupt handlers are oblivious to this distinction and therefore it 
is not
>> >> > +specified in the PLIC device-tree binding.
>> >> > +
>> >> > +While the RISC-V ISA doesn't specify a memory layout for the PLIC, the
>> >> > +"sifive,plic0" device is a concrete implementation of the PLIC that 
contains a
>> >> > +specific memory layout, which is documented in chapter 8 of the SiFive 
U5
>> >> > +Coreplex Series Manual 
<https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>.
>> >> > +
>> >> > +Required properties:
>> >> > +- compatible : "sifive,plic0"
>>
>> I think there was a thread bouncing around somewhere where decided to pick 
the
>> official name of the compatible string to be "sifive,plic-1.0".  The idea 
here
>> is that the PLIC is compatible across all of SiFive's current 
implementations,
>> but there's some limitations in the memory map that will probably cause us to
>> spin a version 2 at some point so we want major version number.  The minor
>> number is just in case we find some errata in the PLIC.
>
> Is 1.0 an actual version number corre

Re: [PATCH 6/8] dt-bindings: interrupt-controller: RISC-V PLIC documentation

[Palmer Dabbelt]

On Wed, 08 Aug 2018 09:15:58 PDT (-0700), robh...@kernel.org wrote:

On Wed, Aug 8, 2018 at 8:59 AM Christoph Hellwig  wrote:


On Wed, Aug 08, 2018 at 08:29:50AM -0600, Rob Herring wrote:
> Version numbers on the individual patches would be nice...

We've never done these in the subsystems I'm involved with.  Too
much clutter in the subject lines for information that is easily
deductable.


Unfortunately not in Gmail which doesn't thread properly. But
patchwork also provides the version tag which I use to sort my
reviews.


> > +Example:
> > +
> > +   plic: interrupt-controller@c00 {
> > +   #address-cells = <0>;
> > +   #interrupt-cells = <1>;
> > +   compatible = "riscv,plic0";
> > +   interrupt-controller;
> > +   interrupts-extended = <
> > +11
> > +11  9
> > +11  9
> > +11  9
> > +11  9>;
>
> I'm confused why this is still here if you are dropping the cpu intc binding?

We need some parent that identifies the core (hart in RISC-V terminology).
The way the code now works is that it just walks up the parent chain
until it finds a CPU node, so it either accepts the legacy intc node
inbetween, or it accepts the cpu node directly as the intc node is pointless.

I guess for the documentation we should instead just point to the
"riscv" cpu nodes instead?


That's not valid and dtc will tell you that. 'interrupts' (via
interrupt-parent) or 'interrupts-extended' has to point to an
'interrupt-controller' node. I guess you could make the cpu nodes
interrupt-controllers. That's a bit strange, but I can't think of a
reason why that wouldn't work.

Just because the cpu-intc is not made to be an irqchip in the kernel
doesn't mean it can't still be represented as an interrupt-controller
in DT. It shouldn't be designed just around how some OS happens to
implement things.


FWIW, I like this approach.  There is an interrupt widget in the hardware, so 
having the device tree represent it seems like a good idea.



> I also noticed the cpu binding refers to "riscv,cpu-intc" as well.
> That needs to be fixed too if there's a change.

Only in the examples.  I'd be fine with dropping them, but let's keep
that separate from the interrupt support.


You need to sort out how this is all tied together and works because
right now you are supporting 2 ways and one is undocumented and the
other is invalid. Changing things later is only going to be more
painful.

Rob

Re: [PATCH 6/8] dt-bindings: interrupt-controller: RISC-V PLIC documentation

[Palmer Dabbelt]

On Wed, 08 Aug 2018 09:15:58 PDT (-0700), robh...@kernel.org wrote:

On Wed, Aug 8, 2018 at 8:59 AM Christoph Hellwig  wrote:


On Wed, Aug 08, 2018 at 08:29:50AM -0600, Rob Herring wrote:
> Version numbers on the individual patches would be nice...

We've never done these in the subsystems I'm involved with.  Too
much clutter in the subject lines for information that is easily
deductable.


Unfortunately not in Gmail which doesn't thread properly. But
patchwork also provides the version tag which I use to sort my
reviews.


> > +Example:
> > +
> > +   plic: interrupt-controller@c00 {
> > +   #address-cells = <0>;
> > +   #interrupt-cells = <1>;
> > +   compatible = "riscv,plic0";
> > +   interrupt-controller;
> > +   interrupts-extended = <
> > +11
> > +11  9
> > +11  9
> > +11  9
> > +11  9>;
>
> I'm confused why this is still here if you are dropping the cpu intc binding?

We need some parent that identifies the core (hart in RISC-V terminology).
The way the code now works is that it just walks up the parent chain
until it finds a CPU node, so it either accepts the legacy intc node
inbetween, or it accepts the cpu node directly as the intc node is pointless.

I guess for the documentation we should instead just point to the
"riscv" cpu nodes instead?


That's not valid and dtc will tell you that. 'interrupts' (via
interrupt-parent) or 'interrupts-extended' has to point to an
'interrupt-controller' node. I guess you could make the cpu nodes
interrupt-controllers. That's a bit strange, but I can't think of a
reason why that wouldn't work.

Just because the cpu-intc is not made to be an irqchip in the kernel
doesn't mean it can't still be represented as an interrupt-controller
in DT. It shouldn't be designed just around how some OS happens to
implement things.


FWIW, I like this approach.  There is an interrupt widget in the hardware, so 
having the device tree represent it seems like a good idea.



> I also noticed the cpu binding refers to "riscv,cpu-intc" as well.
> That needs to be fixed too if there's a change.

Only in the examples.  I'd be fine with dropping them, but let's keep
that separate from the interrupt support.


You need to sort out how this is all tied together and works because
right now you are supporting 2 ways and one is undocumented and the
other is invalid. Changing things later is only going to be more
painful.

Rob

Re: [PATCH 03/11] dt-bindings: interrupt-controller: RISC-V PLIC documentation

[Palmer Dabbelt]

On Wed, 08 Aug 2018 07:16:14 PDT (-0700), robh...@kernel.org wrote:

On Tue, Aug 7, 2018 at 8:17 PM Palmer Dabbelt  wrote:


On Mon, 06 Aug 2018 13:59:48 PDT (-0700), robh...@kernel.org wrote:
> On Thu, Aug 2, 2018 at 4:08 PM Atish Patra  wrote:
>>
>> On 8/2/18 4:50 AM, Christoph Hellwig wrote:
>> > From: Palmer Dabbelt 
>> >
>> > This patch adds documentation for the platform-level interrupt
>> > controller (PLIC) found in all RISC-V systems.  This interrupt
>> > controller routes interrupts from all the devices in the system to each
>> > hart-local interrupt controller.
>> >
>> > Note: the DTS bindings for the PLIC aren't set in stone yet, as we might
>> > want to change how we're specifying holes in the hart list.
>> >
>> > Signed-off-by: Palmer Dabbelt 
>> > [hch: various fixes and updates]
>> > Signed-off-by: Christoph Hellwig 
>> > ---
>> >   .../interrupt-controller/sifive,plic0.txt | 57 +++
>> >   1 file changed, 57 insertions(+)
>> >   create mode 100644 
Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
>> >
>> > diff --git 
a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt 
b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
>> > new file mode 100644
>> > index ..c756cd208a93
>> > --- /dev/null
>> > +++ 
b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
>> > @@ -0,0 +1,57 @@
>> > +SiFive Platform-Level Interrupt Controller (PLIC)
>> > +-
>> > +
>> > +SiFive SOCs include an implementation of the Platform-Level Interrupt 
Controller
>> > +(PLIC) high-level specification in the RISC-V Privileged Architecture
>> > +specification.  The PLIC connects all external interrupts in the system 
to all
>> > +hart contexts in the system, via the external interrupt source in each 
hart.
>> > +
>> > +A hart context is a privilege mode in a hardware execution thread.  For 
example,
>> > +in an 4 core system with 2-way SMT, you have 8 harts and probably at 
least two
>> > +privilege modes per hart; machine mode and supervisor mode.
>> > +
>> > +Each interrupt can be enabled on per-context basis. Any context can claim
>> > +a pending enabled interrupt and then release it once it has been handled.
>> > +
>> > +Each interrupt has a configurable priority. Higher priority interrupts are
>> > +serviced first. Each context can specify a priority threshold. Interrupts
>> > +with priority below this threshold will not cause the PLIC to raise its
>> > +interrupt line leading to the context.
>> > +
>> > +While the PLIC supports both edge-triggered and level-triggered 
interrupts,
>> > +interrupt handlers are oblivious to this distinction and therefore it is 
not
>> > +specified in the PLIC device-tree binding.
>> > +
>> > +While the RISC-V ISA doesn't specify a memory layout for the PLIC, the
>> > +"sifive,plic0" device is a concrete implementation of the PLIC that 
contains a
>> > +specific memory layout, which is documented in chapter 8 of the SiFive U5
>> > +Coreplex Series Manual 
<https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>.
>> > +
>> > +Required properties:
>> > +- compatible : "sifive,plic0"

I think there was a thread bouncing around somewhere where decided to pick the
official name of the compatible string to be "sifive,plic-1.0".  The idea here
is that the PLIC is compatible across all of SiFive's current implementations,
but there's some limitations in the memory map that will probably cause us to
spin a version 2 at some point so we want major version number.  The minor
number is just in case we find some errata in the PLIC.


Is 1.0 an actual version number corresponding to an exact, revision
controlled version of the IP or just something you made up? Looks like
the latter to me and I'm not a fan of s/w folks making up version
numbers for h/w. Standard naming convention is ,-
unless you have good reason to deviate (IP for FPGAs where version
numbers are exposed to customers is one example).


The hardware versioning scheme calls it "riscv,plic0", which is what we were 
originally using.  This PLIC isn't actually defined as a RISC-V spec, so we 
wanted to change it to a "sifive,*" name instead.  Since we were going to 
change the compat string anyway, I thought we'd just introduce a minor number 
to be safe.  Since "0.0" is an awkward number, I figured "1.0" 

Re: [PATCH 03/11] dt-bindings: interrupt-controller: RISC-V PLIC documentation

[Palmer Dabbelt]

On Wed, 08 Aug 2018 07:16:14 PDT (-0700), robh...@kernel.org wrote:

On Tue, Aug 7, 2018 at 8:17 PM Palmer Dabbelt  wrote:


On Mon, 06 Aug 2018 13:59:48 PDT (-0700), robh...@kernel.org wrote:
> On Thu, Aug 2, 2018 at 4:08 PM Atish Patra  wrote:
>>
>> On 8/2/18 4:50 AM, Christoph Hellwig wrote:
>> > From: Palmer Dabbelt 
>> >
>> > This patch adds documentation for the platform-level interrupt
>> > controller (PLIC) found in all RISC-V systems.  This interrupt
>> > controller routes interrupts from all the devices in the system to each
>> > hart-local interrupt controller.
>> >
>> > Note: the DTS bindings for the PLIC aren't set in stone yet, as we might
>> > want to change how we're specifying holes in the hart list.
>> >
>> > Signed-off-by: Palmer Dabbelt 
>> > [hch: various fixes and updates]
>> > Signed-off-by: Christoph Hellwig 
>> > ---
>> >   .../interrupt-controller/sifive,plic0.txt | 57 +++
>> >   1 file changed, 57 insertions(+)
>> >   create mode 100644 
Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
>> >
>> > diff --git 
a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt 
b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
>> > new file mode 100644
>> > index ..c756cd208a93
>> > --- /dev/null
>> > +++ 
b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
>> > @@ -0,0 +1,57 @@
>> > +SiFive Platform-Level Interrupt Controller (PLIC)
>> > +-
>> > +
>> > +SiFive SOCs include an implementation of the Platform-Level Interrupt 
Controller
>> > +(PLIC) high-level specification in the RISC-V Privileged Architecture
>> > +specification.  The PLIC connects all external interrupts in the system 
to all
>> > +hart contexts in the system, via the external interrupt source in each 
hart.
>> > +
>> > +A hart context is a privilege mode in a hardware execution thread.  For 
example,
>> > +in an 4 core system with 2-way SMT, you have 8 harts and probably at 
least two
>> > +privilege modes per hart; machine mode and supervisor mode.
>> > +
>> > +Each interrupt can be enabled on per-context basis. Any context can claim
>> > +a pending enabled interrupt and then release it once it has been handled.
>> > +
>> > +Each interrupt has a configurable priority. Higher priority interrupts are
>> > +serviced first. Each context can specify a priority threshold. Interrupts
>> > +with priority below this threshold will not cause the PLIC to raise its
>> > +interrupt line leading to the context.
>> > +
>> > +While the PLIC supports both edge-triggered and level-triggered 
interrupts,
>> > +interrupt handlers are oblivious to this distinction and therefore it is 
not
>> > +specified in the PLIC device-tree binding.
>> > +
>> > +While the RISC-V ISA doesn't specify a memory layout for the PLIC, the
>> > +"sifive,plic0" device is a concrete implementation of the PLIC that 
contains a
>> > +specific memory layout, which is documented in chapter 8 of the SiFive U5
>> > +Coreplex Series Manual 
<https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>.
>> > +
>> > +Required properties:
>> > +- compatible : "sifive,plic0"

I think there was a thread bouncing around somewhere where decided to pick the
official name of the compatible string to be "sifive,plic-1.0".  The idea here
is that the PLIC is compatible across all of SiFive's current implementations,
but there's some limitations in the memory map that will probably cause us to
spin a version 2 at some point so we want major version number.  The minor
number is just in case we find some errata in the PLIC.


Is 1.0 an actual version number corresponding to an exact, revision
controlled version of the IP or just something you made up? Looks like
the latter to me and I'm not a fan of s/w folks making up version
numbers for h/w. Standard naming convention is ,-
unless you have good reason to deviate (IP for FPGAs where version
numbers are exposed to customers is one example).


The hardware versioning scheme calls it "riscv,plic0", which is what we were 
originally using.  This PLIC isn't actually defined as a RISC-V spec, so we 
wanted to change it to a "sifive,*" name instead.  Since we were going to 
change the compat string anyway, I thought we'd just introduce a minor number 
to be safe.  Since "0.0" is an awkward number, I figured "1.0" 

Re: simplified RISC-V interrupt and clocksource handling v3

[Palmer Dabbelt]

On Sat, 04 Aug 2018 01:23:11 PDT (-0700), Christoph Hellwig wrote:

This series tries adds support for interrupt handling and timers
for the RISC-V architecture.

The basic per-hart interrupt handling implemented by the scause
and sie CSRs is extremely simple and implemented directly in
arch/riscv/kernel/irq.c.  In addition there is a irqchip driver
for the PLIC external interrupt controller, which is called through
the set_handle_irq API, and a clocksource driver that gets its
timer interrupt directly from the low-level interrupt handling.

Compared to previous iterations this version does not try to use an
irqchip driver for the low-level interrupt handling.  This saves
a couple indirect calls and an additional read of the scause CSR
in the hot path, makes the code much simpler and last but not least
avoid the dependency on a device tree for a mandatory architectural
feature.

A git tree is available here (contains a few more patches before
the ones in this series)

git://git.infradead.org/users/hch/riscv.git riscv-irq-simple.3

Gitweb:


http://git.infradead.org/users/hch/riscv.git/shortlog/refs/heads/riscv-irq-simple.3

Changes since v2:
 - actually use SEIE instead of STIE in the plic driver
 - rename the default compat string for the plic to sifive,u5-plic
 - various spelling fixes
 - drop a superflous derefence in the plic driver that is taken care of
   by the following loop
 - drop the patch to document the enable method - not relevant for the
   rest of the series
 - drop the patches for the per-hart timebase frequency - not relevant
   for the rest of the series.
 - use riscv_of_processor_hart in the timer driver

Changes since v1:
 - rename the plic driver to irq-sifive-plic
 - switch to a default compatible of sifive,plic0 (still supporting the
   riscv,plic0 name for compatibility)
 - add a reference for the SiFive PLIC register layout
 - fix plic_toggle addressing for large numbers of hwirqs
 - remove the call to ack_bad_irq
 - use a raw spinlock for plic_toggle_lock
 - use the irq_desc cpumask in the plic enable/disable methods
 - add back OF contexid parsing in the plic driver
 - don't allow COMPILE_TEST builds of the clocksource driver, as it
   depends on 
 - default the clocksource driver to y
 - clean up naming in the clocksource driver
 - remove the MINDELTA and MAXDELTA #defines
 - various DT binding fixes


Thanks!  Modulo the one device tree issue I replied to in patch 3 this looks 
great!  We've already gotten the ACKs to take this through the RISC-V tree, so 
I'm going to put this along with the queued RISC-V patches on our for-next 
branch, including my proposed change for "sifive,plic-1.0" but leaving the 
device tree bindings with #{address,size}-cells=1.


We can always change this, but I'd like to get this out so people can start 
playing with it earlier rather than later.


Thanks to everyone for all the help!

Re: simplified RISC-V interrupt and clocksource handling v3

[Palmer Dabbelt]

On Sat, 04 Aug 2018 01:23:11 PDT (-0700), Christoph Hellwig wrote:

This series tries adds support for interrupt handling and timers
for the RISC-V architecture.

The basic per-hart interrupt handling implemented by the scause
and sie CSRs is extremely simple and implemented directly in
arch/riscv/kernel/irq.c.  In addition there is a irqchip driver
for the PLIC external interrupt controller, which is called through
the set_handle_irq API, and a clocksource driver that gets its
timer interrupt directly from the low-level interrupt handling.

Compared to previous iterations this version does not try to use an
irqchip driver for the low-level interrupt handling.  This saves
a couple indirect calls and an additional read of the scause CSR
in the hot path, makes the code much simpler and last but not least
avoid the dependency on a device tree for a mandatory architectural
feature.

A git tree is available here (contains a few more patches before
the ones in this series)

git://git.infradead.org/users/hch/riscv.git riscv-irq-simple.3

Gitweb:


http://git.infradead.org/users/hch/riscv.git/shortlog/refs/heads/riscv-irq-simple.3

Changes since v2:
 - actually use SEIE instead of STIE in the plic driver
 - rename the default compat string for the plic to sifive,u5-plic
 - various spelling fixes
 - drop a superflous derefence in the plic driver that is taken care of
   by the following loop
 - drop the patch to document the enable method - not relevant for the
   rest of the series
 - drop the patches for the per-hart timebase frequency - not relevant
   for the rest of the series.
 - use riscv_of_processor_hart in the timer driver

Changes since v1:
 - rename the plic driver to irq-sifive-plic
 - switch to a default compatible of sifive,plic0 (still supporting the
   riscv,plic0 name for compatibility)
 - add a reference for the SiFive PLIC register layout
 - fix plic_toggle addressing for large numbers of hwirqs
 - remove the call to ack_bad_irq
 - use a raw spinlock for plic_toggle_lock
 - use the irq_desc cpumask in the plic enable/disable methods
 - add back OF contexid parsing in the plic driver
 - don't allow COMPILE_TEST builds of the clocksource driver, as it
   depends on 
 - default the clocksource driver to y
 - clean up naming in the clocksource driver
 - remove the MINDELTA and MAXDELTA #defines
 - various DT binding fixes


Thanks!  Modulo the one device tree issue I replied to in patch 3 this looks 
great!  We've already gotten the ACKs to take this through the RISC-V tree, so 
I'm going to put this along with the queued RISC-V patches on our for-next 
branch, including my proposed change for "sifive,plic-1.0" but leaving the 
device tree bindings with #{address,size}-cells=1.


We can always change this, but I'd like to get this out so people can start 
playing with it earlier rather than later.


Thanks to everyone for all the help!

Re: [PATCH 03/11] dt-bindings: interrupt-controller: RISC-V PLIC documentation

[Palmer Dabbelt]

On Mon, 06 Aug 2018 13:59:48 PDT (-0700), robh...@kernel.org wrote:

On Thu, Aug 2, 2018 at 4:08 PM Atish Patra  wrote:


On 8/2/18 4:50 AM, Christoph Hellwig wrote:
> From: Palmer Dabbelt 
>
> This patch adds documentation for the platform-level interrupt
> controller (PLIC) found in all RISC-V systems.  This interrupt
> controller routes interrupts from all the devices in the system to each
> hart-local interrupt controller.
>
> Note: the DTS bindings for the PLIC aren't set in stone yet, as we might
> want to change how we're specifying holes in the hart list.
>
> Signed-off-by: Palmer Dabbelt 
> [hch: various fixes and updates]
> Signed-off-by: Christoph Hellwig 
> ---
>   .../interrupt-controller/sifive,plic0.txt | 57 +++
>   1 file changed, 57 insertions(+)
>   create mode 100644 
Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
>
> diff --git 
a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt 
b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
> new file mode 100644
> index ..c756cd208a93
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
> @@ -0,0 +1,57 @@
> +SiFive Platform-Level Interrupt Controller (PLIC)
> +-
> +
> +SiFive SOCs include an implementation of the Platform-Level Interrupt 
Controller
> +(PLIC) high-level specification in the RISC-V Privileged Architecture
> +specification.  The PLIC connects all external interrupts in the system to 
all
> +hart contexts in the system, via the external interrupt source in each hart.
> +
> +A hart context is a privilege mode in a hardware execution thread.  For 
example,
> +in an 4 core system with 2-way SMT, you have 8 harts and probably at least 
two
> +privilege modes per hart; machine mode and supervisor mode.
> +
> +Each interrupt can be enabled on per-context basis. Any context can claim
> +a pending enabled interrupt and then release it once it has been handled.
> +
> +Each interrupt has a configurable priority. Higher priority interrupts are
> +serviced first. Each context can specify a priority threshold. Interrupts
> +with priority below this threshold will not cause the PLIC to raise its
> +interrupt line leading to the context.
> +
> +While the PLIC supports both edge-triggered and level-triggered interrupts,
> +interrupt handlers are oblivious to this distinction and therefore it is not
> +specified in the PLIC device-tree binding.
> +
> +While the RISC-V ISA doesn't specify a memory layout for the PLIC, the
> +"sifive,plic0" device is a concrete implementation of the PLIC that contains 
a
> +specific memory layout, which is documented in chapter 8 of the SiFive U5
> +Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>.
> +
> +Required properties:
> +- compatible : "sifive,plic0"


I think there was a thread bouncing around somewhere where decided to pick the 
official name of the compatible string to be "sifive,plic-1.0".  The idea here 
is that the PLIC is compatible across all of SiFive's current implementations, 
but there's some limitations in the memory map that will probably cause us to 
spin a version 2 at some point so we want major version number.  The minor 
number is just in case we find some errata in the PLIC.



> +- #address-cells : should be <0>
> +- #interrupt-cells : should be <1>
> +- interrupt-controller : Identifies the node as an interrupt controller
> +- reg : Should contain 1 register range (address and length)

The one in the real device tree has two entries.
reg = <0x 0x0c00 0x 0x0400>;

Is it intentional or just incorrect entry left over from earlier days?



> + reg = <0xc00 0x400>;


Looks to me like one has #size-cells and #address-cells set to 2 and
the example is using 1.


Yes.  For some background on how this works: we have a hardware generator that 
has a tree-of-busses abstraction, and each device is attached to some point on 
that tree.  When a device gets attached to the bus, we also generate a device 
tree entry.  For whatever system I generated the example PLIC device tree entry 
from, it must have been attached to a bus with addresses of 32 bits or less, 
which resulted in #address-cells and #size-cells being 1.


Christoph has a HiFive Unleashed, which has a fu540-c000 on it, which is 
probably not what I generated as an example -- the fu540-c000 is a complicated 
configuration, when I mess around with the hardware I tend to use simple ones 
as I'm not really a hardware guy.  I suppose the bus that the PLIC is hanging 
off on the fu540-c000 has addresses wider than 32 bits.  This makes sense, as 
the

Re: [PATCH 03/11] dt-bindings: interrupt-controller: RISC-V PLIC documentation

[Palmer Dabbelt]

On Mon, 06 Aug 2018 13:59:48 PDT (-0700), robh...@kernel.org wrote:

On Thu, Aug 2, 2018 at 4:08 PM Atish Patra  wrote:


On 8/2/18 4:50 AM, Christoph Hellwig wrote:
> From: Palmer Dabbelt 
>
> This patch adds documentation for the platform-level interrupt
> controller (PLIC) found in all RISC-V systems.  This interrupt
> controller routes interrupts from all the devices in the system to each
> hart-local interrupt controller.
>
> Note: the DTS bindings for the PLIC aren't set in stone yet, as we might
> want to change how we're specifying holes in the hart list.
>
> Signed-off-by: Palmer Dabbelt 
> [hch: various fixes and updates]
> Signed-off-by: Christoph Hellwig 
> ---
>   .../interrupt-controller/sifive,plic0.txt | 57 +++
>   1 file changed, 57 insertions(+)
>   create mode 100644 
Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
>
> diff --git 
a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt 
b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
> new file mode 100644
> index ..c756cd208a93
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
> @@ -0,0 +1,57 @@
> +SiFive Platform-Level Interrupt Controller (PLIC)
> +-
> +
> +SiFive SOCs include an implementation of the Platform-Level Interrupt 
Controller
> +(PLIC) high-level specification in the RISC-V Privileged Architecture
> +specification.  The PLIC connects all external interrupts in the system to 
all
> +hart contexts in the system, via the external interrupt source in each hart.
> +
> +A hart context is a privilege mode in a hardware execution thread.  For 
example,
> +in an 4 core system with 2-way SMT, you have 8 harts and probably at least 
two
> +privilege modes per hart; machine mode and supervisor mode.
> +
> +Each interrupt can be enabled on per-context basis. Any context can claim
> +a pending enabled interrupt and then release it once it has been handled.
> +
> +Each interrupt has a configurable priority. Higher priority interrupts are
> +serviced first. Each context can specify a priority threshold. Interrupts
> +with priority below this threshold will not cause the PLIC to raise its
> +interrupt line leading to the context.
> +
> +While the PLIC supports both edge-triggered and level-triggered interrupts,
> +interrupt handlers are oblivious to this distinction and therefore it is not
> +specified in the PLIC device-tree binding.
> +
> +While the RISC-V ISA doesn't specify a memory layout for the PLIC, the
> +"sifive,plic0" device is a concrete implementation of the PLIC that contains 
a
> +specific memory layout, which is documented in chapter 8 of the SiFive U5
> +Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>.
> +
> +Required properties:
> +- compatible : "sifive,plic0"


I think there was a thread bouncing around somewhere where decided to pick the 
official name of the compatible string to be "sifive,plic-1.0".  The idea here 
is that the PLIC is compatible across all of SiFive's current implementations, 
but there's some limitations in the memory map that will probably cause us to 
spin a version 2 at some point so we want major version number.  The minor 
number is just in case we find some errata in the PLIC.



> +- #address-cells : should be <0>
> +- #interrupt-cells : should be <1>
> +- interrupt-controller : Identifies the node as an interrupt controller
> +- reg : Should contain 1 register range (address and length)

The one in the real device tree has two entries.
reg = <0x 0x0c00 0x 0x0400>;

Is it intentional or just incorrect entry left over from earlier days?



> + reg = <0xc00 0x400>;


Looks to me like one has #size-cells and #address-cells set to 2 and
the example is using 1.


Yes.  For some background on how this works: we have a hardware generator that 
has a tree-of-busses abstraction, and each device is attached to some point on 
that tree.  When a device gets attached to the bus, we also generate a device 
tree entry.  For whatever system I generated the example PLIC device tree entry 
from, it must have been attached to a bus with addresses of 32 bits or less, 
which resulted in #address-cells and #size-cells being 1.


Christoph has a HiFive Unleashed, which has a fu540-c000 on it, which is 
probably not what I generated as an example -- the fu540-c000 is a complicated 
configuration, when I mess around with the hardware I tend to use simple ones 
as I'm not really a hardware guy.  I suppose the bus that the PLIC is hanging 
off on the fu540-c000 has addresses wider than 32 bits.  This makes sense, as 
the

[PATCH v2 2/2] spi-nor: add support for is25wp256

[Palmer Dabbelt]

From: "Wesley W. Terpstra" 

This is used of the HiFive Unleashed development board, and follows the
pattern of similar ISSI devices already listed.

Signed-off-by: Wesley W. Terpstra 
Signed-off-by: Palmer Dabbelt 
---
 drivers/mtd/spi-nor/spi-nor.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index aab93463a5e7..f10017b4543d 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1072,6 +1072,8 @@ static const struct flash_info spi_nor_ids[] = {
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
{ "is25wp128",  INFO(0x9d7018, 0, 64 * 1024, 256,
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+   { "is25wp256",  INFO(0x9d7019, 0, 64 * 1024, 512,
+   SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 
/* Macronix */
{ "mx25l512e",   INFO(0xc22010, 0, 64 * 1024,   1, SECT_4K) },
-- 
2.16.4

[PATCH v2 1/2] spi-nor: add support for ISSI's block unlocking scheme

[Palmer Dabbelt]

From: "Wesley W. Terpstra" 

ISSI uses a non-standard scheme to control block protection, with bit 5
of the status registerr controlling an additional block protection bit.
This patch disables all the block protection bits whenever an ISSI chip
is seen.

We might also want to trigger an error when writing SR_TB to these
chips, as it aliases with this extra protection bit in the status
register.  It looks like that's always conditional on SNOR_F_HAS_SR_TB,
so at least what's there is safe.

Signed-off-by: Wesley W. Terpstra 
Signed-off-by: Palmer Dabbelt 
---
 drivers/mtd/spi-nor/spi-nor.c | 43 ++-
 include/linux/mtd/spi-nor.h   |  2 ++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index d9c368c44194..aab93463a5e7 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1515,6 +1515,44 @@ static int macronix_quad_enable(struct spi_nor *nor)
return 0;
 }
 
+/**
+ * issi_unlock() - clear BP[0123] write-protection.
+ * @nor:   pointer to a 'struct spi_nor'
+ *
+ * Bits [2345] of the Status Register are BP[0123].
+ * ISSI chips use a different block protection scheme than other chips.
+ * Just disable the write-protect unilaterally.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int issi_unlock(struct spi_nor *nor)
+{
+   int ret, val;
+   u8 mask = SR_BP0 | SR_BP1 | SR_BP2 | SR_BP3;
+
+   val = read_sr(nor);
+   if (val < 0)
+   return val;
+   if (!(val & mask))
+   return 0;
+
+   write_enable(nor);
+
+   write_sr(nor, val & ~mask);
+
+   ret = spi_nor_wait_till_ready(nor);
+   if (ret)
+   return ret;
+
+   ret = read_sr(nor);
+   if (ret > 0 && !(ret & mask)) {
+   return 0;
+   } else {
+   dev_err(nor->dev, "ISSI Block Protection Bits not cleared\n");
+   return -EINVAL;
+   }
+}
+
 /*
  * Write status Register and configuration register with 2 bytes
  * The first byte will be written to the status register, while the
@@ -2747,6 +2785,9 @@ static int spi_nor_init(struct spi_nor *nor)
spi_nor_wait_till_ready(nor);
}
 
+   if (JEDEC_MFR(nor->info) == SNOR_MFR_ISSI)
+   issi_unlock(nor);
+
if (nor->quad_enable) {
err = nor->quad_enable(nor);
if (err) {
@@ -2926,7 +2967,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
if (ret)
return ret;
 
-   if (nor->addr_width) {
+   if (nor->addr_width && JEDEC_MFR(info) != SNOR_MFR_ISSI) {
/* already configured from SFDP */
} else if (info->addr_width) {
nor->addr_width = info->addr_width;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index e60da0d34cc1..da422a37d383 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -23,6 +23,7 @@
 #define SNOR_MFR_ATMEL CFI_MFR_ATMEL
 #define SNOR_MFR_GIGADEVICE0xc8
 #define SNOR_MFR_INTEL CFI_MFR_INTEL
+#define SNOR_MFR_ISSI  0x9d
 #define SNOR_MFR_MICRONCFI_MFR_ST /* ST Micro <--> Micron */
 #define SNOR_MFR_MACRONIX  CFI_MFR_MACRONIX
 #define SNOR_MFR_SPANSION  CFI_MFR_AMD
@@ -121,6 +122,7 @@
 #define SR_BP0 BIT(2)  /* Block protect 0 */
 #define SR_BP1 BIT(3)  /* Block protect 1 */
 #define SR_BP2 BIT(4)  /* Block protect 2 */
+#define SR_BP3 BIT(5)  /* Block protect 3 (on ISSI chips) */
 #define SR_TB  BIT(5)  /* Top/Bottom protect */
 #define SR_SRWDBIT(7)  /* SR write protect */
 /* Spansion/Cypress specific status bits */
-- 
2.16.4

[PATCH v2 2/2] spi-nor: add support for is25wp256

[Palmer Dabbelt]

From: "Wesley W. Terpstra" 

This is used of the HiFive Unleashed development board, and follows the
pattern of similar ISSI devices already listed.

Signed-off-by: Wesley W. Terpstra 
Signed-off-by: Palmer Dabbelt 
---
 drivers/mtd/spi-nor/spi-nor.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index aab93463a5e7..f10017b4543d 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1072,6 +1072,8 @@ static const struct flash_info spi_nor_ids[] = {
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
{ "is25wp128",  INFO(0x9d7018, 0, 64 * 1024, 256,
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+   { "is25wp256",  INFO(0x9d7019, 0, 64 * 1024, 512,
+   SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 
/* Macronix */
{ "mx25l512e",   INFO(0xc22010, 0, 64 * 1024,   1, SECT_4K) },
-- 
2.16.4

[PATCH v2 1/2] spi-nor: add support for ISSI's block unlocking scheme

[Palmer Dabbelt]

From: "Wesley W. Terpstra" 

ISSI uses a non-standard scheme to control block protection, with bit 5
of the status registerr controlling an additional block protection bit.
This patch disables all the block protection bits whenever an ISSI chip
is seen.

We might also want to trigger an error when writing SR_TB to these
chips, as it aliases with this extra protection bit in the status
register.  It looks like that's always conditional on SNOR_F_HAS_SR_TB,
so at least what's there is safe.

Signed-off-by: Wesley W. Terpstra 
Signed-off-by: Palmer Dabbelt 
---
 drivers/mtd/spi-nor/spi-nor.c | 43 ++-
 include/linux/mtd/spi-nor.h   |  2 ++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index d9c368c44194..aab93463a5e7 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1515,6 +1515,44 @@ static int macronix_quad_enable(struct spi_nor *nor)
return 0;
 }
 
+/**
+ * issi_unlock() - clear BP[0123] write-protection.
+ * @nor:   pointer to a 'struct spi_nor'
+ *
+ * Bits [2345] of the Status Register are BP[0123].
+ * ISSI chips use a different block protection scheme than other chips.
+ * Just disable the write-protect unilaterally.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int issi_unlock(struct spi_nor *nor)
+{
+   int ret, val;
+   u8 mask = SR_BP0 | SR_BP1 | SR_BP2 | SR_BP3;
+
+   val = read_sr(nor);
+   if (val < 0)
+   return val;
+   if (!(val & mask))
+   return 0;
+
+   write_enable(nor);
+
+   write_sr(nor, val & ~mask);
+
+   ret = spi_nor_wait_till_ready(nor);
+   if (ret)
+   return ret;
+
+   ret = read_sr(nor);
+   if (ret > 0 && !(ret & mask)) {
+   return 0;
+   } else {
+   dev_err(nor->dev, "ISSI Block Protection Bits not cleared\n");
+   return -EINVAL;
+   }
+}
+
 /*
  * Write status Register and configuration register with 2 bytes
  * The first byte will be written to the status register, while the
@@ -2747,6 +2785,9 @@ static int spi_nor_init(struct spi_nor *nor)
spi_nor_wait_till_ready(nor);
}
 
+   if (JEDEC_MFR(nor->info) == SNOR_MFR_ISSI)
+   issi_unlock(nor);
+
if (nor->quad_enable) {
err = nor->quad_enable(nor);
if (err) {
@@ -2926,7 +2967,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
if (ret)
return ret;
 
-   if (nor->addr_width) {
+   if (nor->addr_width && JEDEC_MFR(info) != SNOR_MFR_ISSI) {
/* already configured from SFDP */
} else if (info->addr_width) {
nor->addr_width = info->addr_width;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index e60da0d34cc1..da422a37d383 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -23,6 +23,7 @@
 #define SNOR_MFR_ATMEL CFI_MFR_ATMEL
 #define SNOR_MFR_GIGADEVICE0xc8
 #define SNOR_MFR_INTEL CFI_MFR_INTEL
+#define SNOR_MFR_ISSI  0x9d
 #define SNOR_MFR_MICRONCFI_MFR_ST /* ST Micro <--> Micron */
 #define SNOR_MFR_MACRONIX  CFI_MFR_MACRONIX
 #define SNOR_MFR_SPANSION  CFI_MFR_AMD
@@ -121,6 +122,7 @@
 #define SR_BP0 BIT(2)  /* Block protect 0 */
 #define SR_BP1 BIT(3)  /* Block protect 1 */
 #define SR_BP2 BIT(4)  /* Block protect 2 */
+#define SR_BP3 BIT(5)  /* Block protect 3 (on ISSI chips) */
 #define SR_TB  BIT(5)  /* Top/Bottom protect */
 #define SR_SRWDBIT(7)  /* SR write protect */
 /* Spansion/Cypress specific status bits */
-- 
2.16.4

[PATCH v2 0/2] spi-nor: add support for is25wp256

[Palmer Dabbelt]

This adds support for the is25wp256 flash chip, which is on our HiFive
Unleashed board.  Additionally it adds support for ISSI's special
unlocking scheme, which we need to unlock block protection on the whole
chip.

Changes since v1 [<20180804014947.24601-1-pal...@sifive.com>]:
* There are now two patches, as it's logically two separate changes.
* The chip is called is25wp256 instead of is25wp256d, with the
  corresponding commit text also renamed.
* The block size has been changed to match the other similar chips that
  are already supported.
* SPI_NOR_4B_OPCODES is no longer set, to match the other similar chips
  that are are already supported.

[PATCH v2 1/2] spi-nor: add support for ISSI's block unlocking scheme
[PATCH v2 2/2] spi-nor: add support for is25wp256

[PATCH v2 0/2] spi-nor: add support for is25wp256

[Palmer Dabbelt]

This adds support for the is25wp256 flash chip, which is on our HiFive
Unleashed board.  Additionally it adds support for ISSI's special
unlocking scheme, which we need to unlock block protection on the whole
chip.

Changes since v1 [<20180804014947.24601-1-pal...@sifive.com>]:
* There are now two patches, as it's logically two separate changes.
* The chip is called is25wp256 instead of is25wp256d, with the
  corresponding commit text also renamed.
* The block size has been changed to match the other similar chips that
  are already supported.
* SPI_NOR_4B_OPCODES is no longer set, to match the other similar chips
  that are are already supported.

[PATCH v2 1/2] spi-nor: add support for ISSI's block unlocking scheme
[PATCH v2 2/2] spi-nor: add support for is25wp256

Re: [PATCH v2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h

[Palmer Dabbelt]

On Mon, 06 Aug 2018 14:00:53 PDT (-0700), rdun...@infradead.org wrote:

On 08/06/2018 01:42 PM, Palmer Dabbelt wrote:

This file is expected to be included multiple times in the same file in
order to allow the __SYSCALL macro to generate system call tables.  With
a global include guard we end up missing __NR_riscv_flush_icache in the
syscall table, which results in icache flushes that escape the vDSO call
to not actually do anything.

The fix is to move to per-#define include guards, which allows the
system call tables to actually be populated.  Thanks to Macrus Comstedt
for finding and fixing the bug!

I also went ahead and fixed the SPDX header to use a //-style comment,
which I've been told is the canonical way to do it.

Cc: Marcus Comstedt 
Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/include/asm/unistd.h|  5 +
 arch/riscv/include/uapi/asm/syscalls.h | 15 +--
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/include/asm/unistd.h b/arch/riscv/include/asm/unistd.h
index 080fb28061de..508be1780323 100644
--- a/arch/riscv/include/asm/unistd.h
+++ b/arch/riscv/include/asm/unistd.h
@@ -11,6 +11,11 @@
  *   GNU General Public License for more details.
  */

+/*
+ * There is explicitly no include guard here because this file is expected to


to .. be .. included


Thanks.  I'm not going to spin a v3 unless there's more feedback, but I'll 
include it in the PR.





+ * included multiple times.  See uapi/asm/syscalls.h for more info.
+ */
+
 #define __ARCH_WANT_SYS_CLONE
 #include 
 #include 
diff --git a/arch/riscv/include/uapi/asm/syscalls.h 
b/arch/riscv/include/uapi/asm/syscalls.h
index 818655b0d535..690beb002d1d 100644
--- a/arch/riscv/include/uapi/asm/syscalls.h
+++ b/arch/riscv/include/uapi/asm/syscalls.h
@@ -1,10 +1,13 @@
-/* SPDX-License-Identifier: GPL-2.0 */
+// SPDX-License-Identifier: GPL-2.0
 /*
- * Copyright (C) 2017 SiFive
+ * Copyright (C) 2017-2018 SiFive
  */

-#ifndef _ASM__UAPI__SYSCALLS_H
-#define _ASM__UAPI__SYSCALLS_H
+/*
+ * There is explicitly no include guard here because this file is expected to
+ * be included multiple times in order to define the syscall macros via


like that one :)


+ * __SYSCALL.
+ */

 /*
  * Allows the instruction cache to be flushed from userspace.  Despite RISC-V

Re: [PATCH] spi-nor: add support for is25wp256d

[Palmer Dabbelt]

On Mon, 06 Aug 2018 14:05:11 PDT (-0700), marek.va...@gmail.com wrote:

On 08/06/2018 10:58 PM, Palmer Dabbelt wrote:

On Sat, 04 Aug 2018 02:27:54 PDT (-0700), marek.va...@gmail.com wrote:

On 08/04/2018 03:49 AM, Palmer Dabbelt wrote:

From: "Wesley W. Terpstra" 

This is used of the HiFive Unleashed development board.

Signed-off-by: Wesley W. Terpstra 
Signed-off-by: Palmer Dabbelt 
---
 drivers/mtd/spi-nor/spi-nor.c | 47
++-
 include/linux/mtd/spi-nor.h   |  2 ++
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c
b/drivers/mtd/spi-nor/spi-nor.c
index d9c368c44194..e9a3557a3c23 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1072,6 +1072,9 @@ static const struct flash_info spi_nor_ids[] = {
 SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 { "is25wp128",  INFO(0x9d7018, 0, 64 * 1024, 256,
 SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+    { "is25wp256d", INFO(0x9d7019, 0, 32 * 1024, 1024,


Is there a reason for the trailing 'd' in is25wp256d ? I'd drop it.


I'm honestly not sure.  There are data sheets for both of them, but I
don't see much of a difference

   http://www.issi.com/WW/pdf/IS25LP(WP)256D.pdf
   http://www.issi.com/WW/pdf/25LP-WP256.pdf

Following the pattern, I'd expect to see

   { "is25wp256",  INFO(0x9d7019, 0, 64 * 1024, 512,
   SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },

versus

   { "is25wp256d", INFO(0x9d7019, 0, 32 * 1024, 1024,
   SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
SPI_NOR_4B_OPCODES)
   },


They have the same ID ? Do we support the variant without the d already?


Sorry for being a bit vague there.  There is no is25wp256 in the list already, 
but if I follow the pattern from the other similar chips I get a different 
value for is25wp256 than what was proposed in the patch for an is25wp256d.  
From my understanding the different values are just because we picked a 
different block size, which seems possible because the original version of this 
patch was written before the other is25wp devices were added to the list.


What I'm proposing is adding an is25wp256 with the same block size as the other 
similar entries in the list.  Looking at the data sheets they appear to have 
the same values for the "Read Product Identification" instruction.


The data sheets are shared with the is25lp256, which there is an entry for and 
matches my expected ID and block sizes.



So in other words: the d less sections that are larger, and also has the
4B opcodes flag set.  From the documentation in looks like the non-d
version supports 3 and 4 byte opcodes, so I guess it's just a different
physical layout?

In the data sheet for both I see

   "Pages can be erased in groups of 4Kbyte sectors, 32Kbyte blocks,
64Kbyte    blocks, and/or the entire chip"

which indicates to me that maybe we've just selected the larger section
size?  If so then I'll change it to the first one in the new patch.


+    SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ
| SPI_NOR_4B_OPCODES)
+    },

 /* Macronix */
 { "mx25l512e",   INFO(0xc22010, 0, 64 * 1024,   1, SECT_4K) },
@@ -1515,6 +1518,45 @@ static int macronix_quad_enable(struct spi_nor
*nor)
 return 0;
 }

+/**
+ * issi_unlock() - clear BP[0123] write-protection.
+ * @nor:    pointer to a 'struct spi_nor'
+ *
+ * Bits [2345] of the Status Register are BP[0123].
+ * ISSI chips use a different block protection scheme than other chips.
+ * Just disable the write-protect unilaterally.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int issi_unlock(struct spi_nor *nor)
+{
+    int ret, val;
+    u8 mask = SR_BP0 | SR_BP1 | SR_BP2 | SR_BP3;
+
+    val = read_sr(nor);
+    if (val < 0)
+    return val;
+    if (!(val & mask))
+    return 0;
+
+    write_enable(nor);
+
+    write_sr(nor, val & ~mask);
+
+    ret = spi_nor_wait_till_ready(nor);
+    if (ret)
+    return ret;
+
+    ret = read_sr(nor);
+    if (ret > 0 && !(ret & mask)) {
+    dev_info(nor->dev, "ISSI Block Protection Bits cleared\n");
+    return 0;


Is the dev_info() really needed ?


Nope.  I'll spin a v2 pending the above discussion.


+    } else {
+    dev_err(nor->dev, "ISSI Block Protection Bits not cleared\n");
+    return -EINVAL;
+    }
+}


[...]


Thanks!

Re: [PATCH v2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h

[Palmer Dabbelt]

On Mon, 06 Aug 2018 14:00:53 PDT (-0700), rdun...@infradead.org wrote:

On 08/06/2018 01:42 PM, Palmer Dabbelt wrote:

This file is expected to be included multiple times in the same file in
order to allow the __SYSCALL macro to generate system call tables.  With
a global include guard we end up missing __NR_riscv_flush_icache in the
syscall table, which results in icache flushes that escape the vDSO call
to not actually do anything.

The fix is to move to per-#define include guards, which allows the
system call tables to actually be populated.  Thanks to Macrus Comstedt
for finding and fixing the bug!

I also went ahead and fixed the SPDX header to use a //-style comment,
which I've been told is the canonical way to do it.

Cc: Marcus Comstedt 
Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/include/asm/unistd.h|  5 +
 arch/riscv/include/uapi/asm/syscalls.h | 15 +--
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/include/asm/unistd.h b/arch/riscv/include/asm/unistd.h
index 080fb28061de..508be1780323 100644
--- a/arch/riscv/include/asm/unistd.h
+++ b/arch/riscv/include/asm/unistd.h
@@ -11,6 +11,11 @@
  *   GNU General Public License for more details.
  */

+/*
+ * There is explicitly no include guard here because this file is expected to


to .. be .. included


Thanks.  I'm not going to spin a v3 unless there's more feedback, but I'll 
include it in the PR.





+ * included multiple times.  See uapi/asm/syscalls.h for more info.
+ */
+
 #define __ARCH_WANT_SYS_CLONE
 #include 
 #include 
diff --git a/arch/riscv/include/uapi/asm/syscalls.h 
b/arch/riscv/include/uapi/asm/syscalls.h
index 818655b0d535..690beb002d1d 100644
--- a/arch/riscv/include/uapi/asm/syscalls.h
+++ b/arch/riscv/include/uapi/asm/syscalls.h
@@ -1,10 +1,13 @@
-/* SPDX-License-Identifier: GPL-2.0 */
+// SPDX-License-Identifier: GPL-2.0
 /*
- * Copyright (C) 2017 SiFive
+ * Copyright (C) 2017-2018 SiFive
  */

-#ifndef _ASM__UAPI__SYSCALLS_H
-#define _ASM__UAPI__SYSCALLS_H
+/*
+ * There is explicitly no include guard here because this file is expected to
+ * be included multiple times in order to define the syscall macros via


like that one :)


+ * __SYSCALL.
+ */

 /*
  * Allows the instruction cache to be flushed from userspace.  Despite RISC-V

Re: [PATCH] spi-nor: add support for is25wp256d

[Palmer Dabbelt]

On Mon, 06 Aug 2018 14:05:11 PDT (-0700), marek.va...@gmail.com wrote:

On 08/06/2018 10:58 PM, Palmer Dabbelt wrote:

On Sat, 04 Aug 2018 02:27:54 PDT (-0700), marek.va...@gmail.com wrote:

On 08/04/2018 03:49 AM, Palmer Dabbelt wrote:

From: "Wesley W. Terpstra" 

This is used of the HiFive Unleashed development board.

Signed-off-by: Wesley W. Terpstra 
Signed-off-by: Palmer Dabbelt 
---
 drivers/mtd/spi-nor/spi-nor.c | 47
++-
 include/linux/mtd/spi-nor.h   |  2 ++
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c
b/drivers/mtd/spi-nor/spi-nor.c
index d9c368c44194..e9a3557a3c23 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1072,6 +1072,9 @@ static const struct flash_info spi_nor_ids[] = {
 SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 { "is25wp128",  INFO(0x9d7018, 0, 64 * 1024, 256,
 SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+    { "is25wp256d", INFO(0x9d7019, 0, 32 * 1024, 1024,


Is there a reason for the trailing 'd' in is25wp256d ? I'd drop it.


I'm honestly not sure.  There are data sheets for both of them, but I
don't see much of a difference

   http://www.issi.com/WW/pdf/IS25LP(WP)256D.pdf
   http://www.issi.com/WW/pdf/25LP-WP256.pdf

Following the pattern, I'd expect to see

   { "is25wp256",  INFO(0x9d7019, 0, 64 * 1024, 512,
   SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },

versus

   { "is25wp256d", INFO(0x9d7019, 0, 32 * 1024, 1024,
   SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
SPI_NOR_4B_OPCODES)
   },


They have the same ID ? Do we support the variant without the d already?


Sorry for being a bit vague there.  There is no is25wp256 in the list already, 
but if I follow the pattern from the other similar chips I get a different 
value for is25wp256 than what was proposed in the patch for an is25wp256d.  
From my understanding the different values are just because we picked a 
different block size, which seems possible because the original version of this 
patch was written before the other is25wp devices were added to the list.


What I'm proposing is adding an is25wp256 with the same block size as the other 
similar entries in the list.  Looking at the data sheets they appear to have 
the same values for the "Read Product Identification" instruction.


The data sheets are shared with the is25lp256, which there is an entry for and 
matches my expected ID and block sizes.



So in other words: the d less sections that are larger, and also has the
4B opcodes flag set.  From the documentation in looks like the non-d
version supports 3 and 4 byte opcodes, so I guess it's just a different
physical layout?

In the data sheet for both I see

   "Pages can be erased in groups of 4Kbyte sectors, 32Kbyte blocks,
64Kbyte    blocks, and/or the entire chip"

which indicates to me that maybe we've just selected the larger section
size?  If so then I'll change it to the first one in the new patch.


+    SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ
| SPI_NOR_4B_OPCODES)
+    },

 /* Macronix */
 { "mx25l512e",   INFO(0xc22010, 0, 64 * 1024,   1, SECT_4K) },
@@ -1515,6 +1518,45 @@ static int macronix_quad_enable(struct spi_nor
*nor)
 return 0;
 }

+/**
+ * issi_unlock() - clear BP[0123] write-protection.
+ * @nor:    pointer to a 'struct spi_nor'
+ *
+ * Bits [2345] of the Status Register are BP[0123].
+ * ISSI chips use a different block protection scheme than other chips.
+ * Just disable the write-protect unilaterally.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int issi_unlock(struct spi_nor *nor)
+{
+    int ret, val;
+    u8 mask = SR_BP0 | SR_BP1 | SR_BP2 | SR_BP3;
+
+    val = read_sr(nor);
+    if (val < 0)
+    return val;
+    if (!(val & mask))
+    return 0;
+
+    write_enable(nor);
+
+    write_sr(nor, val & ~mask);
+
+    ret = spi_nor_wait_till_ready(nor);
+    if (ret)
+    return ret;
+
+    ret = read_sr(nor);
+    if (ret > 0 && !(ret & mask)) {
+    dev_info(nor->dev, "ISSI Block Protection Bits cleared\n");
+    return 0;


Is the dev_info() really needed ?


Nope.  I'll spin a v2 pending the above discussion.


+    } else {
+    dev_err(nor->dev, "ISSI Block Protection Bits not cleared\n");
+    return -EINVAL;
+    }
+}


[...]


Thanks!

Re: linux-next: Signed-off-by missing for commit in the risc-v tree

[Palmer Dabbelt]

On Sun, 05 Aug 2018 14:37:05 PDT (-0700), Stephen Rothwell wrote:

Hi Palmer,

Commit

  bce17edfe6af ("fixup: ".  " in PLIC docs")

is missing a Signed-off-by from its author and committer.


Oh, sorry about that.  It should be gone, it was just meant as a review 
comment.

Re: linux-next: Signed-off-by missing for commit in the risc-v tree

[Palmer Dabbelt]

On Sun, 05 Aug 2018 14:37:05 PDT (-0700), Stephen Rothwell wrote:

Hi Palmer,

Commit

  bce17edfe6af ("fixup: ".  " in PLIC docs")

is missing a Signed-off-by from its author and committer.


Oh, sorry about that.  It should be gone, it was just meant as a review 
comment.

[PATCH v2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h

[Palmer Dabbelt]

This file is expected to be included multiple times in the same file in
order to allow the __SYSCALL macro to generate system call tables.  With
a global include guard we end up missing __NR_riscv_flush_icache in the
syscall table, which results in icache flushes that escape the vDSO call
to not actually do anything.

The fix is to move to per-#define include guards, which allows the
system call tables to actually be populated.  Thanks to Macrus Comstedt
for finding and fixing the bug!

I also went ahead and fixed the SPDX header to use a //-style comment,
which I've been told is the canonical way to do it.

Cc: Marcus Comstedt 
Signed-off-by: Palmer Dabbelt 
---
 arch/riscv/include/asm/unistd.h|  5 +
 arch/riscv/include/uapi/asm/syscalls.h | 15 +--
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/include/asm/unistd.h b/arch/riscv/include/asm/unistd.h
index 080fb28061de..508be1780323 100644
--- a/arch/riscv/include/asm/unistd.h
+++ b/arch/riscv/include/asm/unistd.h
@@ -11,6 +11,11 @@
  *   GNU General Public License for more details.
  */
 
+/*
+ * There is explicitly no include guard here because this file is expected to
+ * included multiple times.  See uapi/asm/syscalls.h for more info.
+ */
+
 #define __ARCH_WANT_SYS_CLONE
 #include 
 #include 
diff --git a/arch/riscv/include/uapi/asm/syscalls.h 
b/arch/riscv/include/uapi/asm/syscalls.h
index 818655b0d535..690beb002d1d 100644
--- a/arch/riscv/include/uapi/asm/syscalls.h
+++ b/arch/riscv/include/uapi/asm/syscalls.h
@@ -1,10 +1,13 @@
-/* SPDX-License-Identifier: GPL-2.0 */
+// SPDX-License-Identifier: GPL-2.0
 /*
- * Copyright (C) 2017 SiFive
+ * Copyright (C) 2017-2018 SiFive
  */
 
-#ifndef _ASM__UAPI__SYSCALLS_H
-#define _ASM__UAPI__SYSCALLS_H
+/*
+ * There is explicitly no include guard here because this file is expected to
+ * be included multiple times in order to define the syscall macros via
+ * __SYSCALL.
+ */
 
 /*
  * Allows the instruction cache to be flushed from userspace.  Despite RISC-V
@@ -20,7 +23,7 @@
  * caller.  We don't currently do anything with the address range, that's just
  * in there for forwards compatibility.
  */
+#ifndef __NR_riscv_flush_icache
 #define __NR_riscv_flush_icache (__NR_arch_specific_syscall + 15)
-__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache)
-
 #endif
+__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache)
-- 
2.16.4