2022-09-06 10:46:40

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [RFC PATCH 0/2] AX45MP: Add support to non-coherent DMA

Hi All,

On the Andes AX45MP core, cache coherency is a specification option so it
may not be supported. In this case DMA will fail. To get around with this
issue this patch series does the below:

Andes AX45MP core has a Programmable Physical Memory Attributes (PMA)
block that allows dynamic adjustment of memory attributes in the runtime.
It contains a configurable amount of PMA entries implemented as CSR
registers to control the attributes of memory locations in interest. PMA
regions are passed from the cpu core node which are configured as
non-cacheable and non-bufferable with the SBI call.

ax45mp: cpu@0 {
compatible = "andestech,ax45mp", "riscv";
device_type = "cpu";
....
pma-regions = <0x0 0x00000000 0x0 0x14000000>,
<0x0 0x20000000 0x0 0x10000000>,
<0x0 0x58000000 0x0 0x08000000>;
....
};

We provide callbacks to synchronize specific content between memory and
cache. We allocate a global DMA coherent pool (which is marked as
non-cached using PMA) so that DMA memory allocations happens from this
pool and we implement the below callbacks:

- arch_sync_dma_for_device()
- arch_sync_dma_for_cpu()
- arch_dma_alloc()
- arch_dma_free()

Below are the configs that are enabled:

- DMA_GLOBAL_POOL
- ARCH_HAS_SYNC_DMA_FOR_CPU
- ARCH_HAS_SYNC_DMA_FOR_DEVICE

l2cache: cache-controller@13400000 {
compatible = "andestech,ax45mp-cache", "cache";
cache-size = <0x40000>;
cache-line-size = <64>;
cache-sets = <1024>;
cache-unified;
reg = <0x0 0x13400000 0x0 0x100000>;
};

Due to the above approach custom SBI calls have been implemented. The
above implementation is in preparation for adding support for Renesas
RZ/Five SoC which uses the AX45MP core. As with the above approach the
kernel image might not be generic so that it can be used on other
platforms, so sending it as an RFC (without DT binding patches).

OpenSBI implementation isn't upstreamed yet, public repo for access is
available at [0].

[0] https://github.com/renesas-rz/rz_opensbi/tree/work/OpenSBI-PMA

Cheers,
Prabhakar

Lad Prabhakar (2):
riscv: vendors: andes: Add support to configure the PMA regions
riscv: vendors: andes: Add support for non-cohernet dma

arch/riscv/Kbuild | 2 +
arch/riscv/include/asm/sbi.h | 1 +
arch/riscv/vendors/Makefile | 3 +
arch/riscv/vendors/andes/Makefile | 4 +
arch/riscv/vendors/andes/ax45mp.c | 93 ++++++
arch/riscv/vendors/andes/ax45mp_cache.c | 296 ++++++++++++++++++
arch/riscv/vendors/andes/ax45mp_nocache_dma.c | 65 ++++
arch/riscv/vendors/andes/include/proc.h | 9 +
arch/riscv/vendors/andes/include/sbi.h | 27 ++
9 files changed, 500 insertions(+)
create mode 100644 arch/riscv/vendors/Makefile
create mode 100644 arch/riscv/vendors/andes/Makefile
create mode 100644 arch/riscv/vendors/andes/ax45mp.c
create mode 100644 arch/riscv/vendors/andes/ax45mp_cache.c
create mode 100644 arch/riscv/vendors/andes/ax45mp_nocache_dma.c
create mode 100644 arch/riscv/vendors/andes/include/proc.h
create mode 100644 arch/riscv/vendors/andes/include/sbi.h

--
2.25.1


2022-09-06 10:49:29

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [RFC PATCH 2/2] riscv: vendors: andes: Add support for non-cohernet dma

On the AX45MP core, cache coherency is a specification option so it may
not be supported. In this case DMA will fail. As a workaround, firstly we
allocate a global dma coherent pool from which DMA allocations are taken
and marked as non-cacheable in the PMA region as specified in the device
tree. Synchronization callbacks are implemented to synchronize when doing
DMA transactions.

This feature is based on the work posted [0] by Vincent Chen
<[email protected]> for the Andes AndeStart RISC-V CPU.

[0] https://lore.kernel.org/lkml/[email protected]/

Signed-off-by: Lad Prabhakar <[email protected]>
---
arch/riscv/vendors/andes/Makefile | 1 +
arch/riscv/vendors/andes/ax45mp_cache.c | 296 ++++++++++++++++++
arch/riscv/vendors/andes/ax45mp_nocache_dma.c | 65 ++++
arch/riscv/vendors/andes/include/proc.h | 9 +
4 files changed, 371 insertions(+)
create mode 100644 arch/riscv/vendors/andes/ax45mp_cache.c
create mode 100644 arch/riscv/vendors/andes/ax45mp_nocache_dma.c
create mode 100644 arch/riscv/vendors/andes/include/proc.h

diff --git a/arch/riscv/vendors/andes/Makefile b/arch/riscv/vendors/andes/Makefile
index 60fa8226c4a3..f9ec92b92455 100644
--- a/arch/riscv/vendors/andes/Makefile
+++ b/arch/riscv/vendors/andes/Makefile
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0

obj-$(CONFIG_ARCH_R9A07G043) += ax45mp.o
+obj-$(CONFIG_ARCH_R9A07G043) += ax45mp_cache.o ax45mp_nocache_dma.o
diff --git a/arch/riscv/vendors/andes/ax45mp_cache.c b/arch/riscv/vendors/andes/ax45mp_cache.c
new file mode 100644
index 000000000000..6941821e7e4a
--- /dev/null
+++ b/arch/riscv/vendors/andes/ax45mp_cache.c
@@ -0,0 +1,296 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * non-coherent cache functions for AX45MP
+ *
+ * Copyright (C) 2022 Renesas Electronics Corp.
+ *
+ */
+
+#include <linux/cacheinfo.h>
+#include <linux/of_address.h>
+
+#include <asm/sbi.h>
+
+#include "include/sbi.h"
+
+/* D-cache operation */
+#define CCTL_L1D_VA_INVAL 0
+#define CCTL_L1D_VA_WB 1
+
+/* L2 cache */
+#define L2_CACHE_CTL_CEN_MASK 1
+
+/* L2 cache registers */
+#define L2C_REG_CTL_OFFSET 0x8
+#define L2C_REG_C0_CMD_OFFSET 0x40
+#define L2C_REG_C0_ACC_OFFSET 0x48
+#define L2C_REG_STATUS_OFFSET 0x80
+
+/* L2 CCTL status */
+#define CCTL_L2_STATUS_IDLE 0
+
+/* L2 CCTL status cores mask */
+#define CCTL_L2_STATUS_C0_MASK 0xf
+
+/* L2 cache operation */
+#define CCTL_L2_PA_INVAL 0x8
+#define CCTL_L2_PA_WB 0x9
+
+#define L2C_HPM_PER_CORE_OFFSET 0x8
+#define L2C_REG_PER_CORE_OFFSET 0x10
+#define CCTL_L2_STATUS_PER_CORE_OFFSET 4
+
+#define L2C_REG_CN_CMD_OFFSET(n) \
+ (L2C_REG_C0_CMD_OFFSET + ((n) * L2C_REG_PER_CORE_OFFSET))
+#define L2C_REG_CN_ACC_OFFSET(n) \
+ (L2C_REG_C0_ACC_OFFSET + ((n) * L2C_REG_PER_CORE_OFFSET))
+#define CCTL_L2_STATUS_CN_MASK(n) \
+ (CCTL_L2_STATUS_C0_MASK << ((n) * CCTL_L2_STATUS_PER_CORE_OFFSET))
+
+#define MICM_CFG_ISZ_OFFSET 6
+#define MICM_CFG_ISZ_MASK (0x7 << MICM_CFG_ISZ_OFFSET)
+
+#define MDCM_CFG_DSZ_OFFSET 6
+#define MDCM_CFG_DSZ_MASK (0x7 << MDCM_CFG_DSZ_OFFSET)
+
+#define CCTL_REG_UCCTLBEGINADDR_NUM 0x80b
+#define CCTL_REG_UCCTLCOMMAND_NUM 0x80c
+
+#define MCACHE_CTL_CCTL_SUEN_OFFSET 8
+#define MMSC_CFG_CCTLCSR_OFFSET 16
+#define MISA_20_OFFSET 20
+
+#define MCACHE_CTL_CCTL_SUEN_MASK (0x1 << MCACHE_CTL_CCTL_SUEN_OFFSET)
+#define MMSC_CFG_CCTLCSR_MASK (0x1 << MMSC_CFG_CCTLCSR_OFFSET)
+#define MISA_20_MASK (0x1 << MISA_20_OFFSET)
+
+#define MAX_CACHE_LINE_SIZE 256
+
+struct ax45mp_cache_info {
+ bool init_done;
+ int dcache_line_size;
+};
+
+static DEFINE_PER_CPU(struct ax45mp_cache_info, cpu_cache_info) = {
+ .init_done = 0,
+ .dcache_line_size = SZ_64,
+};
+
+static void __iomem *l2c_base;
+
+static uint32_t cpu_get_mcache_ctl_status(void)
+{
+ struct sbiret ret;
+
+ ret = sbi_ecall(SBI_EXT_ANDES, SBI_EXT_ANDES_GET_MCACHE_CTL_STATUS, 0, 0, 0, 0, 0, 0);
+ return ret.value;
+}
+
+static uint32_t cpu_get_micm_cfg_status(void)
+{
+ struct sbiret ret;
+
+ ret = sbi_ecall(SBI_EXT_ANDES, SBI_EXT_ANDES_GET_MICM_CTL_STATUS, 0, 0, 0, 0, 0, 0);
+ return ret.value;
+}
+
+static uint32_t cpu_get_mdcm_cfg_status(void)
+{
+ struct sbiret ret;
+
+ ret = sbi_ecall(SBI_EXT_ANDES, SBI_EXT_ANDES_GET_MDCM_CTL_STATUS, 0, 0, 0, 0, 0, 0);
+ return ret.value;
+}
+
+static uint32_t cpu_get_mmsc_cfg_status(void)
+{
+ struct sbiret ret;
+
+ ret = sbi_ecall(SBI_EXT_ANDES, SBI_EXT_ANDES_GET_MMSC_CTL_STATUS, 0, 0, 0, 0, 0, 0);
+ return ret.value;
+}
+
+static uint32_t cpu_get_misa_cfg_status(void)
+{
+ struct sbiret ret;
+
+ ret = sbi_ecall(SBI_EXT_ANDES, SBI_EXT_ANDES_GET_MISA_CTL_STATUS, 0, 0, 0, 0, 0, 0);
+ return ret.value;
+}
+
+static void fill_cpu_cache_info(struct ax45mp_cache_info *cpu_ci)
+{
+ struct cpu_cacheinfo *this_cpu_ci =
+ get_cpu_cacheinfo(smp_processor_id());
+ struct cacheinfo *this_leaf = this_cpu_ci->info_list;
+ unsigned int i;
+
+ for (i = 0; i < this_cpu_ci->num_leaves ; i++, this_leaf++) {
+ if (this_leaf->type == CACHE_TYPE_DATA)
+ cpu_ci->dcache_line_size = this_leaf->coherency_line_size;
+ }
+
+ cpu_ci->init_done = true;
+}
+
+static inline int get_cache_line_size(void)
+{
+ struct ax45mp_cache_info *cpu_ci =
+ &per_cpu(cpu_cache_info, smp_processor_id());
+
+ if (unlikely(!cpu_ci->init_done))
+ fill_cpu_cache_info(cpu_ci);
+ return cpu_ci->dcache_line_size;
+}
+
+static uint32_t cpu_l2c_get_cctl_status(void)
+{
+ return readl((void *)(l2c_base + L2C_REG_STATUS_OFFSET));
+}
+
+static uint32_t cpu_l2c_ctl_status(void)
+{
+ return readl((void *)(l2c_base + L2C_REG_CTL_OFFSET));
+}
+
+static bool cpu_cache_controlable(void)
+{
+ return (((cpu_get_micm_cfg_status() & MICM_CFG_ISZ_MASK) ||
+ (cpu_get_mdcm_cfg_status() & MDCM_CFG_DSZ_MASK)) &&
+ (cpu_get_misa_cfg_status() & MISA_20_MASK) &&
+ (cpu_get_mmsc_cfg_status() & MMSC_CFG_CCTLCSR_MASK) &&
+ (cpu_get_mcache_ctl_status() & MCACHE_CTL_CCTL_SUEN_MASK));
+}
+
+static void cpu_dcache_wb_range(unsigned long start,
+ unsigned long end,
+ int line_size)
+{
+ bool ucctl_ok = false;
+ unsigned long pa;
+ int mhartid = 0;
+#ifdef CONFIG_SMP
+ mhartid = smp_processor_id();
+#endif
+
+ ucctl_ok = cpu_cache_controlable();
+
+ while (end > start) {
+ if (ucctl_ok) {
+ csr_write(CCTL_REG_UCCTLBEGINADDR_NUM, start);
+ csr_write(CCTL_REG_UCCTLCOMMAND_NUM, CCTL_L1D_VA_WB);
+ }
+
+ if (l2c_base && (cpu_l2c_ctl_status() & L2_CACHE_CTL_CEN_MASK)) {
+ pa = virt_to_phys((void *)start);
+ writel(pa, (void *)(l2c_base + L2C_REG_CN_ACC_OFFSET(mhartid)));
+ writel(CCTL_L2_PA_WB, (void *)(l2c_base + L2C_REG_CN_CMD_OFFSET(mhartid)));
+ while ((cpu_l2c_get_cctl_status() &
+ CCTL_L2_STATUS_CN_MASK(mhartid)) != CCTL_L2_STATUS_IDLE)
+ ;
+ }
+
+ start += line_size;
+ }
+}
+
+static void cpu_dcache_inval_range(unsigned long start,
+ unsigned long end,
+ int line_size)
+{
+ bool ucctl_ok = false;
+ unsigned long pa;
+ int mhartid = 0;
+#ifdef CONFIG_SMP
+ mhartid = smp_processor_id();
+#endif
+
+ ucctl_ok = cpu_cache_controlable();
+
+ while (end > start) {
+ if (ucctl_ok) {
+ csr_write(CCTL_REG_UCCTLBEGINADDR_NUM, start);
+ csr_write(CCTL_REG_UCCTLCOMMAND_NUM, CCTL_L1D_VA_INVAL);
+ }
+
+ if (l2c_base && (cpu_l2c_ctl_status() & L2_CACHE_CTL_CEN_MASK)) {
+ pa = virt_to_phys((void *)start);
+ writel(pa, (void *)(l2c_base + L2C_REG_CN_ACC_OFFSET(mhartid)));
+ writel(CCTL_L2_PA_INVAL,
+ (void *)(l2c_base + L2C_REG_CN_CMD_OFFSET(mhartid)));
+ while ((cpu_l2c_get_cctl_status() &
+ CCTL_L2_STATUS_CN_MASK(mhartid)) != CCTL_L2_STATUS_IDLE)
+ ;
+ }
+
+ start += line_size;
+ }
+}
+
+void cpu_dma_inval_range(unsigned long start, unsigned long end)
+{
+ unsigned long line_size = get_cache_line_size();
+ char cache_buf[2][MAX_CACHE_LINE_SIZE] = { 0 };
+ unsigned long old_start = start;
+ unsigned long old_end = end;
+ unsigned long flags;
+
+ if (unlikely(start == end))
+ return;
+
+ start = start & (~(line_size - 1));
+ end = ((end + line_size - 1) & (~(line_size - 1)));
+
+ local_irq_save(flags);
+ if (unlikely(start != old_start))
+ memcpy(&cache_buf[0][0], (void *)start, line_size);
+
+ if (unlikely(end != old_end))
+ memcpy(&cache_buf[1][0], (void *)(old_end & (~(line_size - 1))), line_size);
+
+ cpu_dcache_inval_range(start, end, line_size);
+
+ if (unlikely(start != old_start))
+ memcpy((void *)start, &cache_buf[0][0], (old_start & (line_size - 1)));
+
+ if (unlikely(end != old_end))
+ memcpy((void *)(old_end + 1),
+ &cache_buf[1][(old_end & (line_size - 1)) + 1],
+ end - old_end - 1);
+
+ local_irq_restore(flags);
+}
+EXPORT_SYMBOL(cpu_dma_inval_range);
+
+void cpu_dma_wb_range(unsigned long start, unsigned long end)
+{
+ unsigned long flags;
+ unsigned long line_size = get_cache_line_size();
+
+ local_irq_save(flags);
+ start = start & (~(line_size - 1));
+ cpu_dcache_wb_range(start, end, line_size);
+ local_irq_restore(flags);
+}
+EXPORT_SYMBOL(cpu_dma_wb_range);
+
+static const struct of_device_id ax45mp_cache_ids[] = {
+ { .compatible = "andestech,ax45mp-cache" },
+ { /* sentinel */ }
+};
+
+static int __init ax45mp_cache_init(void)
+{
+ struct device_node *np;
+
+ np = of_find_matching_node(NULL, ax45mp_cache_ids);
+ if (!np)
+ return -ENODEV;
+
+ l2c_base = of_iomap(np, 0);
+ if (!l2c_base)
+ return -ENOMEM;
+
+ return 0;
+}
+arch_initcall(ax45mp_cache_init);
diff --git a/arch/riscv/vendors/andes/ax45mp_nocache_dma.c b/arch/riscv/vendors/andes/ax45mp_nocache_dma.c
new file mode 100644
index 000000000000..263012f89d0b
--- /dev/null
+++ b/arch/riscv/vendors/andes/ax45mp_nocache_dma.c
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * DMA callbacks implemented for the non-coherent cache
+ * on AX45MP
+ *
+ * Copyright (C) 2022 Renesas Electronics Corp.
+ *
+ */
+
+#include <linux/dma-mapping.h>
+#include <linux/dma-direct.h>
+
+#include "include/proc.h"
+
+static inline void cache_op(phys_addr_t paddr, size_t size,
+ void (*fn)(unsigned long start, unsigned long end))
+{
+ unsigned long start;
+
+ start = (unsigned long)phys_to_virt(paddr);
+ fn(start, start + size);
+}
+
+void arch_sync_dma_for_device(phys_addr_t paddr,
+ size_t size, enum dma_data_direction dir)
+{
+ switch (dir) {
+ case DMA_FROM_DEVICE:
+ cache_op(paddr, size, cpu_dma_inval_range);
+ break;
+ case DMA_TO_DEVICE:
+ case DMA_BIDIRECTIONAL:
+ cache_op(paddr, size, cpu_dma_wb_range);
+ break;
+ default:
+ BUG();
+ }
+}
+
+void arch_sync_dma_for_cpu(phys_addr_t paddr,
+ size_t size, enum dma_data_direction dir)
+{
+ switch (dir) {
+ case DMA_TO_DEVICE:
+ break;
+ case DMA_FROM_DEVICE:
+ case DMA_BIDIRECTIONAL:
+ cache_op(paddr, size, cpu_dma_inval_range);
+ break;
+ default:
+ BUG();
+ }
+}
+
+void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
+ gfp_t gfp, unsigned long attrs)
+{
+ return dma_alloc_from_global_coherent(dev, size, handle);
+}
+
+void arch_dma_free(struct device *dev, size_t size, void *vaddr,
+ dma_addr_t handle, unsigned long attrs)
+{
+ dma_release_from_global_coherent(0, vaddr);
+}
diff --git a/arch/riscv/vendors/andes/include/proc.h b/arch/riscv/vendors/andes/include/proc.h
new file mode 100644
index 000000000000..ace9bcfccf6b
--- /dev/null
+++ b/arch/riscv/vendors/andes/include/proc.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef __RISCV_ANDES_PROC_H
+#define __RISCV_ANDES_PROC_H
+
+void cpu_dma_inval_range(unsigned long start, unsigned long end);
+void cpu_dma_wb_range(unsigned long start, unsigned long end);
+
+#endif
--
2.25.1

2022-09-06 10:50:57

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [RFC PATCH 1/2] riscv: vendors: andes: Add support to configure the PMA regions

The Andes AX45MP core has a Programmable Physical Memory Attributes (PMA)
block that allows dynamic adjustment of memory attributes in the runtime.
It contains a configurable amount of PMA entries implemented as CSR
registers to control the attributes of memory locations in interest.

Below are the memory attributes supported:
* Device, Non-bufferable
* Device, bufferable
* Memory, Non-cacheable, Non-bufferable
* Memory, Non-cacheable, Bufferable
* Memory, Write-back, No-allocate
* Memory, Write-back, Read-allocate
* Memory, Write-back, Write-allocate
* Memory, Write-back, Read and Write-allocate

This patch adds support to configure the memory attributes of the
memory regions as passed from the core node. Currently the OpenSBI code
implements support for "Memory, Non-cacheable, Non-bufferable" option
with SBI_EXT_ANDES_SET_PMA.

More info about PMA (section 10.3):
http://www.andestech.com/wp-content/uploads/AX45MP-1C-Rev.-5.0.0-Datasheet.pdf

Signed-off-by: Lad Prabhakar <[email protected]>
---
Note: the current implementation only supports "Memory, Non-cacheable,
Bufferable" option.
---
arch/riscv/Kbuild | 2 +
arch/riscv/include/asm/sbi.h | 1 +
arch/riscv/vendors/Makefile | 3 +
arch/riscv/vendors/andes/Makefile | 3 +
arch/riscv/vendors/andes/ax45mp.c | 93 ++++++++++++++++++++++++++
arch/riscv/vendors/andes/include/sbi.h | 27 ++++++++
6 files changed, 129 insertions(+)
create mode 100644 arch/riscv/vendors/Makefile
create mode 100644 arch/riscv/vendors/andes/Makefile
create mode 100644 arch/riscv/vendors/andes/ax45mp.c
create mode 100644 arch/riscv/vendors/andes/include/sbi.h

diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild
index afa83e307a2e..d6821f660fc3 100644
--- a/arch/riscv/Kbuild
+++ b/arch/riscv/Kbuild
@@ -5,6 +5,8 @@ obj-$(CONFIG_BUILTIN_DTB) += boot/dts/
obj-y += errata/
obj-$(CONFIG_KVM) += kvm/

+obj-y += vendors/
+
obj-$(CONFIG_ARCH_HAS_KEXEC_PURGATORY) += purgatory/

# for cleaning
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index 2a0ef738695e..10a7c855d125 100644
--- a/arch/riscv/include/asm/sbi.h
+++ b/arch/riscv/include/asm/sbi.h
@@ -37,6 +37,7 @@ enum sbi_ext_id {

/* Vendor extensions must lie within this range */
SBI_EXT_VENDOR_START = 0x09000000,
+ SBI_EXT_ANDES = 0x0900031E,
SBI_EXT_VENDOR_END = 0x09FFFFFF,
};

diff --git a/arch/riscv/vendors/Makefile b/arch/riscv/vendors/Makefile
new file mode 100644
index 000000000000..0a5a5541d2a3
--- /dev/null
+++ b/arch/riscv/vendors/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_ARCH_R9A07G043) += andes/
diff --git a/arch/riscv/vendors/andes/Makefile b/arch/riscv/vendors/andes/Makefile
new file mode 100644
index 000000000000..60fa8226c4a3
--- /dev/null
+++ b/arch/riscv/vendors/andes/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_ARCH_R9A07G043) += ax45mp.o
diff --git a/arch/riscv/vendors/andes/ax45mp.c b/arch/riscv/vendors/andes/ax45mp.c
new file mode 100644
index 000000000000..931cba754f41
--- /dev/null
+++ b/arch/riscv/vendors/andes/ax45mp.c
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AX45MP setup
+ * - PMA
+ *
+ * Copyright (C) 2022 Renesas Electronics Corp.
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/of.h>
+
+#include <asm/sbi.h>
+
+#include "include/sbi.h"
+
+#define ANDES_AX45MP_MAX_PMA_REGIONS 16
+
+struct pma_arg_t {
+ phys_addr_t offset;
+ unsigned long vaddr;
+ size_t size;
+ size_t entry_id;
+};
+
+static long sbi_set_pma(void *arg)
+{
+ phys_addr_t offset = ((struct pma_arg_t *)arg)->offset;
+ unsigned long vaddr = ((struct pma_arg_t *)arg)->vaddr;
+ size_t entry_id = ((struct pma_arg_t *)arg)->entry_id;
+ size_t size = ((struct pma_arg_t *)arg)->size;
+ struct sbiret ret;
+
+ ret = sbi_ecall(SBI_EXT_ANDES, SBI_EXT_ANDES_SET_PMA, offset, vaddr, size, entry_id, 0, 0);
+
+ return ret.value;
+}
+
+static unsigned long cpu_nocache_area_set(unsigned long start,
+ unsigned long size,
+ unsigned long entry_id)
+{
+ struct pma_arg_t pma_arg;
+ unsigned long ret = 0;
+
+ pma_arg.offset = start;
+ pma_arg.size = size;
+ pma_arg.vaddr = start + size;
+ pma_arg.entry_id = entry_id;
+ ret = sbi_set_pma(&pma_arg);
+
+ return ret;
+}
+
+static void ax45mp_configure_pma_regions(struct device_node *np, int count)
+{
+ u64 start, size;
+ unsigned int i;
+
+ for (i = 0 ; i < count ; i++) {
+ of_property_read_u64_index(np, "pma-regions", (i << 1), &start);
+ of_property_read_u64_index(np, "pma-regions", (i << 1) + 1, &size);
+ cpu_nocache_area_set(start, size, i);
+ }
+}
+
+static const struct of_device_id ax45mp_ids[] = {
+ { .compatible = "andestech,ax45mp" },
+ { /* sentinel */ }
+};
+
+static int __init ax45mp_init(void)
+{
+ struct device_node *np;
+ int count;
+
+ np = of_find_matching_node(NULL, ax45mp_ids);
+ if (!np)
+ return -ENODEV;
+
+ count = of_property_count_elems_of_size(np, "pma-regions",
+ sizeof(u32) * 4);
+ if (count < 0)
+ return 0;
+
+ if (count > ANDES_AX45MP_MAX_PMA_REGIONS)
+ return -EINVAL;
+
+ ax45mp_configure_pma_regions(np, count);
+
+ return 0;
+}
+arch_initcall(ax45mp_init);
diff --git a/arch/riscv/vendors/andes/include/sbi.h b/arch/riscv/vendors/andes/include/sbi.h
new file mode 100644
index 000000000000..6dcd215bb5f8
--- /dev/null
+++ b/arch/riscv/vendors/andes/include/sbi.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef __RISCV_ANDES_SBI_H
+#define __RISCV_ANDES_SBI_H
+
+enum sbi_ext_andes_fid {
+ SBI_EXT_ANDES_GET_MCACHE_CTL_STATUS = 0,
+ SBI_EXT_ANDES_GET_MMISC_CTL_STATUS,
+ SBI_EXT_ANDES_SET_MCACHE_CTL,
+ SBI_EXT_ANDES_SET_MMISC_CTL,
+ SBI_EXT_ANDES_ICACHE_OP,
+ SBI_EXT_ANDES_DCACHE_OP,
+ SBI_EXT_ANDES_L1CACHE_I_PREFETCH,
+ SBI_EXT_ANDES_L1CACHE_D_PREFETCH,
+ SBI_EXT_ANDES_NON_BLOCKING_LOAD_STORE,
+ SBI_EXT_ANDES_WRITE_AROUND,
+ SBI_EXT_ANDES_SET_PMA,
+ SBI_EXT_ANDES_FREE_PMA,
+ SBI_EXT_ANDES_PROBE_PMA,
+ SBI_EXT_ANDES_DCACHE_WBINVAL_ALL,
+ SBI_EXT_ANDES_GET_MICM_CTL_STATUS,
+ SBI_EXT_ANDES_GET_MDCM_CTL_STATUS,
+ SBI_EXT_ANDES_GET_MMSC_CTL_STATUS,
+ SBI_EXT_ANDES_GET_MISA_CTL_STATUS,
+};
+
+#endif
--
2.25.1

2022-09-08 00:28:18

by Conor Dooley

[permalink] [raw]
Subject: Re: [RFC PATCH 1/2] riscv: vendors: andes: Add support to configure the PMA regions

On 07/09/2022 22:52, Atish Patra wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
>
> On Tue, Sep 6, 2022 at 3:40 AM <[email protected]
> <mailto:[email protected]>> wrote:
>
> On 06/09/2022 11:21, Lad Prabhakar wrote:
>
>> diff --git a/arch/riscv/include/asm/sbi.h
>> b/arch/riscv/include/asm/sbi.h index 2a0ef738695e..10a7c855d125
>> 100644 --- a/arch/riscv/include/asm/sbi.h +++
>> b/arch/riscv/include/asm/sbi.h @@ -37,6 +37,7 @@ enum sbi_ext_id {
>>
>> /* Vendor extensions must lie within this range */
>> SBI_EXT_VENDOR_START = 0x09000000, + SBI_EXT_ANDES =
>> 0x0900031E, SBI_EXT_VENDOR_END = 0x09FFFFFF, };
>
> Everything else aside, I am very interested in what's happening here.
> I'll take a proper look through things later, but for now:
>
> For PolarFire SoC we have an InterHart Communication SBI EXT that
> would would like to upstream support for. We are not aiming to put
> the driver itself in arch/riscv - it's just a mailbox driver, but I
> would like to use sbi.h for defining the vendor id etc.
>
> I am not sure how this all aligns with:
>> We’ll only accept patches for new modules or extensions if the
>> specifications for those modules or extensions are listed as being
>> “Frozen” or “Ratified” by the RISC-V Foundation. (Developers may,
>> of course, maintain their own Linux kernel trees that contain code
>> for any draft extensions that they wish.)
>>
>> Additionally, the RISC-V specification allows implementors to
>> create their own custom extensions. These custom extensions aren’t
>> required to go through any review or ratification process by the
>> RISC-V Foundation. To avoid the maintenance complexity and
>> potential performance impact of adding kernel code for
>> implementor-specific RISC-V extensions, we’ll only to accept
>> patches for extensions that have been officially frozen or ratified
>> by the RISC-V Foundation. (Implementors, may, of course, maintain
>> their own Linux kernel trees containing code for any custom
>> extensions that they wish.)
>
> Which is in: https://docs.kernel.org/riscv/patch-acceptance.html
> <https://docs.kernel.org/riscv/patch-acceptance.html>
>
> It is unclear to me whether that is just for ISA extensions or if
> that covers SBI extensions too. At least for us, since we don't touch
> arch code there is relatively little friction & there's no concerns
> about reducing the portability of a kernel since it is just a regular
> old driver.
>
>
> It covers the standard SBI extensions as well. However, I don't think
> this includes a vendor extension as there is no freeze or
> ratification associated with vendor extensions.
>
> It would be good to discuss the policy around vendor SBI extensions
> during LPC as well. We also need to discuss the ACPI policy as well.
> We most likely need a BoF to discuss these adhoc topics. I will check
> if we can schedule a BoF in advance.

I did briefly mention this to Palmer on IRC last night, just was busy
today & didn't get a chance to reply here. The indication there was
that yes, this paragraph did cover SBI extensions - which would make
vendor extensions not permitted upstream.

We (microchip) are "only" doing a few ecalls in a driver but this
seems a fair bit more intrusive since it is in arch code. Even if the
answer is a "no" - a no from the horses mouth rather than on IRC &
maybe some rewording of that doc to be clearer would be nice.

I'd be down for a BoF, even if just to get a "no" in person haha

Conor.

>
> I was planning on cornering some people *cough* Palmer *cough* at LPC
> and asking him what his thoughts were there.
>
> FWIW this is what we have been doing:
> https://github.com/linux4microchip/linux/blob/linux-5.15-mchp/drivers/mailbox/mailbox-miv-ihc.c#L27
> <https://github.com/linux4microchip/linux/blob/linux-5.15-mchp/drivers/mailbox/mailbox-miv-ihc.c#L27>
>
> The IP itself has not stabilised yet, so we have not sent any
> patches yet, but we do intend doing so...
>
> But yea, I'll take a properly look at what you're doing here soonTM,
> although at this point it may be the other side of LPC.
>
> btw, where can I get my hands on your hardware?
>
> Thanks, Conor.
>
>
> _______________________________________________ linux-riscv mailing
> list [email protected]
> <mailto:[email protected]>
> http://lists.infradead.org/mailman/listinfo/linux-riscv
> <http://lists.infradead.org/mailman/listinfo/linux-riscv>
>
>
>
> -- Regards, Atish

2022-09-08 08:46:36

by Biju Das

[permalink] [raw]
Subject: RE: [RFC PATCH 1/2] riscv: vendors: andes: Add support to configure the PMA regions

Hi Conor, Atish,

What RISC-V devices you have?

Ours is RISC-V uniprocessor without IO Coherence Port.

Currently USB, ethernet, SDHI/eMMC doesn't work with standard
DMA api's.

On RISC-V world, how do we handle DMA api for uniprocessor without
IO Coherence Port?

Cheers,
Biju



> -----Original Message-----
> From: [email protected] <[email protected]>
> Sent: 08 September 2022 00:38
> To: [email protected]
> Cc: Prabhakar Mahadev Lad <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; Biju Das
> <[email protected]>
> Subject: Re: [RFC PATCH 1/2] riscv: vendors: andes: Add support to
> configure the PMA regions
>
> On 07/09/2022 22:52, Atish Patra wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know
> > the content is safe
> >
> >
> > On Tue, Sep 6, 2022 at 3:40 AM <[email protected]
> > <mailto:[email protected]>> wrote:
> >
> > On 06/09/2022 11:21, Lad Prabhakar wrote:
> >
> >> diff --git a/arch/riscv/include/asm/sbi.h
> >> b/arch/riscv/include/asm/sbi.h index 2a0ef738695e..10a7c855d125
> >> 100644 --- a/arch/riscv/include/asm/sbi.h +++
> >> b/arch/riscv/include/asm/sbi.h @@ -37,6 +37,7 @@ enum sbi_ext_id {
> >>
> >> /* Vendor extensions must lie within this range */
> >> SBI_EXT_VENDOR_START = 0x09000000, + SBI_EXT_ANDES =
> >> 0x0900031E, SBI_EXT_VENDOR_END = 0x09FFFFFF, };
> >
> > Everything else aside, I am very interested in what's happening here.
> > I'll take a proper look through things later, but for now:
> >
> > For PolarFire SoC we have an InterHart Communication SBI EXT that
> > would would like to upstream support for. We are not aiming to put the
> > driver itself in arch/riscv - it's just a mailbox driver, but I would
> > like to use sbi.h for defining the vendor id etc.
> >
> > I am not sure how this all aligns with:
> >> We'll only accept patches for new modules or extensions if the
> >> specifications for those modules or extensions are listed as being
> >> "Frozen" or "Ratified" by the RISC-V Foundation. (Developers may, of
> >> course, maintain their own Linux kernel trees that contain code for
> >> any draft extensions that they wish.)
> >>
> >> Additionally, the RISC-V specification allows implementors to create
> >> their own custom extensions. These custom extensions aren't required
> >> to go through any review or ratification process by the RISC-V
> >> Foundation. To avoid the maintenance complexity and potential
> >> performance impact of adding kernel code for implementor-specific
> >> RISC-V extensions, we'll only to accept patches for extensions that
> >> have been officially frozen or ratified by the RISC-V Foundation.
> >> (Implementors, may, of course, maintain their own Linux kernel trees
> >> containing code for any custom extensions that they wish.)
> >
> > Which is in:
> > https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs
> > .kernel.org%2Friscv%2Fpatch-acceptance.html&amp;data=05%7C01%7Cbiju.da
> > s.jz%40bp.renesas.com%7C7fd3275accdb450e547a08da912a0042%7C53d82571da1
> > 947e49cb4625a166a4a2a%7C0%7C0%7C637981906834865446%7CUnknown%7CTWFpbGZ
> > sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> > D%7C3000%7C%7C%7C&amp;sdata=YAV2Ahz7TFMJJ3wCj%2FAdDuDEcPq0kXXL%2B3o6FA
> > d0%2BUI%3D&amp;reserved=0
> > <https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdoc
> > s.kernel.org%2Friscv%2Fpatch-acceptance.html&amp;data=05%7C01%7Cbiju.d
> > as.jz%40bp.renesas.com%7C7fd3275accdb450e547a08da912a0042%7C53d82571da
> > 1947e49cb4625a166a4a2a%7C0%7C0%7C637981906834865446%7CUnknown%7CTWFpbG
> > Zsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%
> > 3D%7C3000%7C%7C%7C&amp;sdata=YAV2Ahz7TFMJJ3wCj%2FAdDuDEcPq0kXXL%2B3o6F
> > Ad0%2BUI%3D&amp;reserved=0>
> >
> > It is unclear to me whether that is just for ISA extensions or if that
> > covers SBI extensions too. At least for us, since we don't touch arch
> > code there is relatively little friction & there's no concerns about
> > reducing the portability of a kernel since it is just a regular old
> > driver.
> >
> >
> > It covers the standard SBI extensions as well. However, I don't think
> > this includes a vendor extension as there is no freeze or ratification
> > associated with vendor extensions.
> >
> > It would be good to discuss the policy around vendor SBI extensions
> > during LPC as well. We also need to discuss the ACPI policy as well.
> > We most likely need a BoF to discuss these adhoc topics. I will check
> > if we can schedule a BoF in advance.
>
> I did briefly mention this to Palmer on IRC last night, just was busy
> today & didn't get a chance to reply here. The indication there was that
> yes, this paragraph did cover SBI extensions - which would make vendor
> extensions not permitted upstream.
>
> We (microchip) are "only" doing a few ecalls in a driver but this seems a
> fair bit more intrusive since it is in arch code. Even if the answer is a
> "no" - a no from the horses mouth rather than on IRC & maybe some
> rewording of that doc to be clearer would be nice.
>
> I'd be down for a BoF, even if just to get a "no" in person haha
>
> Conor.
>
> >
> > I was planning on cornering some people *cough* Palmer *cough* at LPC
> > and asking him what his thoughts were there.
> >
> > FWIW this is what we have been doing:
> > https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> > ub.com%2Flinux4microchip%2Flinux%2Fblob%2Flinux-5.15-mchp%2Fdrivers%2F
> > mailbox%2Fmailbox-miv-ihc.c%23L27&amp;data=05%7C01%7Cbiju.das.jz%40bp.
> > renesas.com%7C7fd3275accdb450e547a08da912a0042%7C53d82571da1947e49cb46
> > 25a166a4a2a%7C0%7C0%7C637981906834865446%7CUnknown%7CTWFpbGZsb3d8eyJWI
> > joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7
> > C%7C%7C&amp;sdata=seNiuv6EsY1u0SIT33%2F0CWHJu401d5zSaNmVb%2BUHKPM%3D&a
> > mp;reserved=0
> > <https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
> > hub.com%2Flinux4microchip%2Flinux%2Fblob%2Flinux-5.15-mchp%2Fdrivers%2
> > Fmailbox%2Fmailbox-miv-ihc.c%23L27&amp;data=05%7C01%7Cbiju.das.jz%40bp
> > .renesas.com%7C7fd3275accdb450e547a08da912a0042%7C53d82571da1947e49cb4
> > 625a166a4a2a%7C0%7C0%7C637981906834865446%7CUnknown%7CTWFpbGZsb3d8eyJW
> > IjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%
> > 7C%7C%7C&amp;sdata=seNiuv6EsY1u0SIT33%2F0CWHJu401d5zSaNmVb%2BUHKPM%3D&
> > amp;reserved=0>
> >
> > The IP itself has not stabilised yet, so we have not sent any patches
> > yet, but we do intend doing so...
> >
> > But yea, I'll take a properly look at what you're doing here soonTM,
> > although at this point it may be the other side of LPC.
> >
> > btw, where can I get my hands on your hardware?
> >
> > Thanks, Conor.
> >
> >
> > _______________________________________________ linux-riscv mailing
> > list [email protected]
> > <mailto:[email protected]>
> > https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists
> > .infradead.org%2Fmailman%2Flistinfo%2Flinux-riscv&amp;data=05%7C01%7Cb
> > iju.das.jz%40bp.renesas.com%7C7fd3275accdb450e547a08da912a0042%7C53d82
> > 571da1947e49cb4625a166a4a2a%7C0%7C0%7C637981906834865446%7CUnknown%7CT
> > WFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI
> > 6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=AaMGs%2BqRpuw8MOWci3qPvJ1W5U296Vp%2B
> > higFxDSoHmo%3D&amp;reserved=0
> > <https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flist
> > s.infradead.org%2Fmailman%2Flistinfo%2Flinux-riscv&amp;data=05%7C01%7C
> > biju.das.jz%40bp.renesas.com%7C7fd3275accdb450e547a08da912a0042%7C53d8
> > 2571da1947e49cb4625a166a4a2a%7C0%7C0%7C637981906834865446%7CUnknown%7C
> > TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVC
> > I6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=AaMGs%2BqRpuw8MOWci3qPvJ1W5U296Vp%2
> > BhigFxDSoHmo%3D&amp;reserved=0>
> >
> >
> >
> > -- Regards, Atish

2022-09-08 11:58:38

by Prabhakar

[permalink] [raw]
Subject: Re: [RFC PATCH 1/2] riscv: vendors: andes: Add support to configure the PMA regions

Hi Guo,

> > -----Original Message-----
> > From: [email protected] <[email protected]>
> > Sent: 08 September 2022 00:38
> > To: [email protected]
> > Cc: Prabhakar Mahadev Lad <[email protected]>;
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected]; Biju Das
> > <[email protected]>
> > Subject: Re: [RFC PATCH 1/2] riscv: vendors: andes: Add support to
> > configure the PMA regions
> >
> > On 07/09/2022 22:52, Atish Patra wrote:
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you know
> > > the content is safe
> > >
> > >
> > > On Tue, Sep 6, 2022 at 3:40 AM <[email protected]
> > > <mailto:[email protected]>> wrote:
> > >
> > > On 06/09/2022 11:21, Lad Prabhakar wrote:
> > >
> > >> diff --git a/arch/riscv/include/asm/sbi.h
> > >> b/arch/riscv/include/asm/sbi.h index 2a0ef738695e..10a7c855d125
> > >> 100644 --- a/arch/riscv/include/asm/sbi.h +++
> > >> b/arch/riscv/include/asm/sbi.h @@ -37,6 +37,7 @@ enum sbi_ext_id {
> > >>
> > >> /* Vendor extensions must lie within this range */
> > >> SBI_EXT_VENDOR_START = 0x09000000, + SBI_EXT_ANDES =
> > >> 0x0900031E, SBI_EXT_VENDOR_END = 0x09FFFFFF, };
> > >
I am interested to know what is the status of your patch series [0].

[0] https://lore.kernel.org/lkml/[email protected]/T/#m7f4c4cdfb92d6c8672bbfabebda729ce4700e177

Cheers,
Prabhakar

2022-09-08 12:33:40

by Conor Dooley

[permalink] [raw]
Subject: Re: [RFC PATCH 1/2] riscv: vendors: andes: Add support to configure the PMA regions

On 08/09/2022 12:50, Lad, Prabhakar wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Hi Guo,
>
>>> -----Original Message-----
>>> From: [email protected] <[email protected]>
>>> Sent: 08 September 2022 00:38
>>> To: [email protected]
>>> Cc: Prabhakar Mahadev Lad <[email protected]>;
>>> [email protected]; [email protected]; [email protected];
>>> [email protected]; [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected]; Biju Das
>>> <[email protected]>
>>> Subject: Re: [RFC PATCH 1/2] riscv: vendors: andes: Add support to
>>> configure the PMA regions
>>>
>>> On 07/09/2022 22:52, Atish Patra wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>>>> the content is safe
>>>>
>>>>
>>>> On Tue, Sep 6, 2022 at 3:40 AM <[email protected]
>>>> <mailto:[email protected]>> wrote:
>>>>
>>>> On 06/09/2022 11:21, Lad Prabhakar wrote:
>>>>
>>>>> diff --git a/arch/riscv/include/asm/sbi.h
>>>>> b/arch/riscv/include/asm/sbi.h index 2a0ef738695e..10a7c855d125
>>>>> 100644 --- a/arch/riscv/include/asm/sbi.h +++
>>>>> b/arch/riscv/include/asm/sbi.h @@ -37,6 +37,7 @@ enum sbi_ext_id {
>>>>>
>>>>> /* Vendor extensions must lie within this range */
>>>>> SBI_EXT_VENDOR_START = 0x09000000, + SBI_EXT_ANDES =
>>>>> 0x0900031E, SBI_EXT_VENDOR_END = 0x09FFFFFF, };
>>>>
> I am interested to know what is the status of your patch series [0].
>
> [0] https://lore.kernel.org/lkml/[email protected]/T/#m7f4c4cdfb92d6c8672bbfabebda729ce4700e177

Heh, take a look at:
git grep "ERRATA_THEAD*"

This is all can-of-worms territory that we are heading to here, as
the Zicbo* extensions is what is meant to be doing these sorts of
things..


2022-09-08 13:07:12

by Prabhakar

[permalink] [raw]
Subject: Re: [RFC PATCH 1/2] riscv: vendors: andes: Add support to configure the PMA regions

On Thu, Sep 8, 2022 at 1:00 PM <[email protected]> wrote:
>
> On 08/09/2022 12:50, Lad, Prabhakar wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Hi Guo,
> >
> >>> -----Original Message-----
> >>> From: [email protected] <[email protected]>
> >>> Sent: 08 September 2022 00:38
> >>> To: [email protected]
> >>> Cc: Prabhakar Mahadev Lad <[email protected]>;
> >>> [email protected]; [email protected]; [email protected];
> >>> [email protected]; [email protected]; [email protected];
> >>> [email protected]; [email protected];
> >>> [email protected]; [email protected]; Biju Das
> >>> <[email protected]>
> >>> Subject: Re: [RFC PATCH 1/2] riscv: vendors: andes: Add support to
> >>> configure the PMA regions
> >>>
> >>> On 07/09/2022 22:52, Atish Patra wrote:
> >>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
> >>>> the content is safe
> >>>>
> >>>>
> >>>> On Tue, Sep 6, 2022 at 3:40 AM <[email protected]
> >>>> <mailto:[email protected]>> wrote:
> >>>>
> >>>> On 06/09/2022 11:21, Lad Prabhakar wrote:
> >>>>
> >>>>> diff --git a/arch/riscv/include/asm/sbi.h
> >>>>> b/arch/riscv/include/asm/sbi.h index 2a0ef738695e..10a7c855d125
> >>>>> 100644 --- a/arch/riscv/include/asm/sbi.h +++
> >>>>> b/arch/riscv/include/asm/sbi.h @@ -37,6 +37,7 @@ enum sbi_ext_id {
> >>>>>
> >>>>> /* Vendor extensions must lie within this range */
> >>>>> SBI_EXT_VENDOR_START = 0x09000000, + SBI_EXT_ANDES =
> >>>>> 0x0900031E, SBI_EXT_VENDOR_END = 0x09FFFFFF, };
> >>>>
> > I am interested to know what is the status of your patch series [0].
> >
> > [0] https://lore.kernel.org/lkml/[email protected]/T/#m7f4c4cdfb92d6c8672bbfabebda729ce4700e177
>
> Heh, take a look at:
> git grep "ERRATA_THEAD*"
>
> This is all can-of-worms territory that we are heading to here, as
> the Zicbo* extensions is what is meant to be doing these sorts of
> things..
>
Aha thanks for the pointer.

Cheers,
Prabhakar

2022-09-08 13:07:46

by Conor Dooley

[permalink] [raw]
Subject: Re: [RFC PATCH 1/2] riscv: vendors: andes: Add support to configure the PMA regions

On 08/09/2022 09:39, Biju Das wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Hi Conor, Atish,
>
> What RISC-V devices you have?

A bunch ;)

A __couple__ PolarFire SoC boards, HiFive Unleashed, D1 Nezha,
Canaan k210 MAIX something & the VisionFive.

> Ours is RISC-V uniprocessor without IO Coherence Port.

What does "IO Coherence Port" mean? Zicbo*?

> Currently USB, ethernet, SDHI/eMMC doesn't work with standard
> DMA api's.

Sounds pretty similar to the D1 so.

> On RISC-V world, how do we handle DMA api for uniprocessor without
> IO Coherence Port?

If you do mean Zicbo* you're into errata territory there & I don't
know if that'll be acceptable upstream - not for me to make that
call...

Thanks,
Conor.

>
> Cheers,
> Biju
>
>
>
>> -----Original Message-----
>> From: [email protected] <[email protected]>
>> Sent: 08 September 2022 00:38
>> To: [email protected]
>> Cc: Prabhakar Mahadev Lad <[email protected]>;
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected]; Biju Das
>> <[email protected]>
>> Subject: Re: [RFC PATCH 1/2] riscv: vendors: andes: Add support to
>> configure the PMA regions
>>
>> On 07/09/2022 22:52, Atish Patra wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>>> the content is safe
>>>
>>>
>>> On Tue, Sep 6, 2022 at 3:40 AM <[email protected]
>>> <mailto:[email protected]>> wrote:
>>>
>>> On 06/09/2022 11:21, Lad Prabhakar wrote:
>>>
>>>> diff --git a/arch/riscv/include/asm/sbi.h
>>>> b/arch/riscv/include/asm/sbi.h index 2a0ef738695e..10a7c855d125
>>>> 100644 --- a/arch/riscv/include/asm/sbi.h +++
>>>> b/arch/riscv/include/asm/sbi.h @@ -37,6 +37,7 @@ enum sbi_ext_id {
>>>>
>>>> /* Vendor extensions must lie within this range */
>>>> SBI_EXT_VENDOR_START = 0x09000000, + SBI_EXT_ANDES =
>>>> 0x0900031E, SBI_EXT_VENDOR_END = 0x09FFFFFF, };
>>>
>>> Everything else aside, I am very interested in what's happening here.
>>> I'll take a proper look through things later, but for now:
>>>
>>> For PolarFire SoC we have an InterHart Communication SBI EXT that
>>> would would like to upstream support for. We are not aiming to put the
>>> driver itself in arch/riscv - it's just a mailbox driver, but I would
>>> like to use sbi.h for defining the vendor id etc.
>>>
>>> I am not sure how this all aligns with:
>>>> We'll only accept patches for new modules or extensions if the
>>>> specifications for those modules or extensions are listed as being
>>>> "Frozen" or "Ratified" by the RISC-V Foundation. (Developers may, of
>>>> course, maintain their own Linux kernel trees that contain code for
>>>> any draft extensions that they wish.)
>>>>
>>>> Additionally, the RISC-V specification allows implementors to create
>>>> their own custom extensions. These custom extensions aren't required
>>>> to go through any review or ratification process by the RISC-V
>>>> Foundation. To avoid the maintenance complexity and potential
>>>> performance impact of adding kernel code for implementor-specific
>>>> RISC-V extensions, we'll only to accept patches for extensions that
>>>> have been officially frozen or ratified by the RISC-V Foundation.
>>>> (Implementors, may, of course, maintain their own Linux kernel trees
>>>> containing code for any custom extensions that they wish.)
>>>
>>> Which is in:
>>> https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs
>>> .kernel.org%2Friscv%2Fpatch-acceptance.html&amp;data=05%7C01%7Cbiju.da
>>> s.jz%40bp.renesas.com%7C7fd3275accdb450e547a08da912a0042%7C53d82571da1
>>> 947e49cb4625a166a4a2a%7C0%7C0%7C637981906834865446%7CUnknown%7CTWFpbGZ
>>> sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
>>> D%7C3000%7C%7C%7C&amp;sdata=YAV2Ahz7TFMJJ3wCj%2FAdDuDEcPq0kXXL%2B3o6FA
>>> d0%2BUI%3D&amp;reserved=0
>>> <https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdoc
>>> s.kernel.org%2Friscv%2Fpatch-acceptance.html&amp;data=05%7C01%7Cbiju.d
>>> as.jz%40bp.renesas.com%7C7fd3275accdb450e547a08da912a0042%7C53d82571da
>>> 1947e49cb4625a166a4a2a%7C0%7C0%7C637981906834865446%7CUnknown%7CTWFpbG
>>> Zsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%
>>> 3D%7C3000%7C%7C%7C&amp;sdata=YAV2Ahz7TFMJJ3wCj%2FAdDuDEcPq0kXXL%2B3o6F
>>> Ad0%2BUI%3D&amp;reserved=0>
>>>
>>> It is unclear to me whether that is just for ISA extensions or if that
>>> covers SBI extensions too. At least for us, since we don't touch arch
>>> code there is relatively little friction & there's no concerns about
>>> reducing the portability of a kernel since it is just a regular old
>>> driver.
>>>
>>>
>>> It covers the standard SBI extensions as well. However, I don't think
>>> this includes a vendor extension as there is no freeze or ratification
>>> associated with vendor extensions.
>>>
>>> It would be good to discuss the policy around vendor SBI extensions
>>> during LPC as well. We also need to discuss the ACPI policy as well.
>>> We most likely need a BoF to discuss these adhoc topics. I will check
>>> if we can schedule a BoF in advance.
>>
>> I did briefly mention this to Palmer on IRC last night, just was busy
>> today & didn't get a chance to reply here. The indication there was that
>> yes, this paragraph did cover SBI extensions - which would make vendor
>> extensions not permitted upstream.
>>
>> We (microchip) are "only" doing a few ecalls in a driver but this seems a
>> fair bit more intrusive since it is in arch code. Even if the answer is a
>> "no" - a no from the horses mouth rather than on IRC & maybe some
>> rewording of that doc to be clearer would be nice.
>>
>> I'd be down for a BoF, even if just to get a "no" in person haha
>>
>> Conor.
>>
>>>
>>> I was planning on cornering some people *cough* Palmer *cough* at LPC
>>> and asking him what his thoughts were there.
>>>
>>> FWIW this is what we have been doing:
>>> https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
>>> ub.com%2Flinux4microchip%2Flinux%2Fblob%2Flinux-5.15-mchp%2Fdrivers%2F
>>> mailbox%2Fmailbox-miv-ihc.c%23L27&amp;data=05%7C01%7Cbiju.das.jz%40bp.
>>> renesas.com%7C7fd3275accdb450e547a08da912a0042%7C53d82571da1947e49cb46
>>> 25a166a4a2a%7C0%7C0%7C637981906834865446%7CUnknown%7CTWFpbGZsb3d8eyJWI
>>> joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7
>>> C%7C%7C&amp;sdata=seNiuv6EsY1u0SIT33%2F0CWHJu401d5zSaNmVb%2BUHKPM%3D&a
>>> mp;reserved=0
>>> <https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
>>> hub.com%2Flinux4microchip%2Flinux%2Fblob%2Flinux-5.15-mchp%2Fdrivers%2
>>> Fmailbox%2Fmailbox-miv-ihc.c%23L27&amp;data=05%7C01%7Cbiju.das.jz%40bp
>>> .renesas.com%7C7fd3275accdb450e547a08da912a0042%7C53d82571da1947e49cb4
>>> 625a166a4a2a%7C0%7C0%7C637981906834865446%7CUnknown%7CTWFpbGZsb3d8eyJW
>>> IjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%
>>> 7C%7C%7C&amp;sdata=seNiuv6EsY1u0SIT33%2F0CWHJu401d5zSaNmVb%2BUHKPM%3D&
>>> amp;reserved=0>
>>>
>>> The IP itself has not stabilised yet, so we have not sent any patches
>>> yet, but we do intend doing so...
>>>
>>> But yea, I'll take a properly look at what you're doing here soonTM,
>>> although at this point it may be the other side of LPC.
>>>
>>> btw, where can I get my hands on your hardware?
>>>
>>> Thanks, Conor.
>>>
>>>
>>> _______________________________________________ linux-riscv mailing
>>> list [email protected]
>>> <mailto:[email protected]>
>>> https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists
>>> .infradead.org%2Fmailman%2Flistinfo%2Flinux-riscv&amp;data=05%7C01%7Cb
>>> iju.das.jz%40bp.renesas.com%7C7fd3275accdb450e547a08da912a0042%7C53d82
>>> 571da1947e49cb4625a166a4a2a%7C0%7C0%7C637981906834865446%7CUnknown%7CT
>>> WFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI
>>> 6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=AaMGs%2BqRpuw8MOWci3qPvJ1W5U296Vp%2B
>>> higFxDSoHmo%3D&amp;reserved=0
>>> <https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flist
>>> s.infradead.org%2Fmailman%2Flistinfo%2Flinux-riscv&amp;data=05%7C01%7C
>>> biju.das.jz%40bp.renesas.com%7C7fd3275accdb450e547a08da912a0042%7C53d8
>>> 2571da1947e49cb4625a166a4a2a%7C0%7C0%7C637981906834865446%7CUnknown%7C
>>> TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVC
>>> I6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=AaMGs%2BqRpuw8MOWci3qPvJ1W5U296Vp%2
>>> BhigFxDSoHmo%3D&amp;reserved=0>
>>>
>>>
>>>
>>> -- Regards, Atish
>

2022-09-08 13:30:40

by Biju Das

[permalink] [raw]
Subject: RE: [RFC PATCH 1/2] riscv: vendors: andes: Add support to configure the PMA regions

Hi Conor,

Thanks for the feedback.

> Subject: Re: [RFC PATCH 1/2] riscv: vendors: andes: Add support to
> configure the PMA regions
>
> On 08/09/2022 09:39, Biju Das wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know
> > the content is safe
> >
> > Hi Conor, Atish,
> >
> > What RISC-V devices you have?
>
> A bunch ;)
>
> A __couple__ PolarFire SoC boards, HiFive Unleashed, D1 Nezha, Canaan
> k210 MAIX something & the VisionFive.

If standard DMA api works without any issue means, on these platforms
IO Coherence port is enabled in the hardware. So all peripherals
involving DMA work as expected.

> > Ours is RISC-V uniprocessor without IO Coherence Port.
>
> What does "IO Coherence Port" mean? Zicbo*?

The HW will provide coherency between CPU and peripheral.

If Zibco* is uniprocessor, then highly it may not have IO coherence
Port enabled in their design.

Guo, Please confirm.

>
> > Currently USB, ethernet, SDHI/eMMC doesn't work with standard DMA
> > api's.
>
> Sounds pretty similar to the D1 so.
>
> > On RISC-V world, how do we handle DMA api for uniprocessor without IO
> > Coherence Port?
>
> If you do mean Zicbo* you're into errata territory there & I don't know
> if that'll be acceptable upstream - not for me to make that call...

It is not errata for sure. It is a HW design where we don't have
IO cache coherency port enabled in the HW. So looks like it is not
an extension or errata but it is core stuff.

Something similar to incoherency mentioned in below [1] for uniprocessor
Systems.
[1] https://elinux.org/images/8/80/Initializing-riscv.pdf

Cheers,
Biju

> >
> >
> >
> >> -----Original Message-----
> >> From: [email protected] <[email protected]>
> >> Sent: 08 September 2022 00:38
> >> To: [email protected]
> >> Cc: Prabhakar Mahadev Lad <[email protected]>;
> >> [email protected]; [email protected]; [email protected];
> >> [email protected]; [email protected];
> >> [email protected]; [email protected];
> >> [email protected];
> >> [email protected]; [email protected]; Biju Das
> >> <[email protected]>
> >> Subject: Re: [RFC PATCH 1/2] riscv: vendors: andes: Add support to
> >> configure the PMA regions
> >>
> >> On 07/09/2022 22:52, Atish Patra wrote:
> >>> EXTERNAL EMAIL: Do not click links or open attachments unless you
> >>> know the content is safe
> >>>
> >>>
> >>> On Tue, Sep 6, 2022 at 3:40 AM <[email protected]
> >>> <mailto:[email protected]>> wrote:
> >>>
> >>> On 06/09/2022 11:21, Lad Prabhakar wrote:
> >>>
> >>>> diff --git a/arch/riscv/include/asm/sbi.h
> >>>> b/arch/riscv/include/asm/sbi.h index 2a0ef738695e..10a7c855d125
> >>>> 100644 --- a/arch/riscv/include/asm/sbi.h +++
> >>>> b/arch/riscv/include/asm/sbi.h @@ -37,6 +37,7 @@ enum sbi_ext_id {
> >>>>
> >>>> /* Vendor extensions must lie within this range */
> >>>> SBI_EXT_VENDOR_START = 0x09000000, + SBI_EXT_ANDES =
> >>>> 0x0900031E, SBI_EXT_VENDOR_END = 0x09FFFFFF, };
> >>>
> >>> Everything else aside, I am very interested in what's happening here.
> >>> I'll take a proper look through things later, but for now:
> >>>
> >>> For PolarFire SoC we have an InterHart Communication SBI EXT that
> >>> would would like to upstream support for. We are not aiming to put
> >>> the driver itself in arch/riscv - it's just a mailbox driver, but I
> >>> would like to use sbi.h for defining the vendor id etc.
> >>>
> >>> I am not sure how this all aligns with:
> >>>> We'll only accept patches for new modules or extensions if the
> >>>> specifications for those modules or extensions are listed as being
> >>>> "Frozen" or "Ratified" by the RISC-V Foundation. (Developers may,
> >>>> of course, maintain their own Linux kernel trees that contain code
> >>>> for any draft extensions that they wish.)
> >>>>
> >>>> Additionally, the RISC-V specification allows implementors to
> >>>> create their own custom extensions. These custom extensions aren't
> >>>> required to go through any review or ratification process by the
> >>>> RISC-V Foundation. To avoid the maintenance complexity and
> >>>> potential performance impact of adding kernel code for
> >>>> implementor-specific RISC-V extensions, we'll only to accept
> >>>> patches for extensions that have been officially frozen or ratified
> by the RISC-V Foundation.
> >>>> (Implementors, may, of course, maintain their own Linux kernel
> >>>> trees containing code for any custom extensions that they wish.)
> >>>
> >>> Which is in:
> >>>
> >>> It is unclear to me whether that is just for ISA extensions or if
> >>> that covers SBI extensions too. At least for us, since we don't
> >>> touch arch code there is relatively little friction & there's no
> >>> concerns about reducing the portability of a kernel since it is just
> >>> a regular old driver.
> >>>
> >>>
> >>> It covers the standard SBI extensions as well. However, I don't
> >>> think this includes a vendor extension as there is no freeze or
> >>> ratification associated with vendor extensions.
> >>>
> >>> It would be good to discuss the policy around vendor SBI extensions
> >>> during LPC as well. We also need to discuss the ACPI policy as well.
> >>> We most likely need a BoF to discuss these adhoc topics. I will
> >>> check if we can schedule a BoF in advance.
> >>
> >> I did briefly mention this to Palmer on IRC last night, just was busy
> >> today & didn't get a chance to reply here. The indication there was
> >> that yes, this paragraph did cover SBI extensions - which would make
> >> vendor extensions not permitted upstream.
> >>
> >> We (microchip) are "only" doing a few ecalls in a driver but this
> >> seems a fair bit more intrusive since it is in arch code. Even if the
> >> answer is a "no" - a no from the horses mouth rather than on IRC &
> >> maybe some rewording of that doc to be clearer would be nice.
> >>
> >> I'd be down for a BoF, even if just to get a "no" in person haha
> >>
> >> Conor.
> >>
> >>>
> >>> I was planning on cornering some people *cough* Palmer *cough* at
> >>> LPC and asking him what his thoughts were there.
> >>>
> >>> FWIW this is what we have been doing:
> >>>
> >>> The IP itself has not stabilised yet, so we have not sent any
> >>> patches yet, but we do intend doing so...
> >>>
> >>> But yea, I'll take a properly look at what you're doing here soonTM,
> >>> although at this point it may be the other side of LPC.
> >>>
> >>> btw, where can I get my hands on your hardware?
> >>>
> >>> Thanks, Conor.
> >

2022-09-08 13:31:03

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [RFC PATCH 1/2] riscv: vendors: andes: Add support to configure the PMA regions

Hi Biju,

On Thu, Sep 8, 2022 at 3:01 PM Biju Das <[email protected]> wrote:
> > Subject: Re: [RFC PATCH 1/2] riscv: vendors: andes: Add support to
> > configure the PMA regions
> >
> > On 08/09/2022 09:39, Biju Das wrote:
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you know
> > > the content is safe
> > >
> > > Hi Conor, Atish,
> > >
> > > What RISC-V devices you have?
> >
> > A bunch ;)
> >
> > A __couple__ PolarFire SoC boards, HiFive Unleashed, D1 Nezha, Canaan
> > k210 MAIX something & the VisionFive.
>
> If standard DMA api works without any issue means, on these platforms
> IO Coherence port is enabled in the hardware. So all peripherals
> involving DMA work as expected.
>
> > > Ours is RISC-V uniprocessor without IO Coherence Port.
> >
> > What does "IO Coherence Port" mean? Zicbo*?
>
> The HW will provide coherency between CPU and peripheral.
>
> If Zibco* is uniprocessor, then highly it may not have IO coherence
> Port enabled in their design.

Zicbo* is a set of extensions for the instructions.
These cannot be retrofitted to existing silicon, but perhaps they can
be trapped and emulated?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2022-09-08 13:58:35

by Biju Das

[permalink] [raw]
Subject: RE: [RFC PATCH 1/2] riscv: vendors: andes: Add support to configure the PMA regions


Hi Geert,


> Subject: Re: [RFC PATCH 1/2] riscv: vendors: andes: Add support to
> configure the PMA regions
>
> Hi Biju,
>
> On Thu, Sep 8, 2022 at 3:01 PM Biju Das <[email protected]>
> wrote:
> > > Subject: Re: [RFC PATCH 1/2] riscv: vendors: andes: Add support to
> > > configure the PMA regions
> > >
> > > On 08/09/2022 09:39, Biju Das wrote:
> > > > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > > > know the content is safe
> > > >
> > > > Hi Conor, Atish,
> > > >
> > > > What RISC-V devices you have?
> > >
> > > A bunch ;)
> > >
> > > A __couple__ PolarFire SoC boards, HiFive Unleashed, D1 Nezha,
> > > Canaan
> > > k210 MAIX something & the VisionFive.
> >
> > If standard DMA api works without any issue means, on these platforms
> > IO Coherence port is enabled in the hardware. So all peripherals
> > involving DMA work as expected.
> >
> > > > Ours is RISC-V uniprocessor without IO Coherence Port.
> > >
> > > What does "IO Coherence Port" mean? Zicbo*?
> >
> > The HW will provide coherency between CPU and peripheral.
> >
> > If Zibco* is uniprocessor, then highly it may not have IO coherence
> > Port enabled in their design.
>
> Zicbo* is a set of extensions for the instructions.
> These cannot be retrofitted to existing silicon, but perhaps they can be
> trapped and emulated?

Thanks for clarifying this. I just gone through RISC-V CMOs.
https://github.com/riscv/riscv-CMOs/blob/master/specifications/cmobase-v1.0.1.pdf

Cheers,
Biju

2022-09-08 14:39:58

by Conor Dooley

[permalink] [raw]
Subject: Re: [RFC PATCH 1/2] riscv: vendors: andes: Add support to configure the PMA regions

On 08/09/2022 14:01, Biju Das wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Hi Conor,
>
> Thanks for the feedback.
>
>> Subject: Re: [RFC PATCH 1/2] riscv: vendors: andes: Add support to
>> configure the PMA regions
>>
>> On 08/09/2022 09:39, Biju Das wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>>> the content is safe
>>>
>>> Hi Conor, Atish,
>>>
>>> What RISC-V devices you have?
>>
>> A bunch ;)
>>
>> A __couple__ PolarFire SoC boards, HiFive Unleashed, D1 Nezha, Canaan
>> k210 MAIX something & the VisionFive.
>
> If standard DMA api works without any issue means, on these platforms
> IO Coherence port is enabled in the hardware. So all peripherals
> involving DMA work as expected.
>
>>> Ours is RISC-V uniprocessor without IO Coherence Port.
>>
>> What does "IO Coherence Port" mean? Zicbo*?
>
> The HW will provide coherency between CPU and peripheral.
>
> If Zibco* is uniprocessor, then highly it may not have IO coherence
> Port enabled in their design.

Zicbo* are cache management extensions as Geert pointed out.

>
> Guo, Please confirm.
>
>>
>>> Currently USB, ethernet, SDHI/eMMC doesn't work with standard DMA
>>> api's.
>>
>> Sounds pretty similar to the D1 so.
>>
>>> On RISC-V world, how do we handle DMA api for uniprocessor without IO
>>> Coherence Port?
>>
>> If you do mean Zicbo* you're into errata territory there & I don't know
>> if that'll be acceptable upstream - not for me to make that call...
>
> It is not errata for sure. It is a HW design where we don't have
> IO cache coherency port enabled in the HW. So looks like it is not
> an extension or errata but it is core stuff.

If you do non-coherent stuff that is not Zicbo*, the precedence set by
the D1 is errata. As I said to Prabhakar earlier, do a
`git grep "ERRATA_THEAD*`. I am not a maintainer so I don't know the
"rules" about doing cache management without the dedicated extensions
are.

Hope that at least helps,
Conor.


2022-09-08 14:44:42

by Prabhakar

[permalink] [raw]
Subject: Re: [RFC PATCH 1/2] riscv: vendors: andes: Add support to configure the PMA regions

On Thu, Sep 8, 2022 at 3:04 PM <[email protected]> wrote:
>
> On 08/09/2022 14:01, Biju Das wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Hi Conor,
> >
> > Thanks for the feedback.
> >
> >> Subject: Re: [RFC PATCH 1/2] riscv: vendors: andes: Add support to
> >> configure the PMA regions
> >>
> >> On 08/09/2022 09:39, Biju Das wrote:
> >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
> >>> the content is safe
> >>>
> >>> Hi Conor, Atish,
> >>>
> >>> What RISC-V devices you have?
> >>
> >> A bunch ;)
> >>
> >> A __couple__ PolarFire SoC boards, HiFive Unleashed, D1 Nezha, Canaan
> >> k210 MAIX something & the VisionFive.
> >
> > If standard DMA api works without any issue means, on these platforms
> > IO Coherence port is enabled in the hardware. So all peripherals
> > involving DMA work as expected.
> >
> >>> Ours is RISC-V uniprocessor without IO Coherence Port.
> >>
> >> What does "IO Coherence Port" mean? Zicbo*?
> >
> > The HW will provide coherency between CPU and peripheral.
> >
> > If Zibco* is uniprocessor, then highly it may not have IO coherence
> > Port enabled in their design.
>
> Zicbo* are cache management extensions as Geert pointed out.
>
> >
> > Guo, Please confirm.
> >
> >>
> >>> Currently USB, ethernet, SDHI/eMMC doesn't work with standard DMA
> >>> api's.
> >>
> >> Sounds pretty similar to the D1 so.
> >>
> >>> On RISC-V world, how do we handle DMA api for uniprocessor without IO
> >>> Coherence Port?
> >>
> >> If you do mean Zicbo* you're into errata territory there & I don't know
> >> if that'll be acceptable upstream - not for me to make that call...
> >
> > It is not errata for sure. It is a HW design where we don't have
> > IO cache coherency port enabled in the HW. So looks like it is not
> > an extension or errata but it is core stuff.
>
> If you do non-coherent stuff that is not Zicbo*, the precedence set by
> the D1 is errata. As I said to Prabhakar earlier, do a
> `git grep "ERRATA_THEAD*`. I am not a maintainer so I don't know the
> "rules" about doing cache management without the dedicated extensions
> are.
>
Maybe we could have a discussion about this topic at LPC too ;)

Cheers,
Prabhakar

2022-09-08 22:32:19

by Conor Dooley

[permalink] [raw]
Subject: Re: [RFC PATCH 0/2] AX45MP: Add support to non-coherent DMA

Hey,

I had a quick run through this today so if there's discussion
about this next week I at least will have some idea of what I
am talking about...

(I ended up not doing a quick run...)

On 06/09/2022 11:21, Lad Prabhakar wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Hi All,
>
> On the Andes AX45MP core, cache coherency is a specification option so it
> may not be supported. In this case DMA will fail. To get around with this
> issue this patch series does the below:

You say "may not be supported" - is it or is it not supported by the
core on your SoC? Do some of the cheaper SKUs not support it?

From what Biju has said, you need non-coherent DMA for your eMMC, USB
and ethernet controllers to work? To me, that seems like something that
would be quite important to point out here..


> Andes AX45MP core has a Programmable Physical Memory Attributes (PMA)
> block that allows dynamic adjustment of memory attributes in the runtime.
> It contains a configurable amount of PMA entries implemented as CSR
> registers to control the attributes of memory locations in interest. PMA
> regions are passed from the cpu core node which are configured as
> non-cacheable and non-bufferable with the SBI call.
>
> ax45mp: cpu@0 {
> compatible = "andestech,ax45mp", "riscv";
> device_type = "cpu";
> ....
> pma-regions = <0x0 0x00000000 0x0 0x14000000>,
> <0x0 0x20000000 0x0 0x10000000>,
> <0x0 0x58000000 0x0 0x08000000>;
> ....
> };
>
> We provide callbacks to synchronize specific content between memory and
> cache. We allocate a global DMA coherent pool (which is marked as
> non-cached using PMA) so that DMA memory allocations happens from this
> pool and we implement the below callbacks:
>
> - arch_sync_dma_for_device()
> - arch_sync_dma_for_cpu()

These two already exist in arch/riscv/mm/dma-noncoherent.c using the
alternatives mechanism.

> - arch_dma_alloc()
> - arch_dma_free()
>
> Below are the configs that are enabled:
>
> - DMA_GLOBAL_POOL
> - ARCH_HAS_SYNC_DMA_FOR_CPU
> - ARCH_HAS_SYNC_DMA_FOR_DEVICE

For these two see:
arch/riscv/Kconfig & RISCV_DMA_NONCOHERENT

>
> l2cache: cache-controller@13400000 {
> compatible = "andestech,ax45mp-cache", "cache";
> cache-size = <0x40000>;
> cache-line-size = <64>;
> cache-sets = <1024>;
> cache-unified;
> reg = <0x0 0x13400000 0x0 0x100000>;
> };
>
> Due to the above approach custom SBI calls have been implemented. The
> above implementation is in preparation for adding support for Renesas
> RZ/Five SoC which uses the AX45MP core. As with the above approach the
> kernel image might not be generic so that it can be used on other
> platforms, so sending it as an RFC (without DT binding patches).
>
> OpenSBI implementation isn't upstreamed yet, public repo for access is
> available at [0].

When you say "isn't upstreamed yet", what is the actual status? Where in
the process are you or have you not started that yet? Does openSBI even
allow custom extensions to be upstreamed?

>
> [0] https://github.com/renesas-rz/rz_opensbi/tree/work/OpenSBI-PMA
>
> Cheers,
> Prabhakar
>
> Lad Prabhakar (2):
> riscv: vendors: andes: Add support to configure the PMA regions
> riscv: vendors: andes: Add support for non-cohernet dma
>

Anyway, a couple of drive-by comments, having made the wild assumption
that this can be accepted upstream.

> arch/riscv/Kbuild | 2 +
> arch/riscv/include/asm/sbi.h | 1 +
> arch/riscv/vendors/Makefile | 3 +
> arch/riscv/vendors/andes/Makefile | 4 +
> arch/riscv/vendors/andes/ax45mp_cache.c | 296 ++++++++++++++++++

Surely this should be in drivers/soc/andestech, just like the SiFive L2
controller is in drivers/soc/sifive rather in a subdirectory of the
arch?

> arch/riscv/vendors/andes/ax45mp_nocache_dma.c | 65 ++++

This looks like it should be implemented as errata/alternatives just
like the non-coherent stuff on the D1 is done.

> arch/riscv/vendors/andes/include/proc.h | 9 +

And I think this would fall away if implemented as errata/alternatives.

> arch/riscv/vendors/andes/include/sbi.h | 27 ++
> arch/riscv/vendors/andes/ax45mp.c | 93 ++++++

idk where this would go though, if it is even something that is
acceptable, given the policy I linked the other day from:
https://www.kernel.org/doc/html/latest/riscv/patch-acceptance.html#submit-checklist-addendum

There is SiFive specific errata but it is implemented using mimpid etc
rather than compatible/dt. As I said in my initial mails, I am quite
interested in vendor SBI extensions in the kernel. If you did check out
the link I sent, our stuff is a world away from yours - it's isolated to
a driver where we are using SBI ECALLs to communicate with other harts
which are running something other than Linux in AMP configurations.
Pretty much we can do everything we want to do without touching a
single line of code in arch/riscv, so although the statement in that doc
applies to both of us here it does not apply evenly :s

It's all a bit unclear to me what the story is here, because obviously
you are doing things that Zicbo* is meant to do (just like the D1), but
your hardware's design and initial tapeout predates the existance of
Zicbom. Makes me start to wonder, what happens for <insert idea> that
eventually becomes an extension? Where does the line get draw for "you
did something that is not a ratified extension, therefore you are not
permitted upstream"? A line obviously does have to be drawn *somewhere*
and the easiest place to draw that line is "non ratified extensions are
a no-go". But then again, why allow the D1 but not you?

Obviously this is not a runner for someone not using an FPGA or similar,
but the InterHart Communication IP that we are using the SBI ECALLs for
is a fabric core, so we (in theory) could re-write it so that instead of
using an ECALL which routes communication via the e51 "monitor" core we
_could_ write directly to the registers of the IHC block. There's clear
security/isolation benefits for doing things via an ECALL which is why
that method was chosen but if we opened for the direct writes/reads the
driver would be upstream acceptable...

Don't get me wrong, I completely understand why a policy of not allowing
extensions that have not been ratified makes sense - *but* at the same
time if touching arch code is not required it does not feel very much
different to me than adding a driver for a fabric core in the first
place. I mentioned this sort of thing a while back on IRC and Jess made
the point that similar sorts of things are done by some of the Qualcomm
for their remoteproc as we would be doing for ours with the IHC. In your
case, if all of your ECALLs are in drivers/soc - the maintainership
burden for any churn would be on you/Geert etc rather than on the RISC-V
maintainer.

TL;DR of that is maybe a more nuanced policy of "no non-ratified
extensions that touch arch/riscv" could be a possibility but I would
completely understand if a "what's sauce for the goose is sauce for the
gander" approach was taken here and a blanket ban remains in place.

As I have said a bunch of times, this is all just my 2 cents etc and I
am as much of a punter here as you are... but maybe since I am in the
same sort of boat I at least have a fleshed out opinion. ¯\_()_/¯

Hopefully either Palmer can weigh in here or we do get a BoF & the
chance to have a chat about this sort of thing & maybe have a more
nuanced policy - or at the very least something that makes it clearer
that vendor extensions are a complete no-go upstream.

Conor.

> 9 files changed, 500 insertions(+)
> create mode 100644 arch/riscv/vendors/Makefile
> create mode 100644 arch/riscv/vendors/andes/Makefile
> create mode 100644 arch/riscv/vendors/andes/ax45mp.c
> create mode 100644 arch/riscv/vendors/andes/ax45mp_cache.c
> create mode 100644 arch/riscv/vendors/andes/ax45mp_nocache_dma.c
> create mode 100644 arch/riscv/vendors/andes/include/proc.h
> create mode 100644 arch/riscv/vendors/andes/include/sbi.h
>
> --
> 2.25.1
>

2022-09-09 10:59:49

by Prabhakar

[permalink] [raw]
Subject: Re: [RFC PATCH 0/2] AX45MP: Add support to non-coherent DMA

Hi Conor,

On Thu, Sep 8, 2022 at 10:44 PM <[email protected]> wrote:
>
> Hey,
>
> I had a quick run through this today so if there's discussion
> about this next week I at least will have some idea of what I
> am talking about...
>
Thanks for going through this!

> (I ended up not doing a quick run...)
>
:)

> On 06/09/2022 11:21, Lad Prabhakar wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Hi All,
> >
> > On the Andes AX45MP core, cache coherency is a specification option so it
> > may not be supported. In this case DMA will fail. To get around with this
> > issue this patch series does the below:
>
> You say "may not be supported" - is it or is it not supported by the
> core on your SoC? Do some of the cheaper SKUs not support it?
>
Yep, it's not supported by the core on the RZ/Five SoC.

> From what Biju has said, you need non-coherent DMA for your eMMC, USB
> and ethernet controllers to work? To me, that seems like something that
> would be quite important to point out here..
>
Agreed!

>
> > Andes AX45MP core has a Programmable Physical Memory Attributes (PMA)
> > block that allows dynamic adjustment of memory attributes in the runtime.
> > It contains a configurable amount of PMA entries implemented as CSR
> > registers to control the attributes of memory locations in interest. PMA
> > regions are passed from the cpu core node which are configured as
> > non-cacheable and non-bufferable with the SBI call.
> >
> > ax45mp: cpu@0 {
> > compatible = "andestech,ax45mp", "riscv";
> > device_type = "cpu";
> > ....
> > pma-regions = <0x0 0x00000000 0x0 0x14000000>,
> > <0x0 0x20000000 0x0 0x10000000>,
> > <0x0 0x58000000 0x0 0x08000000>;
> > ....
> > };
> >
> > We provide callbacks to synchronize specific content between memory and
> > cache. We allocate a global DMA coherent pool (which is marked as
> > non-cached using PMA) so that DMA memory allocations happens from this
> > pool and we implement the below callbacks:
> >
> > - arch_sync_dma_for_device()
> > - arch_sync_dma_for_cpu()
>
> These two already exist in arch/riscv/mm/dma-noncoherent.c using the
> alternatives mechanism.
>
I need to investigate if this can be re-used.

> > - arch_dma_alloc()
> > - arch_dma_free()
> >
> > Below are the configs that are enabled:
> >
> > - DMA_GLOBAL_POOL
> > - ARCH_HAS_SYNC_DMA_FOR_CPU
> > - ARCH_HAS_SYNC_DMA_FOR_DEVICE
>
> For these two see:
> arch/riscv/Kconfig & RISCV_DMA_NONCOHERENT
>
Thanks for the pointers, SoCs requiring this would just select
RISCV_DMA_NONCOHERENT

> >
> > l2cache: cache-controller@13400000 {
> > compatible = "andestech,ax45mp-cache", "cache";
> > cache-size = <0x40000>;
> > cache-line-size = <64>;
> > cache-sets = <1024>;
> > cache-unified;
> > reg = <0x0 0x13400000 0x0 0x100000>;
> > };
> >
> > Due to the above approach custom SBI calls have been implemented. The
> > above implementation is in preparation for adding support for Renesas
> > RZ/Five SoC which uses the AX45MP core. As with the above approach the
> > kernel image might not be generic so that it can be used on other
> > platforms, so sending it as an RFC (without DT binding patches).
> >
> > OpenSBI implementation isn't upstreamed yet, public repo for access is
> > available at [0].
>
> When you say "isn't upstreamed yet", what is the actual status? Where in
> the process are you or have you not started that yet? Does openSBI even
> allow custom extensions to be upstreamed?
>
TBH we haven't started yet. I'm really new to OpenSBI stuff, not sure
if custom extensions are allowed. I didn't find anything mentioned in
[0]

[0] https://github.com/riscv-software-src/opensbi/blob/master/docs/contributing.md

> >
> > [0] https://github.com/renesas-rz/rz_opensbi/tree/work/OpenSBI-PMA
> >
> > Cheers,
> > Prabhakar
> >
> > Lad Prabhakar (2):
> > riscv: vendors: andes: Add support to configure the PMA regions
> > riscv: vendors: andes: Add support for non-cohernet dma
> >
>
> Anyway, a couple of drive-by comments, having made the wild assumption
> that this can be accepted upstream.
>
:)

> > arch/riscv/Kbuild | 2 +
> > arch/riscv/include/asm/sbi.h | 1 +
> > arch/riscv/vendors/Makefile | 3 +
> > arch/riscv/vendors/andes/Makefile | 4 +
> > arch/riscv/vendors/andes/ax45mp_cache.c | 296 ++++++++++++++++++
>
> Surely this should be in drivers/soc/andestech, just like the SiFive L2
> controller is in drivers/soc/sifive rather in a subdirectory of the
> arch?
>
Agreed, I will move this to soc dir. maybe instead of andestech,
drivers/soc/renesas/rzfive maybe?

> > arch/riscv/vendors/andes/ax45mp_nocache_dma.c | 65 ++++
>
> This looks like it should be implemented as errata/alternatives just
> like the non-coherent stuff on the D1 is done.
>
Agreed (I need to do more reading).

> > arch/riscv/vendors/andes/include/proc.h | 9 +
>
> And I think this would fall away if implemented as errata/alternatives.
>
I need to investigate it.

> > arch/riscv/vendors/andes/include/sbi.h | 27 ++
> > arch/riscv/vendors/andes/ax45mp.c | 93 ++++++
>
> idk where this would go though, if it is even something that is
> acceptable, given the policy I linked the other day from:
> https://www.kernel.org/doc/html/latest/riscv/patch-acceptance.html#submit-checklist-addendum
>
In the worst case we could move this to the lower layer to set up the
PMA regions and can drop this from the kernel.

> There is SiFive specific errata but it is implemented using mimpid etc
> rather than compatible/dt. As I said in my initial mails, I am quite
> interested in vendor SBI extensions in the kernel. If you did check out
> the link I sent, our stuff is a world away from yours - it's isolated to
> a driver where we are using SBI ECALLs to communicate with other harts
> which are running something other than Linux in AMP configurations.
> Pretty much we can do everything we want to do without touching a
> single line of code in arch/riscv, so although the statement in that doc
> applies to both of us here it does not apply evenly :s
>
I agree, as your code doesn't touch the core and just can be
implemented as a standalone driver with ecalls.

> It's all a bit unclear to me what the story is here, because obviously
> you are doing things that Zicbo* is meant to do (just like the D1), but
> your hardware's design and initial tapeout predates the existance of
> Zicbom. Makes me start to wonder, what happens for <insert idea> that
> eventually becomes an extension? Where does the line get draw for "you
> did something that is not a ratified extension, therefore you are not
> permitted upstream"? A line obviously does have to be drawn *somewhere*
> and the easiest place to draw that line is "non ratified extensions are
> a no-go". But then again, why allow the D1 but not you?
>
Yes, with a brief look we are doing something similar to things that
have been done for Zicbo*.

> Obviously this is not a runner for someone not using an FPGA or similar,
> but the InterHart Communication IP that we are using the SBI ECALLs for
> is a fabric core, so we (in theory) could re-write it so that instead of
> using an ECALL which routes communication via the e51 "monitor" core we
> _could_ write directly to the registers of the IHC block. There's clear
> security/isolation benefits for doing things via an ECALL which is why
> that method was chosen but if we opened for the direct writes/reads the
> driver would be upstream acceptable...
>
> Don't get me wrong, I completely understand why a policy of not allowing
> extensions that have not been ratified makes sense - *but* at the same
> time if touching arch code is not required it does not feel very much
> different to me than adding a driver for a fabric core in the first
> place. I mentioned this sort of thing a while back on IRC and Jess made
> the point that similar sorts of things are done by some of the Qualcomm
> for their remoteproc as we would be doing for ours with the IHC. In your
> case, if all of your ECALLs are in drivers/soc - the maintainership
> burden for any churn would be on you/Geert etc rather than on the RISC-V
> maintainer.
>
Agreed a policy as such would keep the core code clean for
non-ratified extensions. I was wondering if we could make use of
staging directory for non-ratified extensions as lots of vendors might
want to have their SoCs upstreamed but are waiting for the extensions
to be ratified. And whenever the extensions are ratified we can always
move it to the core?

> TL;DR of that is maybe a more nuanced policy of "no non-ratified
> extensions that touch arch/riscv" could be a possibility but I would
> completely understand if a "what's sauce for the goose is sauce for the
> gander" approach was taken here and a blanket ban remains in place.
>
> As I have said a bunch of times, this is all just my 2 cents etc and I
> am as much of a punter here as you are... but maybe since I am in the
> same sort of boat I at least have a fleshed out opinion. ¯\_()_/¯
>
Thanks for weighing in.

> Hopefully either Palmer can weigh in here or we do get a BoF & the
> chance to have a chat about this sort of thing & maybe have a more
> nuanced policy - or at the very least something that makes it clearer
> that vendor extensions are a complete no-go upstream.
>
Fingers crossed.

Cheers,
Prabhakar

2022-09-09 23:07:48

by Atish Patra

[permalink] [raw]
Subject: Re: [RFC PATCH 0/2] AX45MP: Add support to non-coherent DMA

On Thu, Sep 8, 2022 at 2:45 PM <[email protected]> wrote:
>
> Hey,
>
> I had a quick run through this today so if there's discussion
> about this next week I at least will have some idea of what I
> am talking about...
>
> (I ended up not doing a quick run...)
>
> On 06/09/2022 11:21, Lad Prabhakar wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Hi All,
> >
> > On the Andes AX45MP core, cache coherency is a specification option so it
> > may not be supported. In this case DMA will fail. To get around with this
> > issue this patch series does the below:
>
> You say "may not be supported" - is it or is it not supported by the
> core on your SoC? Do some of the cheaper SKUs not support it?
>
> From what Biju has said, you need non-coherent DMA for your eMMC, USB
> and ethernet controllers to work? To me, that seems like something that
> would be quite important to point out here..
>
>
> > Andes AX45MP core has a Programmable Physical Memory Attributes (PMA)
> > block that allows dynamic adjustment of memory attributes in the runtime.
> > It contains a configurable amount of PMA entries implemented as CSR
> > registers to control the attributes of memory locations in interest. PMA
> > regions are passed from the cpu core node which are configured as
> > non-cacheable and non-bufferable with the SBI call.
> >
> > ax45mp: cpu@0 {
> > compatible = "andestech,ax45mp", "riscv";
> > device_type = "cpu";
> > ....
> > pma-regions = <0x0 0x00000000 0x0 0x14000000>,
> > <0x0 0x20000000 0x0 0x10000000>,
> > <0x0 0x58000000 0x0 0x08000000>;
> > ....
> > };
> >
> > We provide callbacks to synchronize specific content between memory and
> > cache. We allocate a global DMA coherent pool (which is marked as
> > non-cached using PMA) so that DMA memory allocations happens from this
> > pool and we implement the below callbacks:
> >
> > - arch_sync_dma_for_device()
> > - arch_sync_dma_for_cpu()
>
> These two already exist in arch/riscv/mm/dma-noncoherent.c using the
> alternatives mechanism.
>
> > - arch_dma_alloc()
> > - arch_dma_free()
> >
> > Below are the configs that are enabled:
> >
> > - DMA_GLOBAL_POOL
> > - ARCH_HAS_SYNC_DMA_FOR_CPU
> > - ARCH_HAS_SYNC_DMA_FOR_DEVICE
>
> For these two see:
> arch/riscv/Kconfig & RISCV_DMA_NONCOHERENT
>
> >
> > l2cache: cache-controller@13400000 {
> > compatible = "andestech,ax45mp-cache", "cache";
> > cache-size = <0x40000>;
> > cache-line-size = <64>;
> > cache-sets = <1024>;
> > cache-unified;
> > reg = <0x0 0x13400000 0x0 0x100000>;
> > };
> >
> > Due to the above approach custom SBI calls have been implemented. The
> > above implementation is in preparation for adding support for Renesas
> > RZ/Five SoC which uses the AX45MP core. As with the above approach the
> > kernel image might not be generic so that it can be used on other
> > platforms, so sending it as an RFC (without DT binding patches).
> >
> > OpenSBI implementation isn't upstreamed yet, public repo for access is
> > available at [0].
>
> When you say "isn't upstreamed yet", what is the actual status? Where in
> the process are you or have you not started that yet? Does openSBI even
> allow custom extensions to be upstreamed?
>
> >
> > [0] https://github.com/renesas-rz/rz_opensbi/tree/work/OpenSBI-PMA
> >
> > Cheers,
> > Prabhakar
> >
> > Lad Prabhakar (2):
> > riscv: vendors: andes: Add support to configure the PMA regions
> > riscv: vendors: andes: Add support for non-cohernet dma
> >
>
> Anyway, a couple of drive-by comments, having made the wild assumption
> that this can be accepted upstream.
>
> > arch/riscv/Kbuild | 2 +
> > arch/riscv/include/asm/sbi.h | 1 +
> > arch/riscv/vendors/Makefile | 3 +
> > arch/riscv/vendors/andes/Makefile | 4 +
> > arch/riscv/vendors/andes/ax45mp_cache.c | 296 ++++++++++++++++++
>
> Surely this should be in drivers/soc/andestech, just like the SiFive L2
> controller is in drivers/soc/sifive rather in a subdirectory of the
> arch?
>
> > arch/riscv/vendors/andes/ax45mp_nocache_dma.c | 65 ++++
>
> This looks like it should be implemented as errata/alternatives just
> like the non-coherent stuff on the D1 is done.
>
> > arch/riscv/vendors/andes/include/proc.h | 9 +
>
> And I think this would fall away if implemented as errata/alternatives.
>
> > arch/riscv/vendors/andes/include/sbi.h | 27 ++
> > arch/riscv/vendors/andes/ax45mp.c | 93 ++++++
>
> idk where this would go though, if it is even something that is
> acceptable, given the policy I linked the other day from:
> https://www.kernel.org/doc/html/latest/riscv/patch-acceptance.html#submit-checklist-addendum
>
> There is SiFive specific errata but it is implemented using mimpid etc
> rather than compatible/dt. As I said in my initial mails, I am quite
> interested in vendor SBI extensions in the kernel. If you did check out
> the link I sent, our stuff is a world away from yours - it's isolated to
> a driver where we are using SBI ECALLs to communicate with other harts
> which are running something other than Linux in AMP configurations.
> Pretty much we can do everything we want to do without touching a
> single line of code in arch/riscv, so although the statement in that doc
> applies to both of us here it does not apply evenly :s
>
> It's all a bit unclear to me what the story is here, because obviously
> you are doing things that Zicbo* is meant to do (just like the D1), but
> your hardware's design and initial tapeout predates the existance of
> Zicbom. Makes me start to wonder, what happens for <insert idea> that
> eventually becomes an extension? Where does the line get draw for "you
> did something that is not a ratified extension, therefore you are not
> permitted upstream"? A line obviously does have to be drawn *somewhere*
> and the easiest place to draw that line is "non ratified extensions are
> a no-go". But then again, why allow the D1 but not you?
>
> Obviously this is not a runner for someone not using an FPGA or similar,
> but the InterHart Communication IP that we are using the SBI ECALLs for
> is a fabric core, so we (in theory) could re-write it so that instead of
> using an ECALL which routes communication via the e51 "monitor" core we
> _could_ write directly to the registers of the IHC block. There's clear
> security/isolation benefits for doing things via an ECALL which is why
> that method was chosen but if we opened for the direct writes/reads the
> driver would be upstream acceptable...
>
> Don't get me wrong, I completely understand why a policy of not allowing
> extensions that have not been ratified makes sense - *but* at the same
> time if touching arch code is not required it does not feel very much
> different to me than adding a driver for a fabric core in the first
> place. I mentioned this sort of thing a while back on IRC and Jess made
> the point that similar sorts of things are done by some of the Qualcomm
> for their remoteproc as we would be doing for ours with the IHC. In your
> case, if all of your ECALLs are in drivers/soc - the maintainership
> burden for any churn would be on you/Geert etc rather than on the RISC-V
> maintainer.
>
> TL;DR of that is maybe a more nuanced policy of "no non-ratified
> extensions that touch arch/riscv" could be a possibility but I would
> completely understand if a "what's sauce for the goose is sauce for the
> gander" approach was taken here and a blanket ban remains in place.
>
> As I have said a bunch of times, this is all just my 2 cents etc and I
> am as much of a punter here as you are... but maybe since I am in the
> same sort of boat I at least have a fleshed out opinion. ¯\_()_/¯
>
> Hopefully either Palmer can weigh in here or we do get a BoF & the
> chance to have a chat about this sort of thing & maybe have a more
> nuanced policy - or at the very least something that makes it clearer
> that vendor extensions are a complete no-go upstream.
>

RISC-V BoF is scheduled on Wednesday.

https://lpc.events/event/16/contributions/1389/

> Conor.
>
> > 9 files changed, 500 insertions(+)
> > create mode 100644 arch/riscv/vendors/Makefile
> > create mode 100644 arch/riscv/vendors/andes/Makefile
> > create mode 100644 arch/riscv/vendors/andes/ax45mp.c
> > create mode 100644 arch/riscv/vendors/andes/ax45mp_cache.c
> > create mode 100644 arch/riscv/vendors/andes/ax45mp_nocache_dma.c
> > create mode 100644 arch/riscv/vendors/andes/include/proc.h
> > create mode 100644 arch/riscv/vendors/andes/include/sbi.h
> >
> > --
> > 2.25.1
> >
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv



--
Regards,
Atish